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

trim_trailing_whitespace = false not working #153

Open
jibbers42 opened this issue May 31, 2017 · 34 comments

Comments

@jibbers42
Copy link

commented May 31, 2017

  • 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.

Delete the following condition if it doesn't apply to your case:

If the extension is not picking up the expected configuration for a file:

  • I tried npm install editorconfig -g and ran editorconfig [file-in-question] and the configuration was what I expected. If not, please file on the editorconfig-core-js issue tracker.

Issue

Visual Studio Code editorconfig-vscode
Version 1.12.2 0.9.3

Root .editorconfig File

# Editor configuration, see http://editorconfig.org
root = true

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

[*.md]
max_line_length = off
trim_trailing_whitespace = false

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

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

File opened

./README.md

Expected behavior

trim_trailing_whitespace = false

Actual behavior

trim_trailing_whitespace = true

Additional comments or steps to reproduce

When I add 2 spaces to the end of a line to create a markdown line break and then save the file, the trailing spaces are removed.
OS: Ubuntu 17.04

@mjsarfatti

This comment has been minimized.

Copy link

commented Jun 1, 2017

Same here, OS: macOS Sierra

@af4jm

This comment has been minimized.

Copy link

commented Jun 1, 2017

same here, on both Win10 & Win7, latest VSCode (not Insiders) & latest EditorConfig plugin

@bleuarg

This comment has been minimized.

Copy link

commented Jul 14, 2017

Found the workaround of removing `"files.trimTrailingWhitespace" from user/workspace settings. This way the extension seem to work properly.

@S2-

This comment has been minimized.

Copy link

commented Jul 14, 2017

i like my editor to trim whitespace.
the only problem i have is with markdown files, where whitespace at the end is used for a newline.
to workaround the problem i added this in vscode settings.json

    "files.trimTrailingWhitespace": true,
    "[markdown]": {
        "files.trimTrailingWhitespace": false
    }
@pjsvis

This comment has been minimized.

Copy link

commented Nov 18, 2017

files.trimTrailingWhitespace could do with a debounce setting to allow a bit of thinking time when typing. Often I find the trailing whitespace is trimmed before I finish typing a line.

@RuneMolin

This comment has been minimized.

Copy link

commented Nov 27, 2017

^^^ +1 this. It is so effing annoying

@segevfiner

This comment has been minimized.

Copy link

commented Feb 4, 2018

I guess that to truly fix this someone will have to request an option to override files.trimTrailingWhitespace per TextEditor. Something like adding it as a new property to TextEditorOptions.

A pluasible fix for this, without changing VS Code, can be for this extension to offer its own trim trailing whitespace setting, that will apply when the value in editorconfig is unset and request from the user to unset or offer to automatically unset the VS Code setting for the user.

P.S. Wouldn't the same happen with insert_final_newline and files.insertFinalNewline and files.trimFinalNewlines?

@dietergeerts

This comment has been minimized.

Copy link

commented Mar 13, 2018

@S2- Your proposed solution does not work in the latest VSCode. (1.21.0)

So this plugin doesn't set the setting correctly then. Any idea on when this will be fixed?

@S2-

This comment has been minimized.

Copy link

commented Mar 13, 2018

@dietergeerts still works for me on Version 1.21.0
Commit 9a199d77c82fcb82f39c68bb33c614af01c111ba
Date 2018-03-07T11:04:09.969Z
Shell 1.7.9
Renderer 58.0.3029.110
Node 7.9.0
Architecture x64

@jedmao

This comment has been minimized.

Copy link
Member

commented Mar 13, 2018

@pjsvis that's a great idea. You should submit that to vscode issues.

@S2- your solution sounds like a good fix for this, considering the user settings overrides the EditorConfig setting.

Everyone else, I think a good fix here would be to add the following to your workspace settings to undo the user setting:

    "files.trimTrailingWhitespace": false,

You're basically telling the editor, "Don't worry about trimming trailing whitespace on this project, because I have an EditorConfig configuration that takes care of that."

For all other projects w/o an EditorConfig configuration, your user setting will still apply!

What do you think?

@dietergeerts

This comment has been minimized.

Copy link

commented Mar 19, 2018

@S2- nope, it's not:

  • editorconfig plugin enabled
  • editorconfig file:
root = true

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

[{*.html,*.hbs,*.json,*.yml}]
indent_size = 2

[{*.md}]
trim_trailing_whitespace = false
  • workspace settings:
{
    "html.format.wrapAttributes": "force-aligned",
}
  • user settings:
{

}

VSCode removes trailing whitespaces from .md files

Even with these workspace settings:

{
    "files.trimTrailingWhitespace": false,
    "html.format.wrapAttributes": "force-aligned",
}

And with these workspace settings:

{
  "files.trimTrailingWhitespace": true,
    "[markdown]": {
        "files.trimTrailingWhitespace": false
    },
  "html.format.wrapAttributes": "force-aligned",
}

THE only solution right now is to not use this editorconfig plugin and add all the settings from it to the workspace settings:

{
  // Settings to mimic `.editorconfig` as the plugin has bugs on trim trailing whitespace, 
  // see https://github.com/editorconfig/editorconfig-vscode/issues/153
  "files.encoding": "utf8",
  "editor.insertSpaces": true,
  "editor.tabSize": 4,
  "editor.detectIndentation": false,
  "files.eol": "\n",
  "files.insertFinalNewline": true,
  "files.trimTrailingWhitespace": true,
  "[javascript]": {
    "editor.tabSize": 2,    
  },
  "[handlebars]": {
    "editor.tabSize": 2,    
  },
  "[json]": {
    "editor.tabSize": 2,    
  },
  "[yaml]": {
    "editor.tabSize": 2,    
  },
  "[markdown]": {
    "editor.tabSize": 2,
    "files.trimTrailingWhitespace": false
  },

  // Other project settings
  "html.format.wrapAttributes": "force-aligned",
}
@jedmao

This comment has been minimized.

Copy link
Member

commented Mar 19, 2018

@dietergeerts try:

[*.md]
trim_trailing_whitespace = false
@dietergeerts

This comment has been minimized.

Copy link

commented Jun 7, 2018

@jedmao , why should that matter? [{*.md}] and [*.md] should be the same.

@jedmao

This comment has been minimized.

Copy link
Member

commented Jun 7, 2018

I 100% agree, but the tests don't cover this scenario, so it's not exactly a supported feature. The {} braces are for multiple matches, so there's really no reason to use them unless you're trying to target multiple files or extensions like:

[{*.md,*.yml}]

☝️ that works. You know what also works?

[{*.md,}]

So you can either add this scenario to the tests or just use [{*.md,}] or [*.md] for now. Up to you!

@dietergeerts

This comment has been minimized.

Copy link

commented Nov 11, 2018

If the code was programmed correctly, it should not matter if the singular or plural version was used, as that's a feature of editorconfig….

@jedmao

This comment has been minimized.

Copy link
Member

commented Nov 11, 2018

This is not an issue with this extension, but with the core and the tests written against it.

@tats-u

This comment has been minimized.

Copy link

commented Dec 10, 2018

My similar case is:

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.29.1 0.12.5

Root .editorconfig File

root = true

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

[.editorconfig]
insert_final_newline = true

[*.ps1]
end_of_line = crlf
charset = utf-8-bom
indent_size = 4
insert_final_newline = true

[*.py]
indent_size = 4
insert_final_newline = true

[R*[ab]/file_install_key.txt]
insert_final_newline = false

[*.yml]
indent_size = 2
insert_final_newline = true

[*.md]
trim_trailing_whitespace = false
insert_final_newline = true

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

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

File opened

./README.md

Expected behavior

trim_trailing_whitespace = false

Actual behavior

trim_trailing_whitespace = true

Additional comments or steps to reproduce

@jedmao

This comment has been minimized.

Copy link
Member

commented Dec 11, 2018

If the code was programmed correctly, it should not matter if the singular or plural version was used, as that's a feature of editorconfig….

@dietergeerts it's actually not a feature of EditorConfig. Show me the test that matches that scenario.

@tats-u this appears to be a weird vscode issue in their order of operations, if you will. They are allowing user settings to trump plugin settings. As such, I strongly suggest you short circuit that operation with an md-specific vscode rule until they change the order of operations (if that ever even happens).

{
    "files.trimTrailingWhitespace": true,
    "[markdown]": {
        "editor.wordWrap": "on",
        "editor.quickSuggestions": false,
        "files.trimTrailingWhitespace": false
    }
}

You should probably put ☝️ in your user settings anyway, for projects that don't have an .editorconfig file.

@tats-u

This comment has been minimized.

Copy link

commented Dec 11, 2018

@jedmao
In terms of files.trimTrailingWhiteSpace, I've already found and applied it.

@jedmao

This comment has been minimized.

Copy link
Member

commented Dec 11, 2018

@tats-u do you need any more help then?

@tats-u

This comment has been minimized.

Copy link

commented Dec 12, 2018

@jedmao No, I'm just waiting for the fix of this issue.

@jedmao

This comment has been minimized.

Copy link
Member

commented Dec 12, 2018

You might be waiting a couple years for vscode to fix it on their end. In the meantime, does my solution above solve your immediate needs?

@tats-u

This comment has been minimized.

Copy link

commented Dec 12, 2018

@jedmao Yes, I'm not bothered by this bug now.

@dietergeerts

This comment has been minimized.

Copy link

commented Dec 18, 2018

@jedmao , the docs state that you can have multiple file extensions, and that you can use the , …. A test not being there doesn't mean it's not supported..... (though every feature should be supported though)

@jedmao

This comment has been minimized.

Copy link
Member

commented Dec 18, 2018

@dietergeerts I never said you can't use a ,. The issue is using the {} without also using a ,.

@jedmao

This comment has been minimized.

Copy link
Member

commented Dec 19, 2018

@dietergeerts your issue has nothing to do with this extension, specifically. In fact, it's not even related to editorconfig-core-js. I'm not sure where you're getting the idea that this is a feature of EditorConfig. Please direct me to the documentation that states this feature or request a new EditorConfig feature here.

That said, I'm 100% positive that what you're asking for isn't covered by the core tests, so if that's an issue for you, feel free to file it here.

EditorConfig is an interesting project, because it's really just a specification for which many languages have a core implementation. Those different language implementations are verified all by a common suite of core tests. If the tests pass, the language-specific core is verified as an official core. Editor-specific plugins/extensions use these cores to power their features.

Specification -> Tests -> Core -> Plugin

Many times, issues are reported at the plugin level, when they should be reported at a higher level of the core, the tests or even the specification itself.

Does this clear things up?

@tats-u

This comment has been minimized.

Copy link

commented Dec 21, 2018

Do you really belive this issue is not due to this extension? If so, why don't you send an issue to VSCode itself? If you have already sent one, paste the URL of it.

@dietergeerts

This comment has been minimized.

Copy link

commented Dec 21, 2018

I also wonder why JetBrains can get it right then... It's not because it would not be like 'officially' supported that this plugin can't make that work. Plugins always should keep different scenarios in mind and make sure they work in a lot of scenarios, even the not so default ones...

@tats-u

This comment has been minimized.

Copy link

commented Dec 21, 2018

@dietergeerts JetBrains are a commercial enterprise and IntelliJ is a (paid) product, unlike these.

I think that's why VSCode doesn't allow extensions to overwrite configurations like files.trimAutoWhiteSpace to false.

@jedmao

This comment has been minimized.

Copy link
Member

commented Dec 21, 2018

I don't believe vscode is doing this on purpose. There are three EditorConfig settings that execute on the workspace.onWillSaveTextDocument event.

  • SetEndOfLine()
  • TrimTrailingWhitespace()
  • InsertFinalNewline()

They do their respective jobs correctly, but it doesn't matter, because vscode then executes the built-in save operation, which takes into account the workspace and user settings. In this case, that means they trim trailing whitespace after EditorConfig is done not trimming the whitespace.

I don't think it matters if this is a pre-save operation or a post-save operation, because once that whitespace is gone, it's gone. EditorConfig isn't going to add it back. So the issue that remains is that we need a way to short-circuit the built-in save operation to defer to the EditorConfig settings instead of only consulting workspace / user settings.

This is another reason why EditorConfig being built-in to the editor (as it is in other editors) makes a lot of sense.

Unless there are some new APIs that allow us to do this, by all means, point me in the right direction. As for the last time I looked into this, there wasn't really a solution. I don't think it's a vscode bug per se, but it could be a good new feature request to ask for a way to short-circuit the built-in save operation somehow.

@jedmao

This comment has been minimized.

Copy link
Member

commented Dec 25, 2018

Do you really belive this issue is not due to this extension? If so, why don't you send an issue to VSCode itself? If you have already sent one, paste the URL of it.

microsoft/vscode#65663

@tats-u

This comment has been minimized.

Copy link

commented Dec 25, 2018

@jedmao Thank you for your cooperation. Hmm, the API of Code is too limited...

@jedmao

This comment has been minimized.

Copy link
Member

commented Dec 26, 2018

@tats-u to be fair, the vscode API is limited for good reasons. It would be a wild mess if extension authors could all be messing with built-in save operations. They have to be smart about how they expose APIs to these internals and I think they've done that for the most part. Our job is to make them aware of this particular issue and maybe, if the stars align, EditorConfig support can be baked into vscode. Wouldn't that be nice!

@clystian

This comment has been minimized.

Copy link

commented Jun 27, 2019

some update? can be closed? some ultimate and definitive fix for this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.