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

Cleanup keymap_support.c #64

Merged
merged 7 commits into from
Aug 3, 2024
Merged

Conversation

SiriusStarr
Copy link
Collaborator

Some of this is formatting-related. Do we have a standard formatter to run? And if not, should we?

If the automouse flags are just going to always be set, then we should probably remove all the (non-console/debug) ifdefs? My concern is ending up with some odd combination of flags such that things don't compile because we're accessing something not being defined or whatnot.

Anyways, this all builds and works fine for me, but more eyes on it would be good, obviously.

@SiriusStarr SiriusStarr requested a review from ilc July 26, 2024 22:29
@SiriusStarr SiriusStarr changed the title Cleanup keymap support Cleanup keymap_support.c Jul 26, 2024
Things like Achordion shouldn't be dependent on pointing devices anyways.
This moves all non-hold keys to activate on key-down, so we only need to
think about clearing held states in the key-up case.  It also aborts
further handling of these keycodes once we're done with them.

This fixes the issue with caps word toggle not actually toggling and
getting processed multiple times.  It does prevent users from hooking these
custom keys, but if people know how to do that, they can probably figure
out how to make their own custom key that calls the same code.
This is already defined in `trackpoint/config.h`. If you have input @ilc I'm all ears.
@SiriusStarr SiriusStarr force-pushed the cleanup-keymap-support branch from 684efef to 3fff0f2 Compare July 28, 2024 18:33
@SiriusStarr SiriusStarr mentioned this pull request Jul 29, 2024
Copy link
Collaborator

@ilc ilc left a comment

Choose a reason for hiding this comment

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

I still need to pull this locally for testing. Assuming it's good and the change requested is good. We'll be good to go.

keyboards/svalboard/keymaps/keymap_support.c Outdated Show resolved Hide resolved
@SiriusStarr SiriusStarr requested a review from ilc August 2, 2024 18:52
@ilc
Copy link
Collaborator

ilc commented Aug 3, 2024

LGTM - Running on the board I am using right now.

@ilc ilc merged commit 9d0be11 into svalboard:vial Aug 3, 2024
8 checks passed
@SiriusStarr SiriusStarr deleted the cleanup-keymap-support branch August 5, 2024 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants