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

fix: Change the Linux Ask Sourcery shortcut to ctrl+shift+y #224

Merged
merged 2 commits into from
Sep 19, 2023

Conversation

brendanator
Copy link
Contributor

Fixes: #204

@brendanator brendanator enabled auto-merge (squash) September 18, 2023 16:32
@blakeNaccarato
Copy link

blakeNaccarato commented Sep 18, 2023

The keybinding example suggests setting the key variable in any given keybind to define the default behavior, then to override on a per-platform basis. So maybe you could retain the desired behavior of having Ctrl+Y trigger Ask on Linux while still shunting Windows to Ctrl+Shift+Y by properly setting key to one option, along with the platform overrides?

See your implementation (prior to PR) versus one from the example extension featuring a platform override. Perhaps the Developer: Toggle Keyboard Shortcuts Troubleshooting and some tinkering on a Windows machine would reveal the issue.

I see that the results of running "Preferences: Open Default Keyboard Shortcuts (JSON)" on my machine shows Ctrl+Y for sourcery.chat.ask, so I think the missing default "key" (perhaps that should be set to the safer Ctrl+Shift+Y) in your package.json is the culprit?

The extension developer documentation is pretty sparse in this regard, so I can understand how this went awry. It could be that the Linux keybind is considered the default in absence of key specification, causing some fringe machine configurations getting hit.

@brendanator
Copy link
Contributor Author

Thanks for the great explanation @blakeNaccarato!

I don't actually have access to a windows machine so have been working in the dark. I've taken your advice and set key to ctrl+shift+y and removed linux just to be sure. Hopefully this will work as expected 🤞

@brendanator brendanator merged commit a3fc5ac into main Sep 19, 2023
1 check passed
@brendanator brendanator deleted the brendan/ask-sourcery-windows-shortcut branch September 19, 2023 09:32
@blakeNaccarato
Copy link

Yeah the platform differences have bitten me as well before. Reach out if you need feedback on Windows-related issues going forward.

If Windows testing becomes important in the future, as long as you can write a test for the behavior, you could run tests in CI in a job matrix with this VSCode extension testing Action to get some Windows signal in your workflow. A bit of a hassle for just a keyboard shortcut nag, but it could help if the extension feature set grows a lot.

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.

Consider not binding Ctrl+Y (commonly "Redo") to "Sourcery: Ask Sourcery" aka sourcery.chat.ask
3 participants