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

Move pre_process_record_kb() before process_combo() #20969

Merged
merged 1 commit into from
May 20, 2023

Conversation

sigprof
Copy link
Contributor

@sigprof sigprof commented May 16, 2023

Description

Fix up #20584 to call pre_process_record_kb() (and therefore pre_process_record_user()) before process_combo().

Adding the pre_process_record_kb() call after process_combo() is problematic — pre_process_record_kb() will miss any events which might be a part of some combo: #20584 (comment). Calling pre_process_record_kb() before the combo code would result in a much more logical behavior (pre_process_record_kb() receives all events for raw key or encoder actions before any other processing, including combos).

If we ever need a callback that runs after the combo processing, but before the tapping code, it would need to be called in more places (e.g., in two places where the combo code currently calls action_tapping_process()); but that probably could wait until some real user of such callback appears.

Types of Changes

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

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

The combo code buffers events that might be a part of some combo, and
also generates virtual events for matched combos; if process_combo() is
called first, pre_process_record_kb() would miss those buffered events
(including the events which did not match any combo in the end), and
also won't receive any events generated by combos.  Calling
pre_process_record_kb() before the combo code result in a much more
logical behavior (pre_process_record_kb() receives all events for raw
key or encoder actions before any other processing, including combos).
Copy link
Contributor

@filterpaper filterpaper left a comment

Choose a reason for hiding this comment

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

Tested this change and confirmed that it fixed issues where record was not passed to pre_process_record_kb.

@filterpaper filterpaper mentioned this pull request May 20, 2023
14 tasks
@tzarc tzarc merged commit 21b660f into qmk:develop May 20, 2023
coquizen pushed a commit to coquizen/qmk_firmware that referenced this pull request Jun 22, 2023
autoferrit pushed a commit to SpaceRockMedia/bastardkb-qmk that referenced this pull request Dec 8, 2023
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.

4 participants