Trim surrounding whitespace from line comment token#316763
Open
verifizieren wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR normalizes configured line-comment markers by trimming surrounding whitespace at language configuration resolution time, fixing Toggle Line Comment behavior for whitespace-padded tokens.
Changes:
- Trims string and object-form
lineCommenttokens inResolvedLanguageConfiguration. - Adds a regression test for toggling a whitespace-padded line comment token.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/vs/editor/common/languages/languageConfigurationRegistry.ts |
Normalizes resolved line comment tokens with trim(). |
src/vs/editor/contrib/comment/test/browser/lineCommentCommand.test.ts |
Adds regression coverage for toggling a whitespace-padded token. |
A line comment token configured with leading or trailing whitespace
(e.g. " #") broke Toggle Line Comment: the inserted comment could
never be detected again, so each toggle stacked another comment
instead of removing it.
Whitespace around the marker is not semantically meaningful - spacing
on insertion is controlled by `editor.comments.insertSpace` - so the
token is now trimmed when the comment configuration is resolved. This
also keeps the `${LINE_COMMENT}` snippet variable and notebook inline
variable detection consistent. No built-in language is affected.
Fixes microsoft#249958
dd9448d to
77a8508
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #249958
Problem
When a language configures its
lineCommenttoken with leading or trailing whitespace (e.g." #"), Toggle Line Comment stops toggling: instead of removing an existing comment it keeps stacking new ones (# # # # #).Root cause
In
LineCommentCommand._analyzeLines, the comment is located atfirstNonWhitespaceIndex(lineContent). When the configured token itself begins with whitespace, the inserted comment's leading space is that whitespace, so the detection offset points past it and the needle never matches — the command can never recognize (and therefore never remove) a comment it just inserted.Fix
Trim the line comment token where the comment configuration is resolved (
ResolvedLanguageConfiguration._handleComments). Whitespace around a comment marker is not semantically meaningful — insertion spacing is controlled separately by theeditor.comments.insertSpacesetting — so normalizing it at the source keeps every consumer consistent: comment toggling, the${LINE_COMMENT}snippet variable, and notebook inline-variable detection.This is intentionally a central, minimal fix rather than special-casing the toggle logic. The alternative (handle a whitespace-padded token inside
_analyzeLines) is more code, more blast radius in hot editor logic, and would leave the snippet/notebook consumers still broken — but I'm happy to switch approaches if a maintainer prefers that.Safety
lineCommentvalues (//,#,--,;,',%,@REM, …) have no surrounding whitespace, so no built-in language is affected."", which the existingif (!commentStr)guards already handle gracefully (treated as "no line comment").Tests
Added a regression test in
lineCommentCommand.test.tsasserting toggle reversibility (add, then remove) with a space-padded token.Verified locally (headless Chromium, transpiled build):
Editor Contrib - Line Comment Command— 40 passing, including the new#249958case, no regressionsEditor Contrib - Block Comment Command— 12 passing (related suite sharing the same comment-config resolution), no regressions