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 autoindent when opening a line above #1249

Merged
merged 1 commit into from
Feb 5, 2017

Conversation

inejge
Copy link
Contributor

@inejge inejge commented Jan 26, 2017

Re #862. I rewrote CommandInsertNewLineAbove to mirror CommandInsertNewLineBefore, just using editor.action.insertLineBefore as the editor command to execute.

@inejge
Copy link
Contributor Author

inejge commented Jan 26, 2017

I don't really understand the test failure on first glance, will investigate later.

@xconverge
Copy link
Member

Well if you do a git blame you will get to the reasoning here

#1172

Can you just change the diff to do the indenting correctly?

instead of

diff: new PositionDiff(-1, 0),

do

diff: new PositionDiff(-1, charPositionItShouldBeDueToIndentation),

@johnfn
Copy link
Member

johnfn commented Jan 31, 2017

I think we have a method on TextEditor to determine correct indentation.

@inejge
Copy link
Contributor Author

inejge commented Feb 3, 2017

Can you just change the diff to do the indenting correctly?

Just repositioning is not enough, a properly indented empty line must be inserted for this to work. I've changed the patch accordingly (the gymnastics with replace after setIndentationLevel are necessary because the latter doesn't want to indent a string consisting of a bare "\n" and keep the newline).

When invoking O for the first time, i get "invalid value for set cursor position..." in the console.

The tests are still failing for reasons I don't understand -- they're failing for me even without the patch.

@xconverge
Copy link
Member

The failing test is unrelated, not sure what is going on there, this LGTM now

@xconverge
Copy link
Member

An o and O test that check indentation would be great

@johnfn
Copy link
Member

johnfn commented Feb 5, 2017

This LGTM to me as well. Thank you for the PR, @inejge (and apologies for the slow responses this month!)

@johnfn johnfn merged commit 6dde5ea into VSCodeVim:master Feb 5, 2017
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.

3 participants