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

Fixes #1513: Backspace on middle of whitespace only line fails #1514

Merged
merged 6 commits into from Apr 21, 2017

Conversation

Chillee
Copy link
Member

@Chillee Chillee commented Apr 15, 2017

So I fixed issue #1513. This issue only occurs when Configuration.expandTab is turned on.
The main behavioral decision I made is that if you press backspace on a whitespace only line, it deletes both to the next "tab" as well as all the whitespace after it.

I'm unsure about a couple of things

  1. I'd like to create tests, but from looking through the repo, I don't see how to set Configuration.expandTab to be true while running these tests.

  2. When running the tests locally, I get 56 tests failing, with or without my change. I don't really know what I'm supposed to do there (I thought I would try running the tests on CI, and cool! They passed.).

Cheers,
Horace

@xconverge
Copy link
Member

Hmm yea we are supposed to be ignoring remappings etc when running tests but maybe that is not working so some of your remappings are impacting the tests.

This would be tough to write a test for unless you set the parameter manually and use the old style tests.

Like this one https://github.com/VSCodeVim/Vim/blob/master/test/mode/modeInsert.test.ts#L118

only set Configuration.expandtab = true as the first thing the test does

@Chillee
Copy link
Member Author

Chillee commented Apr 16, 2017

I can't figure out how to set the configuration setting, but it seems like it's already the necessary setting by default, right?

Configuration.expandtab = insertSpaces;

@xconverge
Copy link
Member

ah yea looks like it is set for this case already you are right

@Chillee
Copy link
Member Author

Chillee commented Apr 19, 2017

I have added two new tests, both of which test the new behavior. However, I was running into some issues, and I believe they are related to issues with the test runner.

If you check the new continuous integration, there are 2 new tests, one of which is the other test but with an extra (non-blank) line. The first test works as expected, but the second test fails. I am unable to reproduce the second test's behavior in a non-test environment, and when viewing the running of the test, it seems to display some erratic behavior with regards to the cursor. I suspect this is a bug with the test runner.

@xconverge xconverge merged commit 6efbf92 into VSCodeVim:master Apr 21, 2017
@Chillee Chillee changed the title [WIP] Fixed issue #1513 Fixes #1513: Backspace on middle of whitespace only line fails May 2, 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.

None yet

2 participants