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 keymaps that conflict with non-English keyboards #2535

Merged
merged 2 commits into from Jun 7, 2017

Conversation

Projects
None yet
3 participants
@gnestor
Contributor

gnestor commented May 30, 2017

Fixes #2379

@gnestor gnestor added the type:Bug label May 30, 2017

@gnestor gnestor added this to the 5.1 milestone May 30, 2017

@gnestor

This comment has been minimized.

Show comment
Hide comment
@gnestor

gnestor May 31, 2017

Contributor

@minrk Care to review? This reverts some of #963 which caused conflicts on non-English keyboards.

Contributor

gnestor commented May 31, 2017

@minrk Care to review? This reverts some of #963 which caused conflicts on non-English keyboards.

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Jun 1, 2017

Member

Is it just the ctrl-alt shortcuts or also the cmd-alt shortcuts that cause this problem? I would certainly be sad to lose those.

Member

minrk commented Jun 1, 2017

Is it just the ctrl-alt shortcuts or also the cmd-alt shortcuts that cause this problem? I would certainly be sad to lose those.

@gnestor

This comment has been minimized.

Show comment
Hide comment
@gnestor

gnestor Jun 2, 2017

Contributor

@minrk This is only removing the Cmd-Alt-[ shortcut, not all Cmd-Alt-*

Contributor

gnestor commented Jun 2, 2017

@minrk This is only removing the Cmd-Alt-[ shortcut, not all Cmd-Alt-*

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Jun 2, 2017

Member

Sorry. My question was whether it was ctrl-alt-[ or cmd-alt-[ or both that cause the issue.

Member

minrk commented Jun 2, 2017

Sorry. My question was whether it was ctrl-alt-[ or cmd-alt-[ or both that cause the issue.

@gnestor

This comment has been minimized.

Show comment
Hide comment
@gnestor

gnestor Jun 2, 2017

Contributor

I inquired about that in the issue. Let's see what people say 👍 I think we just need to avoid using shortcuts that involve [ and ], but I saw a one comment about \ and @Carreau suggested that we also disable ones involving /.

Contributor

gnestor commented Jun 2, 2017

I inquired about that in the issue. Let's see what people say 👍 I think we just need to avoid using shortcuts that involve [ and ], but I saw a one comment about \ and @Carreau suggested that we also disable ones involving /.

@Carreau

This comment has been minimized.

Show comment
Hide comment
@Carreau

Carreau Jun 2, 2017

Member

I believe that's anything that touch [,], and alt. Alt is heavily use on french keyboard to type punctuation. You just can't bind by default to anything that uses alt.
That one of the main reason why I'm not an emacs user as most alt-based shortcut were conflicting with typing.

The MacOS keyboard is slightly different, but same battle. Alt is a critical key.

Member

Carreau commented Jun 2, 2017

I believe that's anything that touch [,], and alt. Alt is heavily use on french keyboard to type punctuation. You just can't bind by default to anything that uses alt.
That one of the main reason why I'm not an emacs user as most alt-based shortcut were conflicting with typing.

The MacOS keyboard is slightly different, but same battle. Alt is a critical key.

@gnestor

This comment has been minimized.

Show comment
Hide comment
@gnestor

gnestor Jun 7, 2017

Contributor

Ok, so this PR just comments out the 2 commands that involve alt so I think we can merge this and iterate if necessary.

Contributor

gnestor commented Jun 7, 2017

Ok, so this PR just comments out the 2 commands that involve alt so I think we can merge this and iterate if necessary.

@Carreau Carreau merged commit 4b8cc28 into jupyter:master Jun 7, 2017

4 checks passed

codecov/patch Coverage not affected when comparing 1e18e05...af0a7b5
Details
codecov/project 78.13% (+0.08%) compared to 1e18e05
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Carreau

This comment has been minimized.

Show comment
Hide comment
@Carreau

Carreau Jun 7, 2017

Member

Ok, so this PR just comments out the 2 commands that involve alt so I think we can merge this and iterate if necessary.

Yep, good point.

Member

Carreau commented Jun 7, 2017

Ok, so this PR just comments out the 2 commands that involve alt so I think we can merge this and iterate if necessary.

Yep, good point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment