Re: [PATCH] Implement custom grab key combination definition

["Daniel P. Berrange" <dan berrange com>]

On Mon, Jun 07, 2010 at 11:03:27AM +0200, Michal Novotny wrote:
> On 06/04/2010 05:43 PM, Daniel P. Berrange wrote:
>> On Fri, Jun 04, 2010 at 11:17:31AM +0200, Michal Novotny wrote:
>>> diff --git a/src/vncdisplay.h b/src/vncdisplay.h
>>> index 1b5b297..1f00f5b 100644
>>> --- a/src/vncdisplay.h
>>> +++ b/src/vncdisplay.h
>>> @@ -94,6 +94,8 @@ void            vnc_display_send_keys_ex(VncDisplay *obj, const guint *keyvals,
>>>   					 int nkeyvals, VncDisplayKeyEvent kind);
>>>
>>>   void		vnc_display_send_pointer(VncDisplay *obj, gint x, gint y, int button_mask);
>>> +void		vnc_display_set_grab_keys(VncDisplay *obj, const gchar *key_combo);
>>> +const gchar*	vnc_display_get_grab_keys(VncDisplay *obj);
>>>      
>> I'm not a huge fan of this as an API signature because we're inventing
>> a compound string format that people now have to parse/build.
>>
>> I'm somewhat inclined to think we should be taking a list of modifiers +
>> list of keysyms, but that's not all that pleasant either.
>>
>> eg, to set  Ctrl+Alt+f as the grab sequence, an app could call
>>
>>    int[] grabmods = { GDK_CONTROL_MASK , GDK_MOD1_MASK, NULL }
>>    int[] grabkeys = { GDK_f, NULL };
>>    vnc_display_set_grab_keys(vnc, grabmods, grabkeys);
>>
>>    
>
> Yeah, this is right Daniel but the definition is not that easy for the  
> end user... Or do you think we would parse the key combination  
> (Ctrl+Alt+f) to make the grabmods and grabkeys from it like stated above?

I think it depends on how you think the UI should work.
There are several options

  1. Have a list of pre-defined sequences and let user chose
     between them on a menu
  2. Present a text field and let them enter a string "Ctrl+Alt+f"
  3. Popup a dialog and simply ask them to type in the desired
     sequence & record it.

Personally I'd go for option 3 because it is flexible and easy
to understand. With this, you'd be getting the mods+keysyms
directly, so the API I suggest would be quite straigthforward.

>> Or perhaps a simple data type
>>
>>     typedef struct {
>>        int modmask;
>>        int nkeysyms;
>>        int *keysyms;
>>     } VncGrabSequence;
>>
>>     VncGrabSequence seq = {
>>         .modmask = GDK_CONTROL_MASK | GDK_MOD1_MASK,
>>         .nkeysyms = 1, .keysyms = { GDK_f }
>>     };
>>     vnc_display_set_grab_keys(vnc, seq);

I'm prefering this style more than the example just above it, because I
think this will be easier to deal with in language bindings & properties

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|