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

Feature/add tab mapping #1450

Merged
merged 7 commits into from
Mar 11, 2020
Merged

Conversation

jakeday
Copy link
Contributor

@jakeday jakeday commented Mar 9, 2020

Taking a stab add adding the Visual Mode mapping and adding support for tab and shift tab to indent and outdent when in the visual mode.

Copy link
Member

@glennsl glennsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank @jakeday! Almost perfect execution as well!But I do have one correction below.

I'm also wondering where this keybinding originates from. It doesn't seem to be vscode, which does have something similar but with a different default. And ideally we'd use vscode's defaults.

src/Store/VimStoreConnector.re Show resolved Hide resolved
src/Store/KeyBindingsStoreConnector.re Outdated Show resolved Hide resolved
@glennsl
Copy link
Member

glennsl commented Mar 9, 2020

Also, you need to run esy format to appease the CI.

@jakeday
Copy link
Contributor Author

jakeday commented Mar 10, 2020

@glennsl I don't know why the check failed, but it generated the artifacts. I tested them and they appear to be working correctly.

@leavengood
Copy link
Contributor

The Windows CI tasks seems to be flaky. I had a similar issue last night for my PR, but it worked fine this morning. You just need @glennsl or @bryphe to re-run the CI pipeline.

@glennsl
Copy link
Member

glennsl commented Mar 10, 2020

/azp run

@@ -179,6 +179,16 @@ let start = () => {
command: "workbench.action.files.save",
condition: "editorTextFocus" |> WhenExpr.parse,
},
{
key: "<TAB>",
Copy link
Member

@glennsl glennsl Mar 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still wondering where this keybinding comes from, and leaning towards the default being Ctrol+], since that's the default for vscode. After all, we aim to be vscode/vim hybrid first and foremost.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use tab and shift-tab in VSCode with no extensions to indent and unindent blocks of code, though I can only see the action bound to Ctrl-[] in the docs...but it does work, so not so sure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it a vscodevim thing perhaps?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think so! I use it on clean machines helping students out in programming labs, and its worked across every machine I've tried, so I think it must be default to VSCode since the only plugins they have installed is the Python one.

Copy link
Member

@glennsl glennsl Mar 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah it doesn't work with vscodevim, but does work with a selection if I disable it. And of course it works if I put the cursor at the beginning of the line. So I guess that's just part of the "core" editor experience.

Should we have both bindings then? They aren't entirely identical since the "proper" keybinding will indent the line even without a selection, but that's closer to what's been implemented here anyway.

Also, on macOS the keybinding is Ctrl+Alt+Cmd+8 to outdent and Ctrl+Alt+Cmd+9 to indent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glennsl I added the mappings for the CTRL+] and CTRL+[ for indenting and outdenting. With the current mappings in this PR, we can support both typical use cases for users who will expect to be able to use these shortcuts regardless of which editor they are use to.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jakeday. Could you implement my second suggestion as well? Unless you don't think it's a good idea for some reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glennsl Are you wanting the command to be just "indent" or "editor.action.indent"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like Tab to be just indent and S-Tab to be just outdent, since the latter actually exists as a keybinding in vscode. indent is oddly missing, so we'll just have to infer that one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glennsl Done!

Copy link
Member

@glennsl glennsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great! Thanks @jakeday

Although the mac-specific keybindings don't actually work on my mac because of the weird keyboard layout I have, but that's a more general problem we have to figure out.

@glennsl glennsl merged commit 6293fb2 into onivim:master Mar 11, 2020
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.

None yet

4 participants