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

Undo is broken #208

Closed
2 tasks done
a-ignatov-parc opened this issue Dec 20, 2018 · 26 comments
Closed
2 tasks done

Undo is broken #208

a-ignatov-parc opened this issue Dec 20, 2018 · 26 comments

Comments

@a-ignatov-parc
Copy link

Please fill-in this template.

  • I have a question that is specific to this extension; thus, inappropriate for the main EditorConfig issue tracker.
  • I tried running code --disable-extensions and the issue did NOT present itself.

Issue

Visual Studio Code editorconfig-vscode
Version 1.3.1 0.12.5

Root .editorconfig File

; top-most EditorConfig file
root = true

[*]
end_of_line = lf
insert_final_newline = true
trim_trailing_whitespace = true
charset = utf-8
indent_style = space
indent_size = 4
max_line_length = 120

[*.{js,jsx}]
quote_type = single

[{scripts/plato,scripts/deps}]
quote_type = single

[*.{json,html}]
quote_type = double

[{package.json,.babelrc,*.yaml}]
indent_size = 2

Are there any other relevant .editorconfig files in your project? No

Visual Studio Code Setting Default User Workspace
editor.insertSpaces true true true
editor.tabSize 4 4 4
editor.trimAutoWhitespace true true true
files.autoSave "off" "off" "off"
files.insertFinalNewline false false false
files.trimTrailingWhitespace false false false

File opened

Any file in the project

Expected behavior

Undo/Redo should work as expected

Actual behavior

After saving files Undo/Redo is broken. I need to press Cmd+Z multiple times to revert the previous change. It's like if additional changes were added to the history.

Additional comments or steps to reproduce

After updating to vscode 1.3.0 I'm started seeing this issue. If I disable editorconfig extension everything starts to work as expected.

Possibly related issue microsoft/vscode#64987 (comment)

@maximelebastard
Copy link

Same on MacOS with the latest vscode update. When I type something, and wait a few seconds doing other things than typing code, I have to press Cmd+Z 4 or 5 times to get what I typed removed.

@jednano
Copy link
Member

jednano commented Dec 20, 2018

It sounds like the issue is that EditorConfig is creating undo history, which is not what you expect?

@jednano
Copy link
Member

jednano commented Dec 20, 2018

Is this issue only related to autosave?

@a-ignatov-parc
Copy link
Author

@jedmao, unfortunately, editorconfig in vscode 1.3.X creates more than one record.

For example, I typed foo. Let's imagine that when I press "Save" nothing is reformated because there were no trailing whitespaces and a new line was at the end of the file. Then I press Cmd+Z and nothing happens. Only on next "Undo" foo is deleted. And this is the simplest case.

Sometimes you need to press Cmd+Z twice or even more to start reverting what you've coded.

P.S. autosave is disabled in my config.

@jednano
Copy link
Member

jednano commented Dec 20, 2018

Thanks for the scenario. What if you do have trailing whitespaces though? What would your expectation be then?

I can't say I've run into this myself and I use the plugin every day, but maybe I'm just not sensitive to this behavior. I'll look into it.

@a-ignatov-parc
Copy link
Author

If I type foo with trailing whitespaces it gets removed on save and again there is one extra press before trailing whitespaces is restored and we can do normal undo.

@a-ignatov-parc
Copy link
Author

The worst part of this issue is that redo is broken too.

Here is a case:

  1. asd
  2. asd asd
  3. asd asd asd
  4. Save
  5. Cmd+Z → asd asd asd
  6. Cmd+Z → asd asd
  7. Save
  8. Cmd+Shift+Z → asd asd

And redo history is totally lost at that moment.

@jednano
Copy link
Member

jednano commented Dec 20, 2018

It sounds like removing undo history entirely might solve the problem. Does that sound accurate?

@a-ignatov-parc
Copy link
Author

Sounds good 👍

@jednano
Copy link
Member

jednano commented Dec 26, 2018

@a-ignatov-parc I think microsoft/vscode#65663 has the potential to solve this issue if I can just remove the manual processes that EditorConfig is doing and defer to built-in editor functionality. Let's keep an eye on that issue and see what comes of it, then I can do a follow-up in this project to fix.

@a-ignatov-parc
Copy link
Author

@jedmao sure! Thank you for your time!

@Tyriar
Copy link

Tyriar commented Jan 6, 2019

I'm hitting this and it makes the extension unusable so I need to uninstall. Here's my root config file:

root = true

[*]
charset = utf-8
end_of_line = lf
indent_style = space
indent_size = 2
insert_final_newline = true
trim_trailing_whitespace = true

@jedmao I don't totally understand why you need a new API for this? The editorconfig extension only recently started doing this and my guess is it's happening because you're doing edit(s) which result in the exact same content.

@jednano
Copy link
Member

jednano commented Jan 6, 2019

Right, it only recently started doing this because vscode only recently introduced some new editor rules that overlap with EC. The new API would allow me to remove some edits entirely and let vscode do it all in one operation.

@Tyriar
Copy link

Tyriar commented Jan 6, 2019

But vscode doesn't seem to be making any edits? If it were, the redo stack would break when the extension was disabled, no?

@jednano
Copy link
Member

jednano commented Jan 7, 2019

That's kind of my point. Vscode doesn't do edits, so I can "hitch a ride" so to speak on that process. I can't do it separately, because vscode has the potential to undo what I've done via user/workspace settings. I think microsoft/vscode#65663 describes the issue as much as I can.

@Tyriar
Copy link

Tyriar commented Jan 7, 2019

@jedmao my point is, AFAICT for me that both editor config and vscode settings align, so that would mean the problem is editor config is being naughty?

@jednano
Copy link
Member

jednano commented Jan 7, 2019

For you, sure, but the application of rules still needs to be consolidated with the main save operation.

@jednano
Copy link
Member

jednano commented Jan 7, 2019

EditorConfig uses the exposed API to submit a series of TextEdit operations on save. It's the blessed way that vscode has provided for extensions, so I don't think it's being naughty. Even if it were naughty, it wouldn't make much sense to spend too much time developing a solution that doesn't fix the other related problems in the same stroke, which is what I'm trying to do here. Need to be efficient. That said, feel free to submit a PR of you feel it's more urgent.

@Tyriar
Copy link

Tyriar commented Jan 7, 2019

@jedmao if both VS Code and Editor config settings align, as in my case, both VS Code and Editor Config should do nothing (0 TextEdit operations), and therefore not invalidate the redo stack? Am I missing something?

I see this in the output channel:

src\common\Lifecycle.test.ts: setEndOfLine(LF)
src\common\Lifecycle.test.ts: editor.action.trimTrailingWhitespace

Running editor.action.trimTrailingWhitespace via the command palette does not clear the redo stack.

Running set end of line does (editor says CRLF, .editorconfig says LF), maybe that's a different issue, but this doesn't explain why this would happen on Linux and macOS though.

@Tyriar
Copy link

Tyriar commented Jan 7, 2019

Ok, that's the issue for me:

end_of_line = lf

setEndOfLine(LF) will invalidate the redo stack regardless of whether or not the old EOL format was LF or CRLF.

@Tyriar
Copy link

Tyriar commented Jan 7, 2019

Strangely when calling setEndOfLine(LF) when the editor is set to LF will only invalidate the redo stack when it happens via the extension, not the command palette.

Redo works:

  1. Get some redo
  2. Run Change End of Line Sequence via command palette, LF
  3. cmd+shift+z

Redo does not work:

  1. Get some redo
  2. cmd+s
  3. cmd+shift+z

@jednano
Copy link
Member

jednano commented Jan 8, 2019

@Tyriar everything you're saying is consistent with what I was saying before. You see this line of code? A TextEdit is created to set the end of line. This is the blessed way that vscode wants extension developers to make these kinds of adjustments.

The only way I could imagine to improve this would be to actually look at each line in the text document and see if any lines are NOT consistent with the end_of_line setting and only then would we create a TextEdit, but I don't think that's a great idea, because it's doing too much.

I'm not surprised the command palette works for you, because it uses the built-in trim command, which is exactly what I intend on this plugin using once microsoft/vscode#65663 is addressed.

Bottom line – I absolutely can't do this correctly w/o changes to vscode itself. vscode is the one reading the TextEdit and doing whatever they are doing behind the scenes. If that's inconsistent w/ the command palette then that's on them.

Could I hack something together? Probably, but I'm not comfortable with that solution. I'd rather see the root of the problem solved than get into that territory.

setEndOfLine(LF) will invalidate the redo stack regardless of whether or not the old EOL format was LF or CRLF.

To be clear for everyone reading, TextEdit.setEndOfLine(LF) is a vscode operation.

@jednano
Copy link
Member

jednano commented Jan 8, 2019

@jedmao I don't totally understand why you need a new API for this? The editorconfig extension only recently started doing this and my guess is it's happening because you're doing edit(s) which result in the exact same content.

@Tyriar I just wanted to point out that EditorConfig has been using the same method for years, so if this only recently started happening, it's most likely not EditorConfig's fault. I think it was introduced the moment vscode introduced those new command palette operations that overlap with EditorConfig settings.

Also, your GitHub says you're on the vscode team? Are you working on the vscode side of this issue or is that someone else?

@Tyriar
Copy link

Tyriar commented Jan 8, 2019

everything you're saying is consistent with what I was saying before. You see this line of code? A TextEdit is created to set the end of line. This is the blessed way that vscode wants extension developers to make these kinds of adjustments.

Thanks for the link and info, I think this is the underlying issue: microsoft/vscode#66214

Also, your GitHub says you're on the vscode team? Are you working on the vscode side of this issue or is that someone else?

This isn't my area but I'm trying to expedite getting it fixed so I can use the extension again 😛

@Tyriar
Copy link

Tyriar commented Jan 9, 2019

Provided the fix for microsoft/vscode#66214 works in tomorrow's build, this issue is fixed for me.

@jednano jednano closed this as completed Jan 10, 2019
@philippejer
Copy link

Please note that this issue has been reintroduced recently (around october 2020) and has just been fixed in the insiders build of VSCode (see microsoft/vscode#110141).

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

No branches or pull requests

5 participants