Skip to content

Commit ae2f513

Browse files
committed
Fix race condition in worksheet output
Sometimes, the `ApplyEdits` that we use in the worksheet to insert blank lines so that the worksheet output fits were reordered. The lead to some output of the worksheet being lost, with decorations being inserted after the end of the file. We know keep track of the edits that we want to apply to the worksheet and ensure that they are executed in the same order as the messages were received.
1 parent ffced94 commit ae2f513

File tree

1 file changed

+33
-20
lines changed

1 file changed

+33
-20
lines changed

vscode-dotty/src/worksheet.ts

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,14 @@ class Worksheet implements Disposable {
6363
*/
6464
private canceller?: CancellationTokenSource = undefined
6565

66+
/**
67+
* The edits that should be applied to this worksheet.
68+
*
69+
* This is used to ensure that the blank lines added to fit the output of the worksheet
70+
* are inserted in the same order as the output arrived.
71+
*/
72+
private applyEdits: Promise<void> = Promise.resolve()
73+
6674
constructor(readonly document: vscode.TextDocument, readonly client: BaseLanguageClient) {
6775
}
6876

@@ -73,7 +81,7 @@ class Worksheet implements Disposable {
7381
this.canceller = undefined
7482
}
7583
this._onDidStateChange.dispose()
76-
}
84+
}
7785

7886
/** Remove all decorations, and resets this worksheet. */
7987
private reset(): void {
@@ -83,6 +91,7 @@ class Worksheet implements Disposable {
8391
this.decoratedLines.clear()
8492
this.runVersion = -1
8593
this.margin = this.longestLine() + 5
94+
this.applyEdits = Promise.resolve()
8695
}
8796

8897
/**
@@ -111,6 +120,13 @@ class Worksheet implements Disposable {
111120
return this.canceller != undefined
112121
}
113122

123+
/** Display the output in the worksheet's editor. */
124+
handleMessage(output: WorksheetPublishOutputParams, editor: vscode.TextEditor) {
125+
this.applyEdits = this.applyEdits.then(() => {
126+
this.displayAndSaveResult(output.line - 1, output.content, editor)
127+
})
128+
}
129+
114130
/**
115131
* Run the worksheet in `document`, if a previous run is in progress, it is
116132
* cancelled first.
@@ -160,10 +176,10 @@ class Worksheet implements Disposable {
160176
* @param runResult The result itself.
161177
* @param worksheet The worksheet that receives the result.
162178
* @param editor The editor where to display the result.
163-
* @return A `Thenable` that will insert necessary lines to fit the output
179+
* @return A `Promise` that will insert necessary lines to fit the output
164180
* and display the decorations upon completion.
165181
*/
166-
public displayAndSaveResult(lineNumber: number, runResult: string, editor: vscode.TextEditor) {
182+
public async displayAndSaveResult(lineNumber: number, runResult: string, editor: vscode.TextEditor): Promise<void> {
167183
const resultLines = runResult.trim().split(/\r\n|\r|\n/g)
168184

169185
// The line where the next decoration should be put.
@@ -183,21 +199,18 @@ class Worksheet implements Disposable {
183199
this.runVersion += 1
184200
}
185201

186-
return vscode.workspace.applyEdit(addNewLinesEdit).then(_ => {
187-
for (let line of resultLines) {
188-
const decorationPosition = new vscode.Position(actualLine, 0)
189-
const decorationMargin = this.margin - editor.document.lineAt(actualLine).text.length
190-
const decorationType = this.createDecoration(decorationMargin, line)
191-
const decorationOptions = { range: new vscode.Range(decorationPosition, decorationPosition), hoverMessage: line }
192-
const decoration = new Decoration(decorationType, decorationOptions)
193-
194-
this.decoratedLines.add(actualLine)
195-
this.decorations.push(decoration)
196-
197-
editor.setDecorations(decorationType, [decorationOptions])
198-
actualLine += 1
199-
}
200-
})
202+
await vscode.workspace.applyEdit(addNewLinesEdit);
203+
for (let line of resultLines) {
204+
const decorationPosition = new vscode.Position(actualLine, 0);
205+
const decorationMargin = this.margin - editor.document.lineAt(actualLine).text.length;
206+
const decorationType = this.createDecoration(decorationMargin, line);
207+
const decorationOptions = { range: new vscode.Range(decorationPosition, decorationPosition), hoverMessage: line };
208+
const decoration = new Decoration(decorationType, decorationOptions);
209+
this.decoratedLines.add(actualLine);
210+
this.decorations.push(decoration);
211+
editor.setDecorations(decorationType, [decorationOptions]);
212+
actualLine += 1;
213+
}
201214
}
202215

203216
/**
@@ -409,7 +422,7 @@ export class WorksheetProvider implements Disposable {
409422
* Handle the result of running part of a worksheet.
410423
* This is called when we receive a `worksheet/publishOutput`.
411424
*
412-
* @param message The result of running part of a worksheet.
425+
* @param output The result of running part of a worksheet.
413426
*/
414427
private handleMessage(output: WorksheetPublishOutputParams) {
415428
const editor = vscode.window.visibleTextEditors.find(e => {
@@ -420,7 +433,7 @@ export class WorksheetProvider implements Disposable {
420433
if (editor) {
421434
const worksheet = this.worksheetFor(editor.document)
422435
if (worksheet) {
423-
worksheet.displayAndSaveResult(output.line - 1, output.content, editor)
436+
worksheet.handleMessage(output, editor)
424437
}
425438
}
426439
}

0 commit comments

Comments
 (0)