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 shouldn't affect multiline strings #195010

Closed
Tracked by #195678
kanishkbh opened this issue Oct 6, 2023 · 3 comments · Fixed by #198164
Closed
Tracked by #195678

Trim Trailing Whitespace shouldn't affect multiline strings #195010

kanishkbh opened this issue Oct 6, 2023 · 3 comments · Fixed by #198164
Assignees
Labels
editor-core Editor basic functionality feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@kanishkbh
Copy link

This is regarding the files.trimTrailingWhitespace setting.
While it is possible to set it to False for specific files e.g. markdown, there often cases where we have multiline strings in a code file.

for example:

st.info("""
Line1  
Line2
""")

When files.trimTrailingWhitespace is set to True, it would remove the two whitespaces after Line1, which breaks their markdown rendering as two separate lines.

Would be great to have this setting ignore multilines strings.

@vscodenpa vscodenpa added the stale Issues that have not been triaged in an appropriate amount of time label Oct 13, 2023
@lszomoru lszomoru assigned hediet and unassigned Tyriar and lszomoru Oct 16, 2023
@vscodenpa vscodenpa removed triage-needed stale Issues that have not been triaged in an appropriate amount of time labels Oct 16, 2023
@streetbooy16 streetbooy16 mentioned this issue Oct 16, 2023
@hediet hediet added feature-request Request for new features or functionality diff-editor Diff editor issues labels Oct 16, 2023
@hediet hediet added this to the Backlog milestone Oct 16, 2023
@coreyward
Copy link

+1 — I am running into this as well. I'm writing a test for a function that is expected to handle user input (multiline string) and format it, and I need to test the behavior when there are trailing spaces. I can save the file without formatting, but that's a bit hard to maintain.

@hediet hediet added editor-core Editor basic functionality and removed diff-editor Diff editor issues labels Nov 6, 2023
@vuon9
Copy link
Contributor

vuon9 commented Nov 7, 2023

When working with extensive files and many members, this issue can be quite annoying. Adding an option to enable/disable this behavior would provide more control within the workspace scope.

@333fred
Copy link
Contributor

333fred commented Nov 13, 2023

This is particularly bad when writing tests for parsers and other things where the string content is extremely important. I generally do want to have my trailing whitespace trimmed, but if it can be known syntactically (ie, from VSCode's grammar files, involving no LSP) that the current location is inside a string, VSCode should be able to avoid trimming the trailing content.

@alexdima alexdima assigned alexdima and unassigned hediet Mar 15, 2024
@alexdima alexdima modified the milestones: Backlog, March 2024 Mar 15, 2024
alexdima added a commit that referenced this issue Mar 16, 2024
* Do not trim whitespace when part of strings or regexes

Fixes #195010. If the token to be removed is part of a string or a regex, then we do not want to remove the character, as it is very likely semantically important.

* Address initial feedback:

1. Add a new setting to control whether to trim whitespace in multiline strings/regexes. This setting is then piped through to everywhere that is calling trim whitespace, which is file save editing, the trim whitespace command, and notebooks.
2. Force line tokenization to complete when the setting is enabled so that trim is accurate.

* Don't force tokenization; instead, check to see if there are tokens for a given line and do not format if there are none.

* Look for syntactical tokens

* Fix compilation errors

* Add a test

---------

Co-authored-by: Alex Dima <alexdima@microsoft.com>
@vscodenpa vscodenpa added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Mar 16, 2024
chen-ky pushed a commit to chen-ky/vscode that referenced this issue Mar 18, 2024
)

* Do not trim whitespace when part of strings or regexes

Fixes microsoft#195010. If the token to be removed is part of a string or a regex, then we do not want to remove the character, as it is very likely semantically important.

* Address initial feedback:

1. Add a new setting to control whether to trim whitespace in multiline strings/regexes. This setting is then piped through to everywhere that is calling trim whitespace, which is file save editing, the trim whitespace command, and notebooks.
2. Force line tokenization to complete when the setting is enabled so that trim is accurate.

* Don't force tokenization; instead, check to see if there are tokens for a given line and do not format if there are none.

* Look for syntactical tokens

* Fix compilation errors

* Add a test

---------

Co-authored-by: Alex Dima <alexdima@microsoft.com>
@deepak1556 deepak1556 added the verification-needed Verification of issue is requested label Mar 25, 2024
@dbaeumer dbaeumer added the verified Verification succeeded label Mar 25, 2024
@microsoft microsoft locked and limited conversation to collaborators Jun 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
editor-core Editor basic functionality feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.