Basically actions in menus should show the short label (we assume the menu
position brings contextuality) when available, whereas it will use the longer
label in GUI lacking contextuality.
As a first such usage, the `file-open-recent-*` actions have the file name which
will be opened as short label. This is used in menus since the submenu `File >
Open Recent` ensures that the action which will run is perfectly understandable.
On the other hand, in the action search, the action is named 'Open "<file
name>"' since an action named only with a file name would not be understandable.
Since we now generate actions for GEGL ops, we might as well generate menu items
for these too.
What I did:
- Move the "GEGL Operation…" tool (generic dialog with a drop-down list of all
non-ignored GEGL ops) to Tools menu.
- Create a "GEGL Operations" submenu in Filters > Generic.
- Move "GEGL Graph" to the top of this new submenu.
- Generate a new menu item for each generated action tied to a GEGL plug-in,
alphabetically sorted.
Though the GEGL Operation tool is still useful as a generic dialog, let's
generate also per-operation (the ones not ignored and not already special-cased
in the rest of the GUI) tools and actions.
These "tools" are mostly hidden (e.g. not selectable in toolbox where it would
be a bit useless as they would show with the generic GEGL icon or none), but
they can be searched with the action search, shortcuts can be assigned and they
can be added to menus.
Since the recent changes, these 3 tests are not working:
- tools
- session-2-8-compatibility-multi-window
- session-2-8-compatibility-single-window
(the 2 latters were only working with xvfb anyway)
I could just dig further as I did a bit these last few days, and tweak more and
more the testing code. But I think our current unit-testing framework is just
non-reliable and for all these years, we have spent more time fixing the tests
than actually relying on them to tell us there is a bug in GIMP.
Furthermore creating new tests is so cumbersome that basically none do it.
I have a plan laid out in #9339 for a much better and more reliable unit testing
infrastructure, based on the GIMP executable itself and a very simple syntax to
create new test scenari (so that even non-developers should be able to create
them eventually). So until then, let's disable these tests and stop wasting
time.
Same as ZoomIn/Out keys, these are implemented as secondary shortcuts to actions
"windows-show-display-next" and "windows-show-display-previous" respectively.
See issue #637.
- The `accelerator` variable is a NULL-terminated array, allowing up to 3
accelerators per action (so far, none has more than 2).
- Only the struct GimpProcedureActionEntry still has a single accelerator as I
don't think it makes sense that we change the plug-in API to allow a plug-in
to register more than 1 shortcut for a procedure (e.g. we don't want a plug-in
to just register all possible keys for their procedure!).
Of course, users will still be allowed to register more shortcuts for plug-in
actions through the shortcut dialog. It's only the initialization which
1-shortcut max for procedure actions.
- Remove all actions ending up in "-accel" as these were only a trick to
register more shortcut for a same action. Now we just have the real
possibility (rather than creating bogus duplicate actions). As a consequence,
these actions accelerators have been moved as secondary accelerator to their
main action.
The deleted actions are: "view-zoom-out-accel", "view-zoom-in-accel",
"view-zoom-16-1-accel", "view-zoom-4-1-accel", "view-zoom-2-1-accel" and
"view-zoom-1-1-accel".
… Default Values" buttons in Preferences.
I considered just deleting the "Remove all shortcuts" action, because really I
am wondering how useful (and often used) it is, and if we are not over-featuring
the shortcut code.
I could find the origin commit 6da388be93, from 2006. Basically the use case is
for people who want such a different mapping overall that it's much better to
start from a blank slate (e.g. DVORAK users, like in the original report).
See: https://bugzilla.gnome.org/show_bug.cgi?id=331839
So in the end, I just reimplemented this with newer code.
Also this commit fixes the "Reset Keyboard Shortcuts to Default Values" code.
This was always confusing to people that they had to click "Save" then "Close".
With new code anyway, any change is instantly put into effect, and the only
point of "Save" is to actually store immediately in the shortcutsrc file. But
this is clearly not very clear to people (and it can be done in the Preferences
dialog too).
Instead let's just have a "OK" button. The file will be actually updated on exit
only (if "Save keyboard shortcuts on exit" is checked). And that's it.
The only missing feature would be the ability to cancel the latest changes
before hitting OK, i.e. having a "Cancel" button too. Let's see to do this
later.
Also get rid of various old references to menurc and don't install it anymore to
etc/ (neither the new shortcutsrc as it doesn't look like it brings much value
to do so).
This uses a new concept of "default shortcuts" which must be called only once at
most per action. Any subsequent shortcuts setting are custom settings.
Commenting these lines out is mostly informational as it allows to see in a
quick glimpse in shortcutsrc file which are custom shortcuts or not.
gtk_accel_map_load()/gtk_accel_map_save() are not working with the new
GAction-based code. GtkApplication does not seem to have helper functions to
simply load and save accelerators in a file, so we just implement it ourselves.
A few things are missing right now, namely:
- On parsing, it doesn't handle any kind of duplicate accelerators (possible
especially if someone edited the new shortcutsrc manually).
- On reading, maybe we should only write down the changed (from defaults)
actions, while keeping the old ones commented-out, as menurc used to be. This
is actually useful info both for debugging or even for users who want to look
at this file and see what they changed.
- We should add import code to transform the menurc into shortcutsrc when
updating GIMP, otherwise all custom shortcuts would get lost.
There is still the question on whether we should add the group name too. I think
we should, expecially for plug-in's procedure actions, though right now these
are just added in the GtkApplication's main action group anyway. I'll see later
to refine this.
Now that we don't have any action duplicated in groups based on names (i.e. that
there could just be 2 actions with the same name in different groups), let's
have the GimpAction also store its group.
Use this to fix some code which was now crashing when confirming erasing an
existing shortcut.
In particular on macOS, we want to show some default common actions (see
comments in !837) but gtk_application_prefers_app_menu() docs says that it will
always return FALSE on this OS. So we ignore this call on macOS.
Depending on where it's used, this list of action can be either in the
"dockable" or the "dialogs" action group.
The meson rule is a bit more complicated than I wished it were because of the
ever-so-blocking lack of simple dependency in meson.
Cf. my latest comment: https://github.com/mesonbuild/meson/issues/8123#issuecomment-1496168759
This deals in particular with the dockable issue. Each dockbook needs its own
GimpUiManager and the action groups inside, which are identified by a string
identifier but also by a different user data (allowing to identify the specific
dockbook an action was call from, so that for instance you close the right
dockable).
It also makes the name requirement of plug-in procedure more flexible as a
procedure name clash can only happen between plug-in procedures now (not with
core actions).
This is a continuation of previous work where we are slowly moving to most logic
being in GimpMenuModel so that it works both when we create menus ourselves or
when we let GTK do it for us (e.g. the main menu bar in macOS).
Anders Jonsson