Skip to content

Conversation

@magnus-ISU
Copy link

@magnus-ISU magnus-ISU commented Dec 16, 2021

This refactors a lot of input code to resolve #167 (shortcuts can be set to either uppercase or lowercase). The way it does so is by mostly ignoring the keyCode attribute of an input event and instead using the string key which contains more information and in a more human-readable way. Unfortunately, this currently means it breaks any preconfigured options a user has set and users will need to reconfigure it. Assuming this is unacceptable (I assume it is) I can add in a migration strategy; but I need to know how best that done.

I also removed the old 5.3.0 settings conversion code because its been here long enough I think. In doing so, I was able to simplify some logic. Additionally, vine.co does not really exist anymore so disabling the extension there by default is unneeded I think.

I need testing for numpad keys to see if they still work the same. I wrote this all on a laptop without them.

There are several TODOs marked around the code where I wasn't sure what was going on and need review to tell me if my comments are correct or not.

The version is not bumped, please tell me how to change it. This could be a bump to 0.7.0.0 or something, to signify it is a breaking change and add no migration. Or it could need to be updated to have a migration path, and become 0.6.4.0. Or maybe the project tracks upstream versions directly and only manifest-version should be bumped. I'm not sure.

EDIT: now adds the feature suggested in #119 also

…icated and should be essentially hidden, it is dumb to bypass the formatter
…ally better than keeping the first line right after
… merge with seperate-lower-upper because it changes a lot about settings too
@magnus-ISU magnus-ISU mentioned this pull request Dec 16, 2021
@magnus-ISU
Copy link
Author

Just found out from reading #153 that this tracks upstream very closely and thus probably won't merge this. Time to plan to rewrite everything for chromium lol.

Also upstream is at 7.1 already, hurry up and update this!

@magnus-ISU
Copy link
Author

Upstream PR: igrigorik#907

@magnus-ISU magnus-ISU closed this Dec 16, 2021
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.

Feature Request: differentiate capital and lowercase keypresses for shortcuts

1 participant