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.

Sign up for GitHub

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

Jump to bottom

Align TO() max layers with other keycodes #17989

Merged
merged 1 commit into from
Aug 14, 2022
Merged

Align TO() max layers with other keycodes #17989

merged 1 commit into from
Aug 14, 2022

Conversation

zvecr
Copy link
Member

@zvecr zvecr commented Aug 11, 2022 •
edited
Loading

Description

Alignment as part of the eventual DD migration of keycodes. Fewer special cases will help simplify parsing and validation.

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

@zvecr zvecr requested a review from a team August 11, 2022 01:30
@github-actions github-actions bot added the core label Aug 11, 2022
@zvecr zvecr marked this pull request as ready for review August 11, 2022 23:59
spidey3
spidey3 approved these changes Aug 13, 2022
View reviewed changes
Copy link
Contributor

@spidey3 spidey3 left a comment

Choose a reason for hiding this comment

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

Tested; LGTM

@drashna
Copy link
Member

drashna commented Aug 14, 2022

Closes #17960

@tzarc tzarc merged commit ba04ecf into qmk:develop Aug 14, 2022
@tzarc tzarc added the bug label Aug 14, 2022
@sigprof
Copy link
Contributor

sigprof commented Sep 18, 2022

And apparently this broke VIA again, because the old value of TO(0) was 0x5010 (due to (ON_PRESS << 0x4)), and now it's 0x5000, and 0x5010 is interpreted as TO(16).

https://discord.com/channels/440868230475677696/537514291440517121/1020552010459774986

The TO(0)…TO(9) keycodes are even included on the “Layers” page in VIA (they don't need the Any key to enter).

@sigprof sigprof mentioned this pull request Oct 5, 2022
Closed
14 tasks
This was referenced Oct 19, 2022
Closed
Merged
nolanseaton pushed a commit to nolanseaton/qmk_firmware that referenced this pull request Jan 23, 2023
@zvecr @nolanseaton
Align TO() max layers with other keycodes (qmk#17989)
6bf21d2
@sigprof sigprof mentioned this pull request Jun 8, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spidey3 spidey3 spidey3 approved these changes

@fauxpark fauxpark fauxpark approved these changes

@tzarc tzarc tzarc approved these changes

@drashna drashna drashna approved these changes

Assignees
No one assigned
Labels
bug core enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@zvecr @drashna @sigprof @tzarc @fauxpark @spidey3