Fix code execution, R Markdown, and editor request bugs (#1874)

* fix: preserve R code selections, strings, and process output

* fix: settle R Markdown jobs and preserve quoted paths

* fix: preserve existing files when generation is cancelled

* fix: await RStudio editor edits and correct line coordinates

* test: respect workspace URI normalization on Windows

* fix: restore lint configuration when replacement generation fails

* fix: reject RStudio requests when editor changes cannot apply

Author: renkun-ken

src/cppProperties.ts

@@ -19,10 +19,9 @@ export async function generateCppProperties(): Promise<void> {
             '"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);
 }

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;

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<string | undefined> {
     const currentWorkspaceFolder = getCurrentWorkspaceFolder()?.uri.fsPath;
@@ -17,13 +17,34 @@ export async function createLintrConfig(): Promise<string | undefined> {
             '".lintr" file already exists. Do you want to overwrite?',
             'Yes', 'No'
         );
-        if (overwrite === 'No') {
+        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 });
+        }
+    }
 }

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<void> {
             '".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);
+    }
 }

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;
 }
 

src/rmarkdown/draft.ts

@@ -101,8 +101,7 @@ async function launchTemplatePicker(cwd: string): Promise<TemplateItem | undefin
 }
 
 async function makeDraft(file: string, template: TemplateItem, cwd: string): Promise<string | undefined> {
-    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<void> {
             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<void> {
     } 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 });
     }
 }

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<string>('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, '"')})`);
             }
         }
     }