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

Ignore layer tap interrupts #10409

Closed
wants to merge 1 commit into from

Conversation

buztard
Copy link
Contributor

@buztard buztard commented Sep 23, 2020

Description

This PR adds IGNORE_LAYER_TAP_INTERRUPT for layer taps. This is especially
useful when combined with IGNORE_MOD_TAP_INTERRUPT_PER_KEY.
For example, when you want a layer tap like LT(_FN, KC_F) to trigger f instead of
a layer change, but LT(_RAISE, KC_BSPC) to change the layer when interrupted.

The changes requires IGNORE_LAYER_TAP_INTERRUPT to be defined.
In case #define IGNORE_MOD_TAP_INTERRUPT_PER_KEY is set as well, you'll be
able to reuse the get_ignore_mod_tap_interrupt() function for
layer taps like this:

bool get_ignore_mod_tap_interrupt(uint16_t keycode, keyrecord_t *record) {
    switch (keycode) {
        case SFT_T(KC_SPC):
            return true;
        case LT(_FN, KC_F):
            // interrupt causes 'f'
            return true;
        case LT(_RAISE, KC_BSPC):
            // interrupt causes layer switch
            return false;
        default:
            return false;
    }
}

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

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • 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 Sep 26, 2020

Looks like you actually want the HOLD_ON_OTHER_KEY_PRESS behavior from #9404 for some of your Layer Tap keys? It should do mostly the same thing as the code you are trying to add, just with less delay (the special code for Mod Tap keys, which is actually disabled by defining IGNORE_MOD_TAP_INTERRUPT, does not remove the delay caused by waiting for the tapping term to expire; HOLD_ON_OTHER_KEY_PRESS actually interrupts that delay when another key is pressed).

# ifdef IGNORE_LAYER_TAP_INTERRUPT
if (
# ifdef IGNORE_MOD_TAP_INTERRUPT_PER_KEY
!get_ignore_mod_tap_interrupt(get_event_keycode(record->event, false), record) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of this using the same function here. Would prefer a different function or a generic name for both.

@sigprof
Copy link
Contributor

sigprof commented Oct 4, 2020

The logic here seems to be reversed from IGNORE_MOD_TAP_INTERRUPT.

For mod-tap defining IGNORE_MOD_TAP_INTERRUPT actually disables the extra code which converts some events initially detected as taps into modifier holds (so by default “interrupting” a short mod-tap press by pressing another key together with the mod-tap key results in handling the mod-tap action as a modifier hold, and defining IGNORE_MOD_TAP_INTERRUPT makes the code ignore such interrupts and handle these actions as taps).

For layer-tap the code in this PR is written so that defining IGNORE_LAYER_TAP_INTERRUPT enables the extra code which converts some tap events into layer switching holds — this behavior is opposite to the behavior of IGNORE_MOD_TAP_INTERRUPT, therefore the name of the macro should be changed to reflect its true function. (The alternate solution of changing the default behavior of layer-tap keys to match the behavior of mod-tap keys and then making IGNORE_LAYER_TAP_INTERRUPT revert to the behavior that is currently implemented seems to be much worse.)

@stale
Copy link

stale bot commented Nov 19, 2020

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@stale
Copy link

stale bot commented Dec 19, 2020

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.

@stale stale bot closed this Dec 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants