PR: Editor historystack improvements #5229
Conversation
…vent that posible corruptions raise an error.)
_get_previous_file_index shouldn't modify stack_history, the stack history will be modified when the file close and current_changed is raised Modifing stack_history in _get_previous_file_index cause corruptions in the history when the close of the file is cancelled.
spyder/widgets/editor.py
Outdated
| try: | ||
| return self.id_list.index(self.history[i]) | ||
| except ValueError: | ||
| self.refresh |
There was a problem hiding this comment.
This should be self.refresh(), right?
There was a problem hiding this comment.
Yes, thanks for noticing it
spyder/widgets/editor.py
Outdated
| return self.id_list.index(self.history[i]) | ||
| except ValueError: | ||
| self.refresh | ||
| raise IndexError |
There was a problem hiding this comment.
Do we really need to raise an IndexError here?
There was a problem hiding this comment.
I think it's the expected bevahior, when an element isn't found in a list an IndexError is raised.
The other option will be to return None, when an error occurs
| if len(self.stack_history) > 1: | ||
| last = len(self.stack_history)-1 | ||
| w_id = self.stack_history.pop(last) | ||
| self.stack_history.insert(0, w_id) |
There was a problem hiding this comment.
This was an unnecessary side effect, that could cause the corruption of the historystack order.
This code reordered the stackhiistory before closing the file, the closing could be canceled, an the stackhistory will have a wrong order.
Also this was unnecessary, because when a file is closed the CurrentChanged signal is be emited, and that cause the history_stack to be updated.
There was a problem hiding this comment.
Ok, thanks for the explanation :-)
Fixes: #5226
I think the error was the unnecessary side effect of
_get_previous_file_index, although I wasn't able to reproduce #5226