#| highlighting alternative - #1092
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
juliasilge
left a comment
juliasilge
left a comment
There was a problem hiding this comment.
I really like this approach, and I prefer it to #1084!
I have some inline comments so we are more consistent about what the special comments are, but I'd also like to ask that we make that background/underline/opacity treatment configurable. Can we add a configuration point similar to quarto.cells.background.color here so that folks can turn this off/on? I think boolean and default on is fine, but I would like to give folks a way to opt out of that visual treatment.
| const lines: number[] = []; | ||
| const lastLine = Math.min(blockRange.end.line, document.lineCount - 1); | ||
| for (let i = blockRange.start.line + 1; i <= lastLine; i++) { | ||
| if (!/^\s*# ?\|/.test(document.lineAt(i).text)) { |
There was a problem hiding this comment.
This regex does not match the same lines as Quarto's own option parser. That parser uses optionCommentPattern in cell/options.ts, which expands to /^#\s*\| ?/ for # languages and accepts any whitespace between # and |.
format.ts already imports optionCommentPattern from cell/options.ts so that its code path cannot drift from the option parser. Can we import it here too, instead of making a third copy of this pattern?
| while: (^|\\G)(?!\\s*([\`~]{3,})\\s*$) | ||
| contentName: ${contentName} | ||
| patterns: | ||
| - begin: ^(\\s*)(#\\|) |
There was a problem hiding this comment.
This rule matches only #|, but the engine also accepts interior spaces (optionCommentPattern expands to /^#\s*\| ?/), and the decoration in background.ts accepts # |. A line like # | echo: false gets the darker background but no YAML colors. Can we make this pattern match the same lines as optionCommentPattern?
There was a problem hiding this comment.
Also, this rule is stamped into every embedded language, but #| is only the option prefix for # comment languages. Cells in other languages use --| (sql, lua), //| (js, ojs, cpp), or %| (matlab) and get no YAML highlighting. The reverse also happens: a literal #| line in a css, json, or yaml block is mis-scoped as active YAML. The generator already emits one definition per language, so we can take the prefix per language (see kLangCommentChars in packages/core/src/jupyter/options.ts).
| contentName: ${contentName} | ||
| patterns: | ||
| - begin: ^(\\s*)(#\\|) | ||
| while: ^(\\s*)(#\\|) |
There was a problem hiding this comment.
This rule fires on any #| run in the cell body, not only the leading run, but Quarto only reads options from the leading run, and the new decoration in background.ts stops at the first non-option line. Is it possible for us to handle this the same way Quarto itself does?
```python
x = 1
#| eval: false
```
Maybe not with the declarative TextMate approach, in which case let's just document it.
|
|
||
| function clearEditorHighlightDecorations(editor: vscode.TextEditor) { | ||
| editor.setDecorations(highlightingConfig.backgroundDecoration(), []); | ||
| editor.setDecorations(cellOptionsBackgroundDecoration, []); |
There was a problem hiding this comment.
This clears the block decoration and both new cell option decorations, but not highlightingConfig.inlineBackgroundDecoration(). When the document's language mode changes, the inline backgrounds for `r ...` code stay behind on the non-Quarto document. That omission existed before this PR, but since we are adding clear calls here anyway, can we fix it? Just one line to do so, I believe.
| // to generalize to all languages (//| for js, --| for sql, /*| ... */ | ||
| // for c, etc.), derive the prefix from the block's language using | ||
| // kLangCommentChars/optionCommentPattern in packages/core/src/jupyter/options.ts | ||
| function hashPipeLines( |
There was a problem hiding this comment.
Building on the optionCommentPattern comment I added, the note at lines 256-259 defers //| and --| support, but the pieces already exist and we can bring them in a pretty straightforward way. The block token carries the language (languageNameFromBlock, exported from quarto-core), and kLangCommentChars in cell/options.ts maps each language to its comment prefix. It is private now, so it needs an export. With those two, this function works for js, sql, lua, and ojs cells too, and this third copy of "what is an option line" goes away.
3 participants
Fixes #409
This is an alternative approach to #1084.
This is a duplicate of #1088, which I accidentally merged to main and then reverted. Sorry for confusion.
This PR is a very simple change: it adds a yaml injection language for #| comments. This means that our syntax highlighting will delegate to the yaml language highlighter inside of #| comments.
The quarto.tmLanguage change is generated from the build-lang.js script.
IMO this is a more "correct" approach, if you will. We lose control over how the yaml looks, but we shouldn't really have that control anyway (let yaml be yaml)?
Pros
Cons
Testing note
We could test that the generated tmLanguage delegates
#|properly in some examples, in a non-integrated way, by usingvscode-textmateandvscode-onigurumaas dependencies in the tests, but that seems a little heavy handed so I didn't do that.