Wayne,
I have now fixed this such that UNDO actions are pushed to the UNDO stack
for the associated sheet. All UNDO actions for a given sheet are grouped so
a single Ctrl-Z will undo all components changed in the table (for the
given sheet).
Please find patch _007 attached (must be appli ed atop all previous
patches).
Let me know if you see any other pressing issues.
Regards,
Oliver
On Tue, Apr 18, 2017 at 6:30 AM, Wayne Stambaugh <stambau...@gmail.com>
wrote:
> On 4/17/2017 4:18 PM, Oliver Walters wrote:
> > So how do we proceed here? Is there a 'global' undo stack? If not:
>
> Unfortunately there is no global undo stack. Undo stacks are maintained
> for each unique SCH_SCREEN (schematic file) object.
>
> >
> > A) don't allow changes made in the component table viewer to be undone
> > B) Make an undo entry for each sheet that has changed symbols
> >
> > A) is easier but the user would need to quit-without-save to undo changes
>
> This is less than desirable
>
> >
> > B) is more difficult and doesn't solve the undo operations getting out
> > of order either, as the user could inject another operation on a given
> > sheet.
>
> This would be my preference. Out of order operations are already an
> issue so this solution doesn't make that issue any worse. Undo/redo is
> only available for the current sheet so the user would have to change
> sheets in order to undo anything changed in the component properties table.
>
> >
> > Suggestions?
> >
> > On 18 Apr 2017 01:26, "Wayne Stambaugh" <stambau...@gmail.com
> > <mailto:stambau...@gmail.com>> wrote:
> >
> > On 4/17/2017 10:21 AM, jp charras wrote:
> > > Le 17/04/2017 à 04:11, Oliver Walters a écrit :
> > >> JP, others,
> > >>
> > >> After further investigation, I have worked out why the components
> > with duplicated references were
> > >> displaying incorrectly.
> > >>
> > >> Patch_004 is attached, Thomas can you confirm that it fixes the
> > display for you?
> > >>
> > >> Kind Regards,
> > >> Oliver
> > >>
> > >> On Mon, Apr 17, 2017 at 7:53 AM, Oliver Walters
> > <oliver.henry.walt...@gmail.com <mailto:oliver.henry.walters@
> gmail.com>
> > >
> > > Good work, Oliver!
> > >
> > > I found 2 issues (tested on W7)
> > >
> > > 1 - m_reloadTableButton is not correctly enabled/disabled.
> > > This is due to the way events are managed, and this is OS
> dependent.
> > > To avoid this issue, enable/disable it inside a wxUpdateUIEvent
> > attached to this button.
> > >
> > > 2 - ESC key and ENTER keys do not dismiss the dialog.
> > > This is due to the fact you do not have a wxStdDialogButtonSizer,
> > and no OK and Cancel button.
> > > Please, add it and use the OK button (as usual in a dialog) to
> > transfer changes to schematic (do not
> > > use a wxCloseEvent to manage that), and obviously Cancel just
> > closes the dialog.
> > > To do this transfer, just override TransferDataFromWindow(), that
> > is called by wxWidgets when
> > > closing a dialog by the OK button.
> > >
> > > About other things, undo/redo lists should manage only changes
> > made inside the corresponding sheet,
> > > not in other sheets, to avoid inconsistencies and therefore
> crashes.
> > >
> >
> > This is one of the reasons I've been reluctant to accept code that
> > attempts to change the state of a SCH_SCREEN object other than the
> > current SCH_SCREEN object. It exposes a known flaw in our schematic
> > undo/redo design and I have yet to see anyone update the undo/redo
> > SCH_SCREEN stacks correctly. I see the potential for serious issues
> if
> > you do not keep the undo/redo stacks properly synced. Once you allow
> > the modification of information in the SCH_SCREEN object other than
> the
> > current one, you need to update the undo/redo stack for the
> appropriate
> > SCH_SCREEN object. Otherwise, you wont be able to undo all of the
> > changes correctly.
> >
> > _______________________________________________
> > Mailing list: https://launchpad.net/~kicad-developers
> > <https://launchpad.net/~kicad-developers>
> > Post to : kicad-developers@lists.launchpad.net
> > <mailto:kicad-developers@lists.launchpad.net>
> > Unsubscribe : https://launchpad.net/~kicad-developers
> > <https://launchpad.net/~kicad-developers>
> > More help : https://help.launchpad.net/ListHelp
> > <https://help.launchpad.net/ListHelp>
> >
>
From 124055f368d77d51d45c4ab21331d670749d1d8a Mon Sep 17 00:00:00 2001
From: Oliver Walters <oliver.henry.walt...@gmail.com>
Date: Wed, 19 Apr 2017 00:10:17 +1000
Subject: [PATCH] Fixed UNDO behaviour
- Undo actions are pushed to the appropriate sheet(s)
- Each sheet's actions are grouped together
---
eeschema/bom_table_model.cpp | 11 ++---
eeschema/bom_table_model.h | 2 +-
eeschema/dialogs/dialog_bom_editor.cpp | 78 +++++++++++++++++++++++++++++-----
3 files changed, 71 insertions(+), 20 deletions(-)
diff --git a/eeschema/bom_table_model.cpp b/eeschema/bom_table_model.cpp
index 6e8a0f0..1005a78 100644
--- a/eeschema/bom_table_model.cpp
+++ b/eeschema/bom_table_model.cpp
@@ -1446,9 +1446,9 @@ bool BOM_TABLE_MODEL::HaveFieldsChanged() const
/**
* Returns a list of only those components that have been changed
*/
-std::vector<SCH_COMPONENT*> BOM_TABLE_MODEL::GetChangedComponents()
+std::vector<SCH_REFERENCE> BOM_TABLE_MODEL::GetChangedComponents()
{
- std::vector<SCH_COMPONENT*> components;
+ std::vector<SCH_REFERENCE> components;
for( auto& group : Groups )
{
@@ -1464,12 +1464,7 @@ std::vector<SCH_COMPONENT*> BOM_TABLE_MODEL::GetChangedComponents()
{
for( auto& unit : component->Units )
{
- auto cmp = unit.GetComp();
-
- if( cmp )
- {
- components.push_back( cmp );
- }
+ components.push_back( unit );
}
}
}
diff --git a/eeschema/bom_table_model.h b/eeschema/bom_table_model.h
index 7b160bb..933db5c 100644
--- a/eeschema/bom_table_model.h
+++ b/eeschema/bom_table_model.h
@@ -315,7 +315,7 @@ public:
bool HaveFieldsChanged() const;
- std::vector<SCH_COMPONENT*> GetChangedComponents();
+ std::vector<SCH_REFERENCE> GetChangedComponents();
unsigned int CountChangedComponents();
};
diff --git a/eeschema/dialogs/dialog_bom_editor.cpp b/eeschema/dialogs/dialog_bom_editor.cpp
index 19b0c79..722cbd5 100644
--- a/eeschema/dialogs/dialog_bom_editor.cpp
+++ b/eeschema/dialogs/dialog_bom_editor.cpp
@@ -93,6 +93,18 @@ DIALOG_BOM_EDITOR::~DIALOG_BOM_EDITOR()
//TODO
}
+/* Struct for keeping track of schematic sheet changes
+ * Stores:
+ * SHEET_PATH - Schematic to apply changes to
+ * PICKED_ITEMS_LIST - List of changes to apply
+ */
+
+typedef struct
+{
+ SCH_SHEET_PATH path;
+ PICKED_ITEMS_LIST items;
+} SheetUndoList;
+
/**
* When the component table dialog is closed,
* work out if we need to save any changed.
@@ -124,36 +136,80 @@ bool DIALOG_BOM_EDITOR::TransferDataFromWindow()
if( saveChanges )
{
- /** Create a list of picked items for undo
- * PICKED_ITEMS_LIST contains multiple ITEM_PICKER instances
- * Each ITEM_PICKER contains a component and a command
+ /**
+ * As we may be saving changes across multiple sheets,
+ * we need to first determine which changes need to be made to which sheet.
+ * To this end, we perform the following:
+ * 1. Save the "path" of the currently displayed sheet
+ * 2. Create a MAP of <SheetPath:ChangeList> changes that need to be made
+ * 3. Push UNDO actions to appropriate sheets
+ * 4. Perform all the update actions
+ * 5. Reset the sheet view to the current sheet
*/
- auto pickerList = PICKED_ITEMS_LIST();
+ auto currentSheet = m_parent->GetCurrentSheet();
+
+ //! Create a map of changes required for each sheet
+ std::map<wxString, SheetUndoList> undoSheetMap;
// List of components that have changed
auto changed = m_bom->GetChangedComponents();
ITEM_PICKER picker;
- for( auto cmp : changed )
+ // Iterate through each of the components that were changed
+ for( auto ref : changed )
{
+ // Extract the SCH_COMPONENT* object
+ auto cmp = ref.GetComp();
+
+ wxString path = ref.GetSheetPath().Path();
+
// Push the component into the picker list
picker = ITEM_PICKER( cmp, UR_CHANGED );
picker.SetFlags( cmp->GetFlags() );
- //picker.SetLink( DuplicateStruct( cmp, true ) );
- pickerList.PushItem( picker );
+
+ /*
+ * If there is not currently an undo list for the given sheet,
+ * create an empty one
+ */
+
+ if( undoSheetMap.count( path ) == 0 )
+ {
+ SheetUndoList newList;
+
+ newList.path = ref.GetSheetPath();
+
+ undoSheetMap[path] = newList;
+ }
+
+ auto& pickerList = undoSheetMap[path];
+
+ pickerList.items.PushItem( picker );
}
- if( pickerList.GetCount() > 0 )
+ // Iterate through each sheet that needs updating
+ for( auto it = undoSheetMap.begin(); it != undoSheetMap.end(); ++it )
{
- m_parent->SaveCopyInUndoList( pickerList, UR_CHANGED );
- m_bom->ApplyFieldChanges();
- m_parent->Refresh();
+ auto undo = it->second;
+
+ m_parent->SetCurrentSheet( undo.path );
+ m_parent->SaveCopyInUndoList( undo.items, UR_CHANGED );
+ m_parent->OnModify();
}
+ // Apply all the field changes
+ m_bom->ApplyFieldChanges();
+
+ // Redraw the current sheet and mark as dirty
+ m_parent->Refresh();
m_parent->OnModify();
+
+ // Reset the view to where we left the user
+ m_parent->SetCurrentSheet(currentSheet);
+
+
}
return true;
--
2.7.4
_______________________________________________
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help : https://help.launchpad.net/ListHelp
