Re: [PATCH] Implement custom grab key combination definition

[Michal Novotny <minovotn redhat com>]

On 06/07/2010 11:16 AM, Daniel P. Berrange wrote:
> 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.
>
>    

Ok Daniel. In fact it's great to record it and make it use option 3 but 
I never did the GTK development before so I didn't know how could I do 
that so that I did use option 2 instead. But I see what you mean, in 
fact I'd need to allocate some of my time to learn it how to do it using 
the third option but I have to agree that this way this would be very 
straight-forward. The second thing here is how should this string be 
saved to some configuration file. Like the combination of guints 
presenting the keys pressed?

> > > 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
>    

I see what you mean Daniel and honestly I find it good to implement it 
this way. Since I'm seeing the nkeysyms here I guess it would be ready 
to implement multiple keys to be defined - like Ctrl+Alt+F and 
Ctrl+Alt+f or make this be a combination like Ctrl+a+b or something like 
that. Otherwise I'm seeing no point of having multiple keysyms defined here.

Michal


--
Michal Novotny<minovotn redhat com>, RHCE
Virtualization Team (xen userspace), Red Hat