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

[Keymap] New custom 'super alt' keymap for the Drop ALT #9385

Merged
merged 5 commits into from
Jun 22, 2020

Conversation

ewersp
Copy link
Contributor

@ewersp ewersp commented Jun 12, 2020

Description

Greetings, this is my first PR to qmk_firmware. I've added my own custom keymap for the Drop ALT keyboard, and tried to make sure it's thoroughly documented for others to use as reference.

I've been using it as my daily driver and compiling (on latest Windows 10) all week, so I can confirm that it works to the best of my knowledge. Please let me know if I missed anything!

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

  • N/A

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@sigprof
Copy link
Contributor

sigprof commented Jun 12, 2020

An interesting solution for the left Alt key usage.

However, does the “Left Alt + Backspace → Delete” feature actually work? Looks like the Backspace key is just mapped to KC_DEL on the SUPR layer, therefore both Backspace and Del would be handled using the same code which emits LAlt+Del. To fix this problem, you may define a custom keycode which would just invoke register_code(KC_DEL) and unregister_code(KC_DEL), and map the Backspace key on the SUPR layer to that keycode, so that the Alt modifier would not be registered when pressing that key.

Also I wonder how I could press LAlt+F4 with that keymap while keeping the “super-Alt mode” active. Are you just using the right Alt for this combination (which may not work for keyboard layouts using that key as AltGr to get additional characters)?

There is also a potential issue of pressing some unusual key combinations like Alt+Home and Ctrl+Alt+End (these are used by the Microsoft Remote Desktop Client to pass Ctrl+Esc and Ctrl+Alt+Del to the remote desktop), but this case could be handled by switching the super-Alt mode off temporarily.

@ewersp
Copy link
Contributor Author

ewersp commented Jun 12, 2020

@sigprof Thanks for taking the time to read the code and provide a detailed response! Answers inline below:

Looks like the Backspace key is just mapped to KC_DEL on the SUPR layer

I think I'm just going to remove this, since the Delete key is literally right next to the Backspace key anyway. Initially I added this because on macOS there's no dedicated Delete key, so my muscle memory was used to 'hold a modifier key and press Backspace'.

Also I wonder how I could press LAlt+F4 with

This is a good call out, and I should make it more clear in the readme. Right now it's either LeftAlt+RightAlt+4 (a little odd) or RightAlt+Fn+4 (default). Initially I thought about mapping LeftAlt+4 directly to Alt+F4 but that seemed inconsistent and a little risky. Still would be nice if there was a way to Alt+F4 without needing two hands.

There is also a potential issue of pressing some unusual key combinations like Alt+Home and Ctrl+Alt+End

I couldn't actually think of many LeftAlt commands that this would potentially make more inconvenient, but thanks for letting me know about Remote Desktop (and I'm sure there are more). Like you mentioned, the disable toggle is probably the best option for this (same goes for any AltGr issues, which I also wasn't familiar with).

An unrelated question I did have: initially I was using LT(SUPR, KC_LALT) instead of MO(SUPR) so you could still tap the LeftAlt key if needed, but I noticed that introduced a slight (~50ms?) lag when holding LeftAlt (for example with alt+tab or alt+arrows) so I opted to remove it. I looked at a few of the QMK APIs related to timings/delays, but I don't think any actually affected the inherent delay when using LT (though I could be wrong).

Thanks again.

@sigprof
Copy link
Contributor

sigprof commented Jun 12, 2020

Initially I added this because on macOS there's no dedicated Delete key, so my muscle memory was used to 'hold a modifier key and press Backspace'.

I think this is a valid reason to have a seemingly duplicate mapping. I also have many keys in my Fn layer which seem to be duplicates (including Fn+Backspace → Delete), just because I want to have the same layout for it on all >=60% keyboard sizes.

Still would be nice if there was a way to Alt+F4 without needing two hands.

The usual solution for it is to convert your Caps Lock key into a layer switch (e.g., LT(FUNC, KC_CAPS), so that you can access your FUNC layer from both sides; then, if you have some free space in that layer, you may even add a key which specifically emit Alt+F4, so that you won't need to hold three keys at once. However, see below for additional notes about LT.

BTW, now I checked the behavior of the right Alt key on Windows, and it actually depends on the currently selected keyboard layout. When I have the plain US English layout active (which does not use AltGr), the RAlt+F4 combination closes the window; however, when I switch to the Russian layout (which uses AltGr since some update), RAlt+F4 stops working. Most layouts for various European languages also use AltGr, therefore you must use the left Alt key for shortcuts, and your solution involving RAlt won't work for them.

It could be possible to make the Fn+LAlt press a special case which makes LAlt immediately switch to the LAlt modifier state. This could be easy to do for the case when Fn is pressed before LAlt — just remap LAlt to KC_LALT on your FUNC layer. To handle the case when LAlt is pressed before Fn, you could add MO(FUNC) as another case which causes the LAlt modifier to be registered when LAlt is pressed.

An unrelated question I did have: initially I was using LT(SUPR, KC_LALT) instead of MO(SUPR) so you could still tap the LeftAlt key if needed, but I noticed that introduced a slight (~50ms?) lag when holding LeftAlt (for example with alt+tab or alt+arrows) so I opted to remove it.

If this was not the Alt modifier key, you could even have a way to hold that modifier — a quick tap+hold action on the LT key results in tap+hold of the key specified as the tap action. Unfortunately, that Alt tap messes up things in this case.

And the lag that you observed really exists — it is actually TAPPING_TERM (200 ms by default) since the moment the LT key is pressed. By default the code which decides whether to choose the tap or hold action for a dual-use key such as LT or MT waits until either the dual-use key is released (in which case the tap action is selected), or the tapping term expires (then the hold action is selected), and all other key actions performed during that time are queued and not processed immediately. Only after the tap/hold decision has been finalized, the queued actions are processed.

Enabling the PERMISSIVE_HOLD option makes the lag slightly less noticeable, because it introduces another condition which finalizes the tap/hold decision — if another key is pressed and released after the dual-use key was pressed, the hold action for the dual-use key is selected immediately, even if this happened before the tapping term has expired. In your Alt+Tab case this will mean that your computer will see the first Alt+Tab action when you press and release Tab the first time, which still introduces the delay for the time the Tab key is held down, but is still better than delaying for the whole 200 ms since the press of Alt.

To eliminate the delay completely (well, almost completely — there will still be some delay while QMK emits the Alt press event before Tab, although you can use #define USB_POLLING_INTERVAL_MS 1 to make this delay as small as possible), at the moment you need the code from #8971 (comment) (I really need to overcome my laziness and write some real documentation for that feature, so that I can make a proper PR with it). The HOLD_ON_OTHER_KEY_PRESS option added by that code allows the tap/hold detector to finalize its decision immediately when another key is pressed while the dual-use key is held down. For an LT key this will mean that there will be no observable delay (because the layer switches are not visible to the host); for an MT key only the modifier press event will be delayed (it will be sent just before the next keypress event).

Note that HOLD_ON_OTHER_KEY_PRESS is suitable for keys which are mostly used for their hold actions; if you want to make a regular alphanumeric key into MT or LT (like some people do to get modifier keys on the home row), you probably should not enable HOLD_ON_OTHER_KEY_PRESS for that key, otherwise you may get lots of false hold decisions from normal typing (when you do not release a key fast enough, and the next key is pressed before the previous one has been released). PERMISSIVE_HOLD might be mostly safe in that case, because it is unlikely that you would still hold the previous letter key after you pressed and released the next one.

@ewersp
Copy link
Contributor Author

ewersp commented Jun 13, 2020

Thanks again for the detailed feedback, I'm humbled by the donation of your time! Responses inline below:

However, does the “Left Alt + Backspace → Delete” feature actually work?

Good catch, I didn't notice that technically this wasn't correct, even though the behavior is likely to still work in most cases, it's better to have it sending the right commands. I added an ALT_DEL keycode to sure this.

convert your Caps Lock key into a layer switch

Ah, why didn't I think of this? The caps lock key is literally useless when held down, so this makes perfect sense and opens the door to a lot of possibilities. I'll consider this for my next iteration.

It could be possible to make the Fn+LAlt press a special case

I made these changes and they feel great. Now Fn + LAlt + 4 works properly as alt+f4 so any AltGr users who stumble upon this keymap will be pleased.

Enabling the PERMISSIVE_HOLD option...

Thanks for the detailed overview. I had initially tried permissive hold, but did not update USB_POLLING_INTERVAL_MS and gave up since I felt a bit overwhelmed. But it sounds like your other changes in that branch are exactly what I need. For now I'll patiently await a PR, but that looks very promising to have the best of all worlds.

And lastly, I nearly had a heart attack when I tested the input sequence: Alt (held) -> X (held) -> Alt (release) -> X (release) and saw this output:

KEY-DOWN - QMK: KC_LALT Event key: Alt         Code: AltLeft       KeyCode: 18
KEY-DOWN - QMK: KC_X    Event key: x           Code: KeyX          KeyCode: 88
KEY-UP   - QMK: KC_LALT Event key: Alt         Code: AltLeft       KeyCode: 18 in 886.365ms
KEY-DOWN - QMK: KC_X    Event key: x           Code: KeyX          KeyCode: 88
KEY-UP   - QMK: KC_X    Event key: x           Code: KeyX          KeyCode: 88 in 444.875ms

I was not expecting line 4 where X was pressed again, but after testing with my unmodified RAlt key and noticing the same behavior, I had a huge sigh of relief. I guess I learned that the alt key behaves in mysterious ways.

@sigprof
Copy link
Contributor

sigprof commented Jun 13, 2020

I was not expecting line 4 where X was pressed again

This is apparently an artifact of the Chromium browser engine, and not even specific to the Alt modifier. I tried https://config.qmk.fm/#/test on both Chromium and Firefox on Linux (so it's also not an OS-specific behavior), and I do not see those extra KEY-DOWN events on Firefox, but Chromium emits an extra KEY-DOWN event for the last pressed normal key whenever the state of modifier keys has changed (but with some delay, and you press or release multiple modifier keys during that delay, only one extra event is generated). The “Event key” field in that extra event is updated according to the new modifier state (e.g., it contains a capital letter if you pressed Shift, or an alternate character if you pressed AltGr); maybe this was the reason why Chromium developers added those events. Also, if you press multiple non-modifier keys at the same time, those extra events are generated only for the single non-modifier key which was pressed last, and if you release that key while keeping some other non-modifier keys pressed, the extra events no longer appear.

@ewersp ewersp changed the title New custom 'super alt' keymap for the Drop ALT [Keymap] New custom 'super alt' keymap for the Drop ALT Jun 14, 2020
keyboards/massdrop/alt/keymaps/ewersp/keymap.c Outdated Show resolved Hide resolved
keyboards/massdrop/alt/keymaps/ewersp/keymap.c Outdated Show resolved Hide resolved
keyboards/massdrop/alt/keymaps/ewersp/keymap.c Outdated Show resolved Hide resolved
keyboards/massdrop/alt/keymaps/ewersp/keymap.c Outdated Show resolved Hide resolved
keyboards/massdrop/alt/keymaps/ewersp/keymap.c Outdated Show resolved Hide resolved
keyboards/massdrop/alt/keymaps/ewersp/keymap.c Outdated Show resolved Hide resolved
keyboards/massdrop/alt/keymaps/ewersp/keymap.c Outdated Show resolved Hide resolved
keyboards/massdrop/alt/keymaps/ewersp/keymap.c Outdated Show resolved Hide resolved
@fauxpark fauxpark requested a review from a team June 14, 2020 17:15
@ewersp ewersp force-pushed the master branch 3 times, most recently from e99e359 to 2cd4cb6 Compare June 14, 2020 21:35
@drashna drashna requested a review from a team June 15, 2020 10:01
@drashna drashna requested a review from a team June 18, 2020 04:58
@ewersp ewersp requested review from fauxpark and removed request for a team June 18, 2020 07:07
@ewersp
Copy link
Contributor Author

ewersp commented Jun 22, 2020

Not sure what's up with the unrelated linter failures?

☒ keyboards/1upkeyboards/1up60rgb/1up60rgb.h: LAYOUT_60_ansi: Nested layout macro detected. Matrix data not available!
☒ keyboards/1upkeyboards/1up60rgb/1up60rgb.h: LAYOUT_60_iso: Nested layout macro detected. Matrix data not available!
☒ keyboards/1upkeyboards/1up60rgb/1up60rgb.h: LAYOUT_60_ansi_split_bs_rshift: Nested layout macro detected. Matrix data not available!
☒ keyboards/1upkeyboards/1up60rgb/1up60rgb.h: LAYOUT_60_hhkb: Nested layout macro detected. Matrix data not available!

@tzarc tzarc merged commit 0b09189 into qmk:master Jun 22, 2020
jakobaa pushed a commit to jakobaa/qmk_firmware that referenced this pull request Jul 7, 2020
* New custom 'super alt' keymap for the Drop ALT

* Improvements to 'super alt' keymap based on PR feedback

* Fix flickering LED caps lock bug

* Code cleanup from PR feedback

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

Successfully merging this pull request may close these issues.

5 participants