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

Process leader mappings on key press #7270

Closed
wants to merge 3 commits into from

Conversation

buztard
Copy link
Contributor

@buztard buztard commented Nov 5, 2019

Description

This PR has similar goals as #6580 but uses a different approach.

  • Allows long timeouts and LEADER_PER_KEY_TIMING but you don't have to wait for the timeout to happen.
  • Runs a user defined function on key press and after LEADER_TIMEOUT
  • Allows cancellation of leader mode by key press, defaults to KC_LEAD
  • No need for LEADER_EXTERNS and LEADER_DICTIONARY in user code
  • Optional feature, should not interfere with the current behavior

Here's an example for a user function:

// add some configuration
#ifdef LEADER_ENABLE
#   define LEADER_ON_KEY_PROCESSING

// per key timing and and 1s timeout are vim's defaults ;)
#   define LEADER_PER_KEY_TIMING
#   define LEADER_TIMEOUT 1000

// use escape (or five invalid keypresses) to abort leader mode
#   define LEADER_ABORT KC_ESC

bool leader_process_user(uint16_t *leader_sequence, bool is_timeout) {
    SEQ_ONE_KEY(KC_S) { SEND_STRING("Leader S"); return false; } // finish processing

    // KC_G is not unique, so this one is only valid after the timeout
    SEQ_ONE_KEY(KC_G) { if (is_timeout) { SEND_STRING("Leader with shared prefix G after timeout"); return false; } }

    SEQ_TWO_KEYS(KC_G, KC_G) { SEND_STRING("Leader GG"); return false; }

    return true; // continue processing
}
#endif

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.
  • 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).

@drashna drashna requested a review from a team November 5, 2019 19:56
@@ -982,6 +982,10 @@ void matrix_scan_quantum() {
matrix_scan_combo();
#endif

#if defined(LEADER_ENABLE) && defined(LEADER_ON_KEY_PROCESSING)
Copy link
Member

@drashna drashna Nov 5, 2019

Choose a reason for hiding this comment

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

I think we could just use:

Suggested change
#if defined(LEADER_ENABLE) && defined(LEADER_ON_KEY_PROCESSING)
#if defined(LEADER_ENABLE)

If not, it would be nice ideal if things could be changed so this is called in such a way that it could be use regardless.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this would break existing code because calling matrix_scan_leader() when on key processing is not enabled would finish the leader mode before the user gets the chance to catch it in his matrix_scan_user() with the usual LEADER_DICTIONARY() method.

However, we could move the check to into matrix_scan_leader() to keep the matrix function cleaner.

Otherwise it would break existing code that uses a leader sequence that
contains KC_LEAD
@stale
Copy link

stale bot commented Dec 21, 2019

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.

@drashna drashna requested a review from a team January 18, 2020 04:44
@stale stale bot removed the awaiting changes label Jan 18, 2020
@stale
Copy link

stale bot commented Mar 3, 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 Apr 2, 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 Apr 2, 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.

2 participants