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

bug(developer): [NCAPS SHIFT_K_P] seemed to compile but is invalid #12307

Open
mcdurdin opened this issue Aug 29, 2024 · 3 comments · May be fixed by #12604
Open

bug(developer): [NCAPS SHIFT_K_P] seemed to compile but is invalid #12307

mcdurdin opened this issue Aug 29, 2024 · 3 comments · May be fixed by #12604
Assignees
@markcsinclair
Labels
bug developer/
Milestone
A18S15

Comments

@mcdurdin
Copy link
Member

mcdurdin commented Aug 29, 2024 •
edited
Loading 

store(nul) [NCAPS K_B] [NCAPS SHIFT K_B] [NCAPS K_F] [NCAPS SHIFT K_F] [NCAPS K_P] \
           [NCAPS SHIFT_K_P] [NCAPS K_R] [NCAPS SHIFT K_R] [NCAPS K_Z] [NCAPS SHIFT K_Z]
+ any(nul) > nul

How did this ([NCAPS SHIFT_K_P]) compile previously?!

Originally posted by @mcdurdin in keymanapp/keyboards#3056 (comment)

@markcsinclair one for a unit test?
@mcdurdin mcdurdin added the bug label Aug 29, 2024
@mcdurdin mcdurdin transferred this issue from keymanapp/keyboards Aug 29, 2024
@mcdurdin mcdurdin mentioned this issue Aug 29, 2024
Merged
@mcdurdin mcdurdin added this to the A18S10 milestone Aug 29, 2024
@darcywong00 darcywong00 modified the milestones: A18S10, A18S11 Sep 14, 2024
@darcywong00 darcywong00 modified the milestones: A18S11, A18S12 Sep 28, 2024
@darcywong00 darcywong00 modified the milestones: A18S12, A18S13 Oct 11, 2024
markcsinclair added a commit that referenced this issue Oct 14, 2024
@markcsinclair
chore(developer): add six additional test cases (virtual char key), p…
592ffbd 
…lus three commented-out test cases for issue #12307
@markcsinclair
Copy link
Contributor

It would appear the compiler does not require spaces between the modifiers, or between the modifiers and the key portion, so [CTRLK_A] compiles as if [CTRL K_A], [CTRLALT K_A] as if [CTRL ALT K_A] and [NCAPS SHIFT_K_P] as if [NCAPS SHIFT _K_P] (so this will be a custom key name i.e. _K_P, not key K_P`)

@darcywong00 darcywong00 modified the milestones: A18S13, A18S14 Oct 26, 2024
@markcsinclair
Copy link
Contributor

In addition, there is a space problem with grouping multiple virtual keys. The compiler sees [K_A K_B K_C] as K_A followed by K_ followed by K_.

markcsinclair added a commit that referenced this issue Oct 29, 2024
@markcsinclair
chore(developer): added commented-out unit test for multiple grouped …
8ac4267 
…virtual keys (see #12307)
@markcsinclair markcsinclair mentioned this issue Oct 29, 2024
Merged
@mcdurdin
Copy link
Member Author

Support for series of virtual keys (e.g. [K_A K_B K_C]) is not documented and not supported by us. We should just block them in the compiler, as there is no good use case for them. (This was developed long-ago in anticipation of a possible feature for emitting virtual keys, but emitting virtual keys has been explicitly not supported for many years now.)

@markcsinclair markcsinclair linked a pull request Nov 1, 2024 that will close this issue
Open
@darcywong00 darcywong00 modified the milestones: A18S14, A18S15 Nov 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@markcsinclair markcsinclair

Labels
bug developer/
Projects
Keyman
Status: No status
Milestone
A18S15
3 participants
@mcdurdin @darcywong00 @markcsinclair