Skip to content

Clarify snippet escaping rules #1868

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

Merged
merged 2 commits into from
Jan 8, 2024

Conversation

DanTup
Copy link
Contributor

@DanTup DanTup commented Dec 19, 2023

The current text reads like you can escape $ and } where not strictly necessary, but according to VS Code's behaviour and microsoft/vscode#201059 this is not the case - you may only escape the characters that are required to be escaped, otherwise you'll see backslashes in the output.

VS Code Docs PR: microsoft/vscode-docs#6928

The current text reads like you can escape $ and } where not strictly necessary, but according to VS Code's behaviour and microsoft/vscode#201059 this is not the case - you may only escape the characters that are _required_ to be escaped, otherwise you'll see backslashes in the output.
@dbaeumer
Copy link
Member

dbaeumer commented Jan 8, 2024

Thanks!

@dbaeumer dbaeumer enabled auto-merge (squash) January 8, 2024 18:23
@vscodenpa vscodenpa added this to the December / January 2024 milestone Jan 8, 2024
@dbaeumer dbaeumer merged commit ebd8936 into microsoft:gh-pages Jan 8, 2024
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Jan 11, 2024
We shouldn't escape $ or } in snippet choices because they are not ambiguous - VS Code would show unwanted backslashes.

LSP spec was clarified here: microsoft/language-server-protocol#1868

Fixes #54403

Change-Id: If1134f067a8a73fe5ae678cadfbd2c284913d6c9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345661
Commit-Queue: Phil Quitslund <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Phil Quitslund <[email protected]>
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 this pull request may close these issues.

4 participants