Repository navigation
Fix code execution, R Markdown, and editor request bugs - #1874
Conversation
eitsupi
left a comment
There was a problem hiding this comment.
Reviewed with ChatGPT.
The changes look generally sound, and CI is passing on all three platforms. I found two issues worth addressing before merging:
-
Preserve the existing
.lintrfile if generation failsAfter the user confirms overwriting,
unlinkSync(lintrFilePath)deletes the existing configuration beforelintr::use_lintr()runs. If R is unavailable or the command fails, the user's configuration is permanently lost. Please preserve the original until replacement generation succeeds, and add a regression test for this failure path. -
Check the result of
workspace.applyEdit()Both
insertOrModifyText()anddocumentNew()now awaitworkspace.applyEdit(), which is an improvement. However, the API returnsfalsewhen the edit cannot be applied, without necessarily rejecting the promise. These functions currently report successful completion even when no edit was applied. Please handlefalseexplicitly and add a regression test.
The remaining changes look reasonable to me. In particular, the chunk boundary fixes, UTF-8 decoding, process completion handling, and busy-state cleanup are welcome improvements.
Word-based commands could select the entire document when the cursor was outside a word, and the final R chunk could lose its last code line. This PR fixes those execution bugs along with failures in R Markdown jobs, generated-file prompts, and RStudio editor requests.
.lintrif replacement generation fails or R is unavailable, including partially written output. Await.gitignorewrites and report errors.Added 28 regression tests across these cases.
Validation
git diff --checkpassed.Validated locally on macOS with VS Code 1.141.1. CI is passing on Linux, macOS, and Windows for the latest commit.