wxGUI/history: add pop-up command menu with an item for delete command from history #3310
Conversation
gui/wxpython/history/browser.py
Outdated
| )[0] | ||
| self.showNotification.emit(message=_("Deleting <{}>").format(cmd)) | ||
| try: | ||
| with open(history_path, "r+") as f: |
There was a problem hiding this comment.
I would suggest creating the general function in grassdb/history.py for deleting the entry from the history file (similar as read_history, update_history). Maybe update_history could be renamed to add_entry_to_history ...
There was a problem hiding this comment.
I refactored update_history() func to add or delete command from the history file.
gui/wxpython/history/browser.py
Outdated
| if self._confirmDialog(question, title=_("Delete command")) == wx.ID_YES: | ||
| history_path = get_current_mapset_gui_history_path() | ||
| if history_path: | ||
| del_line_number = self._model.model.GetIndexOfNode( |
There was a problem hiding this comment.
Creating a new method in history/tree.py?
There was a problem hiding this comment.
I added DeleteEntryFromHistory() method.
gui/wxpython/history/browser.py
Outdated
| showTraceback=False, | ||
| ) | ||
| return | ||
| self._model.CreateModel() |
There was a problem hiding this comment.
We can just DeleteNode instead of creating the model from scratch. We could e.g. have two methods in the history/tree.py: renaming UpdateModel to AddEntryToModel and adding a new method DeleteEntryFromModel and we can decide which method to call according to some new "add" or "delete" flag in UpdateHistoryModelByCommand. Just idea. :-)
There was a problem hiding this comment.
I refactored UpdateModel() method to add/delete tree command node to/from model.
| data=data, | ||
| ) | ||
| else: | ||
| self.model.RemoveNode(node) |
There was a problem hiding this comment.
During my testing I investigate that RemoveNode() method of self.model = TreeModel(ModuleNode) instance not working as expected, node is not removed.
grass/gui/wxpython/core/treemodel.py
Lines 142 to 148 in 44e7ebc
There was a problem hiding this comment.
Look at the comment below.
|
@tmszi I have created some changes and posted them as the PR into your fork. I divided the logic of Add and Delete commands as I was also thinking that the Add signal will be much more complex in the future, so it makes sense to have it separately. The problem with RemoveNode was that the node of the Tree differs from the node of the Model. It takes me quite a lot of time to figure that out - quite a betrayal. :-) I also added the delete signal. Except for editing the history log file, it also updates the cmd buffer (that thing was missing in the original code). Please, could I ask you to test it? |
|
Closed by better implementation PR #3342. |
Add pop-up command menu with an item for delete command from history.