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

files.trimTrailingWhitespace does not trim trailing whitespace on save #299

Closed
chrisjaquet opened this issue Aug 4, 2020 · 5 comments · Fixed by #456
Closed

files.trimTrailingWhitespace does not trim trailing whitespace on save #299

chrisjaquet opened this issue Aug 4, 2020 · 5 comments · Fixed by #456
Milestone

Comments

@chrisjaquet
Copy link

The setting files.trimTrailingWhitespace does not seem to trim trailing whitespace when a file is saved. Is this a deliberate decision?

I do not want to format the entire document when saving (using e.g. editor.formatOnSave) as this will affect other file types as well (and I occasionally use custom formatting for clarity).

Extension Details
Name: XML
Id: redhat.vscode-xml
Version: 0.13.0

VSCode Details
Version: 1.47.3 (user setup)
Commit: 91899dcef7b8110878ea59626991a18c8a6a1b3e
Date: 2020-07-23T13:12:49.994Z
Electron: 7.3.2
Chrome: 78.0.3904.130
Node.js: 12.8.1
V8: 7.8.279.23-electron.0
OS: Windows_NT x64 10.0.18363

@angelozerr
Copy link
Contributor

The setting files.trimTrailingWhitespace does not seem to trim trailing whitespace when a file is saved. Is this a deliberate decision?

files.trimTrailingWhitespace format settings has the same behavior than other XML format settings, in other words format is done when you execute it (format document or range formatting). We use files.trimTrailingWhitespace to avoid creating a new settings like xml.format.trimTrailingWhitespace.

But I agree with you, it's strange because vscode description says that it execute this format on save:

image

Perhaps we should have the two formatting settings:

  • xml.format.trimTrailingWhitespaceNoSave (trimTrailingWhitespace without save like today)
  • files.trimTrailingWhitespace (trimTrailingWhitespace when it is saved)

@fbricon @xorye @datho7561 what do you think about that?

@datho7561
Copy link
Contributor

I think it makes sense to be consistent with other language servers/language support. I've checked TypeScript and Java files, and trailing white space is always removed by the formatter. Both these services also remove the trailing whitespace when files.trimTrailingWhitespace is set and the document is saved.

@chrisjaquet
Copy link
Author

Thanks for the feedback. I agree with @datho7561 in that consistency with other languages will prevent user confusion. The main thing that caught me was that the setting (as shown in the screenshot @angelozerr highlighted) did not have an effect when saving files but VSCode indicated that it should.

I do not have an opinion on whether or not there is a separate setting to strip trailing whitespace only while formatting (since in my use case it will be stripped when I save anyway) but I am used to formatting stripping trailing spaces as part of the formatting process in the languages/file types I use.

@chrisjaquet
Copy link
Author

Any updates on this issue? I know it seems trivial but other members in my team are running into the issue now as well.

@datho7561
Copy link
Contributor

The quick way to force trim trailing whitespace on save is to add this to your settings.json:

"[xml]": {
  "files.trimTrailingWhitespace": true
}

This should override the behaviour from vscode-xml. I don't think we should have this behaviour anyways, so I'll make a quick PR to remove it.

datho7561 added a commit to datho7561/vscode-xml that referenced this issue Apr 20, 2021
Do not set `files.trimTrailingWhitespace` to `false` in `package.json`,
so that it instead defaults to whatever the user sets it to.

Closes redhat-developer#299

Signed-off-by: David Thompson <[email protected]>
fbricon pushed a commit that referenced this issue Apr 20, 2021
Do not set `files.trimTrailingWhitespace` to `false` in `package.json`,
so that it instead defaults to whatever the user sets it to.

Closes #299

Signed-off-by: David Thompson <[email protected]>
@fbricon fbricon added this to the 0.17.0 milestone Apr 20, 2021
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 a pull request may close this issue.

4 participants