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

Escaping closing brackets in snippet choices results in backslashes being inserted #201059

Closed
DanTup opened this issue Dec 16, 2023 · 8 comments
Closed
Assignees
Labels

Comments

@DanTup
Copy link
Contributor

DanTup commented Dec 16, 2023

In the docs here it says:

Below is the EBNF (extended Backus-Naur form) for snippets. With \ (backslash), you can escape $, }, and \. Within choice elements, the backslash also escapes comma and pipe characters.

My understanding of this is that } can be escaped with a \ for all kinds of snippets.

However, if I create a snippet that uses choices and escapes a bracket, the backslash appears as-is in the choice:

const doc = await vscode.workspace.openTextDocument({ content: '' });
const editor = await vscode.window.showTextDocument(doc);
const snippet = new vscode.SnippetString("a(${1|{optionA\\})>,optionB|}");
await editor.insertSnippet(snippet, new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 0)));

image

If it is not allowed to escape } for choices, then I think it should be clearer from the docs (and the same docs in LSP).

Note: I can't use the appendChoice() API because these snippets are being returned over LSP from a language server (and such, it's important the docs are accurate and specific, to ensure servers and clients have the same understandings).

@DanTup
Copy link
Contributor Author

DanTup commented Dec 16, 2023

This actually repros without the backslashes when using appendChoice:

const doc = await vscode.workspace.openTextDocument({ content: '' });
const editor = await vscode.window.showTextDocument(doc);
const snippet = new vscode.SnippetString("a(");
snippet.appendChoice(["{optionA})>", "optionB"]);
await editor.insertSnippet(snippet, new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 0)));

Although it's possible that bit will be fixed by #180132

@RedCMD
Copy link
Contributor

RedCMD commented Dec 16, 2023

related: #200424

@jrieken
Copy link
Member

jrieken commented Dec 18, 2023

My understanding of this is that } can be escaped with a \ for all kinds of snippets.

I see how the docs make you believe so but it is a misunderstanding. Generally, only things that have a "syntax-meaning" need to be escaped.

@RedCMD
Copy link
Contributor

RedCMD commented Dec 18, 2023

only things that have a "syntax-meaning" need to be escaped

@jrieken then how come when I escape snippet syntax in #200424 the escape char goes through
but if I don't escape it, the snippet syntax goes through

@jrieken
Copy link
Member

jrieken commented Dec 18, 2023

@RedCMD I don't see how #200424 is related to this issue

@DanTup
Copy link
Contributor Author

DanTup commented Dec 18, 2023

@jrieken

I see how the docs make you believe so but it is a misunderstanding. Generally, only things that have a "syntax-meaning" need to be escaped.

Gotcha - so the only thing to escape in a choice is the pipe | and comma ,? It would be good if the docs could list exactly which characters can be escaped (it sounds like there's no difference between need and can here) for each of the contexts to avoid any ambiguity?

I'm happy to send a PR with my guess at what that is if you'd review it - I'd like to ensure our server (and the LSP spec) are correct
and explicit for the actual behaviour here. Thanks!

DanTup added a commit to DanTup/language-server-protocol that referenced this issue 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.
DanTup added a commit to DanTup/vscode-docs that referenced this issue 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.
gregvanl pushed a commit to microsoft/vscode-docs that referenced this issue Dec 20, 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.
@DanTup
Copy link
Contributor Author

DanTup commented Dec 20, 2023

microsoft/vscode-docs#6928 was merged with doc updates for VS Code. The same change in LSP is open at microsoft/language-server-protocol#1868.

dbaeumer added a commit to microsoft/language-server-protocol that referenced this issue Jan 8, 2024
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.

Co-authored-by: Dirk Bäumer <dirkb@microsoft.com>
@DanTup
Copy link
Contributor Author

DanTup commented Jan 8, 2024

Both the PRs above have been merged.

@DanTup DanTup closed this as completed Jan 8, 2024
@microsoft microsoft locked and limited conversation to collaborators Jun 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants