GNOME Bugzilla – Bug 759850
file-operations: always pop undo manager flag
Last modified: 2016-01-12 19:42:51 UTC
Nautilus undo manager reuses undo information based on a flag that is pushed when an undo operation begins and popped during the operation itself. In the case of trashing / deleting files, the flag was not popped in some situations, which caused the next operations to not be undoable. Flip operands of a logical operator so the flag is always popped, avoiding issues caused by lazy evaluation. A better solution would be to always update the flag internally in the manager, and only peek its value externally.
Created attachment 317863 [details] [review] file-operations: always pop undo manager flag Nautilus undo manager reuses undo information based on a flag that is pushed when an undo operation begins and popped during the operation itself. In the case of trashing / deleting files, the flag was not popped in some situations, which caused the next operations to not be undoable. Flip operands of a logical operator so the flag is always popped, avoiding issues caused by lazy evaluation. A better solution would be to always update the flag internally in the manager, and only peek its value externally.
Review of attachment 317863 [details] [review]: excellent, thanks!
Attachment 317863 [details] pushed as c9135ce - file-operations: always pop undo manager flag
Created attachment 318550 [details] [review] file-undo-manager: handle undo_redo_flag internally Nautilus undo manager reuses undo information based on a flag that marks whether an undo / redo operation is currently being performed. Previously, the flag was managed both internally and externally. This proved to be unnecessary and even harmful, as it led to an unexpected behavior. Remove push and pop functions and handle the flag explicitly, at the start and at the end of an undo / redo operation. Replace external use of pop with the use of a getter function, also introduced in this patch.
Created attachment 318553 [details] [review] file-undo-manager: rename undo_redo_flag Nautilus undo manager previously relied on a stack-like implementation of its operating state. The stack operations (pushing and popping) were removed in a previous commit due to possible unexpected behavior and errors. The flag alone now has an inadequate name for what it represents. Rename the flag to "is_operating", since it is set when the manager is performing an undo / redo operation and unset otherwise. Rename the associated getter function accordingly.
The second patch is just something that I thought might be good for the manager. I think it would make more sense for the flag to be named that way.
Review of attachment 318550 [details] [review]: Wow, excellent, thanks!!
Review of attachment 318553 [details] [review]: yay, I love this code-quality-improvement patches, thanks!
->reopened, it's not resolved fixed yet. It has to be marked as so when pushed. I think you have commits rights now right? If so, feel free to push and mark as fixed :)
Attachment 318550 [details] pushed as d52c523 - file-undo-manager: handle undo_redo_flag internally Attachment 318553 [details] pushed as 0a90763 - file-undo-manager: rename undo_redo_flag