search

mate-desktop / mozo

Menu editor for MATE using the freedesktop.org menu specification
http://www.mate-desktop.org
GNU Lesser General Public License v2.1
28 stars 12 forks source link

Fix item drag & drop creating copies, fix undo, redo and a crash when refreshing #61

Closed gm10 closed 5 years ago

gm10 commented 5 years ago

So I noticed that dragging & dropping a menu entry would duplicate it instead of move it. Turns out there were two moveItem() functions in the same namespace, which explains a lot of the confusion that was had the last time we tried to fix this and probably why it broke again or was never fully fixed.

This also fixes the undo and redo functions (Ctrl+Z and Ctrl+Shift+Z) in general - both broken file accesses plus wrong order of operations - plus in particular in combination with the moveItem() function, where multiple steps had to be merged into a single.

Last but not least it fixes a crash when reloading the menu and cleans up some unused imports and variables I came across.

Remaining issues outside of the scope of this PR: While the Ctrl+Z and Ctrl+Shift+Z seem to be solid with this, I noticed that the context menu "revert" function isn't always getting activated and overall it might be a good idea to add Undo and Redo buttons to the GUI because otherwise the user thinks the Revert button is supposed to handle that (which it is not).

gm10 commented 5 years ago

@lukefromdc the Revert button resets any structural changes to the menu by reverting ~/.config/mate-applications.menu and ~/.config/mate-settings.menu to defaults. I made no changes to that but there's also no problem with it.

The undo/redo options are only available via the keybinds and to some extent via the "Revert to Original" context menu (which is not available after a drag & drop though), hence my suggestion above to add buttons for that.

lukefromdc commented 5 years ago

I do not know those keybindings at all, in fact this is the first 've heard of them

gm10 commented 5 years ago

@vkareh did the DragAction change. As far as I can see the "lag" is caused by the save() function refreshing the menu: https://github.com/mate-desktop/mozo/blob/17b84e25d4a7e51e49206633b3f7057adb81d444/Mozo/MenuEditor.py#L85, and meanwhile the "changed" signal handler doing the same thing a couple of times, too. Certainly something that could be optimized but I'd say outside of the scope of what I was trying to fix here.

Added two additional commits improving the fix further though.

gm10 commented 5 years ago

@vkareh

If I move an item to a different category, it does so successfuly, but when I Undo it, I'm now left with two copies of the item. You mean on the disk? It's due to that hole mechanic of copying & hiding the original rather than modifying it. Frankly didn't understand why that was being done in the first place but menu editing isn't my area of expertise to be honest so I stuck with the general logic that was already there for now, thinking there was probably a good reason it had originally been chosen.

There's also another bug in that with the redo-mechanic and you ending up with a few category-less, non-hidden copies, but to get there you have to do slightly complex moves, doesn't appear on a simple move - undo - redo sequence, so I left that for a rainy day...