Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ignores keypress #30

Closed
pyropeter opened this issue Jan 25, 2011 · 16 comments
Closed

ignores keypress #30

pyropeter opened this issue Jan 25, 2011 · 16 comments
Labels

Comments

@pyropeter
Copy link

My feh 1.11 completely ignores keypresses. The mouse buttons work as usual.
Feh worked perfectly before I updated. (The version I used before still stored the config-files in ~)

@derf
Copy link
Owner

derf commented Jan 25, 2011

Are you using an unusual keyboard layout / X11 keyboard configuration?

What happens when you try to define keybindings in ~/.config/feh/keys, do those work?

@pyropeter
Copy link
Author

keybindings from $XDG_CONFIG_HOME/feh/keys also do not work.

I use CAPS LOCK to switch between german and american layout.

EDIT: I accidentially closed this issue and there seems to be no way (for me) to open it again.

@derf
Copy link
Owner

derf commented Jan 25, 2011

Could you give me the output of xev for a few keypresses that should work with feh, and tell me if xev is reporting the right keys?

This all looks quite weird, so far you're the only one who has reported such a problem.

@pyropeter
Copy link
Author

http://dpaste.org/jsJY/
the output looks fine, it shows the keys I pressed.

@derf
Copy link
Owner

derf commented Jan 25, 2011

Ah... I forgot that X can set more keystates than just Ctrl, Shift and Mod1-Mod5... there will be a commit up shortly, would you mind checking if that fixes the issue for you?

@derf
Copy link
Owner

derf commented Jan 25, 2011

... And in case it doesn't work, try configuring some keybindings with Alt (mod1) like next-img 1-n 1-Right, prev-img 1-p 1-Left etc.
Because it looks like, for whatever reason, X11 always reports that your mod1 key is pressed.

@pyropeter
Copy link
Author

it tried version 1.11-13-g39073b4 (your bugfix), but the problem persists.
also defining a binding for 1-n did not fix it.

@pyropeter
Copy link
Author

it seems like my caps-lock (mod2) is set. (state 0x10)
there is no way to disable it without changing my keyboard configuration. (even pressing caps (layout toggle) or pressing shift-caps (toggles the actual caps-lock functionality) does not unset the bit.)

-   state = kev->state & (ControlMask | Mod1Mask | Mod2Mask | Mod3Mask |
-       Mod4Mask | Mod5Mask);
+   state = kev->state & (ControlMask | Mod1Mask | Mod3Mask |
+       Mod4Mask | Mod5Mask);

fixes the problem, but also disables one to use mod2 for keybindings

@derf
Copy link
Owner

derf commented Jan 26, 2011

Actually, 0x10 is NumLock/Mod2. With that, I was able to reproduce it here :)

I have now reduced the available modifiers to Control, Alt (Mod1) and Super/Windows (Mod4). I think that's still definitely enough.

@pyropeter
Copy link
Author

I see, NumLock makes a lot more sense.
The bug is gone in version 1.11-14-g570db2e, thanks!

@livibetter
Copy link
Contributor

I don't understand why Shift has to be removed. As what I see

ShiftMask -> 0x01
NumLk     -> 0x10
CapLk     -> 0x02

With this way:

state = kev->state & (ControlMask | ShiftMask | Mod1Mask | Mod4Mask);

It shouldn't be having problem with or without either of NumLk or CapLk enabled, and I do have none after I added the Shift key back.

Did I miss something?

@derf
Copy link
Owner

derf commented Jul 31, 2011

Shift is removed because it changes the key code itself.

When you press s, you get XK_s, and when you press S, you get XK_S. So feh does support shift, it just checks it via the keycode and not the modifier mask.

@livibetter
Copy link
Contributor

But feh can't distinguish between Up and Shift+Up.

Right now, after I added it back, I can have both and normal keypress like f and S-F. (The only drawback, you need to adopt the changed keycode, not using S-f, or simply F)

@derf
Copy link
Owner

derf commented Jul 31, 2011

Hm, right. It shouldn't be too hard to allow defining S-Up, while not requiring other Shift keys to be defined with the S- prefix.

But I'm on vacation right now, so although I respond to some bugreports I won't do anything big for the next 9 days. I'll fix it after that .Hope that's acceptable ;)

@livibetter
Copy link
Contributor

How about this?

diff --git a/src/keyevents.c b/src/keyevents.c
index ddfe782..836e794 100644
--- a/src/keyevents.c
+++ b/src/keyevents.c
@@ -55,6 +55,9 @@ static void feh_set_parse_kb_partial(fehkey *key, int index, char *ks) {
                        case 'C':
                                mod = ControlMask;
                                break;
+                       case 'S':
+                               mod = ShiftMask;
+                               break;
                        case '1':
                                mod = Mod1Mask;
                                break;
@@ -69,6 +72,8 @@ static void feh_set_parse_kb_partial(fehkey *key, int index, char *ks) {
        }

        key->keysyms[index] = XStringToKeysym(cur);
+       if (isascii(key->keysyms[index])) 
+               mod &= ~ShiftMask;
        key->keystates[index] = mod;

        if (key->keysyms[index] == NoSymbol)
@@ -346,7 +351,9 @@ void feh_event_handle_keypress(XEvent * ev)

        kev = (XKeyEvent *) ev;
        len = XLookupString(&ev->xkey, (char *) kbuf, sizeof(kbuf), &keysym, NULL);
-       state = kev->state & (ControlMask | Mod1Mask | Mod4Mask);
+       state = kev->state & (ControlMask | ShiftMask | Mod1Mask | Mod4Mask);
+       if (isascii(keysym))
+               state &= ~ShiftMask;

        /* menus are showing, so this is a menu control keypress */
        if (ev->xbutton.window == menu_cover) {

Just drop ShiftMask when the keysym is ASCII?

"F" can still be used and "Up/S-Up" is distinguishable. "S-F" would be just "F" (S is dropped) and "S-f" wouldn't work since S is dropped and keysym would be XK_F, but it's easy to fix this with a conversion, however, who is going to use "S-f"?

(Sorry for interrupting your vacation, it is totally acceptable to deal this after you come back :) )

@derf
Copy link
Owner

derf commented Aug 2, 2011

Looks great, thanks a lot!

Patch commited in 7f2e2a3

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants