Skip to content

fix: handle standalone multiline strings in run selection - #1788

Open
sb123sb123 wants to merge 3 commits into
REditorSupport:mainfrom
sb123sb123:fix/issue-1313-multiline-string-selection
Open

sb123sb123 wants to merge 3 commits into
REditorSupport:mainfrom
sb123sb123:fix/issue-1313-multiline-string-selection

Conversation

@sb123sb123

@sb123sb123 sb123sb123 commented Sep 27, 2026 •

Copy link
Copy Markdown

Closes #1313

Run Selection now continues scanning across lines while a quoted R string remains open, so invoking it from the line that opens a standalone multi-line string selects the complete expression.

For the input lines ['x <- "', 'a', '"'], calling extendSelection(0, ...) returns { startLine: 0, endLine: 2 }.

Validation:

  • npm test -- --run out/test/suite/extendSelection.test.js --timeout 20000 — 38 passing; pretest compile and TypeScript checks passed.
  • npx eslint src --ext ts — 0 errors, 48 existing warnings.
  • git diff --check — passed.

AI assistance was used to investigate and implement the fix.

@eitsupi

eitsupi commented Sep 27, 2026

Copy link
Copy Markdown
Member

Thank you for working on this. However, I'm not sure if this is the right direction.
Considering the following example, what should the result be when executed on the second line?

const doc = ['x <- "', 'a', '"'];

for (const line of [0, 1, 2]) {
    assert.strictEqual(extendSelection(line, f, doc.length).startLine, 0);
    assert.strictEqual(extendSelection(line, f, doc.length).endLine, 2);
}

@sb123sb123

Copy link
Copy Markdown
Author

Thanks for raising this. The expected result is the complete expression: when the cursor is on any of the three lines, Run Selection should select lines 0–2. I pushed commit 0874bb6 with that behavior and updated the regression to check all three starting lines. The focused selection suite passes 38/38, and the TypeScript compile/check passes. The full suite has 259 passing tests; its two session tests need an R executable that is unavailable in my Windows environment.

@eitsupi eitsupi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.
However, I have doubts about introducing a completely new parser just for this feature.

It would require a tremendous amount of effort to fully cover rawastring and other features, and I think it would create a structure that is prone to introducing bugs.

@sb123sb123

Copy link
Copy Markdown
Author

Thank you for the feedback. The latest revision removes the line-prefix helper and limits the change to the reported reproduction with the cursor on the first line (x <- \"). Your three-line case is a useful boundary: I verified that this scoped revision does not select the full string when starting from an interior line, so my earlier statement that all three starting lines were supported was incorrect. Adding that behavior would require lexical-context handling for R strings, including raw strings, which is beyond this patch and the parser direction you cautioned against. The exact issue reproduction passes its regression; the full extension suite reports 259 passing and two session tests failing because the Windows test host cannot resolve an R executable. Would you consider this issue-scoped fix appropriate for #1313?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Run Selection can't handle multi line strings

2 participants