As this is not likely to be used by many, as you write yourself, wouldn't it be better to ship a menurc file that has all shortcuts set to be empty. Similiar to the ps-menurc, which contains the photoshop shortcuts.

If we want to improve shrtocut handling then, we should think about loading and saving such files (and changing the settings on the fly) instead of adding a button which won't be used by many users.

A blank menurc would work just fine. I'm not sure how you would like the bug status to be, so I'll let you handle that.

Thanks,

Maintaining a blank menurc would be more work than adding the suggested button to the Keyboard Editor dialog.

Perhaps rather than adding a button to clear all you could instead use a drop down which allowed users to select from several options such as 
"None" / "Clear All"  
"Defaults"
"Photoshop Style"  (psmenurc)

[Just a suggestion, no need to comment if you prefer to things differently.]

Sounds like a good idea and shouldn't be too hard to implement.

I will set the gnome-love keyword in the hope that we can find someone for this job. There are probably more improvements to our Preferences that could be done

I've decided to look into this enhancement as part of the article I'm writing for Linux Format.  If I don't complete the changes before I submit the article (in two days - short lead times for articles like this) I'll still complete the updates as long as they're acceptable.

I've cscope'd my way around and come up with a plan, but I still have questions.  Here is the basic plan (questions follow):


1. Add entry to PROP enum in app/config/gimpdisplayconfig.c:  PROP_KEYBOARD_SHORTCUTS

2. Add GIMP_CONFIG_INSTALL_PROP_ENUM to app/config/gimpdisplayconfig.c for new menu:
   GIMP_CONFIG_INSTALL_PROP_ENUM (object_class, PROP_KEYBOARD_SHORTCUTS,
                                 "keyboard-shortcuts",
                                 KEYBOARD_SHORTCUTS_BLURB,
                                 GIMP_TYPE_KEYBOARD_SHORTCUTS,
                                 GIMP_KEYBOARD_SHORTCUT_SAVE,
                                 GIMP_PARAM_STATIC_STRINGS);

3. Add a blurb to app/config/gimprc-blurbs.h
#define KEYBOARD_SHORTCUTS_BLURB N_("Sets the status of the keyboard shortcuts")

4. update app/core/core-enums.h
   #define GIMP_TYPE_KEYBOARD_SHORTCUTS (gimp_keyboard_shortcuts_get_type ())
   GType gimp_keyboard_shortcuts_get_type (void) G_GNUC_CONST;
   typedef enum  /*< pdb-skip >*/
   {
     GIMP_KEYBOARD_SHORTCUT_SAVE,   /*< desc="Save Now"     >*/
     GIMP_KEYBOARD_SHORTCUT_RESET,  /*< desc="Reset"        >*/
     GIMP_KEYBOARD_SHORTCUT_CLEAR,  /*< desc="Clear"        >*/
   } GimpKeyboardShortcuts;

5. update app/core/core-enums.c

GType
gimp_view_size_get_type (void)
{
  static const GEnumValue values[] =
  {
    { GIMP_KEYBOARD_SHORTCUT_SAVE, "GIMP_KEYBOARD_SHORTCUT_SAVE", "save" },
    { GIMP_KEYBOARD_SHORTCUT_RESET, "GIMP_KEYBOARD_SHORTCUT_RESET", "reset" },
    { GIMP_KEYBOARD_SHORTCUT_CLEAR, "GIMP_KEYBOARD_SHORTCUT_CLEAR", "clear" },
    { 0, NULL, NULL }
  };

  static const GimpEnumDesc descs[] =
  {
    { GIMP_KEYBOARD_SHORTCUT_SAVE, N_("Save"), NULL },
    { GIMP_KEYBOARD_SHORTCUT_RESET, N_("Reset"), NULL },
    { GIMP_KEYBOARD_SHORTCUT_CLEAR, N_("Clear"), NULL },
    { 0, NULL, NULL }
  };

  static GType type = 0;

  if (! type)
    {
      type = g_enum_register_static ("GimpKeyboardShortcuts", values);
      gimp_enum_set_value_descriptions (type, descs);
    }

  return type;
}

6. Replace buttons in app/dialogs/preferences-dialog.c with
  table = prefs_table_new (2, GTK_CONTAINER (vbox2));
  prefs_enum_combo_box_add (object, "keyboard-shortcuts", 0, 0,
                            _("Keyboard shortcuts status:"),
                            GTK_TABLE (table), 0, size_group);

Questions:
1. How does the option menu selection generate a callback?  Do I save the return value from the call to prefs_enum_combo_box_add() and attach a signal to it?  It's been awhile since I've played with option menus, but I thought the creation of the option menu was supposed to attach a callback that would get passed the option value selected.  If that's true, where do I attach that callback?

More searching:

It looks like I missed adding a switch in    app/config/gimpdisplayconfig.c:gimp_display_config_set_property()    
for PROP_KEYBOARD_SHORTCUTS.  This switch appears to be where the callback should be made. But that means that a static function in app/dialogs/preferences-dialog.c (prefs_menus_save_callback) has to callable from app/config/gimpdisplayconfig.c, which doesn't make any sense.

So I'm guessing that saving the widget id from prefs_enum_combo_box_add() and setting a "changed" signal on it would be the appropriate way to handle this.  Is that right?

Either way, I still need to put a case statement into gimp_display_config_set_property() so I don't get errors when the app starts or you open the preferences dialog, even if the case statement is empty.

Okay, way wrong on my plan.  I tried to do what the other option menus were doing in that pane of the Preferences dialog.  But what they do saves config information.  This enhancement only requires immediate action of some kind.  So a simple gimp_int_combo_box_new() should suffice.  See new question at the bottom of this entry.

So I dropped that big plan and went much simpler:

Note:  all of this happens in app/dialogs/preferences-dialog.c.

1. Add a new horizontal box after the current buttons:
  hbox = gtk_hbox_new (FALSE, 6);
  gtk_box_pack_start (GTK_BOX (vbox2), hbox, FALSE, FALSE, 0);
  gtk_widget_show (hbox);

2. Insert a label for the drop down menu:
  label = gtk_label_new (_("Keyboard Shortcut Status:"));
  gtk_box_pack_start (GTK_BOX (hbox), label, FALSE, FALSE, 0);
  gtk_widget_show (label);

3. Create the drop down menu and connect it to a callback function:
  combo = gimp_int_combo_box_new (
              _("Save Now"),                      KBS_SAVE,
              _("Reset to Default Values"),       KBS_RESET,
              _("Remove All Keyboard Shortcuts"), KBS_CLEAR,
              NULL);
  gimp_int_combo_box_set_active (GIMP_INT_COMBO_BOX (combo), KBS_SAVE);
  gtk_box_pack_start (GTK_BOX (hbox), combo, FALSE, FALSE, 0);
  gtk_widget_show (combo);

  g_signal_connect (combo, "changed",
                    G_CALLBACK (prefs_menus_keyboard_shortcuts),
                    gimp);

4. Add an enum at the top of the file:
   enum {
      KBS_SAVE,
      KBS_RESET,
      KBS_CLEAR
   };

5. Add the callback function:
static void   prefs_menus_keyboard_shortcuts      (GtkWidget  *widget,
                                                   Gimp       *gimp);
static void
prefs_menus_keyboard_shortcuts(GtkWidget *combo,
                               Gimp      *gimp)
{
  gint     value;
  gint     fd;
  GError  *error = NULL;
  gchar   *filename;

  gimp_int_combo_box_get_active (GIMP_INT_COMBO_BOX (combo), &value);

  switch(value)
  {
    case KBS_SAVE:
      menus_save (gimp, TRUE);
      g_message (_("Your keyboard shortcuts have been saved."));
      break;

    case KBS_RESET:
      if (! menus_clear (gimp, &error))
        {
          g_message (error->message);
          g_clear_error (&error);
        }
      else
        {
          g_message (_("Your keyboard shortcuts will be reset to default "
                       "values the next time you start GIMP."));
        }
      break;

    case KBS_CLEAR:
      if (! menus_clear (gimp, &error))
        {
          g_message (error->message);
          g_clear_error (&error);
        }
      else
        {
          filename = gimp_personal_rc_file ("menurc");
          fd = g_open (filename, O_CREAT | O_RDWR);
          close (fd);
          g_free (filename);
          g_message (_("Your keyboard shortcuts will be cleared "
                       "the next time you start GIMP."));
        }
      break;
  }
}

6. Remove the old buttons and their callbacks.

Question:  My understanding was that creating an empty menurc would be sufficient to clear the accelerators.  My code does that, but 
1) the accelerators are there on restart even though the menurc is empty and
2) if you quit after the restart the menurc gets repopulated with the default values.

What else needs to happen here?  Does the menurc need to have at least one entry, or perhaps all the entries need to be cleared (re: set to "")?

A combo box is clearly the wrong widget for this. It must not be used
for triggering actions, that's what buttons are for.

I don't see what's wrong with the current way, just add another button
to clear the shortcuts.

Also, did you try what happens when gimp reads a menurc that is empty?
Doing this has zero effect, GIMP will just use the shortcuts defined
with the actions. I think the only way to acheive this is to iterate
*all* actions and clear the shortcut manually.

A button would be easy, but Alan suggested a drop down menu and Sven said "Sounds good" so I went with a menu instead.  Personally I think the menu looks better, but it might not be as intuitive.

I did experiment with the empty menurc and it indeed is ignored, so just zeroing the menurc is not sufficient. I think you're right about iterating over the actions and clearing them manually.  I'll implement that today and test it.

Thanks for the feedback.

Attaching patch.

1. Added combo box menu for Save, Clear and Remove options for Keyboard Shortcuts in Preferences dialog to app/dialogs/preferences.c.

2. Added combo box callback with case statements for Save, Clear and Remove.  Remove option calls new menus_remove() function and then calls existing menus_save().  Other two options work as before.

3. Added menus_remove() to app/menus/menus.c along with a static iterator function to clear all current accelerators.  Added menus_remove() prototype to app/menus/menus.h.

4. Removed Save and Clear buttons and their callbacks from app/dialogs/preferences-dialog.c.

Code has been tested.  Attaching screenshots of before and after Preferences dialog along with Toolbox File menu before and after running new Remove option.

Created attachment 65183 [details] [review]
Patch file that implements the proposed enhancement to Preferences dialog

Created attachment 65184 [details]
New preferences dialog, with expanded menu.

Created attachment 65185 [details]
Before and after running the new Remove option

I'm pretty sure that Sven meant an option menu is fine for selecting
a preset (like { gimp, photoshop, foobar }), but what you put into
the option menu are actions, which simply don't belong there.

Please use buttons, and just add an additional button plus callback
to the prefs dialog. Apart from that the patch is fine and will go
into CVS after the changes.

(I know 3 buttons are ugly below each other, I think they should go into
a hbox and have shorter labels. Longer descriptions can go into tooltips)

No problem.  Here's a new patch that uses buttons.  Only question is whether I used the right stock icon in the button (I used GIMP_STOCK_CLOSED).  I also put the buttons in an hbox and added tool tips to them.  I just noticed (can't fix right now - on my way out the door, but will fix later) that my tooltips don't honor the tooltip setting in the Preferences dialog.  

Created attachment 65198 [details] [review]
Updated patch that uses buttons instead of drop down menu.

Tooltips are set using gimp_help_set_help_data(), so they honor
the prefs setting. No issue, will fix that when applying the patch.
Thanks :)

I figured that if we move the buttons into a hbox, we will definitely
run into "prefs dialog too large with foobar locale" bugs, so i left
them below each other. I also added a confirmation dialog, since
accidentially removing a well done set of shortcuts can be quite
annoying.

2006-05-11  Michael Natterer  <mitch@gimp.org>

	Applied modified patch from Michael J. Hammel which allows to
	remove all keyboard shortcuts from the menus (fixes bug #331839):

	* app/dialogs/preferences-dialog.c: added "Remove all keyboard
	shortcuts" button to the "Interface" section.

	* app/menus/menus.[ch]: added menus_remove() which does the
	shortcut removal.