-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
Format completion snippet text before escaping #48793
Conversation
return textChanges.applyChanges(syntheticFile.text, changes); | ||
|
||
const allChanges = escapes | ||
? stableSort(concatenate(changes, escapes), (a, b) => compareTextSpans(a.span, b.span)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this might be a dumb question, but is there a chance that we have overlapping text changes, and if so, will this sorting make sure everything works correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overlapping edits can defeat the sorting, but since the formatter deals only in whitespace and semicolons, and the characters we need to escape are only part of tokens, I don’t think it can ever be a problem in practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, though I sort of worry about someone adding a new function to this and forgetting to clear escapes
; right now it's fine since both functions call printUnescapedSnippetList
, though.
* Format snippet text before escaping * Reset `escapes` before printing so printer can be reused
Fixes #48139
Fixes #48215
I briefly tried to make the formatting scanner and/or actual scanner tolerant of
\$
and\\
, but it quickly became apparent that this approach would be easier.