diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2020-03-07 12:44:18 +0000 |
---|---|---|
committer | GitHub <[email protected]> | 2020-03-07 12:44:18 +0000 |
commit | aff82cf7ac172f213cb5dcca637cb2c5332294c1 (patch) | |
tree | cd0c527ac79338d8719a6ce766bf3fd81aaa90f6 /editors/code/src | |
parent | 013e9080564aa497e6de92ae4bd1f162328b3cd8 (diff) | |
parent | 65cecff316e9217eb0f58df189a3f05de5d8d51c (diff) |
Merge #3378
3378: vscode: redesign inlay hints to be capable of handling multiple editors for one source file r=Veetaha a=Veetaha
Fixes: #3008 (inlay corruption with multiple editors for one file).
Fixes: #3319 (unnecessary requests for inlay hints when switching unrelated source files or output/log/console windows)
Also, I don't know how, but the problem described in #3057 doesn't appear for me anymore (maybe it was some fix on the server-side, idk), the inlay hints are displaying right away. Though the last time I checked this it was caused by the server returning an empty array of hints and responding with a very big latency, I am not sure that this redesign actually fixed #3057....
We didn't handle the case when one rust source file is open in multiple editors in vscode (e.g. by manually adding another editor for the same file or by opening an inline git diff view or just any kind of preview within the same file).
The git diff preview is actually quite special because it causes memory leaks in vscode (https://github.com/microsoft/vscode/issues/91782). It is not removed from `visibleEditors` once it is closed. However, this bug doesn't affect the inlay hints anymore, because we don't issue a request and set inlay hints for each editor in isolation. Editors are grouped by their respective files and we issue requests only for files and then update all duplicate editors using the results (so we just update the decorations for already closed git diff preview read-only editors).
Also, note on a hack I had to use. `vscode.TextEdtior` identity is not actually public, its `id` field is not exposed to us. I created a dedicated upstream issue for this (https://github.com/microsoft/vscode/issues/91788).
Regarding #3319: the newly designed hints client doesn't issue requests for type hints when switching the visible editors if it has them already cached (though it does rerender things anyway, but this could be optimized in future if so wanted).
<details>
<summary>Before</summary>
![bug_demo](https://user-images.githubusercontent.com/36276403/75613171-3cd0d480-5b33-11ea-9066-954fb2fb18a5.gif)
</details>
<details>
<summary> After </summary>
![multi-cursor-replace](https://user-images.githubusercontent.com/36276403/75612710-d5b12100-5b2e-11ea-99ba-214b4219e6d3.gif)
</details>
Co-authored-by: Veetaha <[email protected]>
Diffstat (limited to 'editors/code/src')
-rw-r--r-- | editors/code/src/ctx.ts | 12 | ||||
-rw-r--r-- | editors/code/src/inlay_hints.ts | 290 | ||||
-rw-r--r-- | editors/code/src/rust-analyzer-api.ts | 22 | ||||
-rw-r--r-- | editors/code/src/util.ts | 12 |
4 files changed, 202 insertions, 134 deletions
diff --git a/editors/code/src/ctx.ts b/editors/code/src/ctx.ts index b4e983a0c..25ef38aed 100644 --- a/editors/code/src/ctx.ts +++ b/editors/code/src/ctx.ts | |||
@@ -3,7 +3,7 @@ import * as lc from 'vscode-languageclient'; | |||
3 | 3 | ||
4 | import { Config } from './config'; | 4 | import { Config } from './config'; |
5 | import { createClient } from './client'; | 5 | import { createClient } from './client'; |
6 | import { isRustDocument } from './util'; | 6 | import { isRustEditor, RustEditor } from './util'; |
7 | 7 | ||
8 | export class Ctx { | 8 | export class Ctx { |
9 | private constructor( | 9 | private constructor( |
@@ -22,17 +22,15 @@ export class Ctx { | |||
22 | return res; | 22 | return res; |
23 | } | 23 | } |
24 | 24 | ||
25 | get activeRustEditor(): vscode.TextEditor | undefined { | 25 | get activeRustEditor(): RustEditor | undefined { |
26 | const editor = vscode.window.activeTextEditor; | 26 | const editor = vscode.window.activeTextEditor; |
27 | return editor && isRustDocument(editor.document) | 27 | return editor && isRustEditor(editor) |
28 | ? editor | 28 | ? editor |
29 | : undefined; | 29 | : undefined; |
30 | } | 30 | } |
31 | 31 | ||
32 | get visibleRustEditors(): vscode.TextEditor[] { | 32 | get visibleRustEditors(): RustEditor[] { |
33 | return vscode.window.visibleTextEditors.filter( | 33 | return vscode.window.visibleTextEditors.filter(isRustEditor); |
34 | editor => isRustDocument(editor.document), | ||
35 | ); | ||
36 | } | 34 | } |
37 | 35 | ||
38 | registerCommand(name: string, factory: (ctx: Ctx) => Cmd) { | 36 | registerCommand(name: string, factory: (ctx: Ctx) => Cmd) { |
diff --git a/editors/code/src/inlay_hints.ts b/editors/code/src/inlay_hints.ts index 08d3a64a7..e1a82e03e 100644 --- a/editors/code/src/inlay_hints.ts +++ b/editors/code/src/inlay_hints.ts | |||
@@ -1,156 +1,214 @@ | |||
1 | import * as lc from "vscode-languageclient"; | ||
1 | import * as vscode from 'vscode'; | 2 | import * as vscode from 'vscode'; |
2 | import * as ra from './rust-analyzer-api'; | 3 | import * as ra from './rust-analyzer-api'; |
3 | 4 | ||
4 | import { Ctx } from './ctx'; | 5 | import { Ctx, Disposable } from './ctx'; |
5 | import { log, sendRequestWithRetry, isRustDocument } from './util'; | 6 | import { sendRequestWithRetry, isRustDocument, RustDocument, RustEditor } from './util'; |
6 | 7 | ||
7 | export function activateInlayHints(ctx: Ctx) { | ||
8 | const hintsUpdater = new HintsUpdater(ctx); | ||
9 | vscode.window.onDidChangeVisibleTextEditors( | ||
10 | async _ => hintsUpdater.refresh(), | ||
11 | null, | ||
12 | ctx.subscriptions | ||
13 | ); | ||
14 | 8 | ||
15 | vscode.workspace.onDidChangeTextDocument( | 9 | export function activateInlayHints(ctx: Ctx) { |
16 | async event => { | 10 | const maybeUpdater = { |
17 | if (event.contentChanges.length === 0) return; | 11 | updater: null as null | HintsUpdater, |
18 | if (!isRustDocument(event.document)) return; | 12 | onConfigChange() { |
19 | await hintsUpdater.refresh(); | 13 | if (!ctx.config.displayInlayHints) { |
14 | return this.dispose(); | ||
15 | } | ||
16 | if (!this.updater) this.updater = new HintsUpdater(ctx); | ||
20 | }, | 17 | }, |
21 | null, | 18 | dispose() { |
22 | ctx.subscriptions | 19 | this.updater?.dispose(); |
23 | ); | 20 | this.updater = null; |
21 | } | ||
22 | }; | ||
23 | |||
24 | ctx.pushCleanup(maybeUpdater); | ||
24 | 25 | ||
25 | vscode.workspace.onDidChangeConfiguration( | 26 | vscode.workspace.onDidChangeConfiguration( |
26 | async _ => hintsUpdater.setEnabled(ctx.config.displayInlayHints), | 27 | maybeUpdater.onConfigChange, maybeUpdater, ctx.subscriptions |
27 | null, | ||
28 | ctx.subscriptions | ||
29 | ); | 28 | ); |
30 | 29 | ||
31 | ctx.pushCleanup({ | 30 | maybeUpdater.onConfigChange(); |
32 | dispose() { | 31 | } |
33 | hintsUpdater.clear(); | 32 | |
33 | |||
34 | const typeHints = { | ||
35 | decorationType: vscode.window.createTextEditorDecorationType({ | ||
36 | after: { | ||
37 | color: new vscode.ThemeColor('rust_analyzer.inlayHint'), | ||
38 | fontStyle: "normal", | ||
34 | } | 39 | } |
35 | }); | 40 | }), |
36 | 41 | ||
37 | // XXX: we don't await this, thus Promise rejections won't be handled, but | 42 | toDecoration(hint: ra.InlayHint.TypeHint, conv: lc.Protocol2CodeConverter): vscode.DecorationOptions { |
38 | // this should never throw in fact... | 43 | return { |
39 | void hintsUpdater.setEnabled(ctx.config.displayInlayHints); | 44 | range: conv.asRange(hint.range), |
40 | } | 45 | renderOptions: { after: { contentText: `: ${hint.label}` } } |
46 | }; | ||
47 | } | ||
48 | }; | ||
49 | |||
50 | const paramHints = { | ||
51 | decorationType: vscode.window.createTextEditorDecorationType({ | ||
52 | before: { | ||
53 | color: new vscode.ThemeColor('rust_analyzer.inlayHint'), | ||
54 | fontStyle: "normal", | ||
55 | } | ||
56 | }), | ||
41 | 57 | ||
42 | const typeHintDecorationType = vscode.window.createTextEditorDecorationType({ | 58 | toDecoration(hint: ra.InlayHint.ParamHint, conv: lc.Protocol2CodeConverter): vscode.DecorationOptions { |
43 | after: { | 59 | return { |
44 | color: new vscode.ThemeColor('rust_analyzer.inlayHint'), | 60 | range: conv.asRange(hint.range), |
45 | fontStyle: "normal", | 61 | renderOptions: { before: { contentText: `${hint.label}: ` } } |
46 | }, | 62 | }; |
47 | }); | ||
48 | |||
49 | const parameterHintDecorationType = vscode.window.createTextEditorDecorationType({ | ||
50 | before: { | ||
51 | color: new vscode.ThemeColor('rust_analyzer.inlayHint'), | ||
52 | fontStyle: "normal", | ||
53 | }, | ||
54 | }); | ||
55 | |||
56 | class HintsUpdater { | ||
57 | private pending = new Map<string, vscode.CancellationTokenSource>(); | ||
58 | private ctx: Ctx; | ||
59 | private enabled: boolean; | ||
60 | |||
61 | constructor(ctx: Ctx) { | ||
62 | this.ctx = ctx; | ||
63 | this.enabled = false; | ||
64 | } | 63 | } |
64 | }; | ||
65 | 65 | ||
66 | async setEnabled(enabled: boolean): Promise<void> { | 66 | class HintsUpdater implements Disposable { |
67 | log.debug({ enabled, prev: this.enabled }); | 67 | private sourceFiles = new Map<string, RustSourceFile>(); // map Uri -> RustSourceFile |
68 | private readonly disposables: Disposable[] = []; | ||
68 | 69 | ||
69 | if (this.enabled === enabled) return; | 70 | constructor(private readonly ctx: Ctx) { |
70 | this.enabled = enabled; | 71 | vscode.window.onDidChangeVisibleTextEditors( |
72 | this.onDidChangeVisibleTextEditors, | ||
73 | this, | ||
74 | this.disposables | ||
75 | ); | ||
71 | 76 | ||
72 | if (this.enabled) { | 77 | vscode.workspace.onDidChangeTextDocument( |
73 | return await this.refresh(); | 78 | this.onDidChangeTextDocument, |
74 | } else { | 79 | this, |
75 | return this.clear(); | 80 | this.disposables |
76 | } | 81 | ); |
82 | |||
83 | // Set up initial cache shape | ||
84 | ctx.visibleRustEditors.forEach(editor => this.sourceFiles.set( | ||
85 | editor.document.uri.toString(), | ||
86 | { | ||
87 | document: editor.document, | ||
88 | inlaysRequest: null, | ||
89 | cachedDecorations: null | ||
90 | } | ||
91 | )); | ||
92 | |||
93 | this.syncCacheAndRenderHints(); | ||
77 | } | 94 | } |
78 | 95 | ||
79 | clear() { | 96 | dispose() { |
80 | this.ctx.visibleRustEditors.forEach(it => { | 97 | this.sourceFiles.forEach(file => file.inlaysRequest?.cancel()); |
81 | this.setTypeDecorations(it, []); | 98 | this.ctx.visibleRustEditors.forEach(editor => this.renderDecorations(editor, { param: [], type: [] })); |
82 | this.setParameterDecorations(it, []); | 99 | this.disposables.forEach(d => d.dispose()); |
83 | }); | 100 | } |
101 | |||
102 | onDidChangeTextDocument({ contentChanges, document }: vscode.TextDocumentChangeEvent) { | ||
103 | if (contentChanges.length === 0 || !isRustDocument(document)) return; | ||
104 | this.syncCacheAndRenderHints(); | ||
84 | } | 105 | } |
85 | 106 | ||
86 | async refresh() { | 107 | private syncCacheAndRenderHints() { |
87 | if (!this.enabled) return; | 108 | // FIXME: make inlayHints request pass an array of files? |
88 | await Promise.all(this.ctx.visibleRustEditors.map(it => this.refreshEditor(it))); | 109 | this.sourceFiles.forEach((file, uri) => this.fetchHints(file).then(hints => { |
110 | if (!hints) return; | ||
111 | |||
112 | file.cachedDecorations = this.hintsToDecorations(hints); | ||
113 | |||
114 | for (const editor of this.ctx.visibleRustEditors) { | ||
115 | if (editor.document.uri.toString() === uri) { | ||
116 | this.renderDecorations(editor, file.cachedDecorations); | ||
117 | } | ||
118 | } | ||
119 | })); | ||
89 | } | 120 | } |
90 | 121 | ||
91 | private async refreshEditor(editor: vscode.TextEditor): Promise<void> { | 122 | onDidChangeVisibleTextEditors() { |
92 | const newHints = await this.queryHints(editor.document.uri.toString()); | 123 | const newSourceFiles = new Map<string, RustSourceFile>(); |
93 | if (newHints == null) return; | 124 | |
94 | 125 | // Rerendering all, even up-to-date editors for simplicity | |
95 | const newTypeDecorations = newHints | 126 | this.ctx.visibleRustEditors.forEach(async editor => { |
96 | .filter(hint => hint.kind === ra.InlayKind.TypeHint) | 127 | const uri = editor.document.uri.toString(); |
97 | .map(hint => ({ | 128 | const file = this.sourceFiles.get(uri) ?? { |
98 | range: this.ctx.client.protocol2CodeConverter.asRange(hint.range), | 129 | document: editor.document, |
99 | renderOptions: { | 130 | inlaysRequest: null, |
100 | after: { | 131 | cachedDecorations: null |
101 | contentText: `: ${hint.label}`, | 132 | }; |
102 | }, | 133 | newSourceFiles.set(uri, file); |
103 | }, | 134 | |
104 | })); | 135 | // No text documents changed, so we may try to use the cache |
105 | this.setTypeDecorations(editor, newTypeDecorations); | 136 | if (!file.cachedDecorations) { |
106 | 137 | file.inlaysRequest?.cancel(); | |
107 | const newParameterDecorations = newHints | 138 | |
108 | .filter(hint => hint.kind === ra.InlayKind.ParameterHint) | 139 | const hints = await this.fetchHints(file); |
109 | .map(hint => ({ | 140 | if (!hints) return; |
110 | range: this.ctx.client.protocol2CodeConverter.asRange(hint.range), | 141 | |
111 | renderOptions: { | 142 | file.cachedDecorations = this.hintsToDecorations(hints); |
112 | before: { | 143 | } |
113 | contentText: `${hint.label}: `, | 144 | |
114 | }, | 145 | this.renderDecorations(editor, file.cachedDecorations); |
115 | }, | 146 | }); |
116 | })); | 147 | |
117 | this.setParameterDecorations(editor, newParameterDecorations); | 148 | // Cancel requests for no longer visible (disposed) source files |
149 | this.sourceFiles.forEach((file, uri) => { | ||
150 | if (!newSourceFiles.has(uri)) file.inlaysRequest?.cancel(); | ||
151 | }); | ||
152 | |||
153 | this.sourceFiles = newSourceFiles; | ||
118 | } | 154 | } |
119 | 155 | ||
120 | private setTypeDecorations( | 156 | private renderDecorations(editor: RustEditor, decorations: InlaysDecorations) { |
121 | editor: vscode.TextEditor, | 157 | editor.setDecorations(typeHints.decorationType, decorations.type); |
122 | decorations: vscode.DecorationOptions[], | 158 | editor.setDecorations(paramHints.decorationType, decorations.param); |
123 | ) { | ||
124 | editor.setDecorations( | ||
125 | typeHintDecorationType, | ||
126 | this.enabled ? decorations : [], | ||
127 | ); | ||
128 | } | 159 | } |
129 | 160 | ||
130 | private setParameterDecorations( | 161 | private hintsToDecorations(hints: ra.InlayHint[]): InlaysDecorations { |
131 | editor: vscode.TextEditor, | 162 | const decorations: InlaysDecorations = { type: [], param: [] }; |
132 | decorations: vscode.DecorationOptions[], | 163 | const conv = this.ctx.client.protocol2CodeConverter; |
133 | ) { | 164 | |
134 | editor.setDecorations( | 165 | for (const hint of hints) { |
135 | parameterHintDecorationType, | 166 | switch (hint.kind) { |
136 | this.enabled ? decorations : [], | 167 | case ra.InlayHint.Kind.TypeHint: { |
137 | ); | 168 | decorations.type.push(typeHints.toDecoration(hint, conv)); |
169 | continue; | ||
170 | } | ||
171 | case ra.InlayHint.Kind.ParamHint: { | ||
172 | decorations.param.push(paramHints.toDecoration(hint, conv)); | ||
173 | continue; | ||
174 | } | ||
175 | } | ||
176 | } | ||
177 | return decorations; | ||
138 | } | 178 | } |
139 | 179 | ||
140 | private async queryHints(documentUri: string): Promise<ra.InlayHint[] | null> { | 180 | private async fetchHints(file: RustSourceFile): Promise<null | ra.InlayHint[]> { |
141 | this.pending.get(documentUri)?.cancel(); | 181 | file.inlaysRequest?.cancel(); |
142 | 182 | ||
143 | const tokenSource = new vscode.CancellationTokenSource(); | 183 | const tokenSource = new vscode.CancellationTokenSource(); |
144 | this.pending.set(documentUri, tokenSource); | 184 | file.inlaysRequest = tokenSource; |
145 | 185 | ||
146 | const request = { textDocument: { uri: documentUri } }; | 186 | const request = { textDocument: { uri: file.document.uri.toString() } }; |
147 | 187 | ||
148 | return sendRequestWithRetry(this.ctx.client, ra.inlayHints, request, tokenSource.token) | 188 | return sendRequestWithRetry(this.ctx.client, ra.inlayHints, request, tokenSource.token) |
149 | .catch(_ => null) | 189 | .catch(_ => null) |
150 | .finally(() => { | 190 | .finally(() => { |
151 | if (!tokenSource.token.isCancellationRequested) { | 191 | if (file.inlaysRequest === tokenSource) { |
152 | this.pending.delete(documentUri); | 192 | file.inlaysRequest = null; |
153 | } | 193 | } |
154 | }); | 194 | }); |
155 | } | 195 | } |
156 | } | 196 | } |
197 | |||
198 | interface InlaysDecorations { | ||
199 | type: vscode.DecorationOptions[]; | ||
200 | param: vscode.DecorationOptions[]; | ||
201 | } | ||
202 | |||
203 | interface RustSourceFile { | ||
204 | /* | ||
205 | * Source of the token to cancel in-flight inlay hints request if any. | ||
206 | */ | ||
207 | inlaysRequest: null | vscode.CancellationTokenSource; | ||
208 | /** | ||
209 | * Last applied decorations. | ||
210 | */ | ||
211 | cachedDecorations: null | InlaysDecorations; | ||
212 | |||
213 | document: RustDocument; | ||
214 | } | ||
diff --git a/editors/code/src/rust-analyzer-api.ts b/editors/code/src/rust-analyzer-api.ts index c5a010e94..bd6e3ada0 100644 --- a/editors/code/src/rust-analyzer-api.ts +++ b/editors/code/src/rust-analyzer-api.ts | |||
@@ -86,14 +86,20 @@ export interface Runnable { | |||
86 | export const runnables = request<RunnablesParams, Vec<Runnable>>("runnables"); | 86 | export const runnables = request<RunnablesParams, Vec<Runnable>>("runnables"); |
87 | 87 | ||
88 | 88 | ||
89 | export const enum InlayKind { | 89 | |
90 | TypeHint = "TypeHint", | 90 | export type InlayHint = InlayHint.TypeHint | InlayHint.ParamHint; |
91 | ParameterHint = "ParameterHint", | 91 | |
92 | } | 92 | export namespace InlayHint { |
93 | export interface InlayHint { | 93 | export const enum Kind { |
94 | range: lc.Range; | 94 | TypeHint = "TypeHint", |
95 | kind: InlayKind; | 95 | ParamHint = "ParameterHint", |
96 | label: string; | 96 | } |
97 | interface Common { | ||
98 | range: lc.Range; | ||
99 | label: string; | ||
100 | } | ||
101 | export type TypeHint = Common & { kind: Kind.TypeHint }; | ||
102 | export type ParamHint = Common & { kind: Kind.ParamHint }; | ||
97 | } | 103 | } |
98 | export interface InlayHintsParams { | 104 | export interface InlayHintsParams { |
99 | textDocument: lc.TextDocumentIdentifier; | 105 | textDocument: lc.TextDocumentIdentifier; |
diff --git a/editors/code/src/util.ts b/editors/code/src/util.ts index 7c95769bb..95a5f1227 100644 --- a/editors/code/src/util.ts +++ b/editors/code/src/util.ts | |||
@@ -1,7 +1,6 @@ | |||
1 | import * as lc from "vscode-languageclient"; | 1 | import * as lc from "vscode-languageclient"; |
2 | import * as vscode from "vscode"; | 2 | import * as vscode from "vscode"; |
3 | import { strict as nativeAssert } from "assert"; | 3 | import { strict as nativeAssert } from "assert"; |
4 | import { TextDocument } from "vscode"; | ||
5 | 4 | ||
6 | export function assert(condition: boolean, explanation: string): asserts condition { | 5 | export function assert(condition: boolean, explanation: string): asserts condition { |
7 | try { | 6 | try { |
@@ -67,9 +66,16 @@ function sleep(ms: number) { | |||
67 | return new Promise(resolve => setTimeout(resolve, ms)); | 66 | return new Promise(resolve => setTimeout(resolve, ms)); |
68 | } | 67 | } |
69 | 68 | ||
70 | export function isRustDocument(document: TextDocument) { | 69 | export type RustDocument = vscode.TextDocument & { languageId: "rust" }; |
70 | export type RustEditor = vscode.TextEditor & { document: RustDocument; id: string }; | ||
71 | |||
72 | export function isRustDocument(document: vscode.TextDocument): document is RustDocument { | ||
71 | return document.languageId === 'rust' | 73 | return document.languageId === 'rust' |
72 | // SCM diff views have the same URI as the on-disk document but not the same content | 74 | // SCM diff views have the same URI as the on-disk document but not the same content |
73 | && document.uri.scheme !== 'git' | 75 | && document.uri.scheme !== 'git' |
74 | && document.uri.scheme !== 'svn'; | 76 | && document.uri.scheme !== 'svn'; |
75 | } \ No newline at end of file | 77 | } |
78 | |||
79 | export function isRustEditor(editor: vscode.TextEditor): editor is RustEditor { | ||
80 | return isRustDocument(editor.document); | ||
81 | } | ||