Skip to content

Commit 221d3cb

Browse files
committed
Rework multi-line output handling in the worksheet
Even after #5552 there were still situations were the worksheet output was not displayed properly due to all the complications needed to handle multi-line output. Getting this right seems hopeless until vscode gets some better API (microsoft/vscode#63600), so instead in this commit we avoid the problem by only displaying the first line of multi-line output. The complete output is still available by hovering over the decoration which should be good enough.
1 parent 909f10a commit 221d3cb

File tree

1 file changed

+36
-152
lines changed

1 file changed

+36
-152
lines changed

vscode-dotty/src/worksheet.ts

Lines changed: 36 additions & 152 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,6 @@ class Worksheet implements Disposable {
5151
/** All decorations that have been added so far */
5252
private decorations: Decoration[] = []
5353

54-
/** The number of blank lines that have been inserted to fit the output so far. */
55-
private insertedLines: number = 0
56-
57-
/** The lines that contain decorations */
58-
private decoratedLines: Set<number> = new Set<number>()
59-
6054
/** The minimum margin to add so that the decoration is shown after all text. */
6155
private margin: number = 0
6256

@@ -70,46 +64,22 @@ class Worksheet implements Disposable {
7064
*/
7165
private canceller?: CancellationTokenSource = undefined
7266

73-
/**
74-
* The edits that should be applied to this worksheet.
75-
*
76-
* This is used to ensure that the blank lines added to fit the output of the worksheet
77-
* are inserted in the same order as the output arrived.
78-
*/
79-
private applyEdits: Promise<void> = Promise.resolve()
80-
8167
constructor(readonly document: vscode.TextDocument, readonly client: BaseLanguageClient) {
8268
}
8369

8470
dispose() {
8571
this.reset()
86-
if (this.canceller) {
87-
this.canceller.dispose()
88-
this.canceller = undefined
89-
}
9072
this._onDidStateChange.dispose()
9173
}
9274

93-
/** Remove all decorations, and resets this worksheet. */
75+
/** Cancel any current run, remove all decorations, and resets this worksheet. */
9476
private reset(): void {
77+
this.cancel()
78+
9579
this.decorations.forEach(decoration => decoration.decorationType.dispose())
9680
this.decorations = []
97-
this.insertedLines = 0
98-
this.decoratedLines.clear()
9981
this.runVersion = -1
10082
this.margin = this.longestLine() + 5
101-
this.applyEdits = Promise.resolve()
102-
}
103-
104-
/**
105-
* Reset the "worksheet state" (margin and number of inserted lines), and
106-
* return an array of TextEdit that remove the redundant blank lines that have
107-
* been inserted by a previous run.
108-
*/
109-
prepareRun(): TextEdit[] {
110-
const edits = this.removeRedundantBlankLinesEdits()
111-
this.reset()
112-
return edits
11383
}
11484

11585
/** If this worksheet is currently being run, cancel the run. */
@@ -129,9 +99,7 @@ class Worksheet implements Disposable {
12999

130100
/** Display the output in the worksheet's editor. */
131101
handleMessage(output: WorksheetPublishOutputParams, editor: vscode.TextEditor) {
132-
this.applyEdits = this.applyEdits.then(() => {
133-
this.displayAndSaveResult(output.line - 1, output.content, editor)
134-
})
102+
this.displayAndSaveResult(output.line - 1, output.content, editor)
135103
}
136104

137105
/**
@@ -140,29 +108,22 @@ class Worksheet implements Disposable {
140108
*/
141109
run(): Promise<WorksheetRunResult> {
142110
this.cancel()
111+
this.reset()
143112
const canceller = new CancellationTokenSource()
144113
const token = canceller.token
145-
// This ensures that isRunning() returns true.
146-
this.canceller = canceller
114+
this.canceller = canceller // This ensures that isRunning() returns true.
147115

148116
this._onDidStateChange.fire()
149117

150118
return new Promise<WorksheetRunResult>(resolve => {
151-
const textEdits = this.prepareRun()
152-
const edit = new vscode.WorkspaceEdit()
153-
edit.set(this.document.uri, textEdits)
154-
vscode.workspace.applyEdit(edit).then(editSucceeded => {
155-
this.runVersion = this.document.version
156-
if (editSucceeded && !token.isCancellationRequested)
157-
resolve(vscode.window.withProgress({
158-
location: ProgressLocation.Window,
159-
title: "Running worksheet"
160-
}, () => this.client.sendRequest(
161-
WorksheetRunRequest.type, asWorksheetRunParams(this.document), token
162-
)))
163-
else
164-
resolve({ success: false })
165-
})
119+
this.runVersion = this.document.version
120+
resolve(
121+
vscode.window.withProgress({
122+
location: ProgressLocation.Window,
123+
title: "Running worksheet"
124+
}, () => this.client.sendRequest(
125+
WorksheetRunRequest.type, asWorksheetRunParams(this.document), token
126+
)))
166127
}).then(result => {
167128
canceller.dispose()
168129
if (this.canceller === canceller) { // If false, a new run has already started
@@ -181,43 +142,22 @@ class Worksheet implements Disposable {
181142
*
182143
* @param lineNumber The number of the line in the source that produced the result.
183144
* @param runResult The result itself.
184-
* @param worksheet The worksheet that receives the result.
185145
* @param editor The editor where to display the result.
186-
* @return A `Promise` that will insert necessary lines to fit the output
187-
* and display the decorations upon completion.
188146
*/
189-
public async displayAndSaveResult(lineNumber: number, runResult: string, editor: vscode.TextEditor): Promise<void> {
190-
const resultLines = runResult.trim().split(/\r\n|\r|\n/g)
191-
192-
// The line where the next decoration should be put.
193-
// It's the number of the line that produced the output, plus the number
194-
// of lines that we've inserted so far.
195-
let actualLine = lineNumber + this.insertedLines
196-
197-
// If the output has more than one line, we need to insert blank lines
198-
// below the line that produced the output to fit the output.
199-
const addNewLinesEdit = new vscode.WorkspaceEdit()
200-
if (resultLines.length > 1) {
201-
const linesToInsert = resultLines.length - 1
202-
const editPos = new vscode.Position(actualLine + 1, 0) // add after the line
203-
addNewLinesEdit.insert(editor.document.uri, editPos, "\n".repeat(linesToInsert))
204-
this.insertedLines += linesToInsert
205-
// Increase the `runVersion`, because the text edit will increase the document's version
206-
this.runVersion += 1
207-
}
147+
private displayAndSaveResult(lineNumber: number, runResult: string, editor: vscode.TextEditor): void {
148+
const resultLines = runResult.split(/\r\n|\r|\n/g)
208149

209-
await vscode.workspace.applyEdit(addNewLinesEdit);
210-
for (let line of resultLines) {
211-
const decorationPosition = new vscode.Position(actualLine, 0);
212-
const decorationMargin = this.margin - editor.document.lineAt(actualLine).text.length;
213-
const decorationType = this.createDecoration(decorationMargin, line);
214-
const decorationOptions = { range: new vscode.Range(decorationPosition, decorationPosition), hoverMessage: line };
215-
const decoration = new Decoration(decorationType, decorationOptions);
216-
this.decoratedLines.add(actualLine);
217-
this.decorations.push(decoration);
218-
editor.setDecorations(decorationType, [decorationOptions]);
219-
actualLine += 1;
220-
}
150+
if (resultLines.length == 0)
151+
return
152+
153+
const line = editor.document.lineAt(lineNumber)
154+
const decorationOptions = { range: line.range, hoverMessage: runResult }
155+
const decorationMargin = this.margin - line.text.length
156+
const decorationText = resultLines[0] + (resultLines.length > 1 ? "<truncated output>" : "")
157+
const decorationType = this.createDecoration(decorationMargin, decorationText)
158+
const decoration = new Decoration(decorationType, decorationOptions)
159+
this.decorations.push(decoration)
160+
editor.setDecorations(decorationType, [decorationOptions])
221161
}
222162

223163
/**
@@ -226,7 +166,7 @@ class Worksheet implements Disposable {
226166
*
227167
* @param editor The editor where to display the decorations.
228168
*/
229-
public restoreDecorations(editor: vscode.TextEditor) {
169+
restoreDecorations(editor: vscode.TextEditor) {
230170
if (editor.document.version == this.runVersion) {
231171
this.decorations.forEach(decoration => {
232172
editor.setDecorations(decoration.decorationType, [decoration.decorationOptions])
@@ -274,59 +214,6 @@ class Worksheet implements Disposable {
274214

275215
return maxLength
276216
}
277-
278-
/**
279-
* TextEdits to remove the repeated blank lines in the source.
280-
*
281-
* Running a worksheet can insert new lines in the worksheet so that the
282-
* output of a line fits below the line. Before a run, we remove blank
283-
* lines in the worksheet to keep its length under control.
284-
*
285-
* @param worksheet The worksheet where blank lines must be removed.
286-
* @return An array of `TextEdit` that remove the blank lines.
287-
*/
288-
private removeRedundantBlankLinesEdits(): TextEdit[] {
289-
290-
const document = this.document
291-
const lineCount = document.lineCount
292-
let rangesToRemove: vscode.Range[] = []
293-
let rangeStart = 0
294-
let rangeEnd = 0
295-
let inRange = true
296-
297-
function addRange() {
298-
inRange = false
299-
if (rangeStart < rangeEnd) {
300-
rangesToRemove.push(new vscode.Range(rangeStart, 0, rangeEnd, 0))
301-
}
302-
return
303-
}
304-
305-
for (let i = 0; i < lineCount; ++i) {
306-
const isEmpty = document.lineAt(i).isEmptyOrWhitespace && this.hasDecoration(i)
307-
if (inRange) {
308-
if (isEmpty) rangeEnd += 1
309-
else addRange()
310-
} else {
311-
if (isEmpty) {
312-
rangeStart = i
313-
rangeEnd = i + 1
314-
inRange = true
315-
}
316-
}
317-
}
318-
319-
if (inRange) {
320-
rangeEnd = lineCount
321-
addRange()
322-
}
323-
324-
return rangesToRemove.reverse().map(range => vscode.TextEdit.delete(range))
325-
}
326-
327-
private hasDecoration(line: number): boolean {
328-
return this.decoratedLines.has(line)
329-
}
330217
}
331218

332219
export class WorksheetProvider implements Disposable {
@@ -347,17 +234,14 @@ export class WorksheetProvider implements Disposable {
347234
vscode.workspace.onWillSaveTextDocument(event => {
348235
const document = event.document
349236
const worksheet = this.worksheetFor(document)
350-
if (worksheet) {
351-
event.waitUntil(Promise.resolve(worksheet.prepareRun()))
352-
// If the document is not dirty, then `onDidSaveTextDocument` will not
353-
// be called so we need to run the worksheet now.
354-
// On the other hand, if the document _is_ dirty, we should _not_ run
355-
// the worksheet now because the server state will not be synchronized
356-
// with the client state, instead we let `onDidSaveTextDocument`
357-
// handle it.
358-
if (runWorksheetOnSave() && !document.isDirty) {
359-
worksheet.run()
360-
}
237+
// If the document is not dirty, then `onDidSaveTextDocument` will not
238+
// be called so we need to run the worksheet now.
239+
// On the other hand, if the document _is_ dirty, we should _not_ run
240+
// the worksheet now because the server state will not be synchronized
241+
// with the client state, instead we let `onDidSaveTextDocument`
242+
// handle it.
243+
if (worksheet && runWorksheetOnSave() && !document.isDirty) {
244+
worksheet.run()
361245
}
362246
}),
363247
vscode.workspace.onDidSaveTextDocument(document => {

0 commit comments

Comments
 (0)