After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 759850 - file-operations: always pop undo manager flag
file-operations: always pop undo manager flag
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-12-24 15:28 UTC by Razvan Chitu
Modified: 2016-01-12 19:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
file-operations: always pop undo manager flag (1.47 KB, patch)
2015-12-24 15:28 UTC, Razvan Chitu
committed Details | Review
file-undo-manager: handle undo_redo_flag internally (9.03 KB, patch)
2016-01-08 22:03 UTC, Razvan Chitu
committed Details | Review
file-undo-manager: rename undo_redo_flag (7.74 KB, patch)
2016-01-08 22:19 UTC, Razvan Chitu
committed Details | Review

Description Razvan Chitu 2015-12-24 15:28:18 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.
Comment 1 Razvan Chitu 2015-12-24 15:28:21 UTC
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.
Comment 2 Carlos Soriano 2015-12-24 15:29:19 UTC
Review of attachment 317863 [details] [review]:

excellent, thanks!
Comment 3 Carlos Soriano 2015-12-24 15:29:52 UTC
Attachment 317863 [details] pushed as c9135ce - file-operations: always pop undo manager flag
Comment 4 Razvan Chitu 2016-01-08 22:03:08 UTC
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.
Comment 5 Razvan Chitu 2016-01-08 22:19:24 UTC
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.
Comment 6 Razvan Chitu 2016-01-08 22:20:32 UTC
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.
Comment 7 Carlos Soriano 2016-01-11 14:53:58 UTC
Review of attachment 318550 [details] [review]:

Wow, excellent, thanks!!
Comment 8 Carlos Soriano 2016-01-11 14:54:37 UTC
Review of attachment 318553 [details] [review]:

yay, I love this code-quality-improvement patches, thanks!
Comment 9 Carlos Soriano 2016-01-11 14:56:01 UTC
->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 :)
Comment 10 Razvan Chitu 2016-01-12 19:42:43 UTC
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