stanio | 1 Aug 12:14 2011
Picon

[st] shift-insert patch (more general: key.mask and state)

Hi,

I had this issue with shift-insert not working in st which annoyed me a
lot again while setting up a new box today, so I looked into it. 

`kpress` first invokes `kmap` to handle custom keys which override
default behaviour. In `kmap` this condition 
	if(key[i].k == k && (key[i].mask == 0 || key[i].mask & state))
		return (char*)key[i].s;
is always met when you press a key which is defined in config.h in `key`
with 0 mask, e.g.,
	{ XK_Insert,    0, "\033[2~" },

If you want to use XK_Insert with shift, you never come to the place it
is handled in `kpress`. You can remove it from `key` in config.h. This
works with minor side effects, e.g. not able to enter insert mode in vi
by pressing Insert, which I don't do anyway. Might have some effects for
Emacs users?

However, you can also check state == 0 in addition to key[i].mask == 0
in `kmap` which I guess was the intent of the inner parentheses, in
other words "Do mask and state match". 

cheers,
--

-- 
 stanio_
diff -r e9fd465c5dac st.c
--- a/st.c	Sun Jul 31 11:18:06 2011 +0200
(Continue reading)

anonymous | 1 Aug 13:34 2011

Re: [st] shift-insert patch (more general: key.mask and state)

I think my patch is better.  There is no special case for 0 mask and state
and when we match for Ctrl and Shift it works only when they both are
pressed.  With your patch it will work for Ctrl only or for Shift only.

Maybe it can be adopted for dwm so we would be able to remove this
CLEANMASK macro.
diff -r e64c97268f1a st.c
--- a/st.c	Thu Jun 09 18:25:56 2011 +0200
+++ b/st.c	Mon Aug 01 14:31:32 2011 +0300
 <at>  <at>  -1833,8 +1833,9  <at>  <at> 
 char*
 kmap(KeySym k, unsigned int state) {
 	int i;
+
 	for(i = 0; i < LEN(key); i++)
-		if(key[i].k == k && (key[i].mask == 0 || key[i].mask & state))
+		if(key[i].k == k && (state & key[i].mask) == key[i].mask)
 			return (char*)key[i].s;
 	return NULL;
 }
anonymous | 1 Aug 13:52 2011

Re: [st] shift-insert patch (more general: key.mask and state)

On Mon, Aug 01, 2011 at 03:34:46PM +0400, anonymous wrote:
> Maybe it can be adopted for dwm so we would be able to remove this
> CLEANMASK macro.

Patch is attached.  Maybe I missed something because I don't understand
what numlockmask is for. Maybe my patch breaks something or maybe we
can remove all numlockmask stuff?

Ethan Grammatikidis | 1 Aug 14:50 2011

Re: [st] shift-insert patch (more general: key.mask and state)

On Mon, 1 Aug 2011 15:52:37 +0400
anonymous <p37sitdu <at> lavabit.com> wrote:

> On Mon, Aug 01, 2011 at 03:34:46PM +0400, anonymous wrote:
> > Maybe it can be adopted for dwm so we would be able to remove this
> > CLEANMASK macro.
> 
> Patch is attached.  Maybe I missed something because I don't understand
> what numlockmask is for. Maybe my patch breaks something or maybe we
> can remove all numlockmask stuff?

Did you test your patch both with numlock off and on? Years ago a lot of keybindings wouldn't work if numlock
was in the wrong state for the app. Quite nasty, really.

anonymous | 1 Aug 17:52 2011

Re: [st] shift-insert patch (more general: key.mask and state)

On Mon, Aug 01, 2011 at 01:50:33PM +0100, Ethan Grammatikidis wrote:
> On Mon, 1 Aug 2011 15:52:37 +0400
> anonymous <p37sitdu <at> lavabit.com> wrote:
> 
> > On Mon, Aug 01, 2011 at 03:34:46PM +0400, anonymous wrote:
> > > Maybe it can be adopted for dwm so we would be able to remove this
> > > CLEANMASK macro.
> > 
> > Patch is attached.  Maybe I missed something because I don't understand
> > what numlockmask is for. Maybe my patch breaks something or maybe we
> > can remove all numlockmask stuff?
> 
> Did you test your patch both with numlock off and on? Years ago a lot of keybindings wouldn't work if numlock
was in the wrong state for the app. Quite nasty, really.
> 

It works with numlock off and on.  It doesn't clean keycode and matches
it exactly with mask.  Instead it applies mask (with &) and checks if
all bits that are set in the mask are set in keycode.  If your mask
doesn't include numlock, numlock state can't change anything.

What changed is that when you bind Ctrl+A, Ctrl+Shift+A is matched
too. That is why I added "break" in the loop.  It means you should bind
Mod1+Shift+Return first and Mod1+Return later, but it is how it is done
in default config.h so there are no problems.

We still can't remove updatenumlockmask, because it is used in grabbuttons
and grabkeys.  Maybe it can be removed later.

If my patch doesn't break anything, I think it makes code a little cleaner.
(Continue reading)

Aurélien Aptel | 1 Aug 17:55 2011
Picon

Re: [st] shift-insert patch (more general: key.mask and state)

On Mon, Aug 1, 2011 at 1:34 PM, anonymous <p37sitdu <at> lavabit.com> wrote:
> I think my patch is better.  There is no special case for 0 mask and state
> and when we match for Ctrl and Shift it works only when they both are
> pressed.  With your patch it will work for Ctrl only or for Shift only.

When a key in config.h has mask = 0 it's matched with <any modifier>,
not <no modifier>.
Instead we could:

#define XK_NO_MOD  UINT_MAX
#define XK_ANY_MOD 0

When numlock is on, each keypress has a state of 0x10. I suppose
nobody actually use keybindings involving numlock so kmap() could
clear the numlock mask from state before looking for a match in key[].

Aurélien Aptel | 1 Aug 18:14 2011
Picon

Re: [st] shift-insert patch (more general: key.mask and state)

On Mon, Aug 1, 2011 at 5:55 PM, Aurélien Aptel <aurelien.aptel <at> gmail.com> wrote:
> When numlock is on, each keypress has a state of 0x10. I suppose
> nobody actually use keybindings involving numlock so kmap() could
> clear the numlock mask from state before looking for a match in key[].

And handle the special case when mask = XK_NO_MOD.
Also anonymous forgot to attach his second patch.

anonymous | 1 Aug 18:14 2011

Re: [st] shift-insert patch (more general: key.mask and state)

On Mon, Aug 01, 2011 at 05:55:25PM +0200, Aurélien Aptel wrote:
> On Mon, Aug 1, 2011 at 1:34 PM, anonymous <p37sitdu <at> lavabit.com> wrote:
> > I think my patch is better.  There is no special case for 0 mask and state
> > and when we match for Ctrl and Shift it works only when they both are
> > pressed.  With your patch it will work for Ctrl only or for Shift only.
> 
> When a key in config.h has mask = 0 it's matched with <any modifier>,
> not <no modifier>.

With my patch it is matched with any modifier too, because (key & 0) == 0.

Aurélien Aptel | 1 Aug 18:32 2011
Picon

Re: [st] shift-insert patch (more general: key.mask and state)

On Mon, Aug 1, 2011 at 6:14 PM, anonymous <p37sitdu <at> lavabit.com> wrote:
> With my patch it is matched with any modifier too, because (key & 0) == 0.

Yes but it doesn't fix the problem of redefining what XK_Insert sends
while keeping shift + insert to paste.
I've used it in my patch attached to match XK_ANY_MOD. I'd like to
have some feedback/testing before applying it.
Attachment (shift-insert-fix.diff): application/octet-stream, 4178 bytes
anonymous | 1 Aug 18:35 2011

Re: [st] shift-insert patch (more general: key.mask and state)

On Mon, Aug 01, 2011 at 03:52:37PM +0400, anonymous wrote:
> On Mon, Aug 01, 2011 at 03:34:46PM +0400, anonymous wrote:
> Patch is attached. 

Forgot to attach.

To make it work, you should reorder TAGKEYS, so masks with more modifiers are matched first.

#define TAGKEYS(KEY,TAG) \
	{ MODKEY|ControlMask|ShiftMask, KEY, toggletag,  {.ui = 1 << TAG} }, \
	{ MODKEY|ControlMask,           KEY, toggleview, {.ui = 1 << TAG} }, \
	{ MODKEY|ShiftMask,             KEY, tag,        {.ui = 1 << TAG} }, \
	{ MODKEY,    

diff -r b46ae56abe65 dwm.c
--- a/dwm.c	Wed Jul 27 19:59:10 2011 +0200
+++ b/dwm.c	Mon Aug 01 14:48:35 2011 +0300
 <at>  <at>  -42,7 +42,6  <at>  <at> 

 /* macros */
 #define BUTTONMASK              (ButtonPressMask|ButtonReleaseMask)
-#define CLEANMASK(mask)         (mask & ~(numlockmask|LockMask) & (ShiftMask|ControlMask|Mod1Mask|Mod2Mask|Mod3Mask|Mod4Mask|Mod5Mask))
 #define INRECT(X,Y,RX,RY,RW,RH) ((X) >= (RX) && (X) < (RX) + (RW) && (Y) >= (RY) && (Y) < (RY) + (RH))
 #define ISVISIBLE(C)            ((C->tags & C->mon->tagset[C->mon->seltags]))
 #define LENGTH(X)               (sizeof X / sizeof X[0])
 <at>  <at>  -292,7 +291,6  <at>  <at> 
 	XClassHint ch = { 0 };

(Continue reading)


Gmane