From ac971f737ea1dd4f8a5c3fe7ee2d547cc24ceee9 Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Wed, 14 Oct 2026 01:39:27 +0800 Subject: [PATCH 1/7] fix: preserve R code selections, strings, and process output --- src/lineCache.ts | 5 +++ src/rmarkdown/chunks.ts | 60 +++++++++++--------------- src/selection.ts | 1 + src/test/suite/chunks.test.ts | 30 +++++++++++++ src/test/suite/lineCache.test.ts | 9 +++- src/test/suite/selectionWord.test.ts | 19 +++++++++ src/test/suite/util.test.ts | 64 ++++++++++++++++++++++++++++ src/util.ts | 38 +++++++++-------- 8 files changed, 172 insertions(+), 54 deletions(-) create mode 100644 src/test/suite/chunks.test.ts create mode 100644 src/test/suite/selectionWord.test.ts create mode 100644 src/test/suite/util.test.ts diff --git a/src/lineCache.ts b/src/lineCache.ts index 3efb9abd9..82d83811a 100644 --- a/src/lineCache.ts +++ b/src/lineCache.ts @@ -51,6 +51,11 @@ export function cleanLine(text: string): string { let withinQuotes = null; for (let i = 0; i < text.length; i++) { const c = text[i]; + if (withinQuotes && c === '\\') { + cleaned += c; + if (i + 1 < text.length) { cleaned += text[++i]; } + continue; + } if (isQuote(c)) { if (withinQuotes === null) { withinQuotes = c; diff --git a/src/rmarkdown/chunks.ts b/src/rmarkdown/chunks.ts index d5cb8d75e..c802c23f5 100644 --- a/src/rmarkdown/chunks.ts +++ b/src/rmarkdown/chunks.ts @@ -85,13 +85,27 @@ export function getChunks(document: vscode.TextDocument): RMarkdownChunk[] { let line = 0; let chunkId = 0; // One-based index let chunkStartLine: number | undefined = undefined; - let chunkEndLine: number | undefined = undefined; - let codeEndLine: number | undefined = undefined; let chunkLanguage: string | undefined = undefined; let chunkOptions: string | undefined = undefined; let chunkEval: boolean | undefined = undefined; const isRDoc = isRDocument(document); + const finishChunk = (endLine: number, codeEndLine: number) => { + const startLine = chunkStartLine!; + const codeStart = codeEndLine > startLine + ? new vscode.Position(startLine + 1, 0) + : new vscode.Position(startLine, lines[startLine].length); + const codeEnd = codeEndLine > startLine + ? new vscode.Position(codeEndLine, lines[codeEndLine].length) : codeStart; + chunks.push({ + id: chunkId, startLine, endLine, language: chunkLanguage, + options: chunkOptions, eval: chunkEval, + chunkRange: new vscode.Range(startLine, 0, endLine, lines[endLine].length), + codeRange: new vscode.Range(codeStart, codeEnd) + }); + chunkStartLine = undefined; + }; + while (line < lines.length) { if (chunkStartLine === undefined) { if (isChunkStartLine(lines[line], isRDoc)) { @@ -102,44 +116,18 @@ export function getChunks(document: vscode.TextDocument): RMarkdownChunk[] { chunkEval = getChunkEval(chunkOptions); } } else { - // Second condition is for the last chunk in an .R file - const isRDocAndFinalLine = (isRDoc && line === lines.length - 1); - if (isChunkEndLine(lines[line], isRDoc) || isRDocAndFinalLine) { - chunkEndLine = line; - codeEndLine = line - 1; - - // isChunkEndLine looks for `# %%` in `.R` files, so if found, then need to go back one line to mark end of code chunk. - if (isRDoc && !isRDocAndFinalLine) { - chunkEndLine = chunkEndLine - 1; - codeEndLine = chunkEndLine; - line = line - 1; - } - - const chunkRange = new vscode.Range( - new vscode.Position(chunkStartLine, 0), - new vscode.Position(line, lines[line].length) - ); - const codeRange = new vscode.Range( - new vscode.Position(chunkStartLine + 1, 0), - new vscode.Position(codeEndLine, lines[codeEndLine].length) - ); - - chunks.push({ - id: chunkId, // One-based index - startLine: chunkStartLine, - endLine: chunkEndLine, - language: chunkLanguage, - options: chunkOptions, - eval: chunkEval, - chunkRange: chunkRange, - codeRange: codeRange - }); - - chunkStartLine = undefined; + if (isChunkEndLine(lines[line], isRDoc)) { + finishChunk(isRDoc ? line - 1 : line, line - 1); + // An R marker ends the previous chunk and starts the next one, + // including when the marker is the document's final line. + if (isRDoc) { line--; } } } line++; } + if (isRDoc && chunkStartLine !== undefined) { + finishChunk(lines.length - 1, lines.length - 1); + } return chunks; } diff --git a/src/selection.ts b/src/selection.ts index b3bce7f3d..8b2bfed2f 100644 --- a/src/selection.ts +++ b/src/selection.ts @@ -16,6 +16,7 @@ export function getWordOrSelection(): string | undefined { if ((selection.start.line === selection.end.line) && (selection.start.character === selection.end.character)) { const wordRange = currentDocument.getWordRangeAtPosition(selection.start); + if (!wordRange) { return undefined; } text = currentDocument.getText(wordRange); } else { text = currentDocument.getText(textEditor.selection); diff --git a/src/test/suite/chunks.test.ts b/src/test/suite/chunks.test.ts new file mode 100644 index 000000000..3daa6f9cc --- /dev/null +++ b/src/test/suite/chunks.test.ts @@ -0,0 +1,30 @@ +import * as assert from 'assert'; +import * as vscode from 'vscode'; +import { getChunks } from '../../rmarkdown/chunks'; + +suite('R code chunk boundaries', () => { + async function chunkCode(text: string, language = 'r') { + const document = await vscode.workspace.openTextDocument({ language, content: text }); + return getChunks(document).map(chunk => document.getText(chunk.codeRange)); + } + + test('includes the final code line without a trailing newline', async () => { + assert.deepStrictEqual(await chunkCode('# %%\nx <- 1'), ['x <- 1']); + assert.deepStrictEqual(await chunkCode('# %%\nx <- 1\ny <- 2'), ['x <- 1\ny <- 2']); + }); + + test('handles a final chunk marker and adjacent empty chunks', async () => { + assert.deepStrictEqual(await chunkCode('# %%\nx <- 1\n# %%'), ['x <- 1', '']); + assert.deepStrictEqual(await chunkCode('# %%\n# %%\ny <- 2'), ['', 'y <- 2']); + assert.deepStrictEqual(await chunkCode('# %%'), ['']); + }); + + test('empty fenced R Markdown chunks never include their delimiters', async () => { + assert.deepStrictEqual(await chunkCode('```{r}\n```', 'rmd'), ['']); + assert.deepStrictEqual(await chunkCode('```{r}\nx <- 1\n```', 'rmd'), ['x <- 1']); + }); + + test('section headers end an R chunk without dropping its preceding line', async () => { + assert.deepStrictEqual(await chunkCode('# %%\nx <- 1\n# section ----'), ['x <- 1']); + }); +}); diff --git a/src/test/suite/lineCache.test.ts b/src/test/suite/lineCache.test.ts index 7b72c00a3..7cc0da9a5 100644 --- a/src/test/suite/lineCache.test.ts +++ b/src/test/suite/lineCache.test.ts @@ -13,4 +13,11 @@ suite('lineCache Tests', () => { assert.strictEqual(cleanLine('abcde "#abd" # jr2 2r'), 'abcde "#abd"'); assert.strictEqual(cleanLine('abcde "\'#abd\'" #jr 22r" '), 'abcde "\'#abd\'"'); }); -}); \ No newline at end of file + + test('escaped quotes keep hashes inside strings and strip trailing comments', () => { + assert.strictEqual(cleanLine(String.raw`x <- "a\"#b" # comment`), String.raw`x <- "a\"#b"`); + assert.strictEqual(cleanLine(String.raw`x <- 'a\'#b' # comment`), String.raw`x <- 'a\'#b'`); + assert.strictEqual(cleanLine('x <- `a\\`#b` # comment'), 'x <- `a\\`#b`'); + assert.strictEqual(cleanLine(String.raw`x <- "a\\" # comment`), String.raw`x <- "a\\"`); + }); +}); diff --git a/src/test/suite/selectionWord.test.ts b/src/test/suite/selectionWord.test.ts new file mode 100644 index 000000000..c67e96322 --- /dev/null +++ b/src/test/suite/selectionWord.test.ts @@ -0,0 +1,19 @@ +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import * as vscode from 'vscode'; +import { getWordOrSelection } from '../../selection'; + +suite('Word execution selection', () => { + test('a cursor outside a word never selects the entire document', () => { + const sandbox = sinon.createSandbox(); + const getText = sandbox.stub().returns('dangerous_full_document()'); + sandbox.stub(vscode.window, 'activeTextEditor').value({ + selection: new vscode.Selection(0, 0, 0, 0), + document: { getWordRangeAtPosition: () => undefined, getText } + }); + try { + assert.strictEqual(getWordOrSelection(), undefined); + sinon.assert.notCalled(getText); + } finally { sandbox.restore(); } + }); +}); diff --git a/src/test/suite/util.test.ts b/src/test/suite/util.test.ts new file mode 100644 index 000000000..008d10fd7 --- /dev/null +++ b/src/test/suite/util.test.ts @@ -0,0 +1,64 @@ +import * as assert from 'assert'; +import cp from 'child_process'; +import { EventEmitter } from 'events'; +import * as path from 'path'; +import * as os from 'os'; +import * as sinon from 'sinon'; +import * as vscode from 'vscode'; +import * as util from '../../util'; +import { mockExtensionContext } from '../common/mockvscode'; + +suite('R command utilities', () => { + let sandbox: sinon.SinonSandbox; + setup(() => { + sandbox = sinon.createSandbox(); + mockExtensionContext(path.resolve(__dirname, '../../..'), sandbox); + }); + teardown(() => sandbox.restore()); + + test('R literals escape quotes, backslashes and control characters', () => { + assert.strictEqual(util.ToRStringLiteral('a"b\'c\\d\n\r\t\b\f\v', '"'), + String.raw`"a\"b'c\\d\n\r\t\b\f\v"`); + assert.strictEqual(util.ToRStringLiteral('a"b\'c', "'"), String.raw`'a"b\'c'`); + // Editor-path templates can supply either kind of surrounding quote. + assert.strictEqual(util.ToRStringLiteral('a"b\'c', ''), String.raw`a\"b\'c`); + }); + + test('a failed executable settles with its spawn error', async () => { + const result = await Promise.race([ + util.spawnAsync(path.join(os.tmpdir(), 'vscode-r-nonexistent-executable')), + new Promise(resolve => setTimeout(resolve, 500)) + ]); + assert.ok(result, 'spawnAsync must finish when spawning fails'); + assert.strictEqual((result.error as NodeJS.ErrnoException).code, 'ENOENT'); + assert.notStrictEqual(result.status, 0); + }); + + test('collects all output through close and preserves fragmented UTF-8', async () => { + const child = Object.assign(new EventEmitter(), { + pid: 123, + stdout: new EventEmitter(), stderr: new EventEmitter(), kill: sandbox.stub() + }); + sandbox.stub(cp, 'spawn').returns(child as unknown as cp.ChildProcessWithoutNullStreams); + const result = util.spawnAsync('R'); + const bytes = Buffer.from('你好'); + child.stdout.emit('data', bytes.subarray(0, 2)); + child.emit('exit', 0, null); + child.stdout.emit('data', bytes.subarray(2)); + child.stderr.emit('data', Buffer.from('diagnostic')); + child.emit('close', 0, null); + assert.strictEqual((await result).stdout, '你好'); + assert.strictEqual((await result).stderr, 'diagnostic'); + }); + + test('progress propagates synchronous callback exceptions', async () => { + sandbox.stub(vscode.window, 'withProgress').callsFake((_options, task) => + task({ report: () => undefined }, new vscode.CancellationTokenSource().token)); + const clock = sandbox.useFakeTimers(); + const error = new Error('refresh failed'); + const result = util.doWithProgress(() => { throw error; }); + const rejected = assert.rejects(result, candidate => candidate === error); + assert.doesNotThrow(() => clock.tick(1)); + await rejected; + }); +}); diff --git a/src/util.ts b/src/util.ts index 44264be74..2299f6b00 100644 --- a/src/util.ts +++ b/src/util.ts @@ -10,6 +10,7 @@ import * as vscode from 'vscode'; import * as cp from 'child_process'; import { extensionContext } from './extension'; import { randomBytes } from 'crypto'; +import { StringDecoder } from 'string_decoder'; import { createRPathResolverDependencies, findExecutableOnPath, @@ -128,17 +129,15 @@ export function ToRStringLiteral(s: string, quote: string): string { return 'NULL'; } - return (quote + - s.replace(/\\/g, '\\\\') - .replace(/"""/g, `\\${quote}`) - .replace(/\\n/g, '\\n') - .replace(/\\r/g, '\\r') - .replace(/\\t/g, '\\t') - .replace(/\\b/g, '\\b') - .replace(/\\a/g, '\\a') - .replace(/\\f/g, '\\f') - .replace(/\\v/g, '\\v') + - quote); + const escapes: Record = { '\\': '\\\\', '\n': '\\n', '\r': '\\r', '\t': '\\t', + '\b': '\\b', '\x07': '\\a', '\f': '\\f', '\v': '\\v' }; + const escaped = Array.from(s, character => { + if (character === '"' || character === "'") { + return !quote || character === quote ? `\\${character}` : character; + } + return escapes[character] ?? character; + }).join(''); + return quote + escaped + quote; } export async function delay(ms: number): Promise { @@ -272,9 +271,9 @@ export async function doWithProgress(cb: (token?: vscode.CancellationToken, p title: title }; return await vscode.window.withProgress(options, async (progress, token) => { - return await new Promise((resolve) => setTimeout(() => { - const ret = cb(token, progress); - resolve(ret); + return await new Promise((resolve, reject) => setTimeout(() => { + try { resolve(cb(token, progress)); } + catch (error) { reject(error); } })); }); } @@ -503,16 +502,21 @@ export async function spawnAsync(command: string, args?: ReadonlyArray, if (childProcess.pid !== undefined) { result.pid = childProcess.pid; } + const stdoutDecoder = new StringDecoder('utf8'); + const stderrDecoder = new StringDecoder('utf8'); childProcess.stdout?.on('data', (chunk: Buffer) => { - result.stdout += chunk.toString(); + result.stdout += stdoutDecoder.write(chunk); }); childProcess.stderr?.on('data', (chunk: Buffer) => { - result.stderr += chunk.toString(); + result.stderr += stderrDecoder.write(chunk); }); childProcess.on('error', (err: Error) => { result.error = err; }); - childProcess.on('exit', (code, signal) => { + // close also fires for spawn errors and waits for both output pipes. + childProcess.on('close', (code, signal) => { + result.stdout += stdoutDecoder.end(); + result.stderr += stderrDecoder.end(); result.status = code; result.signal = signal; resolve(result); From dcbca1a55fbabfd8a015602cd8916b756d90cb3c Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Wed, 14 Oct 2026 01:39:52 +0800 Subject: [PATCH 2/7] fix: settle R Markdown jobs and preserve quoted paths --- src/rmarkdown/draft.ts | 20 ++--- src/rmarkdown/knit.ts | 49 ++++++------ src/rmarkdown/manager.ts | 112 +++++++++++++-------------- src/rmarkdown/preview.ts | 7 +- src/test/suite/rmarkdown.test.ts | 128 +++++++++++++++++++++++++++++++ 5 files changed, 224 insertions(+), 92 deletions(-) create mode 100644 src/test/suite/rmarkdown.test.ts diff --git a/src/rmarkdown/draft.ts b/src/rmarkdown/draft.ts index e6f40d6a6..66ed93acb 100644 --- a/src/rmarkdown/draft.ts +++ b/src/rmarkdown/draft.ts @@ -101,8 +101,7 @@ async function launchTemplatePicker(cwd: string): Promise { - const fileString = ToRStringLiteral(file, ''); - const cmd = `cat(normalizePath(rmarkdown::draft(file='${fileString}', template='${template.info.id}', package='${template.info.package}', edit=FALSE)))`; + const cmd = `cat(normalizePath(rmarkdown::draft(file=${ToRStringLiteral(file, '"')}, template=${ToRStringLiteral(template.info.id, '"')}, package=${ToRStringLiteral(template.info.package, '"')}, edit=FALSE)))`; return await executeRCommand(cmd, cwd, (e: Error) => { void window.showErrorMessage(e.message); return ''; @@ -136,7 +135,7 @@ export async function newDraft(): Promise { const dir = path.join(parsedPath.dir, parsedPath.name); if (fs.existsSync(dir)) { if (await getConfirmation(`Folder already exists. Are you sure you want to replace the folder?`)) { - fs.rmdirSync(dir, { recursive: true }); + fs.rmSync(dir, { recursive: true, force: true }); } else { return; } @@ -151,12 +150,15 @@ export async function newDraft(): Promise { } else { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'vscode-R-')); const tempFile = path.join(tempDir, 'draft.Rmd'); - const draftPath = await makeDraft(tempFile, template, cwd); - if (draftPath) { - const text = fs.readFileSync(draftPath, 'utf8'); - await workspace.openTextDocument({ language: 'rmd', content: text }) - .then(document => window.showTextDocument(document)); + try { + const draftPath = await makeDraft(tempFile, template, cwd); + if (draftPath) { + const text = fs.readFileSync(draftPath, 'utf8'); + await workspace.openTextDocument({ language: 'rmd', content: text }) + .then(document => window.showTextDocument(document)); + } + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); } - fs.rmdirSync(tempDir, { recursive: true }); } } diff --git a/src/rmarkdown/knit.ts b/src/rmarkdown/knit.ts index cfff466b8..2cc487458 100644 --- a/src/rmarkdown/knit.ts +++ b/src/rmarkdown/knit.ts @@ -116,9 +116,10 @@ export class RMarkdownKnitManager extends RMarkdownManager { let paramObj: IYamlFrontmatter = {}; if (yamlText) { try { - paramObj = yaml.load( - yamlText - ) as IYamlFrontmatter; + const parsed: unknown = yaml.load(yamlText); + if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { + paramObj = parsed as IYamlFrontmatter; + } } catch (e) { console.error(`Could not parse YAML frontmatter for "${docPath}". Error: ${String(e)}`); } @@ -136,26 +137,21 @@ export class RMarkdownKnitManager extends RMarkdownManager { // precedence: // knit > site > configuration - if (yamlParams?.['knit']) { + const formatArg = outputFormat ? `, output_format = ${util.ToRStringLiteral(outputFormat, '"')}` : ''; + if (typeof yamlParams['knit'] === 'string' && yamlParams['knit']) { const knitParam = yamlParams['knit'].trim(); - knitCommand = outputFormat ? - `${knitParam}(${docPath}, output_format = '${outputFormat}')` : - `${knitParam}(${docPath})`; + knitCommand = `(${knitParam})(${docPath}${formatArg})`; } else if (!this.isREADME(docPath) && yamlParams?.['site']) { - knitCommand = outputFormat ? - `rmarkdown::render_site(${docPath}, output_format = '${outputFormat}')` : - `rmarkdown::render_site(${docPath})`; + knitCommand = `rmarkdown::render_site(${docPath}${formatArg})`; } else { const cmd = util.config().get('rmarkdown.knit.command'); if (!cmd) { return; } - knitCommand = outputFormat ? - `${cmd}(${docPath}, output_format = '${outputFormat}')` : - `${cmd}(${docPath})`; + knitCommand = `${cmd}(${docPath}${formatArg})`; } - return knitCommand.replace(/['"]/g, '\''); + return knitCommand; } // check if the workspace of the document is a R Markdown site. @@ -166,8 +162,8 @@ export class RMarkdownKnitManager extends RMarkdownManager { if (!wad) { return; } - const rootFolder = vscode.workspace.workspaceFolders?.[0]?.uri?.fsPath ?? path.dirname(wad); - const indexFile = (await vscode.workspace.findFiles(new vscode.RelativePattern(rootFolder, 'index.{Rmd,rmd, md}'), null, 1))?.[0]; + const rootFolder = vscode.workspace.getWorkspaceFolder(vscode.Uri.file(wad))?.uri.fsPath ?? path.dirname(wad); + const indexFile = (await vscode.workspace.findFiles(new vscode.RelativePattern(rootFolder, 'index.{Rmd,rmd,md}'), null, 1))?.[0]; const siteRoot = path.join(path.dirname(wad), '_site.yml'); // 'Simple' R Markdown websites require all docs to be in the root folder @@ -275,19 +271,22 @@ export class RMarkdownKnitManager extends RMarkdownManager { return; } this.busyUriStore.add(busyPath); - await this.renderDocument( - rDocumentPath, - wad.uri.fsPath, - path.basename(wad.uri.fsPath), - this.getYamlFrontmatter(wad.uri.fsPath), - outputFormat - ); - this.busyUriStore.delete(busyPath); + try { + await this.renderDocument( + rDocumentPath, + wad.uri.fsPath, + path.basename(wad.uri.fsPath), + this.getYamlFrontmatter(wad.uri.fsPath), + outputFormat + ); + } finally { + this.busyUriStore.delete(busyPath); + } } else { if (outputFormat === undefined) { void runTextInTerm(`rmarkdown::render(${rDocumentPath})`); } else { - void runTextInTerm(`rmarkdown::render(${rDocumentPath}, '${outputFormat}')`); + void runTextInTerm(`rmarkdown::render(${rDocumentPath}, ${util.ToRStringLiteral(outputFormat, '"')})`); } } } diff --git a/src/rmarkdown/manager.ts b/src/rmarkdown/manager.ts index e062955ea..8bb1fa5bf 100644 --- a/src/rmarkdown/manager.ts +++ b/src/rmarkdown/manager.ts @@ -2,6 +2,7 @@ import * as util from '../util'; import * as vscode from 'vscode'; import * as cp from 'child_process'; import path = require('path'); +import { StringDecoder } from 'string_decoder'; import { DisposableProcess, spawn } from '../util'; export enum KnitWorkingDirectory { @@ -39,12 +40,11 @@ export abstract class RMarkdownManager { switch (knitDir) { // the directory containing the R Markdown document case KnitWorkingDirectory.documentDirectory: { - return path.dirname(docPath)?.replace(/\\/g, '/')?.replace(/['"]/g, '\\"') ?? undefined; + return path.dirname(docPath); } // the root of the current workspace case KnitWorkingDirectory.workspaceRoot: { - const currentDocumentWorkspace = vscode.workspace.getWorkspaceFolder(vscode.Uri.file(docPath) ?? vscode.window.activeTextEditor?.document?.uri)?.uri?.fsPath ?? undefined; - return currentDocumentWorkspace?.replace(/\\/g, '/')?.replace(/['"]/g, '\\"') ?? undefined; + return vscode.workspace.getWorkspaceFolder(vscode.Uri.file(docPath))?.uri.fsPath; } // the working directory of the attached terminal, NYI // case 'current directory': { @@ -60,6 +60,7 @@ export abstract class RMarkdownManager { let currentProgress = 0; let printOutput = true; + let cancellation: vscode.Disposable | undefined; return await new Promise( (resolve, reject) => { const scriptArgs = args.scriptArgs; @@ -77,7 +78,7 @@ export abstract class RMarkdownManager { // When there's no LANG variable, we should try to set to a UTF-8 compatible one, as R relies // on locale setting (based on LANG) to render certain characters. // See https://github.com/REditorSupport/vscode-R/issues/933 - const env = process.env; + const env = { ...process.env }; if (env.LANG === undefined) { env.LANG = 'en_US.UTF-8'; } @@ -115,75 +116,74 @@ export abstract class RMarkdownManager { this.rMarkdownOutput.appendLine(`==> ${args.rCmd}`); } - childProcess.stdout.on('data', - (data: Buffer) => { - const dat = data.toString('utf8'); - if (printOutput) { - this.rMarkdownOutput.appendLine(dat); - + const stdoutDecoder = new StringDecoder('utf8'); + const stderrDecoder = new StringDecoder('utf8'); + let outputBuffer = ''; + let outputComplete = false; + const receive = (dat: string) => { + // R's output-path marker can span several pipe writes. + // Retain a bounded suffix while streaming ordinary output. + outputBuffer = (outputBuffer + dat).slice(-65536); + if (printOutput) { + this.rMarkdownOutput.appendLine(dat); + } + const percentRegex = /[0-9]+(?=%)/g; + const divisionRegex = /([0-9]+)\/([0-9]+)/g; + const percentRegOutput = dat.match(percentRegex); + if (percentRegOutput) { + for (const item of percentRegOutput) { + const perc = Number(item); + progress?.report({ increment: perc - currentProgress, message: `${perc}%` }); + currentProgress = perc; } - const percentRegex = /[0-9]+(?=%)/g; - const divisionRegex = /([0-9]+)\/([0-9]+)/g; - const percentRegOutput = dat.match(percentRegex); - if (percentRegOutput) { - for (const item of percentRegOutput) { - const perc = Number(item); - progress?.report( - { - increment: perc - currentProgress, - message: `${perc}%` - } - ); - currentProgress = perc; - } - } else { - const divisionRegOutput = divisionRegex.exec(dat); - if (divisionRegOutput) { - const perc = Math.ceil(parseInt(divisionRegOutput[1]) / parseInt(divisionRegOutput[2]) * 100); - progress?.report( - { - increment: perc - currentProgress, - message: `${perc}%` - } - ); - currentProgress = perc; - } + } else { + const divisionRegOutput = divisionRegex.exec(dat); + if (divisionRegOutput) { + const perc = Math.ceil(parseInt(divisionRegOutput[1]) / parseInt(divisionRegOutput[2]) * 100); + progress?.report({ increment: perc - currentProgress, message: `${perc}%` }); + currentProgress = perc; } + } - if (token?.isCancellationRequested) { - if (childProcess) { - resolve(childProcess); - } - } else { - if (args.callback(dat, childProcess)) { - if (childProcess) { - resolve(childProcess); - } - } - } + if (token?.isCancellationRequested) { + reject({ cp: childProcess, wasCancelled: true }); + } else if (!outputComplete && args.callback(outputBuffer, childProcess)) { + outputComplete = true; + if (childProcess) { resolve(childProcess); } } - ); + }; + childProcess.stdout.on('data', (data: Buffer) => { + try { receive(stdoutDecoder.write(data)); } + catch (error) { + this.rMarkdownOutput.appendLine(String(error)); + reject({ cp: childProcess, wasCancelled: false }); + } + }); childProcess.stderr.on('data', (data: Buffer) => { - const dat = data.toString('utf8'); + const dat = stderrDecoder.write(data); if (printOutput) { this.rMarkdownOutput.appendLine(dat); } }); - childProcess.on('exit', (code, signal) => { + childProcess.on('error', error => { + this.rMarkdownOutput.appendLine(error.message); + reject({ cp: childProcess, wasCancelled: false }); + }); + childProcess.on('close', (code, signal) => { this.rMarkdownOutput.appendLine(`[VSC-R] ${fileName} process exited ` + - (signal ? `from signal '${signal}'` : `with exit code ${code || 'null'}`)); - if (code !== 0) { - reject({ cp: childProcess, wasCancelled: false }); - } + (signal ? `from signal '${signal}'` : `with exit code ${code ?? 'null'}`)); + // If no callback recognized the output, even a zero exit code + // cannot produce a preview. Always settle the pending progress. + reject({ cp: childProcess, wasCancelled: false }); }); - token?.onCancellationRequested(() => { + cancellation = token?.onCancellationRequested(() => { reject({ cp: childProcess, wasCancelled: true }); }); } - ); + ).finally(() => { cancellation?.dispose(); }); } protected async knitWithProgress(args: IKnitArgs): Promise { diff --git a/src/rmarkdown/preview.ts b/src/rmarkdown/preview.ts index 6bc1827e0..40e77655f 100644 --- a/src/rmarkdown/preview.ts +++ b/src/rmarkdown/preview.ts @@ -231,8 +231,11 @@ export class RMarkdownPreviewManager extends RMarkdownManager { this.previewStore.get(filePath)?.panel.reveal(); } else { this.busyUriStore.add(filePath); - await this.previewDocument(filePath, fileName, viewer, currentViewColumn); - this.busyUriStore.delete(filePath); + try { + await this.previewDocument(filePath, fileName, viewer, currentViewColumn); + } finally { + this.busyUriStore.delete(filePath); + } } } diff --git a/src/test/suite/rmarkdown.test.ts b/src/test/suite/rmarkdown.test.ts new file mode 100644 index 000000000..ce3573fde --- /dev/null +++ b/src/test/suite/rmarkdown.test.ts @@ -0,0 +1,128 @@ +import * as assert from 'assert'; +import { EventEmitter } from 'events'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import * as vscode from 'vscode'; +import * as util from '../../util'; +import { RMarkdownManager, KnitWorkingDirectory } from '../../rmarkdown/manager'; +import { RMarkdownKnitManager } from '../../rmarkdown/knit'; +import { mockExtensionContext } from '../common/mockvscode'; + +class KnitHarness extends RMarkdownManager { + directory(kind: KnitWorkingDirectory, file: string) { return this.getKnitDir(kind, file); } + knit(callback: (text: string) => boolean) { + this.rPath = 'R'; + return this.knitDocument({ workingDirectory: os.tmpdir(), filePath: '/test.Rmd', + fileName: 'test.Rmd', scriptArgs: {}, scriptPath: '/knit.R', callback }); + } +} + +suite('R Markdown execution', () => { + let sandbox: sinon.SinonSandbox; + setup(() => { + sandbox = sinon.createSandbox(); + mockExtensionContext(path.resolve(__dirname, '../../..'), sandbox); + }); + teardown(() => sandbox.restore()); + + test('working directories preserve literal quotes for spawn and environment values', () => { + const directory = path.join(os.tmpdir(), `project's "report"`); + const manager = new KnitHarness(); + assert.strictEqual(manager.directory(KnitWorkingDirectory.documentDirectory, path.join(directory, 'test.Rmd')), directory); + sandbox.stub(vscode.workspace, 'getWorkspaceFolder').returns({ uri: vscode.Uri.file(directory) } as vscode.WorkspaceFolder); + assert.strictEqual(manager.directory(KnitWorkingDirectory.workspaceRoot, '/test.Rmd'), directory); + }); + + function mockChild() { + const child = Object.assign(new EventEmitter(), { + stdout: new EventEmitter(), stderr: new EventEmitter(), dispose: sandbox.stub() + }) as unknown as util.DisposableProcess; + sandbox.stub(util, 'spawn').returns(child); + return child; + } + + test('knitting rejects an asynchronous spawn failure', async () => { + const child = mockChild(); + const result = new KnitHarness().knit(() => false); + child.emit('error', new Error('ENOENT')); + child.emit('close', -2, null); + const settled = await Promise.race([ + result.then(() => 'resolved', () => 'rejected'), + new Promise(resolve => setTimeout(resolve, 100)) + ]); + assert.strictEqual(settled, 'rejected'); + }); + + test('knitting recognizes a completion marker fragmented across UTF-8 chunks', async () => { + const child = mockChild(); + let output = ''; + const result = new KnitHarness().knit(text => { + const match = /<<>>(.*?)<<>>/s.exec(text); + if (!match) { return false; } + output = match[1]; + return true; + }); + const bytes = Buffer.from('<<>>/tmp/你好.html<<>>'); + child.stdout.emit('data', bytes.subarray(0, 16)); + child.stdout.emit('data', bytes.subarray(16, 22)); + child.stdout.emit('data', bytes.subarray(22)); + child.emit('close', 0, null); + const settled = await Promise.race([ + result.then(() => 'resolved', () => 'rejected'), + new Promise(resolve => setTimeout(resolve, 100)) + ]); + assert.strictEqual(settled, 'resolved'); + assert.strictEqual(output, '/tmp/你好.html'); + }); + + test('successful exit without the expected output marker rejects instead of hanging', async () => { + const child = mockChild(); + const result = new KnitHarness().knit(() => false); + child.emit('exit', 0, null); + child.emit('close', 0, null); + const settled = await Promise.race([ + result.then(() => 'resolved', () => 'rejected'), + new Promise(resolve => setTimeout(resolve, 100)) + ]); + assert.strictEqual(settled, 'rejected'); + }); + + test('knit commands preserve R literals in filenames and custom functions', async () => { + sandbox.stub(util, 'config').returns({ get: () => 'rmarkdown::render' } as unknown as vscode.WorkspaceConfiguration); + const manager = new RMarkdownKnitManager() as unknown as { + getKnitCommand(params: { knit?: string; site: string }, file: string, format?: string): Promise + }; + const file = util.ToRStringLiteral(`/tmp/it's "report".Rmd`, '"'); + assert.strictEqual(await manager.getKnitCommand({ site: '' }, file), `rmarkdown::render(${file})`); + const knit = 'function(input, ...) rmarkdown::render(input, params = list(label = "a\'b"), ...)'; + assert.strictEqual(await manager.getKnitCommand({ site: '', knit }, file, 'html_document'), + `(${knit})(${file}, output_format = "html_document")`); + }); + + test('non-object YAML frontmatter cannot leave knitting permanently busy', async () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'vscode-r-knit-')); + try { + const file = path.join(directory, 'test.Rmd'); + fs.writeFileSync(file, '---\n# comments only\n---\n'); + sandbox.stub(util, 'config').returns({ + get: (key: string) => key === 'rmarkdown.knit.useBackgroundProcess' ? true : undefined + } as unknown as vscode.WorkspaceConfiguration); + sandbox.stub(vscode.window, 'activeTextEditor').value({ + document: { uri: vscode.Uri.file(file), fileName: file, isUntitled: false, isDirty: false } + }); + const manager = new RMarkdownKnitManager() as unknown as { + knitRmd(echo: boolean): Promise; + renderDocument: (literal: string, file: string, name: string, params: unknown) => Promise; + getYamlFrontmatter(file: string): unknown; + }; + assert.deepStrictEqual(manager.getYamlFrontmatter(file), {}); + const render = sandbox.stub(manager, 'renderDocument').rejects(new Error('transient failure')); + await assert.rejects(manager.knitRmd(false), /transient failure/); + render.resolves(); + await manager.knitRmd(false); + sinon.assert.calledTwice(render); + } finally { fs.rmSync(directory, { recursive: true, force: true }); } + }); +}); From fed78c2b6945c4ad3fa4ececd59bc4b3342d9361 Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Wed, 14 Oct 2026 01:40:16 +0800 Subject: [PATCH 3/7] fix: preserve existing files when generation is cancelled --- src/cppProperties.ts | 3 +- src/lintrConfig.ts | 2 +- src/rGitignore.ts | 18 ++++----- src/test/suite/fileGeneration.test.ts | 57 +++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 14 deletions(-) create mode 100644 src/test/suite/fileGeneration.test.ts diff --git a/src/cppProperties.ts b/src/cppProperties.ts index d6807105b..ea83f71af 100644 --- a/src/cppProperties.ts +++ b/src/cppProperties.ts @@ -19,10 +19,9 @@ export async function generateCppProperties(): Promise { '"c_cpp_properties.json" file already exists. Do you want to overwrite?', 'Yes', 'No' ); - if (overwrite === 'No') { + if (overwrite !== 'Yes') { return; } - fs.unlinkSync(outFilePath); } return generateCppPropertiesProc(currentWorkspaceFolder); } diff --git a/src/lintrConfig.ts b/src/lintrConfig.ts index abb7cbe27..4b984cc50 100644 --- a/src/lintrConfig.ts +++ b/src/lintrConfig.ts @@ -17,7 +17,7 @@ export async function createLintrConfig(): Promise { '".lintr" file already exists. Do you want to overwrite?', 'Yes', 'No' ); - if (overwrite === 'No') { + if (overwrite !== 'Yes') { return; } unlinkSync(lintrFilePath); diff --git a/src/rGitignore.ts b/src/rGitignore.ts index 92b9688d0..62fe79065 100644 --- a/src/rGitignore.ts +++ b/src/rGitignore.ts @@ -1,6 +1,6 @@ 'use strict'; -import { writeFile } from 'node:fs'; +import { writeFile } from 'node:fs/promises'; import { existsSync, readFileSync } from 'fs'; import { join } from 'path'; import { window } from 'vscode'; @@ -23,17 +23,13 @@ export async function createGitignore(): Promise { '".gitignore" file already exists. Do you want to overwrite?', 'Yes', 'No' ); - if (overwrite === 'No') { + if (overwrite !== 'Yes') { return; } } - writeFile(ignorePath, ignoreFileContent, (err) => { - try { - if (err) { - void window.showErrorMessage(err.name); - } - } catch (e) { - void window.showErrorMessage(catchAsError(e).message); - } - }); + try { + await writeFile(ignorePath, ignoreFileContent); + } catch (e) { + void window.showErrorMessage(catchAsError(e).message); + } } diff --git a/src/test/suite/fileGeneration.test.ts b/src/test/suite/fileGeneration.test.ts new file mode 100644 index 000000000..0c3bd5569 --- /dev/null +++ b/src/test/suite/fileGeneration.test.ts @@ -0,0 +1,57 @@ +import * as assert from 'assert'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import * as vscode from 'vscode'; +import * as util from '../../util'; +import { createGitignore } from '../../rGitignore'; +import { generateCppProperties } from '../../cppProperties'; +import { createLintrConfig } from '../../lintrConfig'; +import { mockExtensionContext } from '../common/mockvscode'; + +suite('Generated file overwrite prompts', () => { + let sandbox: sinon.SinonSandbox; + let directory: string; + setup(() => { + sandbox = sinon.createSandbox(); + directory = fs.mkdtempSync(path.join(os.tmpdir(), 'vscode-r-overwrite-')); + mockExtensionContext(path.resolve(__dirname, '../../..'), sandbox); + sandbox.stub(util, 'getCurrentWorkspaceFolder').returns({ uri: vscode.Uri.file(directory) } as vscode.WorkspaceFolder); + sandbox.stub(util, 'getRpath').resolves(undefined); + }); + teardown(() => { + sandbox.restore(); + fs.rmSync(directory, { recursive: true, force: true }); + }); + + for (const answer of [undefined, 'No']) { + test(`preserves existing files after ${answer ?? 'dismissal'}`, async () => { + sandbox.stub(vscode.window, 'showWarningMessage').callsFake(() => Promise.resolve(answer) as unknown as Thenable); + const ignorePath = path.join(directory, '.gitignore'); + const cppPath = path.join(directory, '.vscode', 'c_cpp_properties.json'); + const lintrPath = path.join(directory, '.lintr'); + fs.mkdirSync(path.dirname(cppPath)); + fs.writeFileSync(ignorePath, 'user ignore rules'); + fs.writeFileSync(cppPath, 'user compiler settings'); + fs.writeFileSync(lintrPath, 'user lint settings'); + await createGitignore(); + await generateCppProperties(); + await createLintrConfig(); + // Flush any callback-based filesystem writes before inspecting the files. + await new Promise(resolve => setTimeout(resolve, 30)); + assert.strictEqual(fs.readFileSync(ignorePath, 'utf8'), 'user ignore rules'); + assert.strictEqual(fs.readFileSync(cppPath, 'utf8'), 'user compiler settings'); + assert.strictEqual(fs.readFileSync(lintrPath, 'utf8'), 'user lint settings'); + }); + } + + test('failed compiler configuration generation preserves the existing file', async () => { + sandbox.stub(vscode.window, 'showWarningMessage').callsFake(() => Promise.resolve('Yes') as unknown as Thenable); + const cppPath = path.join(directory, '.vscode', 'c_cpp_properties.json'); + fs.mkdirSync(path.dirname(cppPath)); + fs.writeFileSync(cppPath, 'user compiler settings'); + await generateCppProperties(); + assert.strictEqual(fs.readFileSync(cppPath, 'utf8'), 'user compiler settings'); + }); +}); From 6115567ea0c339938371f414135724bf59ab6e1b Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Wed, 14 Oct 2026 01:40:24 +0800 Subject: [PATCH 4/7] fix: await RStudio editor edits and correct line coordinates --- src/rstudioapi.ts | 19 +++++----- src/test/suite/rstudioapi.test.ts | 61 +++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 10 deletions(-) create mode 100644 src/test/suite/rstudioapi.test.ts diff --git a/src/rstudioapi.ts b/src/rstudioapi.ts index b9450cce6..e22388a2e 100644 --- a/src/rstudioapi.ts +++ b/src/rstudioapi.ts @@ -123,7 +123,7 @@ export async function insertOrModifyText(query: RSEditOperation[], id: string | } }); - void workspace.applyEdit(edit); + await workspace.applyEdit(edit); } export async function replaceTextInCurrentSelection(text: string, id: string | null): Promise { @@ -281,13 +281,12 @@ export async function documentNew(text: string, type: string, position: number[] )), text); - void workspace.applyEdit(edit).then(async () => { - const editor = await window.showTextDocument(targetDocument); - editor.selections = [new Selection( - parsePosition(position, targetDocument), - parsePosition(position, targetDocument) - )]; - }); + await workspace.applyEdit(edit); + const editor = await window.showTextDocument(targetDocument); + editor.selections = [new Selection( + parsePosition(position, targetDocument), + parsePosition(position, targetDocument) + )]; } // interface @@ -438,12 +437,12 @@ function normaliseEditText(text: string, editLocation: RSPosition | RSRange, operation: string, targetDocument: TextDocument) { // in a document with lines, does the line position extend past the existing // lines in the document? rstudioapi adds a newline in this case, so must we. - // n_lines is a count, line is 0 indexed position hence + 1 + // RStudio positions are one-based, as is the document's line count. const editStartLine = operation === 'insertText' ? (editLocation as RSPosition)[0] : (editLocation as RSRange).start[0]; if (editStartLine === 'Inf' || - (typeof editStartLine === 'number' && editStartLine + 1 > targetDocument.lineCount && targetDocument.lineCount > 0)) { + (typeof editStartLine === 'number' && editStartLine > targetDocument.lineCount && targetDocument.lineCount > 0)) { return (text + '\n'); } else { return text; diff --git a/src/test/suite/rstudioapi.test.ts b/src/test/suite/rstudioapi.test.ts new file mode 100644 index 000000000..ccfa2e44e --- /dev/null +++ b/src/test/suite/rstudioapi.test.ts @@ -0,0 +1,61 @@ +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import * as vscode from 'vscode'; +import { documentNew, insertOrModifyText } from '../../rstudioapi'; + +suite('RStudio editor request completion', () => { + let sandbox: sinon.SinonSandbox; + setup(() => { sandbox = sinon.createSandbox(); }); + teardown(() => sandbox.restore()); + + function document() { + const uri = vscode.Uri.file('/project/test.R'); + return { uri, lineCount: 1, validatePosition: (position: vscode.Position) => position, + validateRange: (range: vscode.Range) => range } as vscode.TextDocument; + } + + test('insert requests wait for the workspace edit before acknowledging completion', async () => { + const doc = document(); + sandbox.stub(vscode.workspace, 'openTextDocument').resolves(doc); + let finish!: (value: boolean) => void; + sandbox.stub(vscode.workspace, 'applyEdit').returns(new Promise(resolve => { finish = resolve; })); + let completed = false; + const request = insertOrModifyText([{ operation: 'insertText', text: 'x <- 1', location: [1, 1] }], doc.uri.toString()); + void request.then(() => { completed = true; }); + await new Promise(resolve => setImmediate(resolve)); + assert.strictEqual(completed, false); + finish(true); + await request; + assert.strictEqual(completed, true); + }); + + test('new document requests wait for editing and showing the editor', async () => { + const doc = document(); + sandbox.stub(vscode.workspace, 'workspaceFolders').value([{ uri: vscode.Uri.file('/project') }]); + sandbox.stub(vscode.workspace, 'openTextDocument').resolves(doc); + let finish!: (value: boolean) => void; + sandbox.stub(vscode.workspace, 'applyEdit').returns(new Promise(resolve => { finish = resolve; })); + const editor = { document: doc, selections: [] } as unknown as vscode.TextEditor; + const show = sandbox.stub(vscode.window, 'showTextDocument').resolves(editor); + let completed = false; + const request = documentNew('x <- 1', 'R', [1, 1]); + void request.then(() => { completed = true; }); + await new Promise(resolve => setImmediate(resolve)); + assert.strictEqual(completed, false); + sinon.assert.notCalled(show); + finish(true); + await request; + sinon.assert.calledOnce(show); + assert.strictEqual(editor.selections.length, 1); + }); + + test('one-based positions add a newline only beyond the existing document', async () => { + const doc = document(); + sandbox.stub(vscode.workspace, 'openTextDocument').resolves(doc); + const apply = sandbox.stub(vscode.workspace, 'applyEdit').resolves(true); + for (const [line, expected] of [[1, 'x'], [2, 'x\n'], ['Inf', 'x\n']] as const) { + await insertOrModifyText([{ operation: 'insertText', text: 'x', location: [line, 1] }], doc.uri.toString()); + assert.strictEqual((apply.lastCall.args[0] as vscode.WorkspaceEdit).get(doc.uri)[0].newText, expected); + } + }); +}); From a3e43cb0e7baccd9971b497aa3e69b3f25b2d179 Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Wed, 14 Oct 2026 01:49:25 +0800 Subject: [PATCH 5/7] test: respect workspace URI normalization on Windows --- src/test/suite/rmarkdown.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/suite/rmarkdown.test.ts b/src/test/suite/rmarkdown.test.ts index ce3573fde..c34b3fdbf 100644 --- a/src/test/suite/rmarkdown.test.ts +++ b/src/test/suite/rmarkdown.test.ts @@ -31,8 +31,10 @@ suite('R Markdown execution', () => { const directory = path.join(os.tmpdir(), `project's "report"`); const manager = new KnitHarness(); assert.strictEqual(manager.directory(KnitWorkingDirectory.documentDirectory, path.join(directory, 'test.Rmd')), directory); - sandbox.stub(vscode.workspace, 'getWorkspaceFolder').returns({ uri: vscode.Uri.file(directory) } as vscode.WorkspaceFolder); - assert.strictEqual(manager.directory(KnitWorkingDirectory.workspaceRoot, '/test.Rmd'), directory); + const workspaceUri = vscode.Uri.file(directory); + sandbox.stub(vscode.workspace, 'getWorkspaceFolder').returns({ uri: workspaceUri } as vscode.WorkspaceFolder); + // VS Code normalizes Windows drive-letter casing in URI filesystem paths. + assert.strictEqual(manager.directory(KnitWorkingDirectory.workspaceRoot, '/test.Rmd'), workspaceUri.fsPath); }); function mockChild() { From 01eb5464c38869fc656a9dceb1c3d593da373353 Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Wed, 14 Oct 2026 07:31:50 +0800 Subject: [PATCH 6/7] fix: restore lint configuration when replacement generation fails --- src/lintrConfig.ts | 33 +++++++++++++++++---- src/test/suite/fileGeneration.test.ts | 41 +++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/lintrConfig.ts b/src/lintrConfig.ts index 4b984cc50..676c032a5 100644 --- a/src/lintrConfig.ts +++ b/src/lintrConfig.ts @@ -1,9 +1,9 @@ 'use strict'; -import { existsSync, unlinkSync } from 'fs'; +import { existsSync, mkdtempSync, renameSync, rmSync } from 'fs'; import { join } from 'path'; import { window } from 'vscode'; -import { executeRCommand, getCurrentWorkspaceFolder } from './util'; +import { catchAsError, executeRCommand, getCurrentWorkspaceFolder } from './util'; export async function createLintrConfig(): Promise { const currentWorkspaceFolder = getCurrentWorkspaceFolder()?.uri.fsPath; @@ -20,10 +20,31 @@ export async function createLintrConfig(): Promise { if (overwrite !== 'Yes') { return; } - unlinkSync(lintrFilePath); } - return await executeRCommand(`lintr::use_lintr()`, currentWorkspaceFolder, (e: Error) => { - void window.showErrorMessage(e.message); + + let backupDirectory: string | undefined; + let generated = false; + try { + if (existsSync(lintrFilePath)) { + // lintr refuses to overwrite an existing file. Keep the original on + // disk until it has successfully generated the replacement. + backupDirectory = mkdtempSync(join(currentWorkspaceFolder, '.vscode-r-lintr-')); + renameSync(lintrFilePath, join(backupDirectory, '.lintr')); + } + const output = await executeRCommand('lintr::use_lintr()', currentWorkspaceFolder, (error: Error) => { throw error; }); + if (output !== undefined) { + if (!existsSync(lintrFilePath)) { throw new Error('R did not create a .lintr configuration.'); } + generated = true; + } + return output; + } catch (error) { + void window.showErrorMessage(catchAsError(error).message); return ''; - }); + } finally { + if (backupDirectory) { + const backupPath = join(backupDirectory, '.lintr'); + if (!generated && existsSync(backupPath)) { renameSync(backupPath, lintrFilePath); } + rmSync(backupDirectory, { recursive: true, force: true }); + } + } } diff --git a/src/test/suite/fileGeneration.test.ts b/src/test/suite/fileGeneration.test.ts index 0c3bd5569..7fa60aa3f 100644 --- a/src/test/suite/fileGeneration.test.ts +++ b/src/test/suite/fileGeneration.test.ts @@ -54,4 +54,45 @@ suite('Generated file overwrite prompts', () => { await generateCppProperties(); assert.strictEqual(fs.readFileSync(cppPath, 'utf8'), 'user compiler settings'); }); + + test('unavailable R preserves the existing lint configuration after confirming overwrite', async () => { + sandbox.stub(vscode.window, 'showWarningMessage').callsFake(() => Promise.resolve('Yes') as unknown as Thenable); + sandbox.stub(util, 'executeRCommand').resolves(undefined); + const lintrPath = path.join(directory, '.lintr'); + fs.writeFileSync(lintrPath, 'user lint settings'); + assert.strictEqual(await createLintrConfig(), undefined); + assert.strictEqual(fs.readFileSync(lintrPath, 'utf8'), 'user lint settings'); + assert.deepStrictEqual(fs.readdirSync(directory), ['.lintr']); + }); + + test('failed lint generation restores the original even after partial output', async () => { + sandbox.stub(vscode.window, 'showWarningMessage').callsFake(() => Promise.resolve('Yes') as unknown as Thenable); + const error = sandbox.stub(vscode.window, 'showErrorMessage'); + const lintrPath = path.join(directory, '.lintr'); + fs.writeFileSync(lintrPath, 'user lint settings'); + sandbox.stub(util, 'executeRCommand').callsFake((_command, _cwd, fallback) => { + fs.writeFileSync(lintrPath, 'partial lint settings'); + const failure = new Error('lint generation failed'); + if (typeof fallback === 'function') { return Promise.resolve(fallback(failure)); } + throw failure; + }); + assert.strictEqual(await createLintrConfig(), ''); + assert.strictEqual(fs.readFileSync(lintrPath, 'utf8'), 'user lint settings'); + assert.deepStrictEqual(fs.readdirSync(directory), ['.lintr']); + sinon.assert.calledOnce(error); + assert.deepStrictEqual(error.firstCall.args, ['lint generation failed']); + }); + + test('successful lint generation replaces the original and removes its backup', async () => { + sandbox.stub(vscode.window, 'showWarningMessage').callsFake(() => Promise.resolve('Yes') as unknown as Thenable); + const lintrPath = path.join(directory, '.lintr'); + fs.writeFileSync(lintrPath, 'user lint settings'); + sandbox.stub(util, 'executeRCommand').callsFake(() => { + fs.writeFileSync(lintrPath, 'new lint settings'); + return Promise.resolve(''); + }); + assert.strictEqual(await createLintrConfig(), ''); + assert.strictEqual(fs.readFileSync(lintrPath, 'utf8'), 'new lint settings'); + assert.deepStrictEqual(fs.readdirSync(directory), ['.lintr']); + }); }); From e229e1af625364e9debca4726b8ea6ed8de40d61 Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Wed, 14 Oct 2026 07:32:02 +0800 Subject: [PATCH 7/7] fix: reject RStudio requests when editor changes cannot apply --- src/rstudioapi.ts | 12 +++++++--- src/test/suite/rstudioapi.test.ts | 38 ++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/rstudioapi.ts b/src/rstudioapi.ts index e22388a2e..eac242535 100644 --- a/src/rstudioapi.ts +++ b/src/rstudioapi.ts @@ -96,6 +96,12 @@ export async function documentContext(id: string | null): Promise { + if (!await workspace.applyEdit(edit)) { + throw new Error('Could not apply editor changes.'); + } +} + export async function insertOrModifyText(query: RSEditOperation[], id: string | null = null): Promise { @@ -123,7 +129,7 @@ export async function insertOrModifyText(query: RSEditOperation[], id: string | } }); - await workspace.applyEdit(edit); + await applyEditorEdit(edit); } export async function replaceTextInCurrentSelection(text: string, id: string | null): Promise { @@ -135,7 +141,7 @@ export async function replaceTextInCurrentSelection(text: string, id: string | n getLastActiveTextEditor().selection, text ); - await workspace.applyEdit(edit); + await applyEditorEdit(edit); } export function showDialog(message: string): void { @@ -281,7 +287,7 @@ export async function documentNew(text: string, type: string, position: number[] )), text); - await workspace.applyEdit(edit); + await applyEditorEdit(edit); const editor = await window.showTextDocument(targetDocument); editor.selections = [new Selection( parsePosition(position, targetDocument), diff --git a/src/test/suite/rstudioapi.test.ts b/src/test/suite/rstudioapi.test.ts index ccfa2e44e..46fb2b482 100644 --- a/src/test/suite/rstudioapi.test.ts +++ b/src/test/suite/rstudioapi.test.ts @@ -1,7 +1,8 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import * as vscode from 'vscode'; -import { documentNew, insertOrModifyText } from '../../rstudioapi'; +import { documentNew, insertOrModifyText, replaceTextInCurrentSelection } from '../../rstudioapi'; +import { handleEditorRequest } from '../../session'; suite('RStudio editor request completion', () => { let sandbox: sinon.SinonSandbox; @@ -58,4 +59,39 @@ suite('RStudio editor request completion', () => { assert.strictEqual((apply.lastCall.args[0] as vscode.WorkspaceEdit).get(doc.uri)[0].newText, expected); } }); + + test('insert and modify requests report an error when VS Code cannot apply the edit', async () => { + const doc = document(); + sandbox.stub(vscode.workspace, 'openTextDocument').resolves(doc); + sandbox.stub(vscode.workspace, 'applyEdit').resolves(false); + for (const query of [ + [{ operation: 'insertText', text: 'x', location: [1, 1] }], + [{ operation: 'modifyRange', text: 'x', location: { start: [1, 1], end: [1, 2] } }] + ]) { + await assert.rejects(handleEditorRequest('rstudioapi/insert_or_modify_text', { + id: doc.uri.toString(), query + }), /Could not apply editor changes/); + } + }); + + test('a failed new document edit reports an error without showing the editor', async () => { + const doc = document(); + sandbox.stub(vscode.workspace, 'workspaceFolders').value([{ uri: vscode.Uri.file('/project') }]); + sandbox.stub(vscode.workspace, 'openTextDocument').resolves(doc); + sandbox.stub(vscode.workspace, 'applyEdit').resolves(false); + const show = sandbox.stub(vscode.window, 'showTextDocument').resolves({ selections: [] } as unknown as vscode.TextEditor); + await assert.rejects(handleEditorRequest('rstudioapi/document_new', { + text: 'x <- 1', type: 'R', position: [1, 1] + }), /Could not apply editor changes/); + sinon.assert.notCalled(show); + }); + + test('selection replacement rejects an edit that VS Code cannot apply', async () => { + const doc = document(); + sandbox.stub(vscode.window, 'activeTextEditor').value({ + document: doc, selection: new vscode.Selection(0, 0, 0, 1) + }); + sandbox.stub(vscode.workspace, 'applyEdit').resolves(false); + await assert.rejects(replaceTextInCurrentSelection('x', doc.uri.toString()), /Could not apply editor changes/); + }); });