zmkfirmware / zmk

ZMK Firmware Repository
https://zmk.dev/
MIT License
2.67k stars 2.74k forks source link

feat extra MEH and HYPER combined modifier and copilot key support #2341

Open nicenemo opened 3 months ago

nicenemo commented 3 months ago

Add left and right combinations of modifiers such as HYPER and MEH. Adds the copilot key. Closes #2339

Tested on my Cornish-Zen with https://github.com/nicenemo/zmk-config/tree/test_custom_modifier_patch_for_zmk_config

caksoylar commented 2 months ago

Note that I haven't given this as detailed a look as lesshonor yet, but I have some high level thoughts:

nicenemo commented 2 months ago

You don't need to close this PR and open a new one to squash commits—rebasing and force pushing is fine. Adhering to conventional commit guidelines is encouraged. (Don't worry about preserving any of the co-author stuff; I don't care.)

ZMK is not an ultra-fast moving project; there are maybe three people with merge permissions (this does not include me). Your patience with discussions and reviews is appreciated. Taking the extra time to carefully audit your proposed changes for any typos and inconsistencies would be productive.

I just want it to be valuable to many people. The best possible naming is important here. Changing it later is painful. I like your naming solutions for the functions. I will accept your suggestions.

I do understand and respect that some do not see the value of defines for 2 key modifiers.

Regarding the concrete example, I do agree too. I initially did not to keep it similar with the rest.

Regarding the missing back tick, locally I did not get an error.

I did not add the copilot key, initially as I consider it a possible temporary thing. I kept that in my keyboard configuration.

What is the best way to squash the commits on the same branch for this case? Not being git fluent. I looked up several options.

I converted this pull request to a draft for now. I will clear up the linting and do the rebase later today. I have to do other things now.

nicenemo commented 2 months ago

I tested on Windows, Linux(KDE) and i tried iOS. I do not have a Mac so I cannot test that. The iOS tests are inconclusive because of some iOS device issues and I am less familiar with iOS. Hyper and MEH work fine on Windows and Linux. The Copilot key starts a search on some configurations of Windows 11 and will probably start copilot if configured. On Windows 10 it will select the first icon on the Desktop. On Linux higher function key support varies sice. It depends on your Desktop environment configuration, X, Wayland or something more low level. Therefore I did not set that to true or false. Given the previous discussions, and changes I made based on the input , I think this pull request is ready to be reviewed.

Updated my test branch for zmk-config for a corn-ish zen again: https://github.com/nicenemo/zmk-config/tree/test_custom_modifier_patch_for_zmk_config