Skip to content

feat: add an experimental quarto.validate.links.enabled setting for LSP link diagnostics - #1186

Open
nealrichardson wants to merge 6 commits into
quarto-dev:mainfrom
nealrichardson:fix/lsp-markdown-validate
Open

nealrichardson wants to merge 6 commits into
quarto-dev:mainfrom
nealrichardson:fix/lsp-markdown-validate

Conversation

@nealrichardson

@nealrichardson nealrichardson commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1185

The language server's link diagnostics (undefined reference links, missing headers, missing files, unused and duplicate link definitions) could never be turned on. ConfigurationManager.update() rebuilt markdown.validate from the defaults (enabled: false) on every update and never read it from the client. This has been the case since 09dc066 (2023).

Following the discussion below (option 1), the diagnostics are now turned on by a new Quarto setting, and the false positives on Quarto's explicit identifiers are fixed.

Changes

  • New setting quarto.validate.links.enabled (boolean, default false, tagged experimental). It's the only switch for link diagnostics in Quarto documents.
    • The level of each rule, and the links to skip, are still read from VS Code's markdown.validate.* settings: referenceLinks.enabled, fragmentLinks.enabled, fileLinks.enabled, fileLinks.markdownFragmentLinks, unusedLinkDefinitions.enabled, duplicateLinkDefinitions.enabled and ignoredLinks. Missing or invalid values fall back to VS Code's defaults (warning, or hint for unused definitions), so the setting still works if the client doesn't send them.
    • markdown.validate.enabled is ignored. Users who turned it on for .md files don't get new warnings in .qmd files.
    • The setting's description notes the known limitation: links to files that only exist after rendering (such as other.html) are reported as missing, and suggests linking to the .qmd source or using markdown.validate.ignoredLinks.
  • Explicit identifiers are link targets (apps/lsp/src/service/toc.ts). The table of contents used only the identifier generated from a heading's text, so [see](#sec-intro) pointing at ## Introduction {#sec-intro} was reported as "No header found".
    • As in Pandoc, a heading's explicit {#id} (already parsed into the token's attr by quarto-core) replaces the generated identifier, and is reserved so a later generated identifier doesn't reuse it. Besides diagnostics, this fixes go to definition, find references, highlights and # path completions for such headings. They used the generated identifier before, and that identifier doesn't exist in the rendered document.
    • Divs with an identifier (::: {#fig-x}, ::: {.callout-note #nte-x}, ::: {#thm-x}) are now valid fragment targets. They're kept in a separate list from the outline entries, so the outline, folding, smart select and completions don't change. Code cell labels (#| label: fig-x) were already targets.
  • apps/lsp gets its first unit tests (test/config.test.ts, test/link-targets.test.ts, helpers in test/workspace.ts) and a test script, and yarn test-packages now includes quarto-lsp. They cover:
    • the setting off: no diagnostics, even with markdown.validate.enabled: true;
    • the setting on: diagnostics, with levels from markdown.validate.*, and VS Code's defaults when those are missing or invalid;
    • explicit heading identifiers, div identifiers and code cell labels resolving, in the same file and in another file, while #introduction for ## Introduction {#sec-intro} is still flagged;
    • divs staying out of the outline, and find references and go to definition for an explicit identifier.
  • apps/vscode/src/test/lsp-features.test.ts: an end-to-end test turns on quarto.validate.links.enabled, waits for a diagnostic on a broken fragment link in a new fixture (lsp-link-diagnostics.qmd), then turns the setting off and waits for the diagnostic to clear. The existing lsp-features.qmd fixture linked [the section](#first-section) to ## First Section {#sec-first}, a link that's broken in the rendered document; it now links to #sec-first, so the "references to a header" test checks the Pandoc behavior.
  • Two apps/vscode/CHANGELOG.md lines: the new setting, and the explicit identifier fix.

Not covered, for follow-ups

  • Other Quarto cross-reference targets. Identifiers attached to inline or block content that the LSP's block parser doesn't record are still flagged as missing when linked with [..](#id): figures (![](plot.png){#fig-x}), tables (: Caption {#tbl-x}), equations ($$ ... $$ {#eq-x}), listings (code blocks with {#lst-x}), and spans ([text]{#id}). Quarto documents usually reference these with @fig-x, not links, so I left them out. Fixing them needs inline attribute parsing in quarto-core's markdown-it parser.
  • Rendered-output links. Links to files that only exist after rendering (other.html from other.qmd) are reported as missing files. Quarto recommends linking to the .qmd source, so this is lower priority. It's noted in the setting's description.
  • The other markdown.* settings the server models (preferredMdPathExtensionStyle, suggest.paths.*) are still never read from the client.
  • LSP: no-op middleware onDefinition replaces the real definition handler #1139 (the middleware stub that replaces the definition handler) is related but separate, and isn't touched here.

Verification

On Node 24, rebased onto main (a83c5cc):

  • yarn install --frozen-lockfile: lockfile unchanged.
  • yarn build --force: 14/14 tasks.
  • yarn test-packages: 14/14 tasks (13 on main, plus quarto-lsp), including the 11 quarto-lsp tests.
  • yarn lint --continue: 17/17 tasks, 0 errors, 17 warnings, all in quarto, the same as on main.
  • apps/vscode yarn build-test: compiles.
  • yarn test-vscode wasn't run locally: the VS Code tests open GUI windows, and no headless container was available. CI runs the new lsp-features.test.ts test.

🤖 Generated with Claude Code

@posit-snyk-bot

posit-snyk-bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues
✅ Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@nealrichardson
nealrichardson marked this pull request as draft September 29, 2026 19:06
@nealrichardson

Copy link
Copy Markdown
Contributor Author

Converting to draft: the approach needs a decision before this merges.

Once enabled, the link diagnostics give two Quarto-specific false positives:

  • [see](#sec-intro) pointing at an explicit {#sec-intro} ID is flagged "No header found", because the TOC only knows auto-generated heading slugs.
  • A link to rendered output (other.html built from other.qmd) is flagged as a missing file.

As written, this PR reuses VS Code's markdown.validate.* settings. Anyone who has already turned markdown.validate.enabled on for .md files would start getting these false positives in .qmd files too. The LSP's settings request isn't language-scoped, so a [quarto] override can't separate the two.

Options:

  1. A Quarto-specific switch (e.g. quarto.validate.links.enabled, default off), still reading the per-rule levels from markdown.validate.*.
  2. Keep markdown.validate.* as it is, and fix the false positives in a follow-up.
  3. Teach the LSP about explicit {#id} anchors and .qmd → .html targets first, then enable.

Maintainer input welcome on which way to go.

@juliasilge

Copy link
Copy Markdown
Collaborator

Hmmmmm, interesting 👀

I believeI prefer option 1, a quarto.validate.links.enabled setting, off by default, that reads the per-rule levels from markdown.validate.*. I'd also like for us to mark it as "tags": ["experimental"]. What I'm thinking is:

  • Probably some users already set markdown.validate.enabled: true for .md files. With option 2, these users would get new warnings in .qmd files after an update that they didn't ask for them.
  • The explicit ID false positive is not a rare edge case IMO! Many Quarto documents use {#sec-...} IDs for cross-references, so links like [see](#sec-intro) are common. The TOC makes slugs only from the heading text (toSlug(text) in apps/lsp/src/service/toc.ts) and ignores {#id}. Div IDs such as ::: {#fig-x} probably have the same problem.
  • A Quarto setting gives us one place to tell users that the feature is experimental while we improve the file-link rules.

The explicit ID fix can be small if I am understanding it correctly. Tokens in quarto-core already have an attr with an identifier (packages/quarto-core/src/markdown/token.ts). If header tokens set it, I think that you could include the fix in this PR or in a quick follow-up.

From where I sit, I think the .html false positive is less common than the PR description says, since Quarto recommends links to the .qmd source file, and it changes these links at render time. Links to files that exist only after a render, and root-relative / paths, can cause similar problems.

nealrichardson and others added 4 commits October 13, 2026 13:00
ConfigurationManager.update() rebuilt the markdown section from the
defaults on every update, so getDiagnosticsOptions() always returned
enabled: false and the link diagnostics could never be turned on. Also
request the markdown.validate section (VS Code's built-in Markdown
settings) and convert it, falling back to the defaults for missing or
invalid values. Validation stays off unless markdown.validate.enabled
is true.

Add unit tests for apps/lsp (now part of yarn test-packages) and an
end-to-end test in lsp-features.test.ts.

Fixes quarto-dev#1185

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Add an experimental `quarto.validate.links.enabled` setting, off by default,
as the switch for link diagnostics in Quarto documents. The per-rule levels
and ignored links still come from `markdown.validate.*` (falling back to
VS Code's defaults when the client doesn't send them), but
`markdown.validate.enabled` is ignored, so users who turned it on for `.md`
files don't get new warnings in `.qmd` files.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…e LSP

The table of contents only knew the identifier generated from a heading's
text, so `[see](#sec-intro)` pointing at `## Introduction {#sec-intro}` was
reported as "No header found", and completions, go to definition and
references used the wrong identifier. As in Pandoc, an explicit `{#id}` now
replaces the generated one (and is reserved so later generated identifiers
don't reuse it).

Divs with an identifier (`::: {#fig-x}`, callouts, theorems) are now valid
fragment targets too. They're kept apart from the outline entries, so the
document outline, folding and completions are unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@nealrichardson
nealrichardson force-pushed the fix/lsp-markdown-validate branch from 756113b to f921339 Compare October 13, 2026 17:05
…iption

`#markdown.validate.*#` isn't a setting ID, so VS Code wouldn't render it
as a link.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@nealrichardson nealrichardson changed the title fix: read markdown.validate settings in the LSP so link diagnostics can be enabled feat: add an experimental quarto.validate.links.enabled setting for LSP link diagnostics Oct 13, 2026
`lsp-features.qmd` linked `[the section](#first-section)` to
`## First Section {#sec-first}`. Pandoc gives that heading only the
`sec-first` identifier, so the link is broken, and now that the LSP follows
Pandoc, find references no longer counts it. Link to `#sec-first` instead,
and cover references to explicit identifiers in the LSP unit tests.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@nealrichardson

Copy link
Copy Markdown
Contributor Author

🤖 Thanks @juliasilge, this now follows option 1:

  • New setting quarto.validate.links.enabled (default false, tagged experimental). It's the only switch for link diagnostics in Quarto documents. The per-rule levels and ignoredLinks are still read from markdown.validate.*, falling back to VS Code's defaults when they're missing or invalid. markdown.validate.enabled itself is ignored, so people who turned it on for .md files don't get new warnings in .qmd files. The setting's description mentions the rendered-output limitation (other.html) and points to linking the .qmd source or using markdown.validate.ignoredLinks.
  • Explicit IDs are link targets (apps/lsp/src/service/toc.ts). As you suggested, it uses the attr that quarto-core already parses: a heading's {#id} replaces the generated slug, as in Pandoc, so [see](#sec-intro) resolves. As a side effect, go to definition, find references, highlights and # path completions now use the right ID for those headings too. Divs with an ID (::: {#fig-x}, ::: {.callout-note #nte-x}) are valid targets as well. They're kept out of the outline, so the outline, folding and completions don't change.
  • Tests: the LSP gets its first unit tests (setting on/off, level fallbacks, heading/div/cell-label targets in the same file and across files), plus an end-to-end test in lsp-features.test.ts that toggles the setting.

Not covered, left for follow-ups: IDs on inline content the block parser doesn't record (![](x.png){#fig-x}, {#tbl-x}, {#eq-x}, spans). Quarto documents usually reference these with @fig-x, not links, and fixing them needs inline attribute parsing in quarto-core. Links to rendered output (.html) and root-relative / paths aren't covered either. The PR description has the details.

CI is green, so I'm marking this ready for review.

@nealrichardson
nealrichardson marked this pull request as ready for review October 13, 2026 19:34

@juliasilge juliasilge left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you so much for working on this new setting!

Image

I've got two questions that I think are worth raising with the first implementation of this setting here. Let me know what you think!

if (!id) {
return undefined;
}
const slug = pandocSlugifier.fromHeading(id);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

pandocSlugifier.fromHeading(id) changes the explicit identifier, but Pandoc uses an explicit {#id} exactly as the author wrote it. The slugifier makes the text lowercase, removes punctuation such as :, and removes all characters before the first letter.

This causes a couple of problems, I think:

  • # path completion offers the wrong target. For ## Results {#sec-Results}, it offers #sec-results, which is a broken link in the rendered HTML (completion-path.ts:335 uses entry.slug.value).
  • lookup() also slugifies the fragment, so links with the wrong case pass the new link diagnostics. In my manual test, [x](#sec_mixed) for ## Mixed Case {#Sec_Mixed} got no diagnostic.
  • Different identifiers can become the same slug. {#1-intro} becomes intro and collides with the generated identifier of ## Intro. Also, {#fig:plot} matches #figplot.
  • If the slug is empty (for example, ::: {#123}), every fragment that also slugifies to an empty string resolves to that div.

A possible fix would be to keep the explicit identifier verbatim as the slug, and compare it to the raw fragment with no slugifier. We could use the slugifier comparison only for generated identifiers. references.ts:164 also slugifies the fragment before it compares, so it would need the same change.

Do you think this is worth fixing now? I did notice it in my manual playing around here.

const slug = this.#slugifier.fromHeading(fragment);
return this.entries.find(entry => entry.slug.equals(slug));
return this.entries.find(entry => entry.slug.equals(slug))
?? this.anchors.find(entry => entry.slug.equals(slug));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lookup() now returns div anchors, so a link such as [see](#fig-x) to ::: {#fig-x} gets no diagnostic, but navigation to the div does not work. All of the navigation providers make sure that the entry is a heading with isTocHeaderEntry() before they use it, and a div entry has no headerLocation:

  • Go to definition (definitions.ts:87) returns nothing.
  • Cmd/Ctrl+click on the link (document-links.ts:899) opens the file at the top, not at the div.
  • Find references (references.ts:242) does not include the div as the definition.
  • Document highlights (document-highlights.ts:114) do not highlight the div.

Code cell labels (#| label: fig-x) had the same gap before this PR so this is not a new kind of problem, but the CHANGELOG says that div identifiers "are also recognized as valid targets", and users would probably expect navigation from that claim.

We could let these providers use sectionLocation when the entry is not a heading, so that navigation goes to the div (and to code cells). Or alternatively, would you prefer to update the CHANGELOG with something more accurate?

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.

LSP: markdown link diagnostics can never be enabled (markdown.validate is never read from the client)

3 participants