Add custom pair colorization and highlighting for divs#973
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
There was a problem hiding this comment.
This is looking great!
Can we add some basic tests for this, in case we find a change we need to make later? I would think something like a new divs.test.ts that is similar to codeBlocks.test.ts, opening a .qmd and confirming the parser finds a Div token. Pretty cheap to test through MarkdownEngine! If you are motivated to go a little further, you could extract the logic for the depth calculation and getDivMarkerRange regex from activateDivBracketDecorations into a pure function to unit test, but that might not be worth it.
|
|
||
| // Update decorations when document changes | ||
| context.subscriptions.push( | ||
| vscode.workspace.onDidChangeTextDocument(event => { |
There was a problem hiding this comment.
Should we debounce this the same way we do the highlighting decorations in background.ts?
There was a problem hiding this comment.
I went down a bit of a rabbit hole.
- Yes we should not be calling
updateDecorationstoo often because that could cause lag. - BUT! background.ts's debounce was implemented incorrectly -- a new debounced function was getting created on every call of the trigger function (we have to call the same debounced function multiple times for the debounce to take effect).
- Also BUT! I think throttling should be a nicer user experience than debouncing. With throttling, the editor will update as the user is typing, but at a reduced rate, and it will definitely have an update based on the user's last input.
so I made both div-brackets.ts and background.ts use throttling for their updates.
There was a problem hiding this comment.
Also we can't (easily) use lodash's throttle/debounce because it has a fixed delay, whereas we want the delay to be from the config.
There was a problem hiding this comment.
I'm reconsidering my decision to use throttle instead of debounce. I am consulting the #frontend channel in Slack to see if anyone has any thoughts.
There was a problem hiding this comment.
Thanks to @dhruvisompura and @dotNomad for very helpful discussion and resources on Slack around throttling v.s. debouncing.
I tested a 32000 line qmd and did not experience any input lag when creating and removing divs with throttled bracket highlighting. That only says so much, because my work laptop is powerful, but at least there isn't an egregious perf issue here.
In summary, I think throttling is a better user experience than debouncing here!
|
|
||
| ## 1.133.0 | ||
|
|
||
| - Add custom pair colorization and highlighting for divs in qmds (<https://github.com/quarto-dev/quarto/pull/973>). |
There was a problem hiding this comment.
| - Add custom pair colorization and highlighting for divs in qmds (<https://github.com/quarto-dev/quarto/pull/973>). | |
| - Added custom pair colorization and highlighting for divs in qmds (<https://github.com/quarto-dev/quarto/pull/973>). |
Just for consistency in this file

Fixes #670 (except "Navigation"? What does that mean? Do we want divs in the outline?)
Adds colorization to pairs of opening and closing div syntax. It is a custom decorator. I don't believe we can use VSCode's builtin pair colorizer because it relies on opening and closing brackets being preset and distinct, whereas Quarto fenced divs can have any number of colons >2 in them and the closing and opening brackets are the same (other than the trailing class/attribute syntax
{}on the opening bracket).Also fixes the parsing of opening syntax for divs where the class/attribute syntax is empty i.e. exactly
::: {}. This was previously not parsed as opening a div (it was parsed as an inline), the regex expected at least one character inside{}. As a result, folding also works better now.