*** Bug 115483 has been marked as a duplicate of this bug. ***

From my bug against metacity,

You can do it manually by editing the
apps/metacity/keybinding_commands and apps/metacity/global_keybindings
gconf keys using gconf-editor.

This seems fundamentally flawed since I thought GNOME was meant to be
WM independant. Surely generic keybindings like this should be stored
in a non WM-specific part of the gconf tree?

Anyone going to do anything about this? it is an apalling omission
which seriously limits functionality and usability

This needs to be done, either with the current Metacity Gconf bindings or with
the Keybinding tool.

Would it be so hard to just add a few more lines to be edited in
gnome-keybinding-properties

Created attachment 32586 [details] [review]
Draft implementation of biding any key to shell command.

I also missed this functionality, so I implemented it. It's probably far from
perfect -- you need to set the command with gconf and there is limit of 10
shell commands, but it is better then nothing (and in fact all I need).

Maybe the key configuration should be hidden from gnome-keybinding-properties
if you cannot set commands?

Any chances it is going to be included mainstream?

I forgot -- the patch implements this in multimedia-keys part, so unlike in
metacity the special multimedia keys (seen any new keyboard without them?:-)
work OK. And of course it is WM-independant.

*** Bug 162233 has been marked as a duplicate of this bug. ***

*** Bug 170856 has been marked as a duplicate of this bug. ***

Created attachment 50294 [details] [review]
work in progress

I'm working on a better patch. This patch doesn't work well, but I post it to
have  feedback.

That's my first gtk code, and I don't understand gtk_tree_view very well. So I
don't understand why my patch doesn't work :-(

Created attachment 50309 [details] [review]
it works better...

it works far better now. The only problem a have is when I edit a command, text
redered isn't updated (but gconf key is)... close window and open again and the
new command is displayed... don't understand why... but i'm not a tree_view
expert at all !

Any comment ?

version=head and priority=high (because of the patch)

Created attachment 50312 [details] [review]
It works now !!

this is the same patch, but with 3lines more to fix the bug I had.

Any comment ?

Created attachment 50322 [details] [review]
yet another bugfix

Sorry, stupid error in preview patch. Now it is better :)

For the record, there was a discussion on desktop-devel-list around June/July
2005, with pointers to another patch.

http://mail.gnome.org/archives/desktop-devel-list/2005-June/msg00179.html

http://mail.gnome.org/archives/desktop-devel-list/2005-July/msg00007.html

Created attachment 50590 [details] [review]
patch v0.1

this is a rework of patch proposed on mailing list. I keep the same UI, but
move gconf keys to /apps/gnome_settings_daemon/custom_keybindings and adapt
gnome-settings-multimedia-keys to read those keys and run commands.

So it is WM independant.

This patch needs 2 new files to be inclued in
gnome-control-center/capplets/keybindings

Created attachment 50591 [details]
v0.1

Created attachment 50592 [details]
header v0.1

Created attachment 50658 [details] [review]
patch v0.2

bugfix. When adding a new custom binding tree-view shoud select the new row,
but I don't know how to make that.

Created attachment 50659 [details]
custom-binding.c v0.2

Created attachment 50660 [details]
custom-binding.h

Sébastien: what do you think of this?

Xavier: it will have to wait after the 2.12 release since we're UI-frozen...

I've not played with that yet since that's GNOME  material. A screenshoot and
some explanation on how it works would be welcome for people who want to comment
without patching/building the code. I'll put a new comment when playing with the
that.

Created attachment 50687 [details]
screenshot

As you can see, it add a pair of key for each custom binding. The multimedia
daemon has a list of all custom actions and when key is pressed it check if it
is binded to a custom action, if so it exectute the corresponding command.

*** Bug 321913 has been marked as a duplicate of this bug. ***

Created attachment 56732 [details] [review]
Same patch against HEAD

I ported the patch to actual HEAD.

I see a few problems with how it works:

* the dialog looks bad, since there is very little space when opened on 1024x768 for the treeview, due to the new buttons
* not sure if it's me, but the rows in the treeview are invisible, until you pass the mouse over them ??? Then, they become invisible again when the mouse moves to another row.
* it doesn't work for me, might be that I chose bad keybinding

I think any patch that goes in now should have #108689 in mind, since that's an often asked feature within our internal mailing lists.

*** Bug 395754 has been marked as a duplicate of this bug. ***

*** Bug 327110 has been marked as a duplicate of this bug. ***

*** Bug 373926 has been marked as a duplicate of this bug. ***

*** Bug 373939 has been marked as a duplicate of this bug. ***

*** Bug 362578 has been marked as a duplicate of this bug. ***

*** Bug 345885 has been marked as a duplicate of this bug. ***

FYI, and until the patch is cleaned up/merged, here's some docs:
http://live.gnome.org/ControlCenter/CustomKeybindings

Great !

Now, if we are able to bind mouse buttons, there will be no more need of xbindkeys to do that job...

See : http://bugzilla.gnome.org/show_bug.cgi?id=140279

What could be great too would be to be able to bind to command together. For example I have the following lines in my xbindkeyrc file :

"xvkbd -xsendevent -text "\[Alt_L]\[Left]""
   b:8

This lines bind the button 8 of my mouse to <Alt_L> + <Left>... In that case xvkbd do the job, but maybe there is a more elegant way to do that ?

For future reference, I'd like to see a UI for this that doesn't pop up a separate dialog window for adding/editing custom shortcuts but instead uses editable entries in the tree view to achieve this. As Reodrigo mentioned, the proposed UI wastes a lot of space for something that's probably not essential for most. One might also experiment a bit with using right-click context menus instead of buttons for some of the actions.

Cell editors do a really poor job for this, so I went ahead and wrote a patch that uses a popup anyway. One reason why a cell editor is now the right solution here is that you really want to have a separate label/name for the action to display in the list, not just the command, which can look ugly/long/complicated/intimidating. 

With my patch, editing an existing custom shortcut works by clicking on its name, so there is no need for an edit button, at least. Of course, the remove button only lets you remove custom shortcuts, not others.

The patch also supports lockdown of custom shortcuts via the accepted_keys gconf key (there's a patch for that in another bug). When accepted_keys is set, the add button is disabled.

Created attachment 121234 [details]
adding a custom shortcut

Created attachment 121235 [details]
editing a custom shortcut

Created attachment 121236 [details] [review]
patch

+      if (g_file_test ("./gnome-keybinding-properties.glade", G_FILE_TEST_EXISTS))
+        dialog = glade_xml_new ("./gnome-keybinding-properties.glade", NULL, NULL);
+      else  
+        dialog = glade_xml_new (GNOMECC_GLADE_DIR "/gnome-keybinding-properties.glade", NULL, NULL);

Please don't do that.

I also don't particularly like keeping the entire GladeXML around you for the edit window. How about just storing the GtkWindow in a static?

+      if (keys_list[j].cmd_name != NULL)
+        {
+          command = gconf_client_get_string (client, keys_list[j].cmd_name, NULL);
+	}
+      else
+        {
+	  command = NULL;
+	}

Indentation.

+    g_print ("not a custom shortcut\n");

Please remove (or use g_debug if you must).

+  gtk_tree_store_remove (GTK_TREE_STORE (model), iter); if (!gtk_tree_model_iter_has_child (model, &parent))

Missed a NL there.

+static gchar *
+find_free_gconf_key (void)

Please provide for failure here. 1000 *should* be big enough, but better safe than sorry.

+          gtk_tree_view_set_cursor (treeview, path,
+			            gtk_tree_view_get_column (treeview, 0),
+			            FALSE);
+	  update_custom_shortcut (model, &iter);

Indentation.

+	{
+          gtk_widget_grab_focus (GTK_WIDGET (treeview));
+          gtk_tree_view_set_cursor (treeview,
 			    path,
 			    gtk_tree_view_get_column (treeview, 1),
 			    TRUE);
+        }

Same.

+          gtk_tree_view_set_cursor (tree_view, path,
+			            gtk_tree_view_get_column (tree_view, 0),
+			            FALSE);
+	  update_custom_shortcut (model, &iter);

Same.

+  	GladeXML *dialog = data;
+        GtkTreeView *treeview;

Same.

+	else if (response_id == 0)
+          {
+	    add_custom_shortcut (treeview, model);
+	  }
+	else if (response_id == 1)
+          {
+	    selection = gtk_tree_view_get_selection (treeview);
+            if (gtk_tree_selection_get_selected (selection, NULL, &iter))
+	      {
+                remove_custom_shortcut (model, &iter);
+              }
+	  }

Use (predefined?) symbolic ids instead.
Indentation.

+  accepted_keys = gconf_client_get_list (client, 
+                                         GCONF_BINDING_DIR "/accepted_keys", 
+					 GCONF_VALUE_STRING,
+					 NULL);

Indentation.

+      gtk_widget_set_sensitive (WID("add-button"), FALSE);

Please use "WID (...)". Several other occurences.

+	    <widget class="GtkButton" id="okbutton1">
+	      <property name="visible">True</property>
+	      <property name="can_default">True</property>
+	      <property name="can_focus">True</property>
+	      <property name="label">gtk-ok</property>

gtk-apply?

+		  <property name="text" translatable="yes"></property>

Not translatable. Twice.

Still not completely sure about the lock-down stuff. Need to find some time to think it over.

Sorry, I forgot to mention that the patch was not polished or ready for committing. I was just interested in your opinion on the idea in general 

Created attachment 121371 [details] [review]
cleaned up patch

Considering the label you added, yes, the general approach looks valid to me. I suppose you noticed. ;-)

+static gchar *
+find_free_gconf_key (void)
+{
...
+}

This funtion (and the calling one) should take into account that it might not find a free slot. Unlikely, but I'm sure somebody will try it.

+		  <property name="text" translatable="yes"></property>

Not translatable.

+		  <property name="text" translatable="yes"></property>

There was a batch of patches once upon a time to remove these hardcoded settings from glade files. Let's not add them back now.

Created attachment 121389 [details] [review]
next version

I've removed the empty strings and added an error dialog for 'too many custom shortcuts'. I've also improved the formatting of the error dialog somewhat.

+  return glade_xml_new (/*GNOMECC_GLADE_DIR*/ "./gnome-keybinding-properties.glade",
+                        NULL, NULL);

Don't forget to fix this.

grr, yeah. I'll remove that

I've also remove the accepted_keys lockdown stuff, going to file a separate bug about that.

2008-10-26  Matthias Clasen  <mclasen@redhat.com>

        Bug 114796 – binding to arbitrary shell commands

        * gnome-keybinding-properties.c:
        * gnome-keybinding-properties.glade: Add UI for adding and
        removing named custom shortcuts.

*** Bug 562648 has been marked as a duplicate of this bug. ***