Fix indentation of R pipelines via OnTypeFormatting (#2922)

extensions/positron-r/package.json

@@ -222,6 +222,7 @@
     },
     "configurationDefaults": {
       "[r]": {
+        "editor.formatOnType": true,
         "editor.tabSize": 2,
         "editor.wordSeparators": "`~!@#$%^&*()-=+[{]}\\|;:'\",<>/?",
         "editor.smartSelect.selectLeadingAndTrailingWhitespace": false

extensions/positron-r/src/lsp.ts

@@ -19,7 +19,7 @@ import {
 import { Socket } from 'net';
 import { RHelpTopicProvider } from './help';
 import { RLspOutputChannelManager } from './lsp-output-channel-manager';
-
+import { R_DOCUMENT_SELECTORS } from './provider';
 /**
  * The state of the language server.
  */
@@ -39,6 +39,8 @@ export class ArkLsp implements vscode.Disposable {
 	private _client?: LanguageClient;
 
 	private _state: LspState = LspState.uninitialized;
+	private _stateEmitter = new vscode.EventEmitter<LspState>();
+	onDidChangeState = this._stateEmitter.event;
 
 	/** Promise that resolves after initialization is complete */
 	private _initializing?: Promise<void>;
@@ -51,6 +53,11 @@ export class ArkLsp implements vscode.Disposable {
 		private readonly _metadata: positron.RuntimeSessionMetadata
 	) { }
 
+	private setState(state: LspState) {
+		this._state = state;
+		this._stateEmitter.fire(state);
+	}
+
 	/**
 	 * Activate the language server; returns a promise that resolves when the LSP is
 	 * activated.
@@ -101,13 +108,7 @@ export class ArkLsp implements vscode.Disposable {
 			// untitled R files, in-memory R files (e.g. the console), and R / Quarto / R Markdown files on disk.
 			documentSelector: notebookUri ?
 				[{ language: 'r', pattern: notebookUri.path }] :
-				[
-					{ language: 'r', scheme: 'untitled' },
-					{ language: 'r', scheme: 'inmemory' },  // Console
-					{ language: 'r', pattern: '**/*.{r,R}' },
-					{ language: 'r', pattern: '**/*.{qmd,Qmd}' },
-					{ language: 'r', pattern: '**/*.{rmd,Rmd}' },
-				],
+				R_DOCUMENT_SELECTORS,
 			synchronize: notebookUri ?
 				undefined :
 				{
@@ -134,7 +135,7 @@ export class ArkLsp implements vscode.Disposable {
 			// Convert the state to our own enum
 			switch (event.newState) {
 				case State.Starting:
-					this._state = LspState.starting;
+					this.setState(LspState.starting);
 					break;
 				case State.Running:
 					if (this._initializing) {
@@ -146,14 +147,14 @@ export class ArkLsp implements vscode.Disposable {
 						}
 						out.resolve();
 					}
-					this._state = LspState.running;
+					this.setState(LspState.running);
 					break;
 				case State.Stopped:
 					if (this._initializing) {
 						LOGGER.info(`ARK (R ${this._version}) language client init failed`);
 						out.reject('Ark LSP client stopped before initialization');
 					}
-					this._state = LspState.stopped;
+					this.setState(LspState.stopped);
 					break;
 			}
 			LOGGER.info(`ARK (R ${this._version}) language client state changed ${oldState} => ${this._state}`);
@@ -215,6 +216,53 @@ export class ArkLsp implements vscode.Disposable {
 		return this._state;
 	}
 
+	/**
+	 * Wait for the LSP to be connected.
+	 *
+	 * Resolves to `true` once the LSP is connected. Resolves to `false` if the
+	 * LSP has been stopped. Rejects if the LSP fails to start.
+	 */
+	async wait(): Promise<boolean> {
+		switch (this.state) {
+			case LspState.running: return true;
+			case LspState.stopped: return false;
+
+			case LspState.starting: {
+				// Inherit init promise. This can reject if init fails.
+				await this._initializing;
+				return true;
+			}
+
+			case LspState.uninitialized: {
+				const handles = new PromiseHandles<boolean>();
+
+				const cleanup = this.onDidChangeState(state => {
+					let out: boolean;
+					switch (this.state) {
+						case LspState.running: out = true; break;
+						case LspState.stopped: out = false; break;
+						case LspState.uninitialized: return;
+						case LspState.starting: {
+							// Inherit init promise
+							if (this._initializing) {
+								cleanup.dispose();
+								this._initializing.
+									then(() => handles.resolve(true)).
+									catch((err) => handles.reject(err));
+							}
+							return;
+						}
+					}
+
+					cleanup.dispose();
+					handles.resolve(out);
+				});
+
+				return await handles.promise;
+			}
+		}
+	}
+
 	/**
 	 * Registers additional Positron-specific LSP methods. These programmatic
 	 * language features are not part of the LSP specification, and are

extensions/positron-r/src/provider.ts

@@ -17,6 +17,16 @@ import { LOGGER } from './extension';
 import { readLines } from './util';
 import { EXTENSION_ROOT_DIR, MINIMUM_R_VERSION } from './constants';
 
+// We don't give this a type so it's compatible with both the VS Code
+// and the LSP types
+export const R_DOCUMENT_SELECTORS = [
+	{ language: 'r', scheme: 'untitled' },
+	{ language: 'r', scheme: 'inmemory' },  // Console
+	{ language: 'r', pattern: '**/*.{r,R}' },
+	{ language: 'r', pattern: '**/*.{qmd,Qmd}' },
+	{ language: 'r', pattern: '**/*.{rmd,Rmd}' },
+];
+
 /**
  * Discovers R language runtimes for Positron; implements
  * positron.LanguageRuntimeDiscoverer.

extensions/positron-r/src/session.ts

@@ -531,6 +531,16 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa
 		this._lsp.activate(port, this.context);
 	}
 
+	/**
+	 * Wait for the LSP to be connected.
+	 *
+	 * Resolves to `true` once the LSP is connected. Resolves to `false` if the
+	 * LSP has been stopped. Rejects if the LSP fails to start.
+	 */
+	async waitLsp(): Promise<boolean> {
+		return await this._lsp.wait();
+	}
+
 	private async startDap(): Promise<void> {
 		if (this._kernel) {
 			const port = await this.adapterApi!.findAvailablePort([], 25);

extensions/positron-r/src/test/indentation.test.ts

@@ -1,9 +1,11 @@
 import * as vscode from 'vscode';
+import * as positron from 'positron';
 import * as assert from 'assert';
 import * as fs from 'fs';
 import { CURSOR, type, withFileEditor } from './editor-utils';
 import { EXTENSION_ROOT_DIR } from '../constants';
-import { removeLeadingLines } from '../util';
+import { delay, removeLeadingLines } from '../util';
+import { RSession } from '../session';
 
 const snapshotsFolder = `${EXTENSION_ROOT_DIR}/src/test/snapshots`;
 const snippetsPath = `${snapshotsFolder}/indentation-cases.R`;
@@ -30,6 +32,22 @@ suite('Indentation', () => {
 	// it if that's a bug to fix.
 	test('Regenerate and check', async () => {
 		await init();
+
+		// There doesn't seem to be a method that resolves when a language is
+		// both discovered and ready to be started
+		let info;
+		while (true) {
+			try {
+				info = await positron.runtime.getPreferredRuntime('r');
+				break;
+			} catch (_) {
+				await delay(50);
+			}
+		}
+
+		let ses = await positron.runtime.startLanguageRuntime(info!.runtimeId, 'Snapshot tests') as RSession;
+		await ses.waitLsp();
+
 		const expected = fs.readFileSync(snapshotsPath, 'utf8');
 		const current = await regenerateIndentSnapshots();
 
@@ -54,9 +72,22 @@ async function regenerateIndentSnapshots() {
 	for (const snippet of snippets) {
 		const bareSnippet = snippet.split('\n').slice(0, -1).join('\n');
 
-		await withFileEditor(snippet, 'R', async (_editor, doc) => {
+		await withFileEditor(snippet, 'R', async (editor, doc) => {
 			// Type one newline character to trigger indentation
-			await type(doc, `\n${CURSOR}`);
+			await type(doc, '\n');
+
+			await vscode.commands.executeCommand('vscode.executeFormatOnTypeProvider',
+				doc.uri,
+				editor.selection.start,
+				'\n',
+				{
+					insertSpaces: true,
+					tabSize: 4,
+				}
+			);
+
+			await type(doc, `${CURSOR}`);
+
 			const snapshot = removeLeadingLines(doc.getText(), /^$|^#/);
 			snapshots.push(bareSnippet + '\n# ->\n' + snapshot);
 		});

extensions/positron-r/src/test/snapshots/.vscode/settings.json

@@ -1,6 +1,8 @@
 {
     "workbench.localHistory.enabled": false,
-    "editor.insertSpaces": true,
-    "editor.indentSize": 4,
-    "editor.tabSize": 4,
+	"[r]" : {
+		"editor.insertSpaces": true,
+		"editor.indentSize": 4,
+		"editor.tabSize": 4
+	}
 }

extensions/positron-r/src/test/snapshots/indentation-cases.R

@@ -18,13 +18,11 @@ data |>"<>"
 
 # ---
 # Starting a pipeline (one empty line)
-# FIXME
 data |>
     "<>"
 
 # ---
 # Starting a pipeline (multiple empty lines)
-# FIXME
 data |>
 
     "<>"
@@ -53,14 +51,12 @@ data |>
 
 # ---
 # Continuing a one-liner pipeline (comment line)
-# FIXME
 data |>
     fn1() |>
     # foo"<>"
 
 # ---
 # Continuing a one-liner pipeline (after a comment line)
-# FIXME
 data |>
     fn1() |>
     # foo
@@ -75,7 +71,6 @@ data |>
 
 # ---
 # Continuing a multi-liner pipeline
-# FIXME
 data |>
     fn1(
         x,
@@ -84,7 +79,6 @@ data |>
 
 # ---
 # Continuing a multi-liner pipeline (trailing expression)
-# FIXME
 data |>
     fn1(
         x,
@@ -103,20 +97,55 @@ data |>
 
 # ---
 # Dedent after pipeline (multiple lines)
-# FIXME
+# https://github.com/posit-dev/positron/issues/2764
 data |>
     fn1() |>
     fn2(
         "arg"
     )"<>"
 
+# ---
+# Dedent after pipeline (token)
+1 +
+    foo(
+        x
+    ) +
+    bar"<>"
+
+# ---
+# Dedent after pipeline (nested)
+{
+    1 +
+        foo(
+            x
+        ) +
+        bar"<>"
+}
+
 # ---
 # Stickiness of dedent after pipeline
 # https://github.com/posit-dev/positron/issues/1727
 data |>
     fn()
 "<>"
 
+# ---
+# Stickiness of dedent after pipeline (nested)
+{
+    data |>
+        fn()
+    "<>"
+}
+
+# ---
+# Stickiness of dedent after pipeline (nested)
+{
+    fn() %>%
+
+        foo"<>"
+}
+
+
 # ---
 # Stickiness of dedent after pipeline (trailing comment)
 data |>
@@ -146,7 +175,6 @@ for (i in NA) NULL"<>"
 # ---
 # Indent after finished loop (call)
 # https://github.com/posit-dev/positron/issues/1880
-# FIXME
 for (i in 1) fn()"<>"
 
 # ---