getreuer / qmk-keymap

My keymap & reusable QMK gems
Apache License 2.0
288 stars 41 forks source link

[Bug] Word Select does not select backwards on Mac and Windows #26

Closed windhausen closed 1 year ago

windhausen commented 1 year ago

Word Select does not select backwards on Mac and Windows

After following the instructions, compiling and flashing, the marco button only selects foreward if you start in the middle of the word. Same with sentence-select.

Information

Do the keys involved use any of the following features?

https://user-images.githubusercontent.com/57465016/212533786-9bdb71da-1a1c-4ff4-808d-b7ec205e2ef9.mov

I have very little experience with coding, I apologise for the lack of clarity. Any help would be appreciated.

windhausen commented 1 year ago

keymap.c implementation and added folder

#include "features/select_word.h" if (!process_select_word(keycode, record, SELWORD)) { return false; }

Bildschirm­foto 2023-01-15 um 12 21 12
getreuer commented 1 year ago

Thanks for reporting this! I appreciate the clear video demonstrating the problem.

I'm not sure what the cause is, unfortunately. But looking at the Select Word code, I see a few things that might go wrong in certain edge cases, and some things in any case that can be improved.

I just pushed https://github.com/getreuer/qmk-keymap/commit/b9b7d2810b1fa788d5baba28d333306e6886fd28 with these revisions. I don't know that it will fix it in your case, but it is worth a shot! Please pull or re-download the repo to get the latest. Thanks again for raising this issue.

freeform99 commented 1 year ago

I observe the same behavior, even with the patch applied

I am a total newbie at this and not a programmer either, but does the macro actually send ctrl + right when it is first invoked?

This

      if (state == STATE_NONE) {
        send_keyboard_report();
        tap_code(KC_RGHT);
        tap_code(KC_LEFT);
      }

like it sends just a right and a left and not ctrl + right followed by ctrl + left, to go to the beginning of the word.

Thank you for sharing all of this!

freeform99 commented 1 year ago

Just a follow-up.

(Please disregard my prior comment; I see now that the ctrl key is pressed before that if statement.)

It reliably works as intended when I invoke the macro for the first time after flashing or booting, but only on the very first invocation. So I assume that somehow the state is not reset to STATE_NONE at the end of the process.

I should also say that I am on a Moonlander with zsa's forked firmware, so that could play a role as well.

edit: It is not related to the zsa firmware - I just tested it with vanilla qmk and I see the same: it works after booting but then no longer selects the whole word

windhausen commented 1 year ago

Thank you @getreuer for addressing the issue.

I can confirm what @freeform99 is saying. It works only on the very first instance after flashing.

(I use a OLKB Preonic v3 with Vial-QMK)

getreuer commented 1 year ago

Thank you @windhausen and @freeform99 for your help in tracking this down!

Yes, your reading of the code is correct: when the macro is pressed in an unselected state (STATE_NONE), it begins by holding the Ctrl key (or Option on Mac), tapping Right, then tapping Left. I agree with your suspicion that the state isn't getting reset. Another hypothesis: maybe the back-to-back taps on Right and Left is too fast, and the Right tap is getting lost. Unfortunately, I don't have a Mac to test this on myself, so I'm relying on you.

I just pushed https://github.com/getreuer/qmk-keymap/commit/d8afec9fbc3909f309ac10898d01571b23b04b0b with additions in both of these directions:

Delays. The commit adds wait_ms(TAP_CODE_DELAY) between taps, particularly between the Alt+Right and Alt+Left in word mode, to avoid losing the Alt+Right. Maybe this fixes it?

Debug logging. Second, debug logging of the macro's state is added. This logging isn't typed, but sent separately to the console described in Debugging FAQ. The state is logged every time it changes. It looks like this:

Select word: NONE
Select word: WORD
Select word: SELECTED
Select word: WORD
Select word: SELECTED
Select word: NONE
Select word: WORD
Select word: SELECTED
Select word: WORD
Select word: SELECTED
Select word: WORD
Select word: SELECTED
Select word: NONE
Select word: FIRST_LINE
Select word: SELECTED
Select word: NONE

Here is how to interpret it:

State Description
NONE No selection.
SELECTED Macro released with something selected.
WORD Macro held with word(s) selected.
FIRST_LINE Macro held with one line selected.
LINE Macro held with multiple lines selected.

How to enable debug logging: Getting the debug logging enabled is a chore, unfortunately, but it's the best tool for this kind of problem. To enable it, please add CONSOLE_ENABLE = yes in rules.mk, a DB_TOGG key in your keymap to turn it on, and on the computer use a program like hid_listen to view the log messages. See Debugging FAQ for explanation and other options.

I hope that you can test it and let me know how it goes. Thanks again for your help on this bug!

freeform99 commented 1 year ago

I applied the latest changes and the behaviour is unfortunately still the same :(

Below is the debug output of four keypresses of the SELWORD key and a corresponding gif. The first keypress is the very first keypress after plugging in the keyboard. (Also, I am on Windows/Linux.) It never shows NONE.

However, I noticed that when I use they line selection feature, I do get Select word: NONE among the debug messages. (Not shown below.) So something is different between the two.

selword

HID console connected: ZSA Technology Labs Moonlander Mark I (3297:1969:0001)
> 
> r/c 01234567
> 00: 00000000
> 01: 00000000
> 02: 00000000
> 03: 00000000
> 04: 00000000
> 05: 00000000
> 06: 00000000
> 07: 00000000
> 08: 00000000
> 09: 00000000
> 0A: 00001000
> 0B: 00000000
> Select word: WORD
> 
> r/c 01234567
> 00: 00000000
> 01: 00000000
> 02: 00000000
> 03: 00000000
> 04: 00000000
> 05: 00000000
> 06: 00000000
> 07: 00000000
> 08: 00000000
> 09: 00000000
> 0A: 00000000
> 0B: 00000000
> Select word: SELECTED
> 
> r/c 01234567
> 00: 00000000
> 01: 00000000
> 02: 00000000
> 03: 00000000
> 04: 00000000
> 05: 00000000
> 06: 00000000
> 07: 00000000
> 08: 00000000
> 09: 00000000
> 0A: 00001000
> 0B: 00000000
> Select word: WORD
> 
> r/c 01234567
> 00: 00000000
> 01: 00000000
> 02: 00000000
> 03: 00000000
> 04: 00000000
> 05: 00000000
> 06: 00000000
> 07: 00000000
> 08: 00000000
> 09: 00000000
> 0A: 00000000
> 0B: 00000000
> Select word: SELECTED
> 
> r/c 01234567
> 00: 00000000
> 01: 00000000
> 02: 00000000
> 03: 00000000
> 04: 00000000
> 05: 00000000
> 06: 00000000
> 07: 00000000
> 08: 00000000
> 09: 00000000
> 0A: 00001000
> 0B: 00000000
> Select word: WORD
> 
> r/c 01234567
> 00: 00000000
> 01: 00000000
> 02: 00000000
> 03: 00000000
> 04: 00000000
> 05: 00000000
> 06: 00000000
> 07: 00000000
> 08: 00000000
> 09: 00000000
> 0A: 00000000
> 0B: 00000000
> Select word: SELECTED
> 
> r/c 01234567
> 00: 00000000
> 01: 00000000
> 02: 00000000
> 03: 00000000
> 04: 00000000
> 05: 00000000
> 06: 00000000
> 07: 00000000
> 08: 00000000
> 09: 00000000
> 0A: 00001000
> 0B: 00000000
> Select word: WORD
> 
> r/c 01234567
> 00: 00000000
> 01: 00000000
> 02: 00000000
> 03: 00000000
> 04: 00000000
> 05: 00000000
> 06: 00000000
> 07: 00000000
> 08: 00000000
> 09: 00000000
> 0A: 00000000
> 0B: 00000000
> Select word: SELECTED

Hope this helps!

getreuer commented 1 year ago

Wow, thank you for the quick reply and detailed log + gif. Watching the gif, I realize there is a basic limitation that I haven't documented: unfortunately, the macro is unaware of mouse clicks. The macro clears to its "NONE" state when another key—anything besides Shift or the macro itself—is pressed, however, the macro is unaware of clicks on a mouse and anything else external to the keyboard.

For example, input involving the mouse does not clear the state, the state stays in SELECTED:

User input State
(Boot keyboard) NONE
Press SELWORD WORD
Release SELWORD SELECTED
Press SELWORD WORD
Release SELWORD SELECTED
Click mouse SELECTED

But the following keyboard inputs should work as desired, with tapping an arrow key clearing the state to NONE:

User input State
Press Left NONE
Release Left NONE
Press SELWORD WORD
Release SELWORD SELECTED
Press SELWORD WORD
Release SELWORD SELECTED
Press Left NONE
Release Left NONE

Is this consistent with the behavior that you are seeing?

I ran into this limitation also with Caps Word: it would be nice to turn off Caps Word when the mouse is clicked, but there is no way for the keyboard to know this occurred (not without host-side software). A mitigation that I did for Caps Word is an "idle timeout": if the keyboard is idle for X milliseconds, then turn off Caps Word, as the mouse might be in use. I think it would be useful to add a similar idle timeout option for Select Word.

freeform99 commented 1 year ago

I see.

Yes that explains things. If I navigate around with the mouse, press and release shift or ctrl and then hit SELWORD it reliably works as expected.

The idle timeout sounds like a good option. If nothing happened on the keyboard for a certain time the user might have navigated using the mouse. The timeout can be short - the behaviour would be consistent: select words as expected and if I don´t use the keyboard for one second or so then another SELWORD press would no longer add to the selection. IMHO that´s better usability than having to press a (silent) key after every mouse click if I want to use SELWORD from within a word.

getreuer commented 1 year ago

I just pushed https://github.com/getreuer/qmk-keymap/commit/6ca5702fd4c2d0193712a2aa5d3a4c09c26f0aa2 to add an "idle timeout" option to Select Word. This does not fix this bug completely, but mitigates it by clearing the Select Word's state after a period of activity. This improves behavior when a mouse is involved.

To enable idle timeout, please pull or re-download this repo to get the latest. Then define SELECT_WORD_TIMEOUT in units of milliseconds in config.h like:

// When idle, turn off Select Word after 2 seconds.
#define SELECT_WORD_TIMEOUT 2000

and call sentence_case_task() from matrix_scan_user() in keymap.c:

void matrix_scan_user(void) {
  select_word_task();
  // Other tasks...
}

@windhausen and @freeform99 thank you both for your help on this bug!

windhausen commented 1 year ago

Thank you @getreuer for your efforts :)