Commit d5d37bb
Implement public facing
Addresses #8350
- Requires posit-dev/ark#1040
- Blocks quarto-dev/quarto#919
Builds on:
- quarto-dev/quarto#914
- posit-dev/ark#1030
# Summary
This PR adds a new throwable `StatementRangeSyntaxError` to
`positron.d.ts`.
```ts
/**
* An error thrown by a {@link StatementRangeProvider} to indicate that a statement range
* cannot be provided due to a syntax error in the document.
*/
export class StatementRangeSyntaxError extends Error {
/**
* Zero indexed line number where the syntax error occurred.
*/
readonly line?: number;
/**
* Creates a new statement range syntax error.
*
* @param line Zero indexed line number where the syntax error occurred.
*/
constructor(line?: number);
}
```
This is the only public facing API change from this PR.
The idea is that from extension land (`positron-r`, `positron-python`,
and `quarto`) you should be able to throw a `StatementRangeSyntaxError`
from your `provideStatementRange()` provider, with an optional `line`
number of where the syntax error _roughly_ starts. This gets propagated
up through the main thread and into our `Cmd+Enter` handling in
`positronConsoleActions.ts`, where we now detect this special case and:
- Emit an info level notification telling the user that we can't execute
code due to a syntax error. If the `line` number is present, we give
them a button to jump to that line.
- Bail from `Cmd+Enter` handling altogether. We don't even do
one-line-at-a-time execution. #8350 has proved that this is just too
confusing when you are trying to execute code after a syntax error.
Here is what the end result looks like in an R script:
https://github.com/user-attachments/assets/23dca240-06af-40d5-8b1b-a8b24af99259
Note how you can execute R code _above_ the first syntax error, thanks
to posit-dev/ark#1030. It is only _below_ the
first syntax error that we emit this notification and refuse to execute.
It even works in roxygen2 comments where we have a little "subdocument":
https://github.com/user-attachments/assets/ec920753-400d-4493-b534-188dcf4e2a65
And lastly, it works in Quarto, where we have a "virtual document" for
R/Python and map the `line` number from that virtual document back into
the original Qmd, thanks to
quarto-dev/quarto#919 (note that if you try to
run that Quarto PR, you need to remove the version guard on Positron
2026.03.0 manually).
Additionally, quarto-dev/quarto#914 ensures that
you can use StatementRange in any syntax-error-free cell, even if some
other cell in the document has a syntax error.
https://github.com/user-attachments/assets/a01b9ed1-22fe-4169-b088-a8393800d9db
# Design
I considered many different designs along the way, but landed on a model
of:
- Extension host side _models errors as throwable errors_, i.e. with
`StatementRange` and `throw StatementRangeSyntaxError`
```ts
export interface StatementRange {
readonly range: vscode.Range;
readonly code?: string;
}
export class StatementRangeSyntaxError extends Error {
readonly line?: number;
constructor(line?: number);
}
```
- Main thread side _models errors as data_, i.e. a single
`IStatementRange` type made up as:
```ts
type IStatementRange = IStatementRangeSuccess | IStatementRangeRejection
type IStatementRangeRejection = IStatementRangeSyntaxRejection
export interface IStatementRangeSuccess {
readonly kind: StatementRangeKind.Success;
readonly range: IRange;
readonly code?: string;
}
export interface IStatementRangeSyntaxRejection {
readonly kind: StatementRangeKind.Rejection;
readonly rejectionKind: StatementRangeRejectionKind.Syntax;
readonly line?: number;
}
```
Some rationale:
- Backwards compatible and public facing `StatementRange` doesn't change
- Can add new error/rejection variants as needed
- Can't easily model main thread side errors as, well, `Error`s, because
the `StatementRangeSyntaxError` doesn't serialize across the extension
-> main thread boundary well. You basically have to turn it into some
"data" type anyways to do this, and then it doesn't feel worth it to
convert back on the main thread side.
- Ark's custom `StatementRange` LSP Request also now uses the "success"
vs "rejection" model. From Ark's LSP-ish perspective, a "rejection" is
an allowed value that can be returned as an LSP Response. We reserve the
JSONRPC error path for true "holy crap something horrible happened and I
can't complete this request" cases.
The end result looks like this:
<img width="5127" height="4470" alt="test"
src="https://github.com/user-attachments/assets/b341c1dd-edac-45a2-9aed-03a4b05b3c01"
/>
# Prior art
There isn't much prior art for exactly what I'm trying to do here, but
there is a little bit:
- `FileSystemError` with its various flavors. Not quite the same as us
because none of the flavors have any metadata to pass through, like
`line`. That metadata is what steered me away from trying to have one
overarching `StatementRangeError` class that could contain something
like `static SyntaxError(line?: number): StatementRangeError`. I did try
this, but the boilerplate didn't feel worth it.
https://github.com/posit-dev/positron/blob/31c0111d42eaff76f06e1800836663e5765e1df1/src/vscode-dts/vscode.d.ts#L9474-L9486
- `Rejection`, which is used by the `RenameProvider`. Catches an error
from the LSP side of things and converts it into a `Rejection` "data"
type, kind of like we do. Strangely does `RenameProvider & Rejection` as
the return value type.
- Defined here
https://github.com/posit-dev/positron/blob/9754ca9e56524315fa6b2d1981fb6cb82ef5cd7b/src/vs/editor/common/languages.ts#L2127-L2138
- Catching the error here
https://github.com/posit-dev/positron/blob/9754ca9e56524315fa6b2d1981fb6cb82ef5cd7b/src/vs/workbench/api/common/extHostLanguageFeatures.ts#L882-L903
# QA
### Release Notes
#### New Features
- Stepping through code with `Cmd + Enter` now works more reliably when
there are syntax errors in the document:
- For R code:
- [Code _above_ the first syntax
error](posit-dev/ark#1030) can now be executed
reliably with `Cmd + Enter`
- [Code _below_ the first syntax
error](#11907) causes a
notification to pop up that informs you that reliable execution is
impossible and provides you with a button to jump to the syntax error
- For Quarto documents, a syntax error in one chunk no longer affects
the ability to [run code in other
chunks](quarto-dev/quarto#914)
### QA Notes
QA: It would be nice to have two integration tests
```r
1
+ 1 # <put cursor here and execute this, it should work now and send the whole statement>
2 + \ 2
```
```r
1 + \ 1
2
+ 2 # <put cursor here and execute this, it should not send anything, and a notification should pop up>
```
`@:ark` `@:quarto` `@:console`
---------
Signed-off-by: Davis Vaughan <davis@posit.co>
Co-authored-by: Lionel Henry <lionel.hry@proton.me>StatementRangeSyntaxError (#11907)1 parent cf256d6 commit d5d37bb
15 files changed
Lines changed: 688 additions & 60 deletions
File tree
- extensions/positron-r
- src
- test
- src
- positron-dts
- vs
- editor
- common
- standalone
- contrib/positronStatementRange
- browser
- test/browser
- standalone/browser
- workbench
- api/common
- positron
- contrib/positronConsole/browser
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1078 | 1078 | | |
1079 | 1079 | | |
1080 | 1080 | | |
1081 | | - | |
| 1081 | + | |
1082 | 1082 | | |
1083 | 1083 | | |
1084 | 1084 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | | - | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
9 | 18 | | |
10 | 19 | | |
11 | 20 | | |
12 | 21 | | |
13 | 22 | | |
14 | 23 | | |
15 | | - | |
16 | | - | |
17 | | - | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
18 | 38 | | |
19 | 39 | | |
20 | 40 | | |
| |||
38 | 58 | | |
39 | 59 | | |
40 | 60 | | |
41 | | - | |
| 61 | + | |
| 62 | + | |
42 | 63 | | |
43 | 64 | | |
44 | 65 | | |
45 | 66 | | |
46 | 67 | | |
47 | 68 | | |
48 | | - | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
49 | 74 | | |
50 | | - | |
51 | | - | |
52 | | - | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
53 | 95 | | |
54 | | - | |
55 | | - | |
56 | | - | |
57 | | - | |
58 | | - | |
| 96 | + | |
59 | 97 | | |
60 | 98 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1542 | 1542 | | |
1543 | 1543 | | |
1544 | 1544 | | |
| 1545 | + | |
| 1546 | + | |
| 1547 | + | |
1545 | 1548 | | |
1546 | 1549 | | |
1547 | 1550 | | |
| |||
1565 | 1568 | | |
1566 | 1569 | | |
1567 | 1570 | | |
| 1571 | + | |
| 1572 | + | |
| 1573 | + | |
| 1574 | + | |
| 1575 | + | |
| 1576 | + | |
| 1577 | + | |
| 1578 | + | |
| 1579 | + | |
| 1580 | + | |
| 1581 | + | |
1568 | 1582 | | |
| 1583 | + | |
| 1584 | + | |
| 1585 | + | |
| 1586 | + | |
| 1587 | + | |
| 1588 | + | |
1569 | 1589 | | |
1570 | 1590 | | |
1571 | 1591 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2040 | 2040 | | |
2041 | 2041 | | |
2042 | 2042 | | |
| 2043 | + | |
| 2044 | + | |
| 2045 | + | |
| 2046 | + | |
| 2047 | + | |
| 2048 | + | |
| 2049 | + | |
| 2050 | + | |
| 2051 | + | |
2043 | 2052 | | |
2044 | 2053 | | |
2045 | 2054 | | |
| |||
2048 | 2057 | | |
2049 | 2058 | | |
2050 | 2059 | | |
| 2060 | + | |
| 2061 | + | |
| 2062 | + | |
| 2063 | + | |
2051 | 2064 | | |
2052 | 2065 | | |
2053 | 2066 | | |
2054 | | - | |
| 2067 | + | |
| 2068 | + | |
| 2069 | + | |
| 2070 | + | |
| 2071 | + | |
| 2072 | + | |
2055 | 2073 | | |
2056 | 2074 | | |
2057 | 2075 | | |
| |||
2061 | 2079 | | |
2062 | 2080 | | |
2063 | 2081 | | |
| 2082 | + | |
2064 | 2083 | | |
| 2084 | + | |
| 2085 | + | |
| 2086 | + | |
| 2087 | + | |
| 2088 | + | |
| 2089 | + | |
| 2090 | + | |
| 2091 | + | |
| 2092 | + | |
| 2093 | + | |
| 2094 | + | |
| 2095 | + | |
| 2096 | + | |
| 2097 | + | |
| 2098 | + | |
2065 | 2099 | | |
2066 | 2100 | | |
2067 | 2101 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
881 | 881 | | |
882 | 882 | | |
883 | 883 | | |
| 884 | + | |
| 885 | + | |
| 886 | + | |
| 887 | + | |
| 888 | + | |
| 889 | + | |
| 890 | + | |
| 891 | + | |
| 892 | + | |
884 | 893 | | |
885 | 894 | | |
886 | 895 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | | - | |
| 19 | + | |
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
| |||
0 commit comments