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

MacOS Brightness Alias #4836

Merged
merged 9 commits into from
Jan 18, 2019
Merged

MacOS Brightness Alias #4836

merged 9 commits into from
Jan 18, 2019

Conversation

germ
Copy link
Contributor

@germ germ commented Jan 13, 2019

Added aliases to tmk_core for OSX brightness controls (F13, F14)

  • Core
  • Enhancement/Optimization

closes #4750

@vomindoraan
Copy link
Contributor

Possibly related: #2179

@fauxpark
Copy link
Member

These keycodes are too long to be practical in keymaps. I would go with the original proposal to just document this in the existing entries.

Since they are just aliases of Scroll Lock and Pause, they shouldn't have their own entries anyway. It makes it seem like they are separate keycodes.

@vomindoraan
Copy link
Contributor

vomindoraan commented Jan 13, 2019 •
edited
Loading

I agree with Fauxpark. They should be added to the ScrLk and Pause rows as aliases.

I propose KC_BRMU and KC_BRMD for the 7-char keycodes. Reasoning: ideally, they shouldn't start with KC_M since media controls use that convention, and U/D should be at the end for analogy with KC_BRIU/KC_BRID.

@germ
Copy link
Contributor Author

germ commented Jan 13, 2019

Are the ridiculously verbose names alright as long as there's shorthand? I don't mind @vomindoraan 's suggestion for the short codes. Give me a yes or no and I'll update the pull

@fauxpark
Copy link
Member

There are already long names for them - KC_SCROLLLOCK and KC_PAUSE. Just add the short forms into the aliases column (and also in keycodes_basic.md)

@vomindoraan
Copy link
Contributor

They would be, yes, but you'd have to expand the Aliases column quite a bit to fit those in. Imo, forgo the long names and just add KC_BRMU and KC_BRMD to KC_SCROLLLOCK and KC_PAUSE as aliases. Also mention it in the description.

vomindoraan
vomindoraan suggested changes Jan 13, 2019
View reviewed changes
Copy link
Contributor

@vomindoraan vomindoraan left a comment

Choose a reason for hiding this comment

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

By the way, I'd move this block between the consumer page and mouse keys. I think that makes a bit more sense.

tmk_core/common/keycode.h Outdated Show resolved Hide resolved
@germ
Copy link
Contributor Author

germ commented Jan 13, 2019

image

Oh no.

@germ
Copy link
Contributor Author

germ commented Jan 13, 2019

Not ideal in the documentation, but it doesn't need to be expanded. Added the alias to the desc of BrightnessUp/Down

fauxpark
fauxpark reviewed Jan 13, 2019
View reviewed changes
docs/keycodes.md Outdated Show resolved Hide resolved
vomindoraan
vomindoraan reviewed Jan 13, 2019
View reviewed changes
docs/keycodes.md Outdated Show resolved Hide resolved
fauxpark
fauxpark reviewed Jan 13, 2019
View reviewed changes
docs/keycodes.md Outdated Show resolved Hide resolved
vomindoraan
vomindoraan reviewed Jan 13, 2019
View reviewed changes
docs/keycodes.md Outdated Show resolved Hide resolved
@drashna
Copy link
Member

drashna commented Jan 18, 2019

Thanks!

@drashna drashna merged commit 77399bf into qmk:master Jan 18, 2019
@fauxpark fauxpark mentioned this pull request Jan 18, 2019
Merged
13 tasks
@juno juno mentioned this pull request Jan 18, 2019
Merged
13 tasks
akrob pushed a commit to akrob/qmk_firmware that referenced this pull request Jan 19, 2019
Merge remote-tracking branch 'upstream/master'
a8a9bd1 
* upstream/master: (390 commits)
  [Keyboard] handwired/ortho60 Configurator update, readme update, and rules tidy (qmk#4877)
  Fix Encoder documentation (qmk#4861)
  TKC1800 refactor and Configurator visual fixes (qmk#4870)
  Fixed the build break of helix/rev1:OLED_sample caused by PR qmk#4462. (qmk#4874)
  Add cursor keys to top layer (qmk#4876)
  [Keyboard] Fix bootloader size for v60_type_r (qmk#4873)
  Flip definitions of macOS brightness alias
  Grammatical fixes for GPIO Control doc (qmk#4869)
  handwired/ortho5x13: layout macro refactor
  Add new brightness aliases to keycodes_basic.md
  [Keyboard] update VENDOR_ID, PRODUCT_ID, and DEVICE_VER for Duck boards (qmk#4612)
  Add Planck rev1 and rev2, clean up rev3-5 config.h
  Allows Terminal to use ModTap/LayerTap keys (qmk#4586)
  Defined IS_(HOST_)LED_ON/OFF() and improved LED documentation (qmk#4853)
  MacOS Brightness Alias (qmk#4836)
  [Keymap] added custom led effect keymap.c (qmk#4856)
  Simplify split_common Code significantly (qmk#4772)
  Add documentation and fix formating (qmk#4860)
  [Keymap] Adding bdk keymap for ergobox_ez (qmk#4850)
  40percent.club Luddite: Configurator update (qmk#4859)
  ...
dlhextall pushed a commit to dlhextall/qmk_firmware that referenced this pull request May 24, 2019
@germ
MacOS Brightness Alias (qmk#4836)
183dc53 
* Added aliases for OSX brightness

* Updated docs for alises

* Moved aliases, added docs to Brightness section

* Update docs/keycodes.md

Co-Authored-By: germ <[email protected]>

* Update docs/keycodes.md

Co-Authored-By: germ <[email protected]>

* Update keycode.h

* Update keycode.h

* Update keycodes.md

* Update docs/keycodes.md

Co-Authored-By: germ <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fauxpark fauxpark fauxpark left review comments

@vomindoraan vomindoraan vomindoraan requested changes

Assignees
No one assigned
Labels
documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Documentation: macOS Screen Brightness
5 participants
@germ @vomindoraan @fauxpark @drashna @mechmerlin