-
Notifications
You must be signed in to change notification settings - Fork 126
Sketch out formatter for R extension #1686
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
Changes from all commits
f690a2b
73c7e82
6957937
4c7b507
005d7dc
a2fc4f4
b078dd4
59846c3
eb42e1b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (C) 2023 Posit Software, PBC. All rights reserved. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import * as vscode from 'vscode'; | ||
| import * as positron from 'positron'; | ||
| import * as path from 'path'; | ||
| import * as os from 'os'; | ||
| import * as fs from 'fs'; | ||
| import { RRuntime, lastRuntimePath } from './runtime'; | ||
| import { getRunningRRuntime } from './provider'; | ||
| import { timeout } from './util'; | ||
| import { randomUUID } from 'crypto'; | ||
|
|
||
| export async function registerFormatter(context: vscode.ExtensionContext, runtimes: Map<string, RRuntime>) { | ||
|
|
||
| const rDocumentSelector = { scheme: 'file', language: 'r' } as vscode.DocumentSelector; | ||
|
|
||
| context.subscriptions.push( | ||
| vscode.languages.registerDocumentFormattingEditProvider( | ||
| rDocumentSelector, | ||
| new FormatterProvider(runtimes) | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| class FormatterProvider implements vscode.DocumentFormattingEditProvider { | ||
| constructor(public runtimes: Map<string, RRuntime>) { } | ||
|
|
||
| public provideDocumentFormattingEdits(document: vscode.TextDocument): | ||
| vscode.ProviderResult<vscode.TextEdit[]> { | ||
| return this.formatDocument(document, this.runtimes); | ||
| } | ||
|
|
||
| private async formatDocument( | ||
| document: vscode.TextDocument, | ||
| runtimes: Map<string, RRuntime> | ||
| ): Promise<vscode.TextEdit[]> { | ||
| if (!lastRuntimePath) { | ||
| throw new Error(`No running R runtime to provide R package tasks.`); | ||
| } | ||
|
|
||
| const runtime = await getRunningRRuntime(runtimes); | ||
| const id = randomUUID(); | ||
|
|
||
| // We can only use styler on files right now, so write the document to a temp file | ||
| const originalSource = document.getText(); | ||
| const tempdir = os.tmpdir(); | ||
| const fileToStyle = path.basename(document.fileName); | ||
| const stylerFile = path.join(tempdir, `styler-${fileToStyle}`); | ||
| fs.writeFileSync(stylerFile, originalSource); | ||
|
|
||
| // A promise that resolves when the runtime is idle: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sort of code makes me wish for an while (true) {
if (await runtime.nextRuntimeMessage() === foo) {
break
}
}Which I think makes the control flow much more apparent.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That definitely would be nice. Do you want to open an issue describing that kind of change? |
||
| const promise = new Promise<void>(resolve => { | ||
| const disp = runtime.onDidReceiveRuntimeMessage(runtimeMessage => { | ||
| if (runtimeMessage.parent_id === id && | ||
| runtimeMessage.type === positron.LanguageRuntimeMessageType.State) { | ||
| const runtimeMessageState = runtimeMessage as positron.LanguageRuntimeState; | ||
| if (runtimeMessageState.state === positron.RuntimeOnlineState.Idle) { | ||
| resolve(); | ||
| disp.dispose(); | ||
| } | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| // Actual formatting is done by styler | ||
| runtime.execute( | ||
| `styler::style_file('${stylerFile}')`, | ||
| id, | ||
| positron.RuntimeCodeExecutionMode.Silent, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We currently don't respect the silent execution mode. I've fixed this in posit-dev/ark#137 This means we shouldn't have to worry about setting the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for adding that! I will plan to merge this PR and then when I come back to the range formatter, I'll just double check that posit-dev/ark#137 solved the issue. |
||
| positron.RuntimeErrorBehavior.Continue); | ||
|
|
||
| // Wait for the the runtime to be idle, or for the timeout: | ||
| await Promise.race([promise, timeout(2e4, 'waiting for formatting')]); | ||
|
|
||
| // Read the now formatted file and then delete it | ||
| const formattedSource = fs.readFileSync(stylerFile).toString(); | ||
| fs.promises.unlink(stylerFile); | ||
|
|
||
| // Return the formatted source | ||
| const fileStart = new vscode.Position(0, 0); | ||
| const fileEnd = document.lineAt(document.lineCount - 1).range.end; | ||
| return [new vscode.TextEdit(new vscode.Range(fileStart, fileEnd), formattedSource)]; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -277,6 +277,20 @@ export async function* rRuntimeProvider( | |
| } | ||
| } | ||
|
|
||
|
|
||
| export async function getRunningRRuntime(runtimes: Map<string, RRuntime>): Promise<RRuntime> { | ||
| const runningRuntimes = await positron.runtime.getRunningRuntimes('r'); | ||
| if (!runningRuntimes || !runningRuntimes.length) { | ||
| throw new Error('Cannot get running runtime as there is no R interpreter running.'); | ||
| } | ||
| // For now, there will be only one running R runtime: | ||
| const runtime = runtimes.get(runningRuntimes[0].runtimeId); | ||
| if (!runtime) { | ||
| throw new Error(`R runtime '${runningRuntimes[0].runtimeId}' is not registered in the extension host`); | ||
| } | ||
| return runtime; | ||
| } | ||
|
|
||
|
Comment on lines
+281
to
+293
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The formatter is another example of us needing to work with the running runtime in the extension, at least for now while it uses styler. I abstracted out this function for the reusable pieces so now we:
I don't think this is too bad for now but we might need to revisit this approach. |
||
| // directory where this OS is known to keep its R installations | ||
| function rHeadquarters(): string { | ||
| switch (process.platform) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a future PR, I'll add another method for
provideDocumentRangeFormattingEditsbut I'd like to get this reviewed and merged first in a simpler state.