From d846586bc9e0c056a92f8394b6209782238dc5e2 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Thu, 1 Feb 2024 20:38:42 -0500 Subject: [PATCH 1/2] fix: Support multiple tab stops in completions in VSCode Uses the native VSCode support for `SnippetTextEdit`s, but in a semi-hacky way as it's not fully supported yet. --- editors/code/src/commands.ts | 66 +++++++++++++++++++++++++-- editors/code/src/snippets.ts | 87 +++++++++++++++--------------------- 2 files changed, 98 insertions(+), 55 deletions(-) diff --git a/editors/code/src/commands.ts b/editors/code/src/commands.ts index 3d33d255ad491..849fae5cf24b1 100644 --- a/editors/code/src/commands.ts +++ b/editors/code/src/commands.ts @@ -4,7 +4,11 @@ import * as ra from "./lsp_ext"; import * as path from "path"; import type { Ctx, Cmd, CtxInit } from "./ctx"; -import { applySnippetWorkspaceEdit, applySnippetTextEdits } from "./snippets"; +import { + applySnippetWorkspaceEdit, + applySnippetTextEdits, + type SnippetTextDocumentEdit, +} from "./snippets"; import { spawnSync } from "child_process"; import { type RunnableQuickPick, selectRunnable, createTask, createArgs } from "./run"; import { AstInspector } from "./ast_inspector"; @@ -1006,7 +1010,6 @@ export function resolveCodeAction(ctx: CtxInit): Cmd { return; } const itemEdit = item.edit; - const edit = await client.protocol2CodeConverter.asWorkspaceEdit(itemEdit); // filter out all text edits and recreate the WorkspaceEdit without them so we can apply // snippet edits on our own const lcFileSystemEdit = { @@ -1017,16 +1020,71 @@ export function resolveCodeAction(ctx: CtxInit): Cmd { lcFileSystemEdit, ); await vscode.workspace.applyEdit(fileSystemEdit); - await applySnippetWorkspaceEdit(edit); + + // replace all text edits so that we can convert snippet text edits into `vscode.SnippetTextEdit`s + // FIXME: this is a workaround until vscode-languageclient supports doing the SnippeTextEdit conversion itself + // also need to carry the snippetTextDocumentEdits separately, since we can't retrieve them again using WorkspaceEdit.entries + const [workspaceTextEdit, snippetTextDocumentEdits] = asWorkspaceSnippetEdit(ctx, itemEdit); + await applySnippetWorkspaceEdit(workspaceTextEdit, snippetTextDocumentEdits); if (item.command != null) { await vscode.commands.executeCommand(item.command.command, item.command.arguments); } }; } +function asWorkspaceSnippetEdit( + ctx: CtxInit, + item: lc.WorkspaceEdit, +): [vscode.WorkspaceEdit, SnippetTextDocumentEdit[]] { + const client = ctx.client; + + // partially borrowed from https://github.com/microsoft/vscode-languageserver-node/blob/295aaa393fda8ecce110c38880a00466b9320e63/client/src/common/protocolConverter.ts#L1060-L1101 + const result = new vscode.WorkspaceEdit(); + + if (item.documentChanges) { + const snippetTextDocumentEdits: SnippetTextDocumentEdit[] = []; + + for (const change of item.documentChanges) { + if (lc.TextDocumentEdit.is(change)) { + const uri = client.protocol2CodeConverter.asUri(change.textDocument.uri); + const snippetTextEdits: (vscode.TextEdit | vscode.SnippetTextEdit)[] = []; + + for (const edit of change.edits) { + if ( + "insertTextFormat" in edit && + edit.insertTextFormat === lc.InsertTextFormat.Snippet + ) { + // is a snippet text edit + snippetTextEdits.push( + new vscode.SnippetTextEdit( + client.protocol2CodeConverter.asRange(edit.range), + new vscode.SnippetString(edit.newText), + ), + ); + } else { + // always as a text document edit + snippetTextEdits.push( + vscode.TextEdit.replace( + client.protocol2CodeConverter.asRange(edit.range), + edit.newText, + ), + ); + } + } + + snippetTextDocumentEdits.push([uri, snippetTextEdits]); + } + } + return [result, snippetTextDocumentEdits]; + } else { + // we don't handle WorkspaceEdit.changes since it's not relevant for code actions + return [result, []]; + } +} + export function applySnippetWorkspaceEditCommand(_ctx: CtxInit): Cmd { return async (edit: vscode.WorkspaceEdit) => { - await applySnippetWorkspaceEdit(edit); + await applySnippetWorkspaceEdit(edit, edit.entries()); }; } diff --git a/editors/code/src/snippets.ts b/editors/code/src/snippets.ts index d81765649ffb2..fb12125bcd842 100644 --- a/editors/code/src/snippets.ts +++ b/editors/code/src/snippets.ts @@ -3,20 +3,28 @@ import * as vscode from "vscode"; import { assert } from "./util"; import { unwrapUndefinable } from "./undefinable"; -export async function applySnippetWorkspaceEdit(edit: vscode.WorkspaceEdit) { - if (edit.entries().length === 1) { - const [uri, edits] = unwrapUndefinable(edit.entries()[0]); +export type SnippetTextDocumentEdit = [vscode.Uri, (vscode.TextEdit | vscode.SnippetTextEdit)[]]; + +export async function applySnippetWorkspaceEdit( + edit: vscode.WorkspaceEdit, + editEntries: SnippetTextDocumentEdit[], +) { + if (editEntries.length === 1) { + const [uri, edits] = unwrapUndefinable(editEntries[0]); const editor = await editorFromUri(uri); - if (editor) await applySnippetTextEdits(editor, edits); + if (editor) { + edit.set(uri, edits); + await vscode.workspace.applyEdit(edit); + } return; } - for (const [uri, edits] of edit.entries()) { + for (const [uri, edits] of editEntries) { const editor = await editorFromUri(uri); if (editor) { await editor.edit((builder) => { for (const indel of edits) { assert( - !parseSnippet(indel.newText), + !(indel instanceof vscode.SnippetTextEdit), `bad ws edit: snippet received with multiple edits: ${JSON.stringify( edit, )}`, @@ -39,53 +47,30 @@ async function editorFromUri(uri: vscode.Uri): Promise { - for (const indel of edits) { - const parsed = parseSnippet(indel.newText); - if (parsed) { - const [newText, [placeholderStart, placeholderLength]] = parsed; - const prefix = newText.substr(0, placeholderStart); - const lastNewline = prefix.lastIndexOf("\n"); - - const startLine = indel.range.start.line + lineDelta + countLines(prefix); - const startColumn = - lastNewline === -1 - ? indel.range.start.character + placeholderStart - : prefix.length - lastNewline - 1; - const endColumn = startColumn + placeholderLength; - selections.push( - new vscode.Selection( - new vscode.Position(startLine, startColumn), - new vscode.Position(startLine, endColumn), - ), - ); - builder.replace(indel.range, newText); - } else { - builder.replace(indel.range, indel.newText); - } - lineDelta += - countLines(indel.newText) - (indel.range.end.line - indel.range.start.line); - } - }); - if (selections.length > 0) editor.selections = selections; - if (selections.length === 1) { - const selection = unwrapUndefinable(selections[0]); - editor.revealRange(selection, vscode.TextEditorRevealType.InCenterIfOutsideViewport); - } + const edit = new vscode.WorkspaceEdit(); + edit.set(editor.document.uri, toSnippetTextEdits(edits)); + await vscode.workspace.applyEdit(edit); } -function parseSnippet(snip: string): [string, [number, number]] | undefined { - const m = snip.match(/\$(0|\{0:([^}]*)\})/); - if (!m) return undefined; - const placeholder = m[2] ?? ""; - if (m.index == null) return undefined; - const range: [number, number] = [m.index, placeholder.length]; - const insert = snip.replace(m[0], placeholder); - return [insert, range]; +function hasSnippet(snip: string): boolean { + const m = snip.match(/\$\d+|\{\d+:[^}]*\}/); + return m != null; } -function countLines(text: string): number { - return (text.match(/\n/g) || []).length; +function toSnippetTextEdits( + edits: vscode.TextEdit[], +): (vscode.TextEdit | vscode.SnippetTextEdit)[] { + return edits.map((textEdit) => { + // Note: text edits without any snippets are returned as-is instead of + // being wrapped in a SnippetTextEdit, as otherwise it would be + // treated as if it had a tab stop at the end. + if (hasSnippet(textEdit.newText)) { + return new vscode.SnippetTextEdit( + textEdit.range, + new vscode.SnippetString(textEdit.newText), + ); + } else { + return textEdit; + } + }); } From bcf14e27ce9d4331f4685fb759cf135a76ad119c Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Thu, 15 Feb 2024 18:39:17 -0500 Subject: [PATCH 2/2] Work around snippet edits doubling up extra indentation We can't tell vscode to not add in the extra indentation, so we instead opt to remove it from the edits themselves, and then let vscode add it back in. --- editors/code/src/snippets.ts | 71 +++++++++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 2 deletions(-) diff --git a/editors/code/src/snippets.ts b/editors/code/src/snippets.ts index fb12125bcd842..b3982bdf2be41 100644 --- a/editors/code/src/snippets.ts +++ b/editors/code/src/snippets.ts @@ -13,7 +13,7 @@ export async function applySnippetWorkspaceEdit( const [uri, edits] = unwrapUndefinable(editEntries[0]); const editor = await editorFromUri(uri); if (editor) { - edit.set(uri, edits); + edit.set(uri, removeLeadingWhitespace(editor, edits)); await vscode.workspace.applyEdit(edit); } return; @@ -48,7 +48,8 @@ async function editorFromUri(uri: vscode.Uri): Promise { + if (edit instanceof vscode.SnippetTextEdit) { + const snippetEdit: vscode.SnippetTextEdit = edit; + const firstLineEnd = snippetEdit.snippet.value.indexOf("\n"); + + if (firstLineEnd !== -1) { + // Is a multi-line snippet, remove the indentation which + // would be added back in by vscode. + const startLine = editor.document.lineAt(snippetEdit.range.start.line); + const leadingWhitespace = getLeadingWhitespace( + startLine.text, + 0, + startLine.firstNonWhitespaceCharacterIndex, + ); + + const [firstLine, rest] = splitAt(snippetEdit.snippet.value, firstLineEnd + 1); + const unindentedLines = rest + .split("\n") + .map((line) => line.replace(leadingWhitespace, "")) + .join("\n"); + + snippetEdit.snippet.value = firstLine + unindentedLines; + } + + return snippetEdit; + } else { + return edit; + } + }); +} + +// based on https://github.com/microsoft/vscode/blob/main/src/vs/base/common/strings.ts#L284 +function getLeadingWhitespace(str: string, start: number = 0, end: number = str.length): string { + for (let i = start; i < end; i++) { + const chCode = str.charCodeAt(i); + if (chCode !== " ".charCodeAt(0) && chCode !== " ".charCodeAt(0)) { + return str.substring(start, i); + } + } + return str.substring(start, end); +} + +function splitAt(str: string, index: number): [string, string] { + return [str.substring(0, index), str.substring(index)]; +}