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

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

Closed
1 of 6 tasks
windhausen opened this issue Jan 15, 2023 · 11 comments
Closed
1 of 6 tasks

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

windhausen opened this issue Jan 15, 2023 · 11 comments

Comments

@windhausen
Copy link

windhausen commented Jan 15, 2023

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?

  • Achordion (from this repo)
  • Auto Shift
  • Combos
  • Key Overrides
  • Mod-tap or Layer-tap keys
  • Other custom userspace code: select-word-macro
word.select.mov

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

@windhausen
Copy link
Author

windhausen commented Jan 15, 2023

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 added a commit that referenced this issue Jan 29, 2023
Motivated by #26,
this commit makes a few tweaks to avoid what look like bugs in certain
edge case situations. Thanks to @windhausen for reporting this:

* On initial press, always clear the non-Shift mods.
* Change uses of `register_code()` on mod keys to `register_mods()`.
* Change uses of `SEND_STRING` to `tap_code()` calls, since the former
  is not supported on non-Latin layouts.
* Mac, don't use the Ctrl+A / Crtl+E , since KC_A and KC_E might be
  remapped to other meanings if the OS is set to a non-US QWERTY layout.
  Instead use GUI+Left / GUI+Right.
* Streamline to eliminate sending a unnecessary keyboard reports around
  setting and clearing mods.
@getreuer
Copy link
Owner

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 b9b7d28 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
Copy link

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

freeform99 commented Feb 8, 2023

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
Copy link
Author

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 added a commit that referenced this issue Feb 11, 2023
A second commit toward solving the Select Word bug on Mac reported in
#26
Thank you to @windhausen and @freeform99 for their help investigating.

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

* Debug logging of the macro's state, like "Select word: SELECTED". This
  logging is only compiled in when `CONSOLE_ENABLE = yes` is set in
  rules.mk. To see this logging, follow https://docs.qmk.fm/#/faq_debug.
@getreuer
Copy link
Owner

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 d8afec9 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
Copy link

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
Copy link
Owner

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

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 added a commit that referenced this issue Feb 21, 2023
Mitigation for: #26

This commit adds an idle timeout option for Select Word, so that its
state is cleared after being idle for some number of milliseconds. This
mitigates the bug linked above, improving Select Word's behavior when
used together with a mouse.

To enable idle timeout, define `SELECT_WORD_TIMEOUT` in config.h, e.g.:

```
// When idle, turn off Select Word after 2 seconds.
```

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

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

Now that we have gotten to the bottom it, this commit removes the debug
logging and `wait_ms()` calls inserted previously by
d8afec9

Thanks again to @windhausen and @freeform99 for help on this bug.
@getreuer
Copy link
Owner

I just pushed 6ca5702 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
Copy link
Author

Thank you @getreuer for your efforts :)

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

No branches or pull requests

3 participants