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 tap hold confs away from keyboard-level files #14789

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

precondition
Copy link
Contributor

@precondition precondition commented Oct 11, 2021

Description

This moves tap hold configuration settings like IGNORE_MOD_TAP_INTERRUPT
away from keyboard-level files and into the config.h files of the
default and via (it if exists) keymaps.

#define TAP_TERM x is ignored, but I can make a follow-up commit to move it away from keyboard-level files too.

The motivation for this PR is the following rule in the documentation on keyboard PRs:

  • keyboard config.h
    • don't repeat MANUFACTURER in the PRODUCT value
    • no #define DESCRIPTION
    • no Magic Key Options, MIDI Options or HD44780 configuration
    • user preference configurable #defines need to be moved to keymap config.h
    • "DEBOUNCE" instead of "DEBOUNCING_DELAY"
    • bare minimum required code for a board to boot into QMK should be present
      • initialisation code for the matrix and critical devices
      • mirroring existing functionality of a commercial board (like custom keycodes and special animations etc.) should be handled through non-default keymaps
    • Vial-related files or changes will not be accepted, as they are not used by QMK firmware (no Vial-specific core code has been submitted or merged)

Personally speaking, the fact that Dactyl Manuform enables PERMISSIVE_HOLD by default tripped me up because I couldn't understand why my keyboard was acting in a PERMISSIVE_HOLD manner, and thus causing modifier misfires, despite not having enabled it in my keymap config.h.

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

This moves tap hold configuration settings like IGNORE_MOD_TAP_INTERRUPT
away from keyboard-level files and into the config.h files of the
default and via (it if exists) keymaps.

`#define TAP_TERM x` is ignored.
@github-actions github-actions bot added keyboard keymap via Adds via keymap and/or updates keyboard for via support labels Oct 11, 2021
@@ -0,0 +1,3 @@

Copy link
Member

Choose a reason for hiding this comment

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

Need a license header and an include guard.

Same with the rest of the config.h files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whose name should I put in the license header though? The keyboard maker's? Mine?

Copy link
Member

Choose a reason for hiding this comment

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

If unsure, then just QMK is fine.

@stale
Copy link

stale bot commented Jan 3, 2022

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.

@precondition
Copy link
Contributor Author

I'm currently busy with uni exams but I will get back to this, this is not stale.

@drashna drashna added the on hold label Jan 4, 2022
@stale stale bot removed the awaiting changes label Jan 4, 2022
@precondition
Copy link
Contributor Author

Will be in draft until #15741 gets merged.

@precondition precondition marked this pull request as draft May 12, 2022 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keyboard keymap on hold via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants