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

Remove Mac specific keybindings that don't work #15687

Closed
wants to merge 1 commit into from

Conversation

HookyQR
Copy link
Contributor

@HookyQR HookyQR commented Nov 18, 2016

The thing that this attempted to provide has never worked. Removing it allows Dvorak keybindings to work correctly. Previously it caused keybindings to use the QWERTY POSITION of the key rather than the actual key.

eg. On Dvorak, the default for insert comments is "Cmd+z" which would then conflict with the undo shortcut.

The current 'fix' does nothing for 'english' based keyboards such as Dvorak, it in fact makes real weird stuff happen because of the 'reverse' mapping it tries to do.

This change will cause some keybindings to be unreachable on some keyboard layouts, but it is my view that it is better to map to the character than the key, and then allow the user (of those keyboard) to map to the key they want to use. (Ie, the problem with the '/' key on the swiss-german keyboard)

@mention-bot
Copy link

@HookyQR, thanks for your PR! By analyzing the history of the files in this pull request, we identified @alexandrudima and @jrieken to be potential reviewers.

@msftclas
Copy link

Hi @HookyQR, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

@msftclas
Copy link

@HookyQR, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@fakewaffle
Copy link

What's the status of this PR? I'm unable to use VSCode with a Dvorak layout.

@openam
Copy link

openam commented Dec 17, 2016

I'm having issues while using the Dvorak layout as well. I'd love it if this would fix those issues.

@HookyQR
Copy link
Contributor Author

HookyQR commented Dec 17, 2016

Seems the Microsoft team is uninterested. You can follow the steps at #15624 (comment) to fix it yourself. You'll have to do that for each release until they get interested in supporting us outliers. Interestingly, the problem is caused because they wanted to support a different minority group. 🙄

Cos they're so nice about it, the file that needs to be changed has moved. Have updated the referenced comment to reflect.

@HookyQR
Copy link
Contributor Author

HookyQR commented Dec 17, 2016

This will close #1492, #4712, #4619, #15624, #13417 and #6020. It will also likely close #17106, #17023, #16157, #15887 and #15711. Add a keymap extension for the keyboards you're trying to fix with the hacky code this removes and everyone will be happy. For those following 'mentioned' links from those issues. See #15624 (comment) for a resolution to your problems while this PR is ignored.

@alexdima
Copy link
Member

alexdima commented Dec 17, 2016

I am interested in fixing this and I've already started working on it, but breaking other keyboard layouts is not an option. i.e. the proposed changes in this PR would cause other breakages.

Work has already started in https://github.com/Microsoft/node-native-keymap

I plan to investigate using e.code instead of e.keyCode since we now consume a version of Chromium that has implemented it.

@fakewaffle
Copy link

fakewaffle commented Dec 17, 2016 via email

@alexdima
Copy link
Member

Turns out things are more complicated. I have documented my findings in #17521 and that issue will track the work going forward.

@alexdima
Copy link
Member

I am happy to say #17521 is now closed. I will validate tomorrow on a Mac the Dvorak issues to double check that they are indeed resolved.

@alexdima alexdima closed this Mar 23, 2017
@alexdima alexdima added this to the March 2017 milestone Mar 23, 2017
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants