-
-
Notifications
You must be signed in to change notification settings - Fork 40.6k
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
[kle2info] Trim the code in kle2xy
#8955
Merged
Merged
+8
−28
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
skullydazed
reviewed
Apr 28, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the improvements! Can you also submit a PR upstream? https://github.com/skullydazed/kle2xy
cf7fbc6
to
2c233b5
Compare
@skullydazed I updated this PR and put in another across the way. |
This is a slight change. Before, the key_skel would keep the invalid value for future keys. I think this is what was actually intended.
x is the current_x * key_size + (key_size/2) y is the current_y * key_size + (key_size/2) no reason to track both
2c233b5
to
39bc28b
Compare
removed x,y from skel + rebased |
13 tasks
skullydazed
approved these changes
May 9, 2020
fauxpark
approved these changes
May 15, 2020
bitherder
pushed a commit
to bitherder/qmk_firmware
that referenced
this pull request
May 15, 2020
* [kle2jinfo] use min/max instead of if This is a slight change. Before, the key_skel would keep the invalid value for future keys. I think this is what was actually intended. * [kle2info] calculate x x is the current_x * key_size + (key_size/2) y is the current_y * key_size + (key_size/2) no reason to track both
drashna
pushed a commit
to zsa/qmk_firmware
that referenced
this pull request
May 24, 2020
* [kle2jinfo] use min/max instead of if This is a slight change. Before, the key_skel would keep the invalid value for future keys. I think this is what was actually intended. * [kle2info] calculate x x is the current_x * key_size + (key_size/2) y is the current_y * key_size + (key_size/2) no reason to track both
sowbug
pushed a commit
to sowbug/qmk_firmware
that referenced
this pull request
May 24, 2020
* [kle2jinfo] use min/max instead of if This is a slight change. Before, the key_skel would keep the invalid value for future keys. I think this is what was actually intended. * [kle2info] calculate x x is the current_x * key_size + (key_size/2) y is the current_y * key_size + (key_size/2) no reason to track both
fdidron
pushed a commit
to zsa/qmk_firmware
that referenced
this pull request
Jun 12, 2020
* [kle2jinfo] use min/max instead of if This is a slight change. Before, the key_skel would keep the invalid value for future keys. I think this is what was actually intended. * [kle2info] calculate x x is the current_x * key_size + (key_size/2) y is the current_y * key_size + (key_size/2) no reason to track both
turky
pushed a commit
to turky/qmk_firmware
that referenced
this pull request
Jun 13, 2020
* [kle2jinfo] use min/max instead of if This is a slight change. Before, the key_skel would keep the invalid value for future keys. I think this is what was actually intended. * [kle2info] calculate x x is the current_x * key_size + (key_size/2) y is the current_y * key_size + (key_size/2) no reason to track both
jakobaa
pushed a commit
to jakobaa/qmk_firmware
that referenced
this pull request
Jul 7, 2020
* [kle2jinfo] use min/max instead of if This is a slight change. Before, the key_skel would keep the invalid value for future keys. I think this is what was actually intended. * [kle2info] calculate x x is the current_x * key_size + (key_size/2) y is the current_y * key_size + (key_size/2) no reason to track both
sjmacneil
pushed a commit
to sjmacneil/qmk_firmware
that referenced
this pull request
Feb 19, 2021
* [kle2jinfo] use min/max instead of if This is a slight change. Before, the key_skel would keep the invalid value for future keys. I think this is what was actually intended. * [kle2info] calculate x x is the current_x * key_size + (key_size/2) y is the current_y * key_size + (key_size/2) no reason to track both
BorisTestov
pushed a commit
to BorisTestov/qmk_firmware
that referenced
this pull request
May 23, 2024
* [kle2jinfo] use min/max instead of if This is a slight change. Before, the key_skel would keep the invalid value for future keys. I think this is what was actually intended. * [kle2info] calculate x x is the current_x * key_size + (key_size/2) y is the current_y * key_size + (key_size/2) no reason to track both
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is not aimed to change functionality. Just trimming the code a little to hopefully be more understandable
Description
2 commits:
current_x
,current_y
Use Min, Max
the
label_style
can be between 0 and 9.If a value is too high, it is set to 9.
The old code would put the correct value in
current_key
, but would put the incorrect value inskel
(i.e. default values for the current key).Simplifying the bounds checking to a single expression allowes both
skel
andcurrent_key
to be set with the correct value.Same goes for
font_size
.remove
current_{x,y}
row
andcolumn
are the location of a key inu
units.u
being the size of a key.x
andy
are the location of the center of the key inmm
.y = current_row * key_size + key_height * key_size /2
you can have this value either by adding
key_size
every row, or by calculating this on the fly.It seemed simpler to me to not have (and maintain) the 2 extra variables
Types of Changes
Checklist
I ran before and after with the test kle.txt and the larger alice keyboard layout.
The output was the same.
Since info does not output the
x, y
inmm
, I also ran alice keyboard with print statements of therow, col, x, y
values both before and after to ensure the calculated values were the same as the maintained values.