Skip to content

Merge PR #159 (grid-table detection) with main; renumber to Q-2-38#215

Merged
cscheid merged 5 commits into
mainfrom
feature/grid-table-error-merge
May 19, 2026
Merged

Merge PR #159 (grid-table detection) with main; renumber to Q-2-38#215
cscheid merged 5 commits into
mainfrom
feature/grid-table-error-merge

Conversation

@cscheid
Copy link
Copy Markdown
Member

@cscheid cscheid commented May 19, 2026

Picks up #159 and resolves the conflicts that accumulated on main since it was filed.

Summary

Test plan

  • cargo xtask verify (full — Rust workspace, hub-client build/test, WASM, trace-viewer): 8951/8951 Rust tests pass.
  • tree-sitter test in crates/tree-sitter-qmd/tree-sitter-markdown: 490/490.
  • End-to-end check via cargo run --bin pampa -- <grid-table.qmd>: emits [Q-2-38] Grid tables are not supported with the expected Ariadne layered layout (red multi-line span, faded block-quote prefixes).

Note for @rundel

Heads up — the only judgment call I made was the renumber from Q-2-36 → Q-2-38, since both Q-2-36 and Q-2-37 had been taken by other PRs that landed after #159 was filed. Plan is to merge from this branch; #159 can stay open until then or be closed in favor of this one.

🤖 Generated with Claude Code

rundel and others added 4 commits May 4, 2026 21:55
Quarto Markdown intentionally doesn't support Pandoc-style grid
tables. Today they fall through as paragraphs (silently mis-parsed)
or generic ERROR nodes — no diagnostic that names the construct or
captures its text.

tree-sitter-qmd
  - Add a GRID_TABLE external token to scanner.c. parse_plus is
    refactored to disambiguate after the leading `+`: `+-`, `+=`, or
    `++` enters parse_grid_table_after_first_plus(); space/tab/EOL
    keeps the existing list-marker path.
  - parse_grid_table_after_first_plus greedily consumes a `+...+`
    border followed by one or more `|...|` body / border lines,
    walking nested block-quote prefixes via the open_blocks stack
    so grids inside `>` / `> >` are captured correctly.
  - grammar.js wires `grid_table` into _block_not_section.
  - 5 new corpus tests in test/corpus/grid_table.txt cover plain,
    single-quoted, double-nested-with-intermediate-paragraph,
    lone-border (negative), and `+====+` separator forms.

pampa
  - native_visitor in pandoc/treesitter.rs handles `grid_table`:
    builds a Q-2-36 DiagnosticMessage and falls back to a
    `Block::RawBlock { format: "qmd-grid-table" }` so the AST stays
    well-formed.
  - The diagnostic uses a layered Ariadne rendering:
      1. multi-line main label spans the whole table (provides
         `╭─▶ … ╰─` corners and the red highlight on table content);
      2. empty-content `add_detail_at` labels on interior body lines
         force Ariadne to display them instead of eliding to `┆`;
      3. `add_faded_at` labels on `>` block-quote prefixes after the
         opening border render those columns in Ariadne's grey
         (matching the prefix on line 1, which falls outside every
         label).
  - 5 new integration tests in tests/test_grid_table_error.rs.

quarto-error-reporting
  - New `DetailKind::Faded` variant + `add_faded_at` builder method.
    Maps to Ariadne's `unimportant_color` via a named constant
    ARIADNE_UNIMPORTANT_COLOR (mirror of ariadne 0.6.0's private
    `Config::unimportant_color`, currently `Color::Fixed(249)`).
  - Two generic improvements that this feature needed:
      - Skip `Label::with_message` when detail content is empty so
        empty labels don't draw a \`╰── \` arrow row in the snippet.
      - Set `Label::with_order(end_offset)` on every label so labels
        sort by line and stay in one Ariadne group instead of
        producing duplicated snippet blocks.
  - error_catalog.json registers Q-2-36 ("Grid tables are not
    supported"). quarto-lsp-core::DetailKind folds Faded into Note.

Side effect — regenerating parser.c shifts LR state numbers, so
crates/pampa/resources/error-corpus/_autogen-table.json was
regenerated via crates/pampa/scripts/build_error_table.ts (the
documented Merr workflow in crates/pampa/CLAUDE.md). Skipping this
step silently breaks every autogen-table-driven diagnostic
including Q-2-10.

Verification
  - tree-sitter test: 475/475 corpus
  - cargo nextest run --workspace: 8390/8390
  - cargo xtask verify --skip-hub-build --skip-hub-tests: clean
  - cargo xtask lint: 677 files clean

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The grid-table commit added DetailKind::Faded to quarto-error-reporting
and updated quarto-lsp-core to fold it into Note for non-Ariadne
contexts, but missed the parallel match in the WASM client's
diagnostic_to_json. CI's hub-client build (wasm32-unknown-unknown)
failed with E0004 non-exhaustive-patterns.

Map Faded -> "note" in JsonDiagnosticDetail, matching the convention
already used in quarto-lsp-core::types::From<DetailKind>.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts:
#	crates/tree-sitter-qmd/tree-sitter-markdown/grammar.js
#	crates/tree-sitter-qmd/tree-sitter-markdown/src/grammar.json
#	crates/tree-sitter-qmd/tree-sitter-markdown/src/parser.c
#	crates/tree-sitter-qmd/tree-sitter-markdown/src/scanner.c
Mechanical merge plus one renumber:

* parser.c regenerated from the auto-merged grammar.js + scanner.c
  (combines main's CAPTION_START external + PR #159's GRID_TABLE
  external; tree-sitter test 490/490 passes).
* _autogen-table.json regenerated via crates/pampa/scripts/build_error_table.ts
  (8 LR-state numbers shifted by the new grammar).
* Renumbered the PR's grid-table feature from Q-2-36 to Q-2-38:
  - Q-2-36 was claimed on main by PR #197 (knitr-style chunk options).
  - Q-2-37 was claimed on main by PR #207 (line break in link destination).
  - Updates: error_catalog.json (key + docs_url), treesitter.rs
    (.with_code on grid_table emission), test_grid_table_error.rs
    (assertions + test names).

Verification:
* cargo xtask verify (full, including hub-client build/test/WASM) — green.
* End-to-end: `cargo run --bin pampa -- <grid-table.qmd>` emits
  `[Q-2-38] Grid tables are not supported` with the expected
  layered Ariadne layout.
Main moved forward while #215 was in review:
* #209 (multi-line attribute lists on images/spans) claimed Q-2-38
  for "Unclosed Attribute Specifier" — the slot we'd previously
  picked for grid tables.
* #213 (bare `<` as Str token) shifted the parser further.

Mechanical pieces (same as first pass):
* parser.c regenerated via `tree-sitter generate` (auto-merged
  grammar.js / scanner.c picked up the new tokens from both
  features cleanly; tree-sitter test 513/513 passes).
* _autogen-table.json regenerated via build_error_table.ts.

Renumber: Q-2-38 → Q-2-39 for the grid-table feature (next free
slot). Updates the catalog (key + docs_url), `treesitter.rs`
(.with_code), and `test_grid_table_error.rs` (assertions + test
names q_2_38 → q_2_39).

Also: changed the user-facing recommendation from "Use a pipe
table instead" to "Use a list table instead" — pipe tables don't
support block content in cells, but list tables (and grid tables)
do, so the substitution is closer to a drop-in replacement.

Verification: `cargo xtask verify` (full, including hub-client
build + WASM + trace-viewer + q2-preview-spa) — all green.
End-to-end pampa run on a grid-table fixture emits
`[Q-2-39] Grid tables are not supported` with the new
recommendation text.
@cscheid
Copy link
Copy Markdown
Member Author

cscheid commented May 19, 2026

Refreshed: main moved forward (#209, #213 landed) while this PR was in review, which:

Also changed the user-facing recommendation in the diagnostic message from "Use a pipe table instead" to "Use a list table instead", since pipe tables don't support block content in cells but list tables and grid tables both do — a closer drop-in replacement.

Full cargo xtask verify (Rust workspace, hub-client build/tests, WASM, trace-viewer, q2-preview-spa) green.

@cscheid cscheid merged commit f32f3ec into main May 19, 2026
4 checks passed
@cscheid cscheid deleted the feature/grid-table-error-merge branch May 19, 2026 21:08
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.

2 participants