GNOME Bugzilla – Bug 685559
New file action "file-close"
Last modified: 2012-12-14 18:49:10 UTC
There are currently a file-close-all action, which would close all images. To close a single image, the only action is "view-close". But this latter would close a toolbox if it has the focus, which is very annoying, because toolboxes often have the focus, and when you work with several files, you usually want to be able to open/close images very fast with a shortcut. So you end up closing your toolboxes by mistake when working fast. I don't remove the view-close action, because I guess it can also have its usefulness in some cases. But I think a new "file-close" action which affects *only* the images would be worth adding. So wherever you have focus in, when running file-close (likely with a shortcut), it will affect the toplevel image. For the rest, it obviously uses the same calls to close an image, which implies all the normal checks (if dirty, it pops-up whether you want to save/not-save/cancel). I implemented this on my public branch. Repository: git://git.tuxfamily.org/gitroot/gimpgirin/gimp.git Branch: girin Commits: 09d3b02959897087733b41b4045a4f1a6feb6b9d e9f43a0729e7ae57efe065e636b82aa0d2052e17
Created attachment 229820 [details] [review] New file-close action Attached is the patch. I have also reworded the existing view-close to say "Close Window" (instead of simply "Close" - "Close the current window") previously, in order to better different with he new action which says "Close Image" - "Close the current image"). I believe this new action brings a *lot* in usability. Current view-close is extremely frustrating (constantly closes the tool boxes by mistake). Can I commit it on master?
Created attachment 230023 [details] [review] Updated patch
Mitch: about the discussion yesterday where you said that you were considering getting rid of the old view-close action. I thought of it and realized that you can already close a tool box if you have focus by the normal shortcut of your window manager to close a window (most often alt-f4), or simply by the normal UI or your window manager (often a small cross at the top decoration). So there is indeed no real need for us to add an additional action (other than acknowledging the signal from the window manager when it wants to close a view). The only different need is closing an image (hence this new patch). As a consequence, after much thought, I agree that we may get rid of view-close, and there should not even be any real feature lost. Of course if someone has a workflow where one opens/closes often toolboxes and would have liked to be able to customize the shortcut easier than changing it system-wide, this is the only case where they might disagree with the change. Anyway that's your call. As long as we can include at least the saner behavior of my new action, I am happy. :-D So if you want to remove the old one, and replace it by the new (or not removing the old one, but make the new one default in the menu instead), just tell me, and I'll make the change.
Created attachment 230176 [details] [review] view-close replaced by file-close Hey Mitch, some news on checking the code: 1/ I realized that there is another action which is exactly same as view-close: dock-close (both call the same callback window_close_cmd_callback()). Yet for some reason, none of the dock actions are made visible to the user (so unless one goes and edit manually menurc, they are not customizable because not in the shortcut UI). I would say that either we want this dock-action group, and we show them; or we don't want them, and we remove the code (hence not much, but that's still a few less code to load at startup :-). 2/ I wrote some code to kill view-close, replace it by file-close in the menu, give file-close the same default shortcut (ctrl-w), and last but not least, update the menurc file so that if the user had set a custom view-close in 2.x, then we change it into a custom file-close when updating from 2.x to 2.9. This last part in particular can be a base for a better update tool than just copying file (like here, changing action name without deleting user customization). Cf. additional patch. Hoping it makes the case advance. :-)
Ok, finally reviewed it. The menurc migration code kicks ass :) But I think "view-close" is the right name for the action, but it should *always* close the active image window, and never a dock. file-close is imo confusing because an image can have multiple views. We should keep the migration code anyway :)
Hi Michael > that's good for me. As we will not be using the migration code for this feature, I will make them 2 separate commits and push this.
Ok. So I pushed the way you said on master. Also I am so completely sorry. I made a small mistake and an include was missing (was triggering a warning but would still work). So I made a small fix commit. ------------------------------ commit 8a935f05a07a89279538f638d5d43f165d34a6e6 Author: Jehan <jehan@girinstud.io> Date: Wed Dec 12 16:05:39 2012 +0900 actions: forgot an include which was triggering an "implicit declaration" warning. commit 2ad8634c06814219728bb10fbe59a63afc42fdea Author: Jehan <jehan@girinstud.io> Date: Mon Nov 26 00:18:00 2012 +0900 Bug 685559 - view-close action modified to close only an image view. view-close was closing also toolbox docks if they had the focus. Now this action will close only the current active image view (if any), whatever the window which has actual focus. Additionally all other view actions are available on dock focus. -------------------------------- Do we consider this was a bug, hence should we push this to 2.8 as well? For the migration infrastructure, I'll do a separate commit. Also I think it would be worth cleaning up a little the actions now that we have this migration code, hence can do it without even the users losing their customization. For instance, you decided for view-close. But there is a file-close-all. Should it be renamed to view-close-all for consistency? And why does the quit action is file-quit?! This has nothing file-related. Also all the dock-* actions are loaded but they are not available from UI and customizable only by editing directly the menurc by hand. If we actually consider they are not relevant anymore, why not delete them? And so on.
For information, I opened the ticket Bug 690079 for discussion inconsistencies in action names.
Hi Michael, I saw you canceled part of the solution with: ------------- commit 0ff07fa385f401a7f9758d724a06304840f2f8c0 Author: Michael Natterer <mitch@gimp.org> Date: Wed Dec 12 23:26:46 2012 +0100 app: can't call view_actions_setup() from file_actions_setup() This breaks Ctrl+W from docks to close the active display, we'll need another solution for this. ------------ Sorry for this one, I thought it was ok because I saw other _actions_setup() called this way. For instance window_actions_setup() is called from view_actions_setup(). So why is it a problem to call view_actions_setup() from file_actions_setup()? Looks like a good idea to me. From a user point of view, you want to be able to act on the active image, even when a dock has focus (and they often have because you touch options, tools, layers, etc. I guess that reopens the ticket then. What do you suggest instead?
Because window_actions_setup() is just a utility function, whereas "view", "file" etc are their own action groups. The "view" group is not added to docks on purpose. I suggest we discuss that on IRC, since I'm not entirely sure what to do.
And fixed again: commit b234f5f879f1aeec704904f0c85562290b283b46 Author: Michael Natterer <mitch@gimp.org> Date: Fri Dec 14 09:54:17 2012 +0100 app: add the "view" action group to all docks so all image and view related shortcuts work globally now. app/menus/menus.c | 2 ++ 1 file changed, 2 insertions(+)