From e724a9bff3ae318139e5161971ed9aeaf665aa0c Mon Sep 17 00:00:00 2001 From: Jehan Date: Wed, 8 Mar 2023 13:01:21 +0100 Subject: [PATCH] app: GimpActionGroup is not a GtkGroup anymore, but a GimpObject descendant! I've hesitated if we should make it a GActionGroup, whose API handles actions fully through name strings. I didn't see what it brings us short-term compared to the additional work, so for now it's just a class of its own, not implementing GActionGroup interface. Maybe later. --- app/actions/plug-in-actions.c | 125 ---------------------------------- app/widgets/gimpactiongroup.c | 74 +++++++++++--------- app/widgets/gimpactiongroup.h | 7 +- 3 files changed, 48 insertions(+), 158 deletions(-) diff --git a/app/actions/plug-in-actions.c b/app/actions/plug-in-actions.c index 835ae86840..5054a48d57 100644 --- a/app/actions/plug-in-actions.c +++ b/app/actions/plug-in-actions.c @@ -51,26 +51,15 @@ /* local function prototypes */ -static void plug_in_actions_menu_branch_added (GimpPlugInManager *manager, - GFile *file, - const gchar *menu_path, - const gchar *menu_label, - GimpActionGroup *group); static void plug_in_actions_register_procedure (GimpPDB *pdb, GimpProcedure *procedure, GimpActionGroup *group); static void plug_in_actions_unregister_procedure (GimpPDB *pdb, GimpProcedure *procedure, GimpActionGroup *group); -static void plug_in_actions_menu_path_added (GimpPlugInProcedure *proc, - const gchar *menu_path, - GimpActionGroup *group); static void plug_in_actions_add_proc (GimpActionGroup *group, GimpPlugInProcedure *proc); -static void plug_in_actions_build_path (GimpActionGroup *group, - const gchar *translated); - /* private variables */ @@ -96,24 +85,6 @@ plug_in_actions_setup (GimpActionGroup *group) plug_in_actions, G_N_ELEMENTS (plug_in_actions)); - for (list = gimp_plug_in_manager_get_menu_branches (manager); - list; - list = g_slist_next (list)) - { - GimpPlugInMenuBranch *branch = list->data; - - plug_in_actions_menu_branch_added (manager, - branch->file, - branch->menu_path, - branch->menu_label, - group); - } - - g_signal_connect_object (manager, - "menu-branch-added", - G_CALLBACK (plug_in_actions_menu_branch_added), - group, 0); - for (list = manager->plug_in_procedures; list; list = g_slist_next (list)) @@ -175,21 +146,6 @@ plug_in_actions_update (GimpActionGroup *group, /* private functions */ -static void -plug_in_actions_menu_branch_added (GimpPlugInManager *manager, - GFile *file, - const gchar *menu_path, - const gchar *menu_label, - GimpActionGroup *group) -{ - gchar *full; - - full = g_strconcat (menu_path, "/", menu_label, NULL); - plug_in_actions_build_path (group, full); - - g_free (full); -} - static void plug_in_actions_register_procedure (GimpPDB *pdb, GimpProcedure *procedure, @@ -199,10 +155,6 @@ plug_in_actions_register_procedure (GimpPDB *pdb, { GimpPlugInProcedure *plug_in_proc = GIMP_PLUG_IN_PROCEDURE (procedure); - g_signal_connect_object (plug_in_proc, "menu-path-added", - G_CALLBACK (plug_in_actions_menu_path_added), - group, 0); - if (plug_in_proc->menu_label && ! plug_in_proc->file_proc) { @@ -225,10 +177,6 @@ plug_in_actions_unregister_procedure (GimpPDB *pdb, { GimpPlugInProcedure *plug_in_proc = GIMP_PLUG_IN_PROCEDURE (procedure); - g_signal_handlers_disconnect_by_func (plug_in_proc, - plug_in_actions_menu_path_added, - group); - if (plug_in_proc->menu_label && ! plug_in_proc->file_proc) { @@ -248,25 +196,11 @@ plug_in_actions_unregister_procedure (GimpPDB *pdb, } } -static void -plug_in_actions_menu_path_added (GimpPlugInProcedure *plug_in_proc, - const gchar *menu_path, - GimpActionGroup *group) -{ -#if 0 - g_print ("%s: %s (%s)\n", G_STRFUNC, - gimp_object_get_name (plug_in_proc), menu_path); -#endif - - plug_in_actions_build_path (group, menu_path); -} - static void plug_in_actions_add_proc (GimpActionGroup *group, GimpPlugInProcedure *proc) { GimpProcedureActionEntry entry; - GList *list; entry.name = gimp_object_get_name (proc); entry.icon_name = gimp_viewable_get_icon_name (GIMP_VIEWABLE (proc)); @@ -279,13 +213,6 @@ plug_in_actions_add_proc (GimpActionGroup *group, gimp_action_group_add_procedure_actions (group, &entry, 1, plug_in_run_cmd_callback); - for (list = proc->menu_paths; list; list = g_list_next (list)) - { - const gchar *original = list->data; - - plug_in_actions_build_path (group, original); - } - if (proc->image_types_val) { GimpContext *context = gimp_get_user_context (group->gimp); @@ -309,55 +236,3 @@ plug_in_actions_add_proc (GimpActionGroup *group, tooltip); } } - -static void -plug_in_actions_build_path (GimpActionGroup *group, - const gchar *path_translated) -{ - GimpContext *context = gimp_get_user_context (group->gimp); - GHashTable *path_table; - gchar *copy_translated; - gchar *p2; - - path_table = g_object_get_data (G_OBJECT (group), "plug-in-path-table"); - - if (! path_table) - { - path_table = g_hash_table_new_full (g_str_hash, g_str_equal, - g_free, NULL); - - g_object_set_data_full (G_OBJECT (group), "plug-in-path-table", - path_table, - (GDestroyNotify) g_hash_table_destroy); - } - - copy_translated = g_strdup (path_translated); - - p2 = strrchr (copy_translated, '/'); - - if (p2 && ! g_hash_table_lookup (path_table, copy_translated)) - { - GimpAction *action; - gchar *label; - - label = p2 + 1; - -#if 0 - g_print ("adding plug-in submenu '%s' (%s)\n", - copy_translated, label); -#endif - - action = gimp_action_impl_new (copy_translated, label, NULL, NULL, NULL, context); - gimp_action_group_add_action (group, action); - g_object_unref (action); - - g_hash_table_insert (path_table, g_strdup (copy_translated), action); - - *p2 = '\0'; - - /* recursively call ourselves with the last part of the path removed */ - plug_in_actions_build_path (group, copy_translated); - } - - g_free (copy_translated); -} diff --git a/app/widgets/gimpactiongroup.c b/app/widgets/gimpactiongroup.c index 98964244a7..5c61fa531a 100644 --- a/app/widgets/gimpactiongroup.c +++ b/app/widgets/gimpactiongroup.c @@ -74,7 +74,7 @@ static void gimp_action_group_get_property (GObject *object, GParamSpec *pspec); -G_DEFINE_TYPE (GimpActionGroup, gimp_action_group, GTK_TYPE_ACTION_GROUP) +G_DEFINE_TYPE (GimpActionGroup, gimp_action_group, GIMP_TYPE_OBJECT) static guint signals[LAST_SIGNAL] = { 0, }; @@ -128,6 +128,7 @@ gimp_action_group_class_init (GimpActionGroupClass *klass) static void gimp_action_group_init (GimpActionGroup *group) { + group->actions = NULL; } static void @@ -310,19 +311,18 @@ gimp_action_group_new (Gimp *gimp, const gchar * gimp_action_group_get_name (GimpActionGroup *group) { - return gtk_action_group_get_name ((GtkActionGroup *) group); + return gimp_object_get_name (GIMP_OBJECT (group)); } void -gimp_action_group_add_action (GimpActionGroup *action_group, +gimp_action_group_add_action (GimpActionGroup *group, GimpAction *action) { - gtk_action_group_add_action ((GtkActionGroup *) action_group, - (GtkAction *) action); + gimp_action_group_add_action_with_accel (group, action, NULL); } void -gimp_action_group_add_action_with_accel (GimpActionGroup *action_group, +gimp_action_group_add_action_with_accel (GimpActionGroup *group, GimpAction *action, const gchar *accelerator) { @@ -330,35 +330,51 @@ gimp_action_group_add_action_with_accel (GimpActionGroup *action_group, /* Making sure all our Gimp*Action classes are also GAction. */ g_return_if_fail (G_IS_ACTION (action)); - g_action_map_add_action (G_ACTION_MAP (action_group->gimp->app), G_ACTION (action)); - if ((accelerator != NULL && g_strcmp0 (accelerator, "") != 0)) - gimp_action_set_accels (action, (const char*[]) { accelerator, NULL }); + if (! g_list_find (group->actions, action)) + { + group->actions = g_list_prepend (group->actions, action); - /* TODO: remove the old logic with GtkAction. */ - gtk_action_group_add_action_with_accel ((GtkActionGroup *) action_group, - (GtkAction *) action, NULL); + g_action_map_add_action (G_ACTION_MAP (group->gimp->app), G_ACTION (action)); + + if ((accelerator != NULL && g_strcmp0 (accelerator, "") != 0)) + gimp_action_set_accels (action, (const char*[]) { accelerator, NULL }); + } } void -gimp_action_group_remove_action (GimpActionGroup *action_group, +gimp_action_group_remove_action (GimpActionGroup *group, GimpAction *action) { - gtk_action_group_remove_action ((GtkActionGroup *) action_group, - (GtkAction *) action); + group->actions = g_list_remove (group->actions, action); + + /* TODO GAction: we should also check if the action is still present in + * another group (maybe if each action keeps track of its own groups, or with + * gimp_ui_manager_find_action(), or a "action-removed" signal tracked by the + * GimpUIManager which would verify other groups). + * If it's not in any group anymore, we should remove the action with + * g_action_map_remove_action(). + */ } GimpAction * gimp_action_group_get_action (GimpActionGroup *group, const gchar *action_name) { - return (GimpAction *) gtk_action_group_get_action ((GtkActionGroup *) group, - action_name); + for (GList *iter = group->actions; iter; iter = iter->next) + { + GimpAction *action = iter->data; + + if (g_strcmp0 (gimp_action_get_name (action), action_name) == 0) + return action; + } + + return NULL; } GList * gimp_action_group_list_actions (GimpActionGroup *group) { - return gtk_action_group_list_actions ((GtkActionGroup *) group); + return g_list_copy (group->actions); } GList * @@ -836,19 +852,8 @@ void gimp_action_group_remove_action_and_accel (GimpActionGroup *group, GimpAction *action) { - const gchar *action_name; - const gchar *group_name; - gchar *accel_path; - - action_name = gimp_action_get_name (action); - group_name = gimp_action_group_get_name (group); - accel_path = g_strconcat ("/", group_name, "/", - action_name, NULL); - - gtk_accel_map_change_entry (accel_path, 0, 0, FALSE); - + gimp_action_set_accels (action, (const char*[]) { NULL }); gimp_action_group_remove_action (group, action); - g_free (accel_path); } void @@ -1166,5 +1171,12 @@ gimp_action_group_set_action_always_show_image (GimpActionGroup *group, return; } - gtk_action_set_always_show_image ((GtkAction *) action, always_show_image); +#if 0 + /* TODO GAction: currently a no-op, though I think it was already a no-op to + * us, at least for proxy images. We could still use this for other menu items + * and button icons. Let's investigate further later to decide whether we + * should get rid of all related code or instead make it work as expected. + */ + gtk_action_set_always_show_image ((GtkAction *) action, TRUE); +#endif } diff --git a/app/widgets/gimpactiongroup.h b/app/widgets/gimpactiongroup.h index 3af0056522..d4cccb5aab 100644 --- a/app/widgets/gimpactiongroup.h +++ b/app/widgets/gimpactiongroup.h @@ -21,6 +21,7 @@ #ifndef __GIMP_ACTION_GROUP_H__ #define __GIMP_ACTION_GROUP_H__ +#include "core/gimpobject.h" #define GIMP_TYPE_ACTION_GROUP (gimp_action_group_get_type ()) #define GIMP_ACTION_GROUP(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GIMP_TYPE_ACTION_GROUP, GimpActionGroup)) @@ -34,7 +35,7 @@ typedef struct _GimpActionGroupClass GimpActionGroupClass; struct _GimpActionGroup { - GtkActionGroup parent_instance; + GimpObject parent_instance; Gimp *gimp; gchar *label; @@ -43,11 +44,13 @@ struct _GimpActionGroup gpointer user_data; GimpActionGroupUpdateFunc update_func; + + GList *actions; }; struct _GimpActionGroupClass { - GtkActionGroupClass parent_class; + GimpObjectClass parent_class; GHashTable *groups;