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 auto-indent on paste, second line and after #83386

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

eliot-akira
Copy link

@eliot-akira eliot-akira commented Oct 27, 2019

This PR fixes #32320, fixes #31255.

In the method AutoIndentOnPaste.trigger, there's a logic to re-indent pasted lines based on editor tab/indent size (and optionally editor.detectIndentation). At the end of that calculation is a loop that re-indents the second line and after: it was adding unnecessary indents if any of the original lines had no indentation.

To illustrate, pasting the following:

<div>
  <i></i>
</div>

..was resulting in:

<div>
    <i></i>
  </div>

The only way for the user to fix this when it happened, was to undo once after pasting.

This PR adds a check to handle that case, by not adding indent if a pasted line had no indent to start with. In the above example, that's the third line.

It was somewhat tricky to reproduce the issue - although many people have reported variations of the symptoms in HTML, CSS, JS, etc. The common precondition seems to be when the tab/indent sizes detected for the document are different from the ones detected from the pasted lines. That leads to the loop that re-indents the second line and after.

@eliot-akira
Copy link
Author

The failing integration tests are due to an unrelated network issue in the CI pipeline itself, it seems.

@eliot-akira
Copy link
Author

eliot-akira commented Oct 28, 2019

I've been trying to find a reliable way to reproduce the issue that this PR attempts to fix.

On a clean install with default settings (editor.autoIndent and .detectIndentation set true, and .tabSize at 4):

  • Create new file

  • Enter

    <div>
      test
    </div>
  • Save as test.html, close and reopen.

    Sometimes this step is not necessary(?), especially when .detectIndentation is false

  • Copy & paste results in:

    <div>
        test
      </div>
  • A single undo operation corrects the indentation back to original

Notably, in this example the editor's default tab size 4 is different from the pasted block's detected indent 2. I believe this is related to the issue.

At the beginning of AutoIndentOnPaste.trigger, there's a call to model.getOptions (which leads to textModel.resolveOptions) that gets the "starting" indent size. At the end of the auto-indent logic, since the pasted block's indent is different, it goes into the "reindent second line and after" loop - which adds unnecessary indent (the last line of the pasted block in the example).

With the proposed change in this PR, the paste produces the expected result:

<div>
    test
</div>

I hope that helps clarify the issue and the motivation/aim of the PR. For more reported examples, please see issue #32320.


EDIT: I found that the proposed change in this PR is not the right solution (yet).

If the pasted block is itself indented, the change will remove it.. This:

    <div>
        test
    </div>

..when pasted becomes:

<div>
    test
</div>

So, I'll need to rethink this - I had a feeling this "solution" was too simple to be true. :) It seems the check needs to be relative to the starting indent size of the pasted block.

OK, I believe I got it - please see next commit.

@eliot-akira
Copy link
Author

Nope, those commits are not correct either. OK, I'll continue trying to solve this.

@eliot-akira eliot-akira changed the title Prevent re-indenting lines with no indent Fix auto-indent on paste, second line and after Oct 31, 2019
@alexdima alexdima assigned rebornix and unassigned alexdima Feb 4, 2020
@rebornix rebornix added bug Issue identified by VS Code Team member as probable bug editor-autoindent Editor auto indentation issues labels Oct 8, 2020
@rebornix rebornix removed the bug Issue identified by VS Code Team member as probable bug label Nov 11, 2020
@rebornix
Copy link
Member

@eliot-akira thanks for your contribution and great effort! I love reading your thoughts and analysis into this problem and indeed it's really hard. May I ask what's the current state of this pr now?

@eliot-akira
Copy link
Author

@rebornix Thank you for reading over this prior effort. To be honest, during the last few commits I wasn't sure anymore if I was going in the right direction. I think this PR is too out of date to pursue further, and can be closed.

But, I believe it was getting closer to the crux of the issue (at least a variant of it), that it's related to the logic of "reindent second line and after".

@joaomoreno joaomoreno changed the base branch from master to main February 15, 2021 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editor-autoindent Editor auto indentation issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Paste indents twice with HTML formatting Autoindent on paste causes wrong indentation (JavaScript)
3 participants