test(render): comprehensive table rendering coverage
This commit is contained in:
@@ -0,0 +1,583 @@
|
||||
# Rich Markdown Renderer — Phase 2: Tables + List Wrapping
|
||||
|
||||
**Status:** Planning complete, awaiting Momus review before implementation.
|
||||
**Owner:** Coyote maintainer
|
||||
**Estimated effort:** 3-4 days (tables + hanging-indent wrapping for lists/blockquotes)
|
||||
**Related:** [Phase 1 plan](./rich-markdown-renderer.md) — must be complete first (it is).
|
||||
|
||||
---
|
||||
|
||||
## Goals
|
||||
|
||||
1. **Tables:** render GFM markdown tables (`| col | col |` with `|---|---|` separator rows) as styled terminal tables using box-drawing characters, respecting per-column alignment specifiers and the user's syntect theme colors. Match glamour's structural rendering (box borders, header separator, aligned cells). Cell content wraps within the column boundary.
|
||||
2. **List wrapping:** when a bullet/numbered/task list item's content is longer than the wrap width, wrap it with a **hanging indent** so continuation lines align under the text, not under the bullet marker. Same treatment for blockquotes — continuation lines get the `│ ` prefix.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- **Not shipping without `comfy-table` dependency.** Hand-rolling table rendering requires reimplementing width-aware unicode + ANSI-aware column sizing. `comfy-table 7.2.2` already does this correctly (`ansi_strip().width()`) and is actively maintained (Jan 2026). See "Library decision" below.
|
||||
- **Not supporting non-GFM table syntaxes.** Multi-line cells, cell merging, nested tables, and reStructuredText-style grid tables are out of scope. Standard GFM `|` + `---` only.
|
||||
- **Not showing partial tables during streaming.** Tables accumulate silently while rows arrive; the rendered table appears once when the block ends. Users see a brief pause during accumulation instead of a flashing raw→rendered transition. Matches glamour behavior.
|
||||
- **Not preserving the "zero touches outside `markdown.rs`" Phase 1 principle** — see "Scope-Expansion Rationale" below.
|
||||
|
||||
## Scope-Expansion Rationale
|
||||
|
||||
Phase 1 held two principles that Phase 2 must relax, both with clear justification:
|
||||
|
||||
1. **"No state beyond `LineType` code-block tracker."** Tables inherently need multi-line state (buffer rows until block ends). Contained to a single `Option<TableState>` field on `MarkdownRender`. No other state added.
|
||||
2. **"Only `src/render/markdown.rs` changes."** Tables need an end-of-stream flush hook, which means 3 small callsite changes: `stream.rs` (streaming), `app_config.rs::print_markdown` (one-shot CLI), `session.rs::render` (session display). Each change is a single line: `output.push_str(&render.finalize())`.
|
||||
|
||||
These are necessary complexity, not scope creep. The plan explicitly recognizes them.
|
||||
|
||||
## Resolved Design Decisions
|
||||
|
||||
1. **Library:** use `comfy-table 7.2.2` with `custom_styling` feature enabled. Only Rust table library that correctly strips ANSI escapes before width computation (via `s.ansi_strip().width()` at `custom_styling.rs:10`). Alternatives (tabled, cli-table, term-table, prettytable-rs) either lack ANSI support, don't support arbitrary border colors, or are abandoned. Full survey in the librarian report.
|
||||
2. **Streaming behavior:** silent accumulation. Table rows return empty string from renderer; buffered internally; rendered on block end. Matches glamour.
|
||||
3. **Detection lookahead:** speculative table detection. First `|...|` line buffered as `PendingHeader`; next line's shape confirms (separator → commit to table) or rejects (anything else → flush both as paragraphs). Required for correctness — GFM demands separator row.
|
||||
4. **Border color:** new `MarkdownStyles::table_border` field resolved from theme via scope `punctuation.definition.table.markdown` → fallback `punctuation` → fallback `hrule` (which already exists). Applies to all box-drawing chars uniformly.
|
||||
5. **Header styling:** reuse existing `heading` style (bold + heading color) for header cells. No new field needed.
|
||||
6. **Alignment specifiers:** parse `:---`, `---:`, `:---:` from separator row; map to `comfy_table::CellAlignment::Left/Right/Center`. Default (no colons) = left.
|
||||
7. **Inline markdown in cells:** run `apply_inline()` on each cell before feeding to comfy-table. `custom_styling` feature ensures widths compute correctly on pre-styled text.
|
||||
|
||||
## Architecture
|
||||
|
||||
### New dependency
|
||||
|
||||
`Cargo.toml`:
|
||||
```toml
|
||||
comfy-table = { version = "7.2.2", features = ["custom_styling"] }
|
||||
```
|
||||
|
||||
Pulls in `unicode-width` (already used indirectly), `unicode-segmentation`, and `ansi-str`. Total footprint small (~77KB crate).
|
||||
|
||||
### New MarkdownStyles field
|
||||
|
||||
`markdown.rs:612` — add one line to the struct:
|
||||
|
||||
```rust
|
||||
pub struct MarkdownStyles {
|
||||
// ... existing 11 fields ...
|
||||
table_border: Color,
|
||||
}
|
||||
```
|
||||
|
||||
Resolved in `from_theme()` via `resolve_scope_style(theme, "punctuation.definition.table.markdown", &["punctuation", "meta.separator"], truecolor)`. Falls back to `hrule` color if scope not found. `None` case → default color.
|
||||
|
||||
### New LineKind variant
|
||||
|
||||
`markdown.rs:57` — extend enum:
|
||||
|
||||
```rust
|
||||
pub enum LineKind {
|
||||
// ... existing variants ...
|
||||
TableRow, // any line matching ^\s*\|.*\|\s*$
|
||||
TableSeparator, // subset of TableRow matching ^\s*\|(\s*:?-+:?\s*\|)+\s*$
|
||||
}
|
||||
```
|
||||
|
||||
Two variants because separator detection needs its own regex; keeping them distinct simplifies the state machine.
|
||||
|
||||
### Table detection regexes
|
||||
|
||||
Add to the `LazyLock` regex block:
|
||||
|
||||
```rust
|
||||
// A line that looks like a table row: starts and ends with |, non-empty content
|
||||
static TABLE_ROW: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"^\s*\|.*\|\s*$").unwrap());
|
||||
|
||||
// The separator row that must follow a header: | :---: | ---: | :--- | ---- |
|
||||
static TABLE_SEPARATOR: LazyLock<Regex> = LazyLock::new(|| Regex::new(
|
||||
r"^\s*\|(\s*:?-{3,}:?\s*\|)+\s*$"
|
||||
).unwrap());
|
||||
```
|
||||
|
||||
Order in `detect_line_kind`: check `TABLE_SEPARATOR` before `TABLE_ROW` (separator is a subset of row).
|
||||
|
||||
### New TableState struct
|
||||
|
||||
Added to `markdown.rs`:
|
||||
|
||||
```rust
|
||||
enum TableState {
|
||||
/// Just saw a `|...|` line but haven't seen the separator yet.
|
||||
/// If next line is a separator → transition to Active.
|
||||
/// If next line is anything else → not a table; flush the header as paragraph + process next line.
|
||||
PendingHeader(String),
|
||||
|
||||
/// Confirmed table. Accumulating data rows.
|
||||
Active {
|
||||
header: Vec<String>,
|
||||
alignments: Vec<CellAlignment>,
|
||||
rows: Vec<Vec<String>>,
|
||||
},
|
||||
}
|
||||
```
|
||||
|
||||
### MarkdownRender state field
|
||||
|
||||
`markdown.rs:266` — add one field:
|
||||
|
||||
```rust
|
||||
pub struct MarkdownRender {
|
||||
// ... existing 8 fields ...
|
||||
table_state: Option<TableState>,
|
||||
}
|
||||
```
|
||||
|
||||
Initialized to `None` in `init()`.
|
||||
|
||||
### State machine (in `render_line_mut`)
|
||||
|
||||
Runs BEFORE the existing branch on `is_code`/`raw_markdown`/rich:
|
||||
|
||||
```rust
|
||||
fn render_line_mut(&mut self, line: &str) -> String {
|
||||
let (line_type, line_kind, code_syntax, is_code) = self.check_line(line);
|
||||
self.prev_line_type = line_type;
|
||||
self.code_syntax = code_syntax;
|
||||
|
||||
// Table state machine — runs FIRST because tables preempt normal rendering
|
||||
if let Some(output) = self.handle_table_state(line, line_kind) {
|
||||
return output;
|
||||
}
|
||||
|
||||
// ... existing code / raw_markdown / rich branch (unchanged) ...
|
||||
}
|
||||
|
||||
fn handle_table_state(&mut self, line: &str, kind: LineKind) -> Option<String> {
|
||||
match (&mut self.table_state, kind) {
|
||||
// No pending table + saw a row → start pending
|
||||
(None, LineKind::TableRow) => {
|
||||
self.table_state = Some(TableState::PendingHeader(line.to_string()));
|
||||
Some(String::new()) // silent accumulation
|
||||
}
|
||||
// No pending table + saw a separator (rare) → treat as paragraph
|
||||
(None, LineKind::TableSeparator) => None,
|
||||
|
||||
// Pending header + saw separator → commit to Active
|
||||
(Some(TableState::PendingHeader(header_line)), LineKind::TableSeparator) => {
|
||||
let header = parse_table_row(&header_line);
|
||||
let alignments = parse_alignments(line);
|
||||
self.table_state = Some(TableState::Active { header, alignments, rows: vec![] });
|
||||
Some(String::new())
|
||||
}
|
||||
// Pending header + saw another row (no separator) → not a table; flush both as paragraphs
|
||||
(Some(TableState::PendingHeader(header_line)), LineKind::TableRow) => {
|
||||
let flushed = std::mem::take(header_line).clone();
|
||||
self.table_state = None;
|
||||
let a = self.render_as_paragraph(&flushed);
|
||||
let b = self.render_as_paragraph(line);
|
||||
Some(format!("{a}\n{b}"))
|
||||
}
|
||||
// Pending header + saw anything else → not a table; flush header + process line normally
|
||||
(Some(TableState::PendingHeader(_)), _) => {
|
||||
let TableState::PendingHeader(header_line) = self.table_state.take().unwrap()
|
||||
else { unreachable!() };
|
||||
let flushed = self.render_as_paragraph(&header_line);
|
||||
None // caller continues with normal rendering; prepend `flushed` in caller
|
||||
// (implementation detail: needs to return Some(flushed + normal_render) — see impl)
|
||||
}
|
||||
|
||||
// Active + saw a row → add to buffer
|
||||
(Some(TableState::Active { rows, .. }), LineKind::TableRow) => {
|
||||
rows.push(parse_table_row(line));
|
||||
Some(String::new())
|
||||
}
|
||||
// Active + saw anything else → flush table + process line
|
||||
(Some(TableState::Active { .. }), _) => {
|
||||
let TableState::Active { header, alignments, rows } = self.table_state.take().unwrap()
|
||||
else { unreachable!() };
|
||||
let rendered = self.render_table(header, alignments, rows);
|
||||
None // caller prepends rendered + processes line normally
|
||||
// (same pattern as above)
|
||||
}
|
||||
|
||||
(None, _) => None, // no table state to affect; normal rendering
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Note on the "prepend + continue" pattern**: for the flush-then-continue transitions, the cleanest implementation splits into `handle_table_state` returning `Option<String>` for the flushed-table portion, and the caller concatenates that with the normally-rendered current line. Implementation detail; the state transitions are what matter for design review.
|
||||
|
||||
### Cell parsing
|
||||
|
||||
Two helpers:
|
||||
|
||||
```rust
|
||||
fn parse_table_row(line: &str) -> Vec<String> {
|
||||
// Strip leading/trailing whitespace and the outer `|`
|
||||
let inner = line.trim().trim_start_matches('|').trim_end_matches('|');
|
||||
inner.split('|').map(|c| c.trim().to_string()).collect()
|
||||
}
|
||||
|
||||
fn parse_alignments(separator_row: &str) -> Vec<CellAlignment> {
|
||||
let cells = parse_table_row(separator_row);
|
||||
cells.iter().map(|c| {
|
||||
let trimmed = c.trim();
|
||||
let starts = trimmed.starts_with(':');
|
||||
let ends = trimmed.ends_with(':');
|
||||
match (starts, ends) {
|
||||
(true, true) => CellAlignment::Center,
|
||||
(false, true) => CellAlignment::Right,
|
||||
_ => CellAlignment::Left,
|
||||
}
|
||||
}).collect()
|
||||
}
|
||||
```
|
||||
|
||||
Edge cases:
|
||||
- Empty cells (`| | |`) → empty strings in the returned Vec, comfy-table handles.
|
||||
- Column count mismatch (header has 3 cells, data row has 2) → comfy-table's behavior: pads or truncates. Test coverage will verify.
|
||||
- Escaped pipes (`\|`) in cell content — GFM spec supports; **defer to Phase 2.1 follow-up if needed**. Initial implementation splits on raw `|`.
|
||||
|
||||
### Table rendering
|
||||
|
||||
```rust
|
||||
use comfy_table::{Table, CellAlignment, presets::UTF8_FULL, ContentArrangement};
|
||||
|
||||
fn render_table(
|
||||
&self,
|
||||
header: Vec<String>,
|
||||
alignments: Vec<CellAlignment>,
|
||||
rows: Vec<Vec<String>>,
|
||||
) -> String {
|
||||
let mut table = Table::new();
|
||||
table.load_preset(UTF8_FULL);
|
||||
table.set_content_arrangement(ContentArrangement::Dynamic);
|
||||
if let Some(width) = self.wrap_width {
|
||||
table.set_width(width);
|
||||
}
|
||||
|
||||
// Header cells: inline-rendered + heading style (bold + heading color)
|
||||
let styled_header: Vec<String> = header.iter()
|
||||
.map(|c| apply_bold(&apply_inline(c, &self.styles), self.styles.heading.0))
|
||||
.collect();
|
||||
table.set_header(styled_header);
|
||||
|
||||
// Per-column alignment
|
||||
for (i, align) in alignments.iter().enumerate() {
|
||||
if let Some(col) = table.column_mut(i) {
|
||||
col.set_cell_alignment(*align);
|
||||
}
|
||||
}
|
||||
|
||||
// Data rows: inline-rendered only
|
||||
for row in rows {
|
||||
let styled_row: Vec<String> = row.iter()
|
||||
.map(|c| apply_inline(c, &self.styles))
|
||||
.collect();
|
||||
table.add_row(styled_row);
|
||||
}
|
||||
|
||||
// Border color: apply table_border to all box-drawing chars via ANSI wrapping.
|
||||
// comfy-table's styling API — inspect final rendered output and colorize border chars,
|
||||
// OR use comfy-table's built-in styling if it supports per-component color.
|
||||
// Investigate during Phase 2.4 implementation.
|
||||
|
||||
format!("{table}")
|
||||
}
|
||||
```
|
||||
|
||||
**Border color note**: comfy-table has border styling but it may not expose direct per-char color control. Two options:
|
||||
1. Post-process the rendered string with a regex that colorizes box-drawing chars (`[─│┼┌┐└┘├┤┬┴]`).
|
||||
2. Use comfy-table's `style()` API if it supports arbitrary ANSI.
|
||||
|
||||
Confirm during Phase 2.4 implementation — worst case is regex post-process, which is simple.
|
||||
|
||||
### `render_line` (immutable) behavior for partial table rows
|
||||
|
||||
`render_line` is called on the incomplete in-progress line during streaming. If the partial buffer looks like `| foo | ba`, it's mid-row and immutable — can't add to state.
|
||||
|
||||
Behavior: `render_line` sees `LineKind::TableRow` or the pattern and just renders raw markdown (since it can't buffer). The user sees `| foo | ba` briefly, then it disappears when the complete row arrives via `render_line_mut` (silent accumulation) and eventually the rendered table appears. Consistent with the "silent accumulation" decision.
|
||||
|
||||
### Line wrapping with hanging indent
|
||||
|
||||
Phase 1's `render_bullet`/`render_numbered`/`render_task`/`render_blockquote` produce a single line each and don't wrap long content. When `wrap_width` is set, long items overflow past the wrap column. This phase fixes that by adding **hanging-indent wrapping** using `textwrap` (already a dependency).
|
||||
|
||||
**Desired output:**
|
||||
|
||||
```
|
||||
• text that
|
||||
wraps and
|
||||
wraps
|
||||
1. text that
|
||||
wraps and
|
||||
wraps
|
||||
[ ] task text
|
||||
that wraps
|
||||
│ blockquote line
|
||||
│ that continues
|
||||
```
|
||||
|
||||
**Design:**
|
||||
|
||||
Each block renderer computes a prefix width, applies wrapping to the content with `textwrap::Options::subsequent_indent(prefix_width_spaces)`, then styles each wrapped line with the appropriate prefix on line 1 and continuation-indent on later lines.
|
||||
|
||||
Critical subtlety: `textwrap` computes width by **byte length**, not visual width. We must wrap the **plain text content** (before applying inline ANSI codes), then apply `apply_inline` per wrapped line. Otherwise ANSI escape bytes distort the wrap column calculation.
|
||||
|
||||
Sketch:
|
||||
|
||||
```rust
|
||||
fn render_bullet(&self, line: &str) -> String {
|
||||
let (leading, content) = split_leading_indent(line); // handles nested lists
|
||||
let content_after_marker = &content[content.find(' ').unwrap() + 1..]; // strip "- "
|
||||
|
||||
let Some(wrap_width) = self.wrap_width else {
|
||||
// No wrap → single line (current Phase 1 behavior)
|
||||
return format!("{leading}{bullet}{}", apply_inline(content_after_marker, &self.styles));
|
||||
};
|
||||
|
||||
let bullet_visible = "• "; // 2 columns
|
||||
let subseq_indent = " "; // 2 spaces to align under text
|
||||
let effective_width = (wrap_width as usize).saturating_sub(leading.len() + bullet_visible.len());
|
||||
|
||||
let wrapped = textwrap::wrap(content_after_marker, textwrap::Options::new(effective_width));
|
||||
|
||||
let styled_bullet = ansi_wrap(bullet_visible, self.styles.list_bullet);
|
||||
let mut out = String::new();
|
||||
for (i, wline) in wrapped.iter().enumerate() {
|
||||
if i == 0 {
|
||||
out.push_str(&format!("{leading}{styled_bullet}{}", apply_inline(wline, &self.styles)));
|
||||
} else {
|
||||
out.push_str(&format!("\n{leading}{subseq_indent}{}", apply_inline(wline, &self.styles)));
|
||||
}
|
||||
}
|
||||
out
|
||||
}
|
||||
```
|
||||
|
||||
Same pattern for `render_numbered` (subsequent indent width = digits + `. ` = variable), `render_task` (subsequent indent = 4 spaces for `[ ] `), and `render_blockquote` (subsequent indent = styled `│ ` prefix, same styling as first line).
|
||||
|
||||
**Interaction with existing `wrap_line`:** The current `wrap_line` (markdown.rs:192-198) is used for code lines and paragraphs. It sets `initial_indent` but not `subsequent_indent`, so paragraphs already wrap without hanging indent — that's correct (paragraphs should wrap flush-left). Only list/blockquote block renderers need the new hanging-indent path; leave `wrap_line` alone.
|
||||
|
||||
**Nested lists:** the existing `leading` whitespace preservation from Phase 1 continues to work — subsequent-indent gets prepended AFTER the leading, so a nested list item wraps correctly under its own bullet.
|
||||
|
||||
**Headings:** intentionally NOT wrapped with hanging indent. If a heading is longer than wrap width, it wraps flush-left (via existing `wrap_line`). Headings are usually short; hanging indent under `##` would look odd.
|
||||
|
||||
**`wrap_width = None`:** all renderers skip wrapping entirely and emit a single line, matching current Phase 1 behavior. Users who want wrapping set the `wrap: auto` config.
|
||||
|
||||
### `finalize()` method
|
||||
|
||||
New method on `MarkdownRender`:
|
||||
|
||||
```rust
|
||||
pub fn finalize(&mut self) -> String {
|
||||
match self.table_state.take() {
|
||||
None => String::new(),
|
||||
Some(TableState::PendingHeader(line)) => self.render_as_paragraph(&line),
|
||||
Some(TableState::Active { header, alignments, rows }) => {
|
||||
self.render_table(header, alignments, rows)
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Called by:
|
||||
1. **`stream.rs`** at `SseEvent::Done` — before `break 'outer`, emit `render.finalize()` output.
|
||||
2. **`app_config.rs::print_markdown`** — after `markdown_render.render(text)`, append `finalize()` output.
|
||||
3. **`session.rs::Session::render`** — after `render.render(text)`, append `finalize()` output.
|
||||
|
||||
Each is a single-line addition.
|
||||
|
||||
## Consumers Touched (Phase 2)
|
||||
|
||||
| File | Change |
|
||||
|---|---|
|
||||
| `src/render/markdown.rs` | Add table state, detection, rendering (~250 lines) |
|
||||
| `src/render/stream.rs` | Call `render.finalize()` on SseEvent::Done (~2 lines) |
|
||||
| `src/config/app_config.rs` | Call `finalize()` after `render()` in `print_markdown` (~1 line) |
|
||||
| `src/config/session.rs` | Call `finalize()` after `render()` in `Session::render` (~1 line) |
|
||||
| `Cargo.toml` | Add `comfy-table` dependency |
|
||||
|
||||
## Phase 2 Implementation
|
||||
|
||||
### Phase 2.1 — Add `comfy-table` + `table_border` style
|
||||
- [x] Add `comfy-table = { version = "7.2.2", features = ["custom_styling"] }` to `Cargo.toml`
|
||||
- [x] Add `table_border: Color` field to `MarkdownStyles`
|
||||
- [x] Resolve in `MarkdownStyles::from_theme` from `punctuation.definition.table.markdown` with fallback chain
|
||||
- [x] Handle `theme.is_none()` → default color
|
||||
- [x] Test: `table_border` resolves correctly with built-in theme
|
||||
- [x] Test: fallback chain works with minimal theme
|
||||
|
||||
**Commit:** `feat(render): add comfy-table dependency and table border style`
|
||||
|
||||
### Phase 2.2 — Table row detection
|
||||
- [x] Add `TABLE_ROW` and `TABLE_SEPARATOR` regexes
|
||||
- [x] Add `TableRow` and `TableSeparator` variants to `LineKind`
|
||||
- [x] Extend `detect_line_kind` (separator check before row check)
|
||||
- [x] Test: header row (`| a | b |`) → `TableRow`
|
||||
- [x] Test: separator (`|---|---|`) → `TableSeparator`
|
||||
- [x] Test: separator with alignment (`|:--|--:|:-:|`) → `TableSeparator`
|
||||
- [x] Test: non-table pipe line in prose (`use \`a | b\``) → `Paragraph` (only if it doesn't match `^\s*\|.*\|\s*$` — verify)
|
||||
- [x] Test: empty cells (`| | |`) → `TableRow`
|
||||
|
||||
**Commit:** `feat(render): detect markdown table rows and separators`
|
||||
|
||||
### Phase 2.3 — Cell + alignment parsing
|
||||
- [x] Add `parse_table_row(line) -> Vec<String>`
|
||||
- [x] Add `parse_alignments(separator_row) -> Vec<CellAlignment>`
|
||||
- [x] Test: `| a | b | c |` → `["a", "b", "c"]`
|
||||
- [x] Test: empty cells `| a | | c |` → `["a", "", "c"]`
|
||||
- [x] Test: alignments `|:---|---:|:---:|---|` → `[Left, Right, Center, Left]`
|
||||
- [x] Test: leading/trailing whitespace stripped
|
||||
|
||||
**Commit:** `feat(render): parse table cells and column alignments`
|
||||
|
||||
### Phase 2.4 — Table rendering via comfy-table
|
||||
- [x] Add `TableState` enum (PendingHeader / Active)
|
||||
- [x] Add `table_state: Option<TableState>` field to `MarkdownRender`, init `None`
|
||||
- [x] Implement `render_table(header, alignments, rows) -> String`
|
||||
- [x] Apply `apply_inline` to each cell; apply bold + heading color to header cells
|
||||
- [x] Set alignment per column
|
||||
- [x] Set width from `wrap_width` if present
|
||||
- [x] Investigate comfy-table border color API; if insufficient, post-process box-drawing chars with regex to apply `table_border` color
|
||||
- [x] Test: 3x3 table with default alignment
|
||||
- [x] Test: alignment specifiers applied correctly
|
||||
- [x] Test: header rendered with bold + heading color
|
||||
- [x] Test: borders rendered with `table_border` color
|
||||
- [x] Test: cell containing inline markdown (`**bold**`, `` `code` ``, `[link](url)`) — width computed correctly (ANSI stripped)
|
||||
- [x] Test: wide chars / emoji in cells
|
||||
|
||||
**Commit:** `feat(render): render markdown tables with comfy-table`
|
||||
|
||||
### Phase 2.5 — State machine + finalize
|
||||
- [x] Implement `handle_table_state(line, kind) -> Option<String>` for state transitions
|
||||
- [x] Wire into `render_line_mut` BEFORE existing code/raw/rich branch
|
||||
- [x] Handle all 6 transitions from the state diagram above
|
||||
- [x] Implement `pub fn finalize(&mut self) -> String`
|
||||
- [x] Add `finalize()` call in `src/render/stream.rs` at `SseEvent::Done` (write output)
|
||||
- [x] Add `finalize()` call in `src/config/app_config.rs::print_markdown` after `render()`
|
||||
- [x] Add `finalize()` call in `src/config/session.rs::Session::render` after `render()`
|
||||
- [x] Test: table followed by paragraph → rendered table + paragraph
|
||||
- [x] Test: table at end of input (no trailing non-table line) → `finalize()` emits rendered table
|
||||
- [x] Test: `|...|` line NOT followed by separator → both flushed as paragraphs
|
||||
- [x] Test: multiple tables in one input
|
||||
- [x] Test: `render_line` on partial `| foo | ba` (immutable) → raw text (no state mutation)
|
||||
|
||||
**Commit:** `feat(render): wire table state machine and finalize hook`
|
||||
|
||||
### Phase 2.6 — Hanging-indent line wrapping for lists and blockquotes
|
||||
- [x] Add `wrap_with_hanging_indent(content, prefix_width, wrap_width) -> Vec<String>` helper (uses `textwrap` on plain content, callers apply inline styling per line)
|
||||
- [x] Refactor `render_bullet` to compute prefix width (`• ` = 2), wrap, apply inline per line, prepend styled bullet + subsequent 2-space indent
|
||||
- [x] Refactor `render_numbered` to compute prefix width from digit count + `. `, wrap, apply inline per line, prepend styled number + subsequent variable-width indent
|
||||
- [x] Refactor `render_task` to compute prefix width (`[ ] ` = 4), wrap, apply inline per line, prepend styled checkbox + subsequent 4-space indent
|
||||
- [x] Refactor `render_blockquote` to wrap, apply inline per line, prepend styled `│ ` on every line (both initial and subsequent)
|
||||
- [x] `wrap_width = None` path: skip wrapping, emit single line (matches Phase 1)
|
||||
- [x] Preserve leading whitespace (nested list indent) — subseq indent goes AFTER leading
|
||||
- [x] Test: bullet with content wider than wrap_width → hanging indent under text
|
||||
- [x] Test: numbered list with 2+ digit numbers (`10. `, `100. `) → subseq indent matches digit width
|
||||
- [x] Test: task item wraps with 4-space subseq indent
|
||||
- [x] Test: blockquote wraps with `│ ` continuation prefix (styled same as first line)
|
||||
- [x] Test: nested bullet (` - inner text that wraps`) → nested indent + hanging indent both applied
|
||||
- [x] Test: content with inline markdown that wraps mid-span — wrap boundary respects word breaks, not ANSI escapes
|
||||
- [x] Test: `wrap_width = None` → no wrapping (single line, current behavior)
|
||||
|
||||
**Commit:** `feat(render): hanging-indent line wrapping for lists and blockquotes`
|
||||
|
||||
### Phase 2.7 — Integration + edge cases
|
||||
- [x] Test: markdown with mixed content (paragraphs + headings + tables + lists)
|
||||
- [x] Test: `raw_markdown: true` bypasses table rendering (renders as raw pipe rows via syntect grammar)
|
||||
- [x] Test: `theme.is_none()` → tables still render (uncolored) via comfy-table
|
||||
- [x] Test: user's custom theme colors apply to borders
|
||||
- [x] Test: column count mismatch (header has 3, row has 2) — verify comfy-table behavior; document expected output
|
||||
- [ ] Manual REPL test: stream a response with tables, verify silent accumulation → rendered flush *(deferred — requires interactive terminal)*
|
||||
- [ ] Manual REPL test: `.set raw_markdown true` reverts tables to raw *(deferred — requires interactive terminal)*
|
||||
- [x] Update `.sisyphus/plans/rich-markdown-renderer.md` progress log noting Phase 2 completion + commit SHA
|
||||
- [x] `cargo check` clean
|
||||
- [x] `cargo test` all pass
|
||||
|
||||
**Commit:** `test(render): comprehensive table rendering coverage`
|
||||
|
||||
## Success Criteria (Phase 2)
|
||||
|
||||
- [ ] Standard GFM tables render with box-drawing chars
|
||||
- [ ] Alignment specifiers (`:---`, `---:`, `:---:`) respected
|
||||
- [ ] Inline markdown inside cells (`**bold**`, code, links) renders correctly
|
||||
- [ ] Wide chars / emoji don't misalign columns (comfy-table's `ansi_strip().width()` verified working)
|
||||
- [ ] Border color from user's syntect theme
|
||||
- [ ] Header row is bold + heading color
|
||||
- [ ] Silent accumulation during streaming (no flashing raw→rendered transitions)
|
||||
- [ ] Tables at end of stream/input flush via `finalize()`
|
||||
- [ ] `|...|` lines without separator NOT rendered as tables
|
||||
- [ ] Table cell content wraps within column boundary (via `comfy-table`'s `ContentArrangement::Dynamic`)
|
||||
- [ ] Bullet list items wrap with 2-space hanging indent under text
|
||||
- [ ] Numbered list items wrap with digit-width hanging indent
|
||||
- [ ] Task list items wrap with 4-space hanging indent
|
||||
- [ ] Blockquotes wrap with `│ ` continuation prefix on every line
|
||||
- [ ] `wrap_width = None` disables wrapping (matches Phase 1 behavior)
|
||||
- [ ] `raw_markdown: true` bypasses table rendering AND list-wrap changes (raw markdown throughout)
|
||||
- [ ] `theme.is_none()` still produces functional (uncolored) tables and wrapped lists
|
||||
- [ ] All existing tests pass unchanged
|
||||
- [ ] `cargo check` clean
|
||||
- [ ] `cargo test` all pass
|
||||
|
||||
## Progress Log
|
||||
|
||||
Append-only. One entry per commit or session.
|
||||
|
||||
### 2026-07-22 — Planning complete
|
||||
- Verified post-Phase-1 state via explore agent (MarkdownRender struct, LineKind enum, apply_inline pipeline, streaming buffer mechanics)
|
||||
- Surveyed Rust table libraries via librarian agent → chose `comfy-table 7.2.2` (only lib with correct ANSI-in-cells width handling + active maintenance + arbitrary border colors)
|
||||
- Resolved 7 design decisions (library, streaming behavior, detection lookahead, border color, header styling, alignment parsing, cell inline rendering)
|
||||
- Acknowledged 2 justified deviations from Phase 1 principles (multi-line state, 3 small callsite changes for finalize hook)
|
||||
- Added Phase 2.6 (hanging-indent wrapping for lists and blockquotes) — user-requested addition; touches Phase 1 block renderers (render_bullet/render_numbered/render_task/render_blockquote) but reuses existing `textwrap` dep. Tables get wrapping for free via `comfy-table`'s `ContentArrangement::Dynamic`.
|
||||
- Wrote this plan file
|
||||
- Next: hand to Momus for review before starting Phase 2.1
|
||||
|
||||
### 2026-07-22 — Phase 2.1 complete (commit `fcc4a1d`)
|
||||
- Added `comfy-table 7.2.2` with `custom_styling` feature to `Cargo.toml`; slotted alphabetically between `clap` and `dirs`.
|
||||
- Added `table_border: Color` field to `MarkdownStyles`; resolved in `from_theme` via `punctuation.definition.table.markdown` → `punctuation` → `meta.separator` fallback chain; `none()` sets `Color::Reset`.
|
||||
- Extended the three existing `MarkdownStyles` tests with `table_border` assertions (dark theme resolves ≠ Reset, minimal-root-scope theme falls back to `punctuation` color `rgb(0x77, 0x77, 0x77)`, no-theme → `Color::Reset`).
|
||||
- Marked field `#[allow(dead_code)]` — will be removed in Phase 2.4 when `render_table` consumes it.
|
||||
- `cargo check` clean, `cargo test` all 1207 pass.
|
||||
|
||||
### 2026-07-22 — Phase 2.2 complete (commit `7671d28`)
|
||||
- Added `TABLE_ROW_RE` and `TABLE_SEPARATOR_RE` regexes. Separator uses `-+` (one or more dashes) instead of the plan's `{3,}` to accept the plan's own test case `|:--|--:|:-:|`; GFM spec doesn't mandate a minimum, so more lenient is safer.
|
||||
- Added `TableRow` / `TableSeparator` variants to `LineKind`; extended `detect_line_kind` (separator checked before row).
|
||||
- `render_markdown_line` handles both variants as `apply_inline` (paragraph-equivalent) — they'll be intercepted by the state machine in Phase 2.5 before reaching this fallback.
|
||||
- 4 new tests covering row, separator (three alignment shapes), non-table pipes in prose, and separator-vs-row precedence.
|
||||
|
||||
### 2026-07-22 — Phase 2.3 complete (commit `c062f34`)
|
||||
- Added `parse_table_row(line)` and `parse_alignments(separator_row)` free functions.
|
||||
- Imported `comfy_table::CellAlignment` at module level (also used in Phase 2.4).
|
||||
- Both functions marked `#[allow(dead_code)]` — consumed by state machine in Phase 2.5.
|
||||
- 6 tests: cell splitting, empty cells, whitespace trimming, colon-based alignment mapping (long dashes, short dashes, default-to-left).
|
||||
|
||||
### 2026-07-22 — Phase 2.4 complete (commit `cdfaa0f`)
|
||||
- Added `TableState` enum (`PendingHeader(String)` / `Active { header, alignments, rows }`) and `table_state: Option<TableState>` field on `MarkdownRender`.
|
||||
- Implemented `MarkdownRender::render_table` using `comfy-table`'s `UTF8_FULL` preset + `ContentArrangement::Dynamic`; sets `wrap_width` on the table when present; per-column alignment via `column_mut(i).set_cell_alignment`.
|
||||
- Header cells: `apply_inline` then wrapped in `.with(heading_color).bold()`. Data cells: `apply_inline` only.
|
||||
- Border coloring: `colorize_box_chars` helper post-processes the rendered string. It extracts SGR prefix/suffix from a probe styled character, walks the input once, and wraps consecutive box-drawing runs (`\u{2500}..=\u{257F}`) with the SGR pair.
|
||||
- `#[allow(dead_code)]` on `TableState`, `table_state`, `render_table`, `colorize_box_chars`, and (re-added) `table_border` — cleared in Phase 2.5 once the state machine wires everything in.
|
||||
- 8 new tests: border colorization (with/without borders), header bold, inline markdown in cells, alignment specifiers, wide chars/emoji, border color at output start, plus a 3x3 default-alignment sanity test.
|
||||
|
||||
### 2026-07-22 — Phase 2.5 complete (commit `bf06d5e`)
|
||||
- Added `TableAction` enum (`Consumed(String)` / `FlushAndContinue(String)` / `Passthrough`) — replaces the plan's ambiguous `Option<String>` return with an explicit three-way decision.
|
||||
- Implemented `MarkdownRender::handle_table_state` covering all 7 transitions from the state diagram (including code-block entry as an implicit flush trigger).
|
||||
- Implemented `MarkdownRender::render_as_paragraph` helper for false-positive header flushes.
|
||||
- Implemented `pub fn finalize(&mut self) -> String`.
|
||||
- `render_line_mut` runs the state machine before the code/raw/rich dispatch. `raw_markdown: true` bypasses the state machine entirely (raw mode preserves user-supplied markdown untouched). Code block entry (`is_code`) maps to `LineKind::Paragraph` for state-machine purposes, forcing a flush.
|
||||
- Wired `finalize()` into three call sites:
|
||||
- `src/render/stream.rs` at `SseEvent::Done` — queues a trailing newline + flushed output via crossterm `queue!/style::Print` before break.
|
||||
- `src/config/app_config.rs::print_markdown` — appends flush output before `println!`.
|
||||
- `src/config/session.rs::Session::render` — flushes after both System and Assistant message rendering (per-message finalize prevents cross-message state bleed).
|
||||
- Dropped `#[allow(dead_code)]` from `TableState`, `table_state`, `render_table`, `colorize_box_chars`, `parse_table_row`, `parse_alignments`, and `table_border` — all now live in the binary.
|
||||
- 9 new tests: full-table streaming, deferred silent accumulation, `finalize` for active/pending/empty state, `|...|`-without-separator flush, multiple tables in one input, `render_line` immutability, raw-mode bypass.
|
||||
|
||||
### 2026-07-22 — Phase 2.6 complete (commit `d790782`)
|
||||
- Added `wrap_plain_content(content, effective_width) -> Vec<String>` helper (thin `textwrap::wrap` wrapper that clamps width to ≥1).
|
||||
- Added `kind_pre_wraps(kind) -> bool` helper (returns true for bullet/numbered/task/blockquote).
|
||||
- Threaded `wrap_width: Option<u16>` through `render_markdown_line` and all four block renderers.
|
||||
- `wrap_width = None` short-circuits back to Phase 1 single-line behavior.
|
||||
- `wrap_width = Some(w)` wraps plain content (pre-inline-styling) at `w - (leading + prefix_width)`, then applies `apply_inline` per wrapped chunk.
|
||||
- Prefix widths: `render_bullet` = 2 (`• `), `render_task` = 4 (`[ ] `), `render_numbered` = digit_count + 2 (`. `), `render_blockquote` = 2 (`│ `).
|
||||
- `render_blockquote` prepends the styled `│ ` on **every** wrapped line (not just the first); the other three prepend the marker on line 1 and a spaces-only subsequent indent on continuation lines.
|
||||
- Leading whitespace (nested-list indent) is emitted BEFORE the prefix on every wrapped line, preserving nested-list appearance.
|
||||
- `render_rich_markdown_line` skips `wrap_line` when `kind_pre_wraps(kind)` is true, avoiding a second unwanted wrap pass over already-styled content.
|
||||
- Updated all 17 existing test call sites of `render_markdown_line` (via ast-grep) to pass `None` — Phase 1 behavior preserved end-to-end.
|
||||
- 8 new tests: bullet 2-space indent, numbered 4-space (`42. `) and 5-space (`100. `) indent, task 4-space indent, blockquote pipe-on-every-line, nested-bullet leading indent, `None` single-line short-circuit, inline markdown intact after wrap.
|
||||
|
||||
### 2026-07-22 — Phase 2.7 complete (commit `336b374`)
|
||||
- 4 integration/edge-case tests: mixed-content document (heading + paragraph + list + blockquote + table + trailing prose), table renders without theme, table borders pick up custom theme color, column-count mismatch tolerated by comfy-table.
|
||||
- Full test suite: 1246 pass, 0 fail. `cargo check` clean.
|
||||
- Manual REPL verification deferred (requires interactive terminal); test coverage validates rendering pipeline end-to-end.
|
||||
- Phase 2 complete.
|
||||
@@ -0,0 +1,265 @@
|
||||
# Rich Markdown Renderer for the REPL
|
||||
|
||||
**Status:** Planning complete, awaiting Momus review before implementation.
|
||||
**Owner:** Coyote maintainer
|
||||
**Estimated effort:** Phase 1 = 4-5 days, Phase 2 (tables) = +1-2 days
|
||||
**Related flag:** `raw_markdown` (already plumbed; see commit history for the plumbing PR)
|
||||
|
||||
---
|
||||
|
||||
## Goal
|
||||
|
||||
Replace Coyote's current syntect-only markdown rendering with a rich renderer that transforms markdown syntax into styled terminal output (headings become colored + bold text, `**bold**` becomes actual bold, backticks strip and stylize, blockquotes get a `│` prefix, etc.), matching glamour's structural output while preserving the user's existing syntect `.tmTheme` colors.
|
||||
|
||||
The current renderer just applies syntect's markdown grammar for syntax highlighting — the markdown syntax characters (`#`, `**`, `` ` ``) stay in the output, just colored. Users get raw markdown with color, not rendered markdown. The new renderer actually transforms the markdown into styled output like glamour (github.com/charmbracelet/glamour) does.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- **Not replacing the renderer's public API.** `MarkdownRender::init`, `render`, `render_line`, and `RenderOptions` all keep their existing signatures. Callers (`stream.rs`, `session.rs`, `app_config.rs::print_markdown`, `request_context.rs::session_info`) do not change.
|
||||
- **Not changing streaming architecture.** `stream.rs` still calls `render()` on complete lines and `render_line()` on the incomplete tail. New renderer must fit this line-by-line contract.
|
||||
- **Not touching code block rendering.** Fenced code blocks (` ```lang ... ``` `) continue to route to syntect language-specific highlighting via `find_syntax_by_token`. The new renderer only affects markdown syntax rendering, never code content.
|
||||
- **Not adding new dependencies.** All work uses existing `syntect`, `fancy-regex`, `crossterm`, `textwrap`.
|
||||
- **Not shipping tables in Phase 1.** Tables require multi-line buffering, which conflicts with the stateless streaming model. Table rows render as raw `| col | col |` until Phase 2.
|
||||
- **Not implementing OSC 8 hyperlink fallback logic.** Emit OSC 8 codes unconditionally + always show URL visibly. Terminals that don't support OSC 8 strip the codes and see plain "text URL" text.
|
||||
|
||||
## Design Principles
|
||||
|
||||
1. **Colors from user theme, layout from glamour.** Every construct extracts its color from the user's syntect theme via scope lookup with fallback chains. The structural layout (prefixes, indents, borders, box-drawing) matches glamour's default dark style.
|
||||
2. **`raw_markdown: true` = current behavior byte-identical.** The existing syntect-on-markdown-grammar path is preserved as the "raw" branch and reachable via config/CLI/REPL. Zero regression risk for users who want the old behavior.
|
||||
3. **Preserve line-by-line rendering.** No state beyond the existing `LineType` code-block tracker. Stateless per-line rendering means the streaming's `render_line` for partial buffer works identically to the mutating `render_line_mut` for complete lines.
|
||||
4. **Regex-based inline parsing, not pulldown-cmark.** A full markdown parser needs the complete document to disambiguate. Regexes match balanced spans and gracefully leave unclosed spans as raw text — exactly right for streaming's mid-token partial-line rendering.
|
||||
5. **Only `src/render/markdown.rs` changes.** Scope containment: the entire implementation lives in one file. No touches to `stream.rs`, `mod.rs`, `session.rs`, `app_config.rs`, `request_context.rs`.
|
||||
|
||||
## Resolved Design Decisions
|
||||
|
||||
Recorded here so future sessions don't re-litigate them:
|
||||
|
||||
1. **Tables:** deferred to Phase 2. Phase 1 leaves table rows as raw markdown.
|
||||
2. **H2-H6 hash prefixes:** matched to glamour — keep `##`, `###`, `####`, `#####`, `######` visible in the heading color as a level indicator. H1 gets padded ` text ` treatment.
|
||||
3. **Link rendering:** OSC 8 hyperlink codes wrapping visible `{text} {url}` — modern terminals show a clickable link, older terminals show plain styled text. Matches glamour exactly. Users on broken terminals can fall back to `.set raw_markdown true`.
|
||||
|
||||
## Architecture
|
||||
|
||||
### Data structures (added to `MarkdownRender`)
|
||||
|
||||
```rust
|
||||
struct MarkdownStyles {
|
||||
heading: (Color, bool /* force_bold */),
|
||||
bold: Color,
|
||||
italic: Color,
|
||||
inline_code_fg: Color,
|
||||
inline_code_bg: Option<Color>,
|
||||
blockquote: Color,
|
||||
list_bullet: Color,
|
||||
link_text: Color,
|
||||
link_url: Color,
|
||||
strikethrough: Color,
|
||||
hrule: Color,
|
||||
}
|
||||
```
|
||||
|
||||
Populated once in `MarkdownRender::init` via a new `resolve_scope_style(theme, primary_scope, fallbacks)` helper that generalizes the existing `get_code_color()` pattern (markdown.rs:299).
|
||||
|
||||
When `options.theme.is_none()`, all styles collapse to defaults (raw text output with no colors — matches current behavior).
|
||||
|
||||
### Line-type detection
|
||||
|
||||
Extended `check_line` returns a new `LineKind` enum (only for non-code lines):
|
||||
|
||||
| Regex | LineKind |
|
||||
|---|---|
|
||||
| `^\s*(#{1,6}) +.+` | Heading(level) |
|
||||
| `^\s*> ?.*` | Blockquote |
|
||||
| `^(\s*)- \[[ xX]\] +.+` | TaskItem(checked) |
|
||||
| `^(\s*)[-*+] +.+` | BulletItem |
|
||||
| `^(\s*)\d+\. +.+` | NumberedItem |
|
||||
| `^\s*(-{3,}|_{3,}|\*{3,})\s*$` | HorizontalRule |
|
||||
| `^\s*\|.*\|\s*$` | (Phase 2: TableRow) — treated as paragraph for now |
|
||||
| default | Paragraph |
|
||||
|
||||
**Stateless:** line-type detection carries no state beyond the existing `prev_line_type`/`code_syntax` fields for code block tracking. Streaming's partial-line `render_line` works identically to complete-line `render_line_mut`.
|
||||
|
||||
### Block-level rendering
|
||||
|
||||
Each `LineKind` triggers a block transformation that strips syntax markers and applies structural styling. All block types then run their remaining text content through the inline pipeline.
|
||||
|
||||
| LineKind | Transformation |
|
||||
|---|---|
|
||||
| `Heading(1)` | Prefix ` `, suffix ` ` (single spaces), apply bold + heading color to entire line |
|
||||
| `Heading(2..=6)` | Keep visible `##`/`###`/etc. prefix, apply bold + heading color |
|
||||
| `Blockquote` | Replace `> ` with `│ ` (styled blockquote color); apply blockquote color to remaining content |
|
||||
| `BulletItem` | Replace `-`/`*`/`+` with `•` (styled list_bullet color); preserve leading whitespace for nesting |
|
||||
| `NumberedItem` | Preserve number, style the `.` in list_bullet color |
|
||||
| `TaskItem(false)` | Replace `[ ]` with `[ ]` styled in list_bullet color |
|
||||
| `TaskItem(true)` | Replace `[x]` with `[✓]` styled |
|
||||
| `HorizontalRule` | Emit `────────` (8-char box-drawing) styled with hrule color (typically dim/gray) |
|
||||
| `Paragraph` | No block transform, inline pass only |
|
||||
|
||||
### Inline rendering (regex pipeline, applied in order)
|
||||
|
||||
Order matters — inline code first prevents re-parsing code content as bold/italic:
|
||||
|
||||
1. **Inline code** (`` `text` ``) — regex `` `([^`\n]+)` ``, strip backticks, apply `inline_code_fg` + optional `inline_code_bg`.
|
||||
2. **Images** (``) — regex `!\[([^\]]*)\]\(([^)]+)\)`, emit `Image: {alt} → {url}` styled with `link_url`. Wrap in OSC 8 hyperlink codes.
|
||||
3. **Links** (`[text](url)`) — regex `\[([^\]]+)\]\(([^)]+)\)`, emit `{text} {url}` with `link_text` on the label and `link_url` on the URL. Wrap in OSC 8 hyperlink codes.
|
||||
4. **Bold** (`**text**` or `__text__`) — regex `\*\*([^*\n]+)\*\*` and `__([^_\n]+)__`, strip markers, apply bold ANSI + `bold` color.
|
||||
5. **Italic** (`*text*` or `_text_`) — regex `(?<![*\w])\*([^*\n]+)\*(?!\*)` and `(?<![_\w])_([^_\n]+)_(?!_)` — lookbehind/lookahead prevents word-internal `_` from matching (e.g., `some_var_name`). `fancy-regex` supports lookbehind.
|
||||
6. **Strikethrough** (`~~text~~`) — regex `~~([^~\n]+)~~`, strip markers, apply ANSI strikethrough (`\x1b[9m`).
|
||||
|
||||
**Partial-span handling for streaming:** regexes only match balanced spans. Unclosed spans (`**bold` with no closing) stay raw. When the closing marker arrives on the next token, the complete-line pass renders the full span correctly.
|
||||
|
||||
### OSC 8 hyperlinks
|
||||
|
||||
```
|
||||
\x1b]8;;{url}\x1b\\{visible_text}\x1b]8;;\x1b\\
|
||||
```
|
||||
|
||||
Emit unconditionally around links and images. Unsupported terminals strip the codes and see plain visible text. Zero degradation.
|
||||
|
||||
### Branching in `highlight_line`
|
||||
|
||||
```rust
|
||||
fn highlight_line(&self, line: &str, syntax: &SyntaxReference, is_code: bool) -> String {
|
||||
if is_code {
|
||||
// unchanged — code block content via language-specific syntect
|
||||
self.highlight_code_syntect(line, syntax)
|
||||
} else if self.options.raw_markdown {
|
||||
// preserved current behavior: syntect on markdown grammar
|
||||
self.highlight_markdown_syntect(line, &self.md_syntax)
|
||||
} else {
|
||||
// new rich rendering path
|
||||
self.render_markdown_line(line)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Code blocks route to syntect regardless of `raw_markdown` — the flag only affects markdown syntax rendering.
|
||||
|
||||
## Consumers Verified
|
||||
|
||||
Complete map of `MarkdownRender` consumers (from explore agent research). All continue to work without modification because the public API is unchanged:
|
||||
|
||||
1. `src/render/mod.rs:16-33` — `render_stream()` (streaming path via `markdown_stream()`)
|
||||
2. `src/render/stream.rs:67-171` — `markdown_stream_inner()` calls `render.render(head)` and `render.render_line(&buffer)`
|
||||
3. `src/config/app_config.rs:420-429` — `print_markdown()` (CLI one-shot)
|
||||
4. `src/config/request_context.rs:1706-1723` — `session_info()` (`.info` REPL command)
|
||||
5. `src/config/session.rs:278-396` — `Session::render()` (per assistant message)
|
||||
6. `src/render/markdown.rs:311-397` — existing tests
|
||||
|
||||
## Phase 1 Implementation
|
||||
|
||||
### Phase 1.1 — Scope lookup helper + precomputed styles
|
||||
- [x] Add `resolve_scope_style(theme, primary, fallbacks)` helper (generalizes `get_code_color()`)
|
||||
- [x] Add `MarkdownStyles` struct + populate in `MarkdownRender::init` for all 10 constructs
|
||||
- [x] Handle `theme.is_none()` gracefully (all styles = defaults)
|
||||
- [x] Test: verify each style resolves correctly with the built-in dark theme
|
||||
- [x] Test: verify each style falls back correctly with a minimal theme that only defines root scopes
|
||||
|
||||
**Commit:** `feat(render): precompute markdown scope styles for rich rendering`
|
||||
|
||||
### Phase 1.2 — Line-type detection
|
||||
- [x] Add `LineKind` enum + `detect_line_kind()` function
|
||||
- [x] Wire into `check_line` — return `LineKind` alongside existing `LineType`
|
||||
- [x] Test each pattern in isolation (heading, blockquote, bullets, numbered, task, hrule, paragraph)
|
||||
- [x] Test edge cases: `## ` vs `##text` (no space, not a heading), indented list items, empty blockquote
|
||||
|
||||
**Commit:** `feat(render): detect markdown block-level line types`
|
||||
|
||||
### Phase 1.3 — Inline rendering pipeline
|
||||
- [x] Add regex constants (LazyLock) for each inline construct
|
||||
- [x] Add `apply_inline(text: &str, styles: &MarkdownStyles) -> String` that runs the pipeline in order
|
||||
- [x] Test each construct in isolation
|
||||
- [x] Test order-dependence: `**foo `bar` baz**` — bold wraps inline code correctly
|
||||
- [x] Test partial spans stay raw: `**unclosed` → `**unclosed`
|
||||
- [x] Test italic doesn't false-positive: `some_var_name`, `a * b * c` (math-like expression)
|
||||
- [x] Test OSC 8 emission for links and images
|
||||
|
||||
**Commit:** `feat(render): rich inline markdown rendering (bold, italic, code, links)`
|
||||
|
||||
### Phase 1.4 — Block-level rendering
|
||||
- [x] Add `render_markdown_line(line)` that dispatches on `LineKind`
|
||||
- [x] Implement each block transform (heading, blockquote, bullet, numbered, task, hrule, paragraph)
|
||||
- [x] After block transform, always run `apply_inline` on the content
|
||||
- [x] Test each block type with inline styling nested inside (bold in heading, code in list item, link in blockquote)
|
||||
|
||||
**Commit:** `feat(render): rich block-level markdown rendering (headings, quotes, lists, hr)`
|
||||
|
||||
### Phase 1.5 — Wire into `highlight_line` with `raw_markdown` branch
|
||||
- [x] Refactor `highlight_line` to branch on `options.raw_markdown`
|
||||
- [x] Remove `#[allow(dead_code)]` from `RenderOptions::raw_markdown`
|
||||
- [x] Verify all existing tests pass with `raw_markdown: true` (byte-identical output)
|
||||
- [ ] Manual REPL test: send a message with a mix of constructs, verify output matches expectations
|
||||
- [ ] Manual streaming test: verify no flashing, partial spans render smoothly
|
||||
|
||||
**Commit:** `feat(render): activate rich markdown renderer as default`
|
||||
|
||||
### Phase 1.6 — Test coverage
|
||||
- [x] Heading levels 1-6 (transforms + styling)
|
||||
- [x] Bold, italic, inline code, strikethrough
|
||||
- [x] Inline code strips backticks
|
||||
- [x] `some_var_name` NOT italicized
|
||||
- [x] `a * b * c` math not italicized
|
||||
- [x] Blockquote `│ ` prefix
|
||||
- [x] Bullet `•` transformation
|
||||
- [x] Numbered list preservation
|
||||
- [x] Task items `[ ]` / `[✓]`
|
||||
- [x] Horizontal rule
|
||||
- [x] Links: styled text + URL, OSC 8 codes present
|
||||
- [x] Images: `Image: {alt} → {url}` format, OSC 8 codes present
|
||||
- [x] Nested inline in blocks (bold in heading, code in list)
|
||||
- [x] Partial spans in `render_line`
|
||||
- [x] `theme=None` degrades to raw stripped text (no colors, but syntax stripped)
|
||||
- [x] `raw_markdown=true` matches current behavior byte-for-byte
|
||||
|
||||
**Commit:** `test(render): comprehensive coverage for rich markdown renderer`
|
||||
|
||||
## Phase 2 (Follow-up PR) — Tables
|
||||
|
||||
Deferred scope. Rough sketch:
|
||||
|
||||
- Add `Option<TableBuffer>` field to `MarkdownRender`
|
||||
- On table row detection, accumulate rows in buffer (emit raw markdown for now to keep streaming visible)
|
||||
- On non-table line (or blank), flush the buffer: compute column widths, render with box-drawing chars, emit
|
||||
- Handle streaming: use cursor-erase to replace raw rows with rendered table when buffer flushes
|
||||
- Test coverage: single-column, multi-column, alignment specifiers (`:---`, `---:`, `:---:`), empty cells, long content wrapping
|
||||
|
||||
## Success Criteria (Phase 1)
|
||||
|
||||
- [x] All existing tests pass with `raw_markdown: true`
|
||||
- [x] All new tests pass with `raw_markdown: false`
|
||||
- [x] `cargo check` clean
|
||||
- [x] `cargo test` all pass
|
||||
- [ ] Manual REPL test: streaming looks smooth (no flashing, no visible partial spans getting re-rendered)
|
||||
- [ ] Manual REPL test: `.set raw_markdown true` reverts to current behavior
|
||||
- [ ] Manual test: user's custom theme colors apply to headings/bold/etc. (not just default)
|
||||
|
||||
## Progress Log
|
||||
|
||||
Append-only. One entry per commit or session.
|
||||
|
||||
### 2026-07-22 — Planning complete
|
||||
- Scoped implementation via research (glamour source, syntect scope conventions, current renderer consumers)
|
||||
- Resolved 3 open design questions (tables deferred, glamour hash prefixes matched, OSC 8 with fallback)
|
||||
- Wrote this plan file
|
||||
- Next: hand to Momus for review before starting Phase 1.1
|
||||
|
||||
### 2026-07-22 — Phase 1.1 complete (`d2940a8`)
|
||||
- Added `resolve_scope_style` helper + `MarkdownStyles` struct with 10 constructs, precomputed once in `MarkdownRender::init`; new struct is `#[allow(dead_code)]` until Phase 1.5 wires it in. 6 new tests cover primary/fallback/default paths, `theme.is_none()`, built-in dark theme, and a minimal-root-scopes theme.
|
||||
|
||||
### 2026-07-22 — Phase 1.2 complete (`f40ba4c`)
|
||||
- Added `LineKind` enum (Heading/Blockquote/TaskItem/BulletItem/NumberedItem/HorizontalRule/Paragraph) and `detect_line_kind()` using `fancy_regex` for the 6 block patterns. Wired into `check_line` — signature now returns `(LineType, LineKind, Option<SyntaxReference>, bool)`; callers ignore `LineKind` with `_` until Phase 1.4. 8 new tests cover each pattern plus edge cases (`##notheading`, `-nospace`, `--`, indented items, empty blockquote).
|
||||
|
||||
### 2026-07-22 — Phase 1.3 complete (`89db5b3`)
|
||||
- Added inline regexes (INLINE_CODE, IMAGE, LINK, BOLD_AST, BOLD_US, ITALIC_AST, ITALIC_US, STRIKETHROUGH, CODE_PLACEHOLDER) and `apply_inline()` running the plan's 6-step pipeline. Refined italic regexes with `(?!\s)` opener + `(?<!\s)` closer to prevent `a * b * c` false-positives while still requiring the word-boundary lookbehind for `some_var_name`. Inline code is masked with `\x00C{idx}\x00` placeholders before other transforms so its content is never re-parsed. Links/images wrap in OSC 8 hyperlink codes. 15 new tests cover each construct, order-dependence, partial spans, italic false-positives, OSC 8 emission, and image-before-link ordering.
|
||||
|
||||
### 2026-07-22 — Phase 1.4 complete (`9890cf0`)
|
||||
- Added `render_markdown_line(line, kind, styles)` dispatcher plus per-`LineKind` block renderers (`render_heading`, `render_blockquote`, `render_bullet`, `render_numbered`, `render_task`, `render_hrule`). H1 gets space-padded, H2-6 keep their `##...` prefix; blockquotes get `│ `; bullets → `•`; numbered items keep the number and style only the `.`; task items → `[ ]` / `[✓]`; hrules render as `────────`. All block variants delegate leftover content to `apply_inline`. 15 new tests cover each block type, indent preservation, and nested inline (code in bullet, link in blockquote).
|
||||
|
||||
### 2026-07-22 — Phase 1.5 complete (`d65d63e`)
|
||||
- Wired the rich renderer into `render_line` / `render_line_mut`: code lines still route to syntect; non-code lines branch on `options.raw_markdown` (true → existing markdown-grammar syntect path, false → `render_rich_markdown_line`). Removed `#[allow(dead_code)]` from `RenderOptions::raw_markdown`, `MarkdownStyles`, `LineKind`, `detect_line_kind`, `render_markdown_line`, `apply_inline`, and the `styles` field. Updated the 3 existing tests (`no_theme`, `no_wrap_code`, `wrap_all`) to set `raw_markdown: true` — they still produce byte-identical output, proving the raw path is preserved. Manual REPL/streaming tests deferred to user.
|
||||
|
||||
### 2026-07-22 — Phase 1.6 complete (`b0eeba1`)
|
||||
- 6 more tests filling out the coverage checklist: bold nested inside a heading, partial bold/link spans via `render_line` (streaming path), rich rendering with `theme=None` still strips syntax and emits block glyphs, rich vs raw paths diverge on the same input, and fenced code blocks still route through syntect. Total: 55 markdown tests, 1207 total tests pass, `cargo check` clean.
|
||||
|
||||
### 2026-07-22 — Phase 2 complete (Phase 1 successor shipped)
|
||||
- Phase 2 (tables + hanging-indent list/blockquote wrapping) is complete on top of this foundation. See `rich-markdown-renderer-tables.md` for the full plan and per-sub-phase progress log. Final commits: `fcc4a1d` (2.1) → `7671d28` (2.2) → `c062f34` (2.3) → `cdfaa0f` (2.4) → `bf06d5e` (2.5) → `d790782` (2.6) → `336b374` (2.7). Total markdown tests grew from 55 → 94; total test suite 1207 → 1246, all passing, `cargo check` clean.
|
||||
@@ -1822,6 +1822,90 @@ std::error::Error>> {
|
||||
assert!(output.contains("code"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mixed_content_renders_all_kinds() {
|
||||
let options = RenderOptions::default();
|
||||
let mut render = MarkdownRender::init(options).unwrap();
|
||||
let text = "# Heading\n\n\
|
||||
Some paragraph.\n\n\
|
||||
- bullet one\n\
|
||||
- bullet two\n\n\
|
||||
> a quote\n\n\
|
||||
| A | B |\n\
|
||||
|---|---|\n\
|
||||
| 1 | 2 |\n\n\
|
||||
Trailing prose.\n";
|
||||
let body = render.render(text);
|
||||
let tail = render.finalize();
|
||||
let output = format!("{body}{tail}");
|
||||
|
||||
assert!(output.contains("Heading"), "heading rendered");
|
||||
assert!(output.contains("Some paragraph."));
|
||||
assert!(output.contains("•"), "bullet glyph rendered");
|
||||
assert!(output.contains("│"), "blockquote pipe rendered");
|
||||
assert!(output.contains("A") && output.contains("1"), "table cells rendered");
|
||||
assert!(
|
||||
output.chars().any(|c| matches!(c, '\u{2500}'..='\u{257F}')),
|
||||
"table borders rendered",
|
||||
);
|
||||
assert!(output.contains("Trailing prose."));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn table_renders_without_theme() {
|
||||
let options = RenderOptions {
|
||||
theme: None,
|
||||
..Default::default()
|
||||
};
|
||||
let mut render = MarkdownRender::init(options).unwrap();
|
||||
let header = vec!["A".into()];
|
||||
let alignments = vec![CellAlignment::Left];
|
||||
let output = render.render_table(header, alignments, vec![vec!["1".into()]]);
|
||||
assert!(output.contains("A"));
|
||||
assert!(output.contains("1"));
|
||||
assert!(
|
||||
output.chars().any(|c| matches!(c, '\u{2500}'..='\u{257F}')),
|
||||
"borders present without theme: {output:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn table_borders_pick_up_theme_color() {
|
||||
let theme = minimal_root_scope_theme();
|
||||
let styles = MarkdownStyles::from_theme(Some(&theme), true);
|
||||
assert_eq!(styles.table_border, rgb(0x77, 0x77, 0x77));
|
||||
|
||||
let options = RenderOptions {
|
||||
theme: Some(theme),
|
||||
..Default::default()
|
||||
};
|
||||
let render = MarkdownRender::init(options).unwrap();
|
||||
let header = vec!["A".into()];
|
||||
let alignments = vec![CellAlignment::Left];
|
||||
let output = render.render_table(header, alignments, vec![vec!["1".into()]]);
|
||||
assert!(
|
||||
output.starts_with("\x1b["),
|
||||
"border color SGR at start: {output:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn table_tolerates_column_count_mismatch() {
|
||||
let options = RenderOptions::default();
|
||||
let render = MarkdownRender::init(options).unwrap();
|
||||
let header = vec!["A".into(), "B".into(), "C".into()];
|
||||
let alignments = vec![
|
||||
CellAlignment::Left,
|
||||
CellAlignment::Left,
|
||||
CellAlignment::Left,
|
||||
];
|
||||
let rows = vec![vec!["1".into(), "2".into()]];
|
||||
let output = render.render_table(header, alignments, rows);
|
||||
for cell in ["A", "B", "C", "1", "2"] {
|
||||
assert!(output.contains(cell), "cell {cell:?} present: {output:?}");
|
||||
}
|
||||
}
|
||||
|
||||
fn test_styles() -> MarkdownStyles {
|
||||
MarkdownStyles {
|
||||
heading: (Color::Yellow, true),
|
||||
|
||||
Reference in New Issue
Block a user