xcf_save_prop() starts with va_start(), hence must end with va_end(). Yet any of
the xcf_write*_check_error() macros could end this function abruptly. Therefore
I'm adding a "cleanup" argument to the macros, allowing to add any code
necessary to clean the current function before returning.
Five icons in the Layer dockable were being replaced by GTK defaults at
runtime. A "gimp-" prefix was added so that GIMP's version would always
be used. A few dialogues were fixed to use constants rather than
hardcoding the filename, to make it easier to update the icon in the
future.
Replaces GimpPickableInterface's pixel_to_srgb () functions with
pixel_to_rgb(). Now GimpRGB's values should be in the correct
image color space from the beginning of the process.
This fixes:
> app/core/gimpitemlist.c:632:7: warning: ‘g_pattern_match_string’ is deprecated: Use 'g_pattern_spec_match_string' instead [-Wdeprecated-declarations]
The new function appeared in GLib 2.70 which is our current minimum GLib
requirement, so the replacement is fine.
Generated actions are not necessarily application-wide actions ("app." prefix of
detailed action name). As is the case here, they can be actions specific to a
group only.
With our old code, we needed dummy actions for every submenu. This is not needed
anymore. Actions are only for end menu items (items which actually do something,
not just open a submenu).
Get rid of them all, as well as the code to ignore any action ending with such
suffixes in action-listing pieces of code.
Especially some remnants of when Gimp(Toggle|Radio)Action were not descendants
of GimpActionImpl and had to reimplement GimpAction interface.
Also fixing a few details here and there.
This helps make sure we break circular references as soon as possible when
destroying an action group. I also steal the pointer before freeing objects to
make sure the list is empty immediately and any further signal from the actions
won't affect this group.
Note that it will only work for a model attached to a GimpMenu. In particular,
it won't work for the menu bar set to a GtkApplication with gtk_application_set_menubar().
In other words, it won't work on macOS where we let the OS handle the menu.
Use this for the "View > Zoom" and "View > Flip & Rotate" submenus which used to
display the zoom level and rotation angle. This regression is now fixed.
Though it actually worked in some cases, and failed in others (I have not
figured out which build option exactly makes the format with hole work anyway),
printf manual actually explicitly says:
> There may be no gaps in the numbers of arguments specified using '$'; for
> example, if arguments 1 and 3 are specified, argument 2 must also be specified
> somewhere in the format string.
This explained while it was crashing GIMP (again, only with some build options,
or lack of build options):
> *** invalid %N$ use detected ***
There were 2 possible solutions for this: either ensure that the order number is
always used, but that defeats the purpose of plural localization. Instead I
reorder the argument so that the file name (which must always be shown for sure)
is first in arguments. This way, even if the order number is sometimes omitted
(be it in English or in any language), we avoid the crash.
gimp_rectangle_options_fixed_rule_changed is called when either the
checkbox or the dropdown for Fixed Size is changed.
However, the "fixed-size-active" property was not updated until after
it ran, so the toggle behavior is inverted.
This checks to see if the toggle was changed and updates the boolean
before setting the size field as (in)sensitive.
Just "Layers" or "Patterns" was always very confusing. It was even worse when
both tools and dialogs had nearly6 the same name (for instance "Gradient" was
the tool action, but "Gradients" was the dialog).
Now these dialog actions will be labelled more obviously in the action search,
such as "Layers Dialog" or "Gradients Dialog".
Of course, the short name will stay in contextualized menus, such as in the
"Windows > Dockable Dialogs" top submenu, or in the Dockbooks' "Add Tab"
submenu.
While the short label can be "Gradient" for instance, because it is in the
submenu "Tools > Paint Tools", this is a confusing label say in the action
search.
Now the longer label will be used in there and will say:
> Activate tool "Gradient"
"edit-paste-as-new-image-short" and "vectors-selection-to-vectors-short" were
just duplicate of the action named the same, except for the "-short" suffix, and
the only point was to have different labels.
Not though that this time, it was not enough to conclude that the action in a
menu shoud have the short variant. These were both used differently depending on
the menu.
Instead I added the concept of "label-variant" attribute in .ui menu files. When
the "long" variant is set, then we simply use the longer label.
There is still one more "-short" action: "tools-by-color-select-short", but I am
a still unsure how to handle this one.
All Gimp*ActionEntry (except GimpProcedureActionEntry) now have a short_label
member.
This commit doesn't add any new short label yet. It just fixes the struct usage,
and fixes a few localization contexts here and there when I saw such broken
strings.
I also fixed a few gradient editor action strings which were not proper labels
(like "splitmidpoint" or "splituniform", or missing uppercase, etc.).
The short labels are just the file names as we have all the context in the menu
to understand what these actions do (in Windows menu, ordered by view position
and with a `Ctrl+n` shortcut next to each, increasing by position).
In the action search for instance, we keep a 'Show "<image name>"' label.
Also I'm adding a relevant tooltip to further explain what each of these actions
do, using the view position.
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.
Jehan