On Wed, Aug 3, 2016 at 10:52 PM, Peter Hutterer
<peter.hutte...@who-t.net> wrote:
> On Wed, Aug 03, 2016 at 10:13:48PM -0700, Ping Cheng wrote:
>> xsetwacom accepts keys in X11 KeySyms format. To make the interface
>> user friendly, we mapped a set of modifiers to aliases that can be
>> recognized by most users. But, there are a few less recognizable or
>> confusing keys/modifiers left. This patch tells users where to find
>> the X11 KeySyms code they are looking for.
>>
>> Return key is added as a bonus (by-product ;).
>
> No, please don't add this in the same patch. it's completely unrelated and
> for stable branch cherry-picking, etc. hiding these things in the same patch
> is a bad idea.
>
> Golden rule: when you need to write "Also..." in a commit message you
> probably need two patches :)
>
> Same goes for typo fixes (which IMO you can just push directly, they're
> usually safe anyway).
>
>> Signed-off-by: Ping Cheng <pi...@wacom.com>
>> Tested-by: Kelly Price <strredw...@gmail.com>
>> ---
>>  man/xsetwacom.man | 13 +++++++------
>>  tools/xsetwacom.c |  4 ++++
>>  2 files changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/man/xsetwacom.man b/man/xsetwacom.man
>> index 1930d67..8276e32 100644
>> --- a/man/xsetwacom.man
>> +++ b/man/xsetwacom.man
>> @@ -109,17 +109,18 @@ button-number will produce a press of X11 button 3 
>> (i.e. right click).
>>  Action mappings allow button presses to perform many events. They take the 
>> form
>>  of a string of keywords and arguments.
>>
>> -The "key" keyword is following by a list of key names. These can optionally
>> +The "key" keyword is followed by a list of key names. These can optionally
>>  be preceded by "+" for press and "-" for release. If +/- is not given,
>>  press-and-release is assumed, except for modifier keys which are left 
>> pressed.
>> -Key names can be X11 KeySyms or some aliases such as 'shift' or 'f1' (the
>> +Key names can be X11 KeySyms code or some aliases such as 'shift' or 'f1' 
>> (the
>
> don't use keysyms code, that's confusing because key code is a specific term
> in X.
> "Key names can be an X11 KeySym or ...."
>
>>  full list can be seen with the
>>  .B list modifiers
>> -command).
>> +command). To assign a key that is not in the modifiers list, look for its 
>> KeySyms
>> +code in keysymdef.h under /usr/include/X11 on your system.
>
> hmm, that makes me wonder why we have special treatment for modifiers. IIRC
> they're just special so we can use "shift" instead of "Shift_L". but we
> should parse the actual keysym as well. maybe a future patch.
> either way:

Well, modifier used to mean the key won't do anything by itself until
another(other)
key(s) is pressed. So, modifiers would always have two separate
states: pressed or released.

"Key +shift a -shift" is different from "Key shift a shift"

You made those individual states available to all keys although

"key +a  +shift b -shift- a" is the same as "key a +shift b -shift"

 With that change, all keys could be treated the same way by xsetwacom.

> "To assign a key that is not in the modifiers list use the KeySym in
> /usr/include/X11/keysymdef.h with the XK_ prefix removed.
> For exampmle, XK_BackSpace should be specified as "BackSpace".
>
>> -For example, "key +a +shift b -shift -a" converts the button into a series 
>> of
>> -keystrokes, in this example "press a, press shift, press and release b,
>> -release shift, release a".
>> +For example, "key +a +shift b -shift -a 0xff0d" converts the button into a
>> +series of keystrokes, in this example "press a, press shift, press and 
>> release b,
>> +release shift, release a, then enter".
>
> whoah, does this actually work? I didn't know that we parsed hex codes
> correctly.

Yeah, it was there at the beginning. Since it was too long ago, we
just forgot what we've done...

>>  The "button" keyword is similar except that its arguments are X11 button
>>  numbers.
>> diff --git a/tools/xsetwacom.c b/tools/xsetwacom.c
>> index 2f75d78..aa5f7a1 100644
>> --- a/tools/xsetwacom.c
>> +++ b/tools/xsetwacom.c
>> @@ -605,6 +605,8 @@ static struct modifier specialkeys[] = {
>>
>>       {"backspace", "BackSpace"}, {"Backspace", "BackSpace"},
>>
>> +     {"return", "Return"},
>> +
>
> as said above, this needs to be a separate patch (feel free to push, no need
> for re-review here if it's just that one line anyway). But do make a note
> in the commmit message about the "Enter" vs "Return" controversy :)

Done and pushed.

>>       {"tab", "Tab"},
>>
>>       {"PgUp", "Prior"}, {"PgDn", "Next"},
>> @@ -937,6 +939,8 @@ static void list_mod(Display *dpy)
>>       m = specialkeys;
>>       while(m->name)
>>               printf("        %s\n", m++->name);
>> +
>> +     printf("If you don't see your special key listed above, please use its 
>> keysyms code or refer to man page for details.\n");
>
> "Keys not listed above can be specified via their KeySym. See the man page
> for details."

It will be in the updated patch (v2) and posted here for review.

Thank you for your comments.

Ping

------------------------------------------------------------------------------
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to