From 3ed55e91c18b0e2aef9760d46594cea38841ca3f Mon Sep 17 00:00:00 2001 From: JDonaghy Date: Sat, 5 Sep 2026 16:58:40 +0000 Subject: [PATCH 1/5] fix(#821): adopt quadraui render_markdown_to_styled for hover popups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hover popups (EditorHoverPopup / PanelHoverPopup) now style markdown via quadraui::compose::markdown::render_markdown_to_styled instead of vimcode's hand-rolled MdStyle-to-color span walk (render.rs's now-deleted hover_line_to_styled_text / markdown_rendered_to_quadraui_lines, ~120 lines). core::markdown.rs (pulldown-cmark) is untouched and still used by the editor-buffer inline highlighter and markdown-preview buffers — genuinely out of scope per the issue, and not functionally substitutable there (tree-sitter code highlighting + bare-URL scanning it does are reused here, not duplicated). EditorHoverPopup/PanelHoverPopup now store the raw markdown source plus a theme-independent structure (plain line_text, resolved links, tree-sitter code_highlights) computed once at show time via the new core::markdown::hover_markdown_structure. render.rs recomputes styled spans from that same source at paint time with the live theme, then overlays vimcode's own tree-sitter highlighting onto fenced code blocks (quadraui's renderer is deliberately language-agnostic; its own doc says as much). One real feature gap: quadraui only recognizes `[text](url)` links, not bare `http://`/`https://` autolinks. core::markdown::linkify_bare_urls is the compatibility shim that closes it locally (rewrites bare URLs into bracketed links before quadraui ever parses them) pending a proper upstream fix. Distinct per-level heading colors (Theme::md_heading1/2/3) are not preserved — quadraui renders every heading as bold + a larger line_scales factor, uniformly colored; accepted, documented divergence. New black-box tests, both backends (verified failing red with the linkify_bare_urls call removed, since quadraui's parser never turns unbracketed text into a link without it): - tui_main::shell_app::driver_editor_hover_renders_code_and_bare_url_link_via_quadraui_markdown - gtk::testing::editor_popups::editor_hover_popup_renders_code_and_bare_url_link_via_quadraui_markdown_on_gtk Co-Authored-By: Claude Sonnet 5 --- src/core/engine/ext_panel.rs | 63 +++---- src/core/engine/mod.rs | 35 +++- src/core/engine/panels.rs | 71 +------- src/core/engine/tests.rs | 4 +- src/core/markdown.rs | 319 +++++++++++++++++++++++++++++++++++ src/gtk/testing.rs | 58 +++++++ src/render.rs | 289 +++++++++++++------------------ src/tui_main/panels.rs | 4 +- src/tui_main/shell_app.rs | 110 ++++++++++++ 9 files changed, 667 insertions(+), 286 deletions(-) diff --git a/src/core/engine/ext_panel.rs b/src/core/engine/ext_panel.rs index df76868f..1bf14bf8 100644 --- a/src/core/engine/ext_panel.rs +++ b/src/core/engine/ext_panel.rs @@ -628,12 +628,15 @@ impl Engine { item_index: usize, markdown: &str, ) { - let rendered = crate::core::markdown::render_markdown(markdown); - let links = Self::extract_hover_links(&rendered); + let markdown = crate::core::markdown::linkify_bare_urls(markdown); + let (line_text, links, code_highlights) = + crate::core::markdown::hover_markdown_structure(&markdown); // Dismiss any active editor hover to avoid overlapping popups. self.dismiss_editor_hover(); self.panel_hover = Some(PanelHoverPopup { - rendered, + markdown, + line_text, + code_highlights, links, panel_name: panel_name.to_string(), item_id: item_id.to_string(), @@ -1135,46 +1138,30 @@ impl Engine { take_focus: bool, add_goto_links: bool, ) { - let mut rendered = crate::core::markdown::render_markdown(markdown); - let mut links = Self::extract_hover_links(&rendered); + let mut full_markdown = markdown.to_string(); // Append "Go to" navigation links after actual LSP content (vim mode only). + // Emitted as real `[label](url)` markdown — quadraui's renderer + // (adopted below, #821) parses these into clickable links itself, + // so no manual span bookkeeping is needed here. if add_goto_links && !self.is_vscode_mode() { let goto = self.lsp_goto_links(); if !goto.is_empty() { - use crate::core::markdown::{MdSpan, MdStyle}; - // Separator line. - rendered.lines.push(String::new()); - rendered.spans.push(Vec::new()); - rendered.code_highlights.push(Vec::new()); - // Build: "Go to Definition (:gd) | Type Definition (:gy) | ..." - // "Go to" is default fg; labels are link-colored and clickable. - let nav_line_idx = rendered.lines.len(); - let mut nav_text = String::from("Go to "); - let mut nav_spans = Vec::new(); + full_markdown.push_str("\n\nGo to "); for (i, (label, keybind, url)) in goto.iter().enumerate() { if i > 0 { - nav_text.push_str(" | "); + full_markdown.push_str(" | "); } - let start = nav_text.len(); - nav_text.push_str(label); - let end = nav_text.len(); - nav_spans.push(MdSpan { - start_byte: start, - end_byte: end, - style: MdStyle::Link, - }); - links.push((nav_line_idx, start, end, url.to_string())); - nav_text.push_str(&format!(" (:{})", keybind)); + full_markdown.push_str(&format!("[{label}]({url}) (:{keybind})")); } - rendered.lines.push(nav_text); - rendered.spans.push(nav_spans); - rendered.code_highlights.push(Vec::new()); } } - let popup_width = rendered - .lines + let full_markdown = crate::core::markdown::linkify_bare_urls(&full_markdown); + let (line_text, links, code_highlights) = + crate::core::markdown::hover_markdown_structure(&full_markdown); + + let popup_width = line_text .iter() .map(|l| l.chars().count()) .max() @@ -1187,7 +1174,9 @@ impl Engine { // Dismiss any active panel hover to avoid overlapping popups. self.dismiss_panel_hover_now(); self.editor_hover = Some(EditorHoverPopup { - rendered, + markdown: full_markdown, + line_text, + code_highlights, links, anchor_line, anchor_col, @@ -1283,7 +1272,7 @@ impl Engine { "j" | "Down" => { // Scroll down — stop when last line is visible if let Some(hover) = &mut self.editor_hover { - let max_scroll = hover.rendered.lines.len().saturating_sub(20); + let max_scroll = hover.line_text.len().saturating_sub(20); if hover.scroll_top < max_scroll { hover.scroll_top += 1; } @@ -1413,7 +1402,7 @@ impl Engine { /// Returns true if the popup was scrolled. pub fn editor_hover_scroll(&mut self, delta: i32) -> bool { if let Some(hover) = &mut self.editor_hover { - let max_scroll = hover.rendered.lines.len().saturating_sub(20); + let max_scroll = hover.line_text.len().saturating_sub(20); if delta > 0 { let new = (hover.scroll_top + delta as usize).min(max_scroll); if new != hover.scroll_top { @@ -1437,7 +1426,7 @@ impl Engine { /// from `quadraui::dispatch_mouse_drag` into this call (#215). pub fn editor_hover_set_scroll(&mut self, new_offset: usize) -> bool { if let Some(hover) = &mut self.editor_hover { - let max_scroll = hover.rendered.lines.len().saturating_sub(20); + let max_scroll = hover.line_text.len().saturating_sub(20); let clamped = new_offset.min(max_scroll); if clamped != hover.scroll_top { hover.scroll_top = clamped; @@ -1459,9 +1448,9 @@ impl Engine { pub fn hover_selection_text(&self) -> Option { let hover = self.editor_hover.as_ref()?; let text = if let Some(ref sel) = hover.selection { - sel.extract_text(&hover.rendered.lines) + sel.extract_text(&hover.line_text) } else { - hover.rendered.lines.join("\n") + hover.line_text.join("\n") }; if text.is_empty() { None diff --git a/src/core/engine/mod.rs b/src/core/engine/mod.rs index 3e7ce08b..af0547b9 100644 --- a/src/core/engine/mod.rs +++ b/src/core/engine/mod.rs @@ -1452,9 +1452,19 @@ pub struct ContextMenuItem { #[derive(Debug, Clone)] #[allow(dead_code)] pub struct PanelHoverPopup { - /// Rendered markdown lines + spans (reuses the existing markdown module). - pub rendered: crate::core::markdown::MdRendered, - /// Clickable link URLs extracted from LinkUrl spans: (line_idx, start_byte, end_byte, url). + /// Raw markdown source (already passed through + /// `core::markdown::linkify_bare_urls`). Styled at paint time via + /// `quadraui::compose::markdown::render_markdown_to_styled` with the + /// active theme (#821) — see `EditorHoverPopup::markdown`'s doc for the + /// full rationale. + pub markdown: String, + /// Plain per-line text (markdown syntax stripped) — used for the + /// popup's empty-content check and content-width sizing. + pub line_text: Vec, + /// Per-line tree-sitter highlights for fenced code-block lines. See + /// `EditorHoverPopup::code_highlights`. + pub code_highlights: Vec>, + /// Clickable link URLs: (line_idx, start_byte, end_byte, url). pub links: Vec<(usize, usize, usize, String)>, /// Panel name this hover belongs to. pub panel_name: String, @@ -1492,8 +1502,23 @@ pub enum EditorHoverSource { #[derive(Debug, Clone)] #[allow(dead_code)] pub struct EditorHoverPopup { - /// Rendered markdown content. - pub rendered: crate::core::markdown::MdRendered, + /// Raw markdown source (already passed through + /// `core::markdown::linkify_bare_urls`, and with any "Go to" nav links + /// appended). Styled at paint time via + /// `quadraui::compose::markdown::render_markdown_to_styled` with the + /// active theme — see `render.rs`'s `markdown_hover_to_quadraui_lines` + /// (#821). + pub markdown: String, + /// Plain per-line text (markdown syntax stripped) — used for scroll + /// bounds, clipboard copy, and selection extraction. Computed once, at + /// show time, via `core::markdown::hover_markdown_structure` (theme- + /// independent, so it doesn't drift from what's painted regardless of + /// theme changes while the popup is open). + pub line_text: Vec, + /// Per-line tree-sitter highlights for fenced code-block lines (empty + /// for non-code-block lines). Byte offsets are relative to the code + /// line's own raw text — see `hover_markdown_structure`'s doc. + pub code_highlights: Vec>, /// Clickable link regions: (line_idx, start_byte, end_byte, url). pub links: Vec<(usize, usize, usize, String)>, /// Buffer line where the hover is anchored (0-indexed). diff --git a/src/core/engine/panels.rs b/src/core/engine/panels.rs index 77cfcdce..cb5c5100 100644 --- a/src/core/engine/panels.rs +++ b/src/core/engine/panels.rs @@ -1547,73 +1547,10 @@ impl Engine { result } - /// Extract clickable links from rendered markdown. - /// - /// Two sources of click regions are handled: - /// - /// 1. **Markdown links** — each `Link` span (the label text) is paired with - /// the following `LinkUrl` span on the same line. The click region covers - /// the label; the URL drives dispatch. Command URIs displayed as - /// `:Name?args` are restored to `command:Name?args`. - /// - /// 2. **Bare URLs** — standalone `LinkUrl` spans (emitted by `render_markdown` - /// for plain `http://` / `https://` text) become their own click regions. - /// The span text is the URL itself, so no reconstruction is needed beyond - /// the same `:` → `command:` prefix check used for markdown link URLs. - pub(crate) fn extract_hover_links( - rendered: &crate::core::markdown::MdRendered, - ) -> Vec<(usize, usize, usize, String)> { - use crate::core::markdown::MdStyle; - let mut links = Vec::new(); - for (line_idx, line_spans) in rendered.spans.iter().enumerate() { - let Some(line) = rendered.lines.get(line_idx) else { - continue; - }; - // Walk every span on this line. - let mut span_iter = line_spans.iter().peekable(); - while let Some(span) = span_iter.next() { - if span.style == MdStyle::Link { - // Paired markdown link: look for the following LinkUrl span. - let url = span_iter - .peek() - .filter(|next| next.style == MdStyle::LinkUrl) - .and_then(|next| { - if next.end_byte <= line.len() { - Some(&line[next.start_byte..next.end_byte]) - } else { - None - } - }); - if let Some(url_text) = url { - // Command URIs display as ":Name?args" — restore prefix. - let url = if url_text.starts_with(':') { - format!("command{}", url_text) - } else { - url_text.to_string() - }; - if is_safe_url(&url) { - // Click region = the Link label span. - links.push((line_idx, span.start_byte, span.end_byte, url)); - } - } - } else if span.style == MdStyle::LinkUrl && span.end_byte <= line.len() { - // Standalone LinkUrl span (bare URL or the URL display of a - // markdown link). Make the span itself clickable. - let url_text = &line[span.start_byte..span.end_byte]; - // Restore command: prefix if displayed as ":Name?args". - let url = if url_text.starts_with(':') { - format!("command{}", url_text) - } else { - url_text.to_string() - }; - if is_safe_url(&url) { - links.push((line_idx, span.start_byte, span.end_byte, url)); - } - } - } - } - links - } + // Link extraction from hover markdown moved to + // `core::markdown::hover_markdown_structure` (#821 — hover popups adopt + // quadraui's `render_markdown_to_styled`, which resolves link ranges + // itself instead of vimcode re-pairing `Link`/`LinkUrl` spans). /// Execute an LSP navigation command from a hover popup link. /// Moves the cursor to the given position before invoking the LSP request. diff --git a/src/core/engine/tests.rs b/src/core/engine/tests.rs index 3c48c0a4..8c89a6a9 100644 --- a/src/core/engine/tests.rs +++ b/src/core/engine/tests.rs @@ -16031,7 +16031,7 @@ fn test_panel_hover_show_and_dismiss() { assert_eq!(ph.panel_name, "test_panel"); assert_eq!(ph.item_id, "item_1"); assert_eq!(ph.item_index, 0); - assert!(!ph.rendered.lines.is_empty()); + assert!(!ph.line_text.is_empty()); e.dismiss_panel_hover_now(); assert!(e.panel_hover.is_none()); } @@ -16043,7 +16043,7 @@ fn test_panel_hover_links_extracted() { let ph = e.panel_hover.as_ref().unwrap(); // Should have at least one link extracted from the markdown assert!( - !ph.links.is_empty() || !ph.rendered.lines.is_empty(), + !ph.links.is_empty() || !ph.line_text.is_empty(), "hover should have rendered content" ); } diff --git a/src/core/markdown.rs b/src/core/markdown.rs index 665a38dd..770febe1 100644 --- a/src/core/markdown.rs +++ b/src/core/markdown.rs @@ -581,6 +581,242 @@ pub fn render_markdown(input: &str) -> MdRendered { } } +// ─── Hover-popup markdown (quadraui-backed, #821) ──────────────────────────── +// +// `render_markdown` above (and the `MdRendered`/`MdSpan`/`MdStyle` types it +// returns) remain the engine's markdown pipeline for the editor-buffer inline +// highlighter (`src/render.rs`'s `md_inline_spans` — compensates for no +// tree-sitter markdown injection, out of scope for #821) and for markdown +// *preview* buffers (`Engine::open_markdown_preview*`). Neither of those +// consumes styled colour, so they stay on the local pulldown-cmark pipeline. +// +// The **hover-popup** path (`EditorHoverPopup` / `PanelHoverPopup`) instead +// adopts `quadraui::compose::markdown::render_markdown_to_styled` — the +// shared, cross-backend markdown renderer quadraui ships (quadraui#262) — +// rather than hand-rolling the markdown-style-enum-to-color span walk vimcode +// used to do in `render.rs` (`hover_line_to_styled_text`, +// `markdown_rendered_to_quadraui_lines`, now removed). Structure (plain line +// text, link ranges) is theme-independent, so it's computed once here, at +// hover-show time; the *styled* spans (which need the live `render::Theme`, +// not available in `core::`) are recomputed at paint time in `render.rs` from +// the same markdown string. +// +// One real feature gap vs. the local `render_markdown`: quadraui's parser +// only recognizes `[text](url)` links, not bare `http://`/`https://` +// autolinks (verified against the pinned rev — see `linkify_bare_urls` +// below, which is the compatibility shim for that gap; a proper upstream +// fix should still be filed against quadraui). Tree-sitter code-block syntax +// highlighting is also quadraui-agnostic by design (its own doc: "Tree-sitter- +// capable callers opt into per-language highlighting" via the `code_blocks` +// side-channel) — `hover_markdown_structure` below does exactly that, +// reusing the same `MdCodeHighlight`/`Syntax` machinery `render_markdown` +// uses for markdown-preview code blocks. Distinct heading colors +// (`Theme::md_heading1/2/3`) are *not* preserved — quadraui renders every +// heading level as bold + a larger `line_scales` factor, with no per-level +// color; this is an accepted, documented divergence, not a bug. + +/// Wrap bare `http://` / `https://` URLs in `[url](url)` markdown link +/// syntax so quadraui's `render_markdown_to_styled` (which only recognizes +/// bracketed links) still makes them clickable. A URL immediately preceded +/// by `(` is assumed to already be a link destination (`[text](url)`) and is +/// left alone. Fenced code blocks are skipped entirely (their contents +/// should never be rewritten). Adapted from `scan_bare_urls` above, which +/// performs the equivalent scan for the local pulldown-cmark pipeline. +pub fn linkify_bare_urls(input: &str) -> String { + let mut out = String::with_capacity(input.len()); + let mut in_code_block = false; + for (i, line) in input.split('\n').enumerate() { + if i > 0 { + out.push('\n'); + } + if line.trim_start().starts_with("```") { + in_code_block = !in_code_block; + out.push_str(line); + continue; + } + if in_code_block { + out.push_str(line); + continue; + } + linkify_bare_urls_in_line(line, &mut out); + } + out +} + +fn linkify_bare_urls_in_line(line: &str, out: &mut String) { + let bytes = line.as_bytes(); + let len = bytes.len(); + let mut i = 0usize; + while i < len { + let scheme_len = if bytes[i..].starts_with(b"https://") { + Some(8usize) + } else if bytes[i..].starts_with(b"http://") { + Some(7usize) + } else { + None + }; + let Some(scheme_len) = scheme_len else { + let ch = line[i..].chars().next().unwrap(); + out.push(ch); + i += ch.len_utf8(); + continue; + }; + if i > 0 && bytes[i - 1] == b'(' { + // Already a markdown link destination — leave untouched. + let ch = line[i..].chars().next().unwrap(); + out.push(ch); + i += ch.len_utf8(); + continue; + } + let start = i; + let mut j = i + scheme_len; + while j < len && !bytes[j].is_ascii_whitespace() { + j += 1; + } + while j > start + scheme_len { + match bytes[j - 1] { + b'.' | b',' | b')' => j -= 1, + _ => break, + } + } + let url = &line[start..j]; + out.push('['); + out.push_str(url); + out.push_str("]("); + out.push_str(url); + out.push(')'); + i = j; + } +} + +/// `(line_text, links, code_highlights)` — see [`hover_markdown_structure`]'s +/// doc for what each element means. +pub type HoverMarkdownStructure = ( + Vec, + Vec<(usize, usize, usize, String)>, + Vec>, +); + +/// The theme-independent half of hover-popup markdown rendering: plain +/// per-line text, validated+dispatch-ready link ranges, and per-line +/// tree-sitter code-block highlights. Computed once when a hover popup is +/// shown (`Engine::show_editor_hover` / `show_panel_hover`); the styled +/// (theme-colored) `quadraui::StyledText` spans are computed separately, at +/// paint time, in `render.rs`. +/// +/// `markdown` should already have been passed through [`linkify_bare_urls`]. +/// +/// Returns `(line_text, links, code_highlights)`: +/// - `line_text[i]` — plain text of rendered line `i` (markdown syntax +/// stripped), used for scroll-bound math, clipboard copy, and selection +/// extraction. +/// - `links` — `(line_idx, start_byte, end_byte, url)`, byte ranges into +/// `line_text[line_idx]`; only `is_safe_url`-passing schemes are kept. +/// - `code_highlights[i]` — tree-sitter highlight spans for fenced +/// code-block line `i` (empty for non-code-block lines or unrecognized +/// languages). Byte offsets are relative to that line's own code text +/// (no code-rail prefix) — see `render.rs`'s `overlay_code_highlights`, +/// which applies these directly to the *last* span of a code-block +/// `StyledText` line (always the raw code span; the two before it are the +/// code-rail indent + bar, per quadraui's `render_code_content`). +pub fn hover_markdown_structure(markdown: &str) -> HoverMarkdownStructure { + let rendered = quadraui::compose::markdown::render_markdown_to_styled( + markdown, + &quadraui::Theme::default(), + ); + + let links = rendered + .links + .iter() + .filter_map(|(line_idx, range, url)| { + // Command URIs are never displayed in shortened ":Name?args" form + // by any current producer, but restore the prefix defensively — + // matches the pre-#821 `extract_hover_links` behaviour. + let url = match url.strip_prefix(':') { + Some(rest) => format!("command:{rest}"), + None => url.clone(), + }; + crate::core::engine::is_safe_url(&url).then_some(( + *line_idx, + range.start, + range.end, + url, + )) + }) + .collect(); + + let code_highlights = hover_code_highlights(&rendered); + + (rendered.line_text, links, code_highlights) +} + +/// Tree-sitter-highlight every fenced code block in `rendered` (via +/// `rendered.code_blocks`), producing per-line highlight spans keyed to each +/// code-content line's *own* raw text (i.e. the last span of that line's +/// `StyledText` — see `hover_markdown_structure`'s doc). +fn hover_code_highlights( + rendered: &quadraui::compose::markdown::RenderedMarkdown, +) -> Vec> { + let mut out: Vec> = vec![Vec::new(); rendered.lines.len()]; + + for cb in &rendered.code_blocks { + let Some(lang) = cb.lang.as_deref().and_then(SyntaxLanguage::from_name) else { + continue; + }; + let content_start = cb.fence_open + 1; + let content_end = cb.fence_close.unwrap_or(rendered.lines.len()); + if content_start >= content_end { + continue; + } + + let raw_lines: Vec<&str> = (content_start..content_end) + .map(|i| { + rendered + .lines + .get(i) + .and_then(|st| st.spans.last()) + .map(|s| s.text.as_str()) + .unwrap_or("") + }) + .collect(); + let code_text = raw_lines.join("\n"); + + let mut syntax = Syntax::new_for_language(lang); + let highlights = syntax.parse(&code_text); + + let mut line_byte_starts = Vec::with_capacity(raw_lines.len()); + let mut offset = 0usize; + for raw in &raw_lines { + line_byte_starts.push(offset); + offset += raw.len() + 1; // +1 for the joining '\n' + } + + for (start, end, scope) in &highlights { + let raw_line_idx = match line_byte_starts.binary_search(start) { + Ok(i) => i, + Err(i) => i.saturating_sub(1), + }; + let out_line_idx = content_start + raw_line_idx; + if out_line_idx >= out.len() { + continue; + } + let line_start = line_byte_starts[raw_line_idx]; + let local_start = start.saturating_sub(line_start); + let local_end = (*end - line_start).min(raw_lines[raw_line_idx].len()); + if local_start >= local_end { + continue; + } + out[out_line_idx].push(MdCodeHighlight { + start_byte: local_start, + end_byte: local_end, + scope: scope.clone(), + }); + } + } + + out +} + // ─── Tests ─────────────────────────────────────────────────────────────────── #[cfg(test)] @@ -757,4 +993,87 @@ mod tests { "code_highlights length must match lines length" ); } + + // ─── Hover-popup markdown (quadraui-backed, #821) ─────────────────── + + #[test] + fn linkify_bare_urls_wraps_a_bare_url() { + let out = linkify_bare_urls("See https://example.com for details"); + assert_eq!( + out, + "See [https://example.com](https://example.com) for details" + ); + } + + #[test] + fn linkify_bare_urls_leaves_existing_markdown_links_alone() { + let out = linkify_bare_urls("see [label](https://example.com) here"); + assert_eq!(out, "see [label](https://example.com) here"); + } + + #[test] + fn linkify_bare_urls_skips_fenced_code_blocks() { + let out = linkify_bare_urls("```\nsee https://example.com\n```"); + assert_eq!(out, "```\nsee https://example.com\n```"); + } + + #[test] + fn linkify_bare_urls_strips_trailing_punctuation() { + let out = linkify_bare_urls("visit https://example.com, please."); + assert_eq!( + out, + "visit [https://example.com](https://example.com), please." + ); + } + + #[test] + fn hover_markdown_structure_extracts_bold_code_and_link() { + let (line_text, links, _) = + hover_markdown_structure("**bold** and `code` and [label](https://example.com)"); + assert_eq!(line_text.len(), 1); + assert_eq!(line_text[0], "bold and code and label"); + assert_eq!(links.len(), 1); + assert_eq!(links[0].3, "https://example.com"); + let (_, s, e) = (links[0].0, links[0].1, links[0].2); + assert_eq!(&line_text[0][s..e], "label"); + } + + #[test] + fn hover_markdown_structure_extracts_bare_url_after_linkify() { + let markdown = linkify_bare_urls("See https://example.com for details"); + let (line_text, links, _) = hover_markdown_structure(&markdown); + assert_eq!(line_text[0], "See https://example.com for details"); + assert_eq!(links.len(), 1); + assert_eq!(links[0].3, "https://example.com"); + } + + #[test] + fn hover_markdown_structure_rejects_unsafe_url_scheme() { + let (_, links, _) = hover_markdown_structure("[click](javascript:alert(1))"); + assert!( + links.is_empty(), + "javascript: URLs must not become clickable links" + ); + } + + #[test] + fn hover_markdown_structure_highlights_fenced_code_block() { + let (line_text, _, code_highlights) = + hover_markdown_structure("```rust\nfn main() { let x = 42; }\n```"); + let code_line_idx = line_text + .iter() + .position(|l| l.contains("fn main")) + .expect("expected code line"); + assert!( + !code_highlights[code_line_idx].is_empty(), + "expected tree-sitter highlights for Rust code block, got none" + ); + assert!( + code_highlights[code_line_idx] + .iter() + .any(|h| h.scope == "keyword"), + "expected 'keyword' scope in highlights: {:?}", + code_highlights[code_line_idx] + ); + } } diff --git a/src/gtk/testing.rs b/src/gtk/testing.rs index 6b663a34..59ee4589 100644 --- a/src/gtk/testing.rs +++ b/src/gtk/testing.rs @@ -3713,6 +3713,64 @@ mod editor_popups { ); assert!(pw > 0.0 && ph > 0.0); } + + /// #821: hover popups adopt `quadraui::compose::markdown::render_markdown_to_styled` + /// instead of vimcode's hand-rolled `MdStyle`-to-color span walk. GTK twin + /// of `tui_main::shell_app::tests:: + /// driver_editor_hover_renders_code_and_bare_url_link_via_quadraui_markdown`: + /// same markdown, same three acceptance-criteria features (bold, inline + /// code, links), checked through GTK's own black-box surface — + /// `screen_contains` for the paint proof (markdown syntax must not leak), + /// `editor_hover_link_rects` for the link (a cached hit-region, painted by + /// production code, not a hardcoded rect — see that field's own doc). + /// + /// quadraui only recognizes `[text](url)` links, not bare `http://` + /// autolinks; `core::markdown::linkify_bare_urls` rewrites the source + /// markdown before quadraui ever parses it so the bare URL below still + /// becomes a real, clickable link on both backends. + /// + /// **RED against an unfixed tree:** comment out the `linkify_bare_urls` + /// call in `Engine::show_editor_hover` and `editor_hover_link_rects` comes + /// back without the bare-URL entry — quadraui's renderer never turns + /// unbracketed text into a link. + #[test] + fn editor_hover_popup_renders_code_and_bare_url_link_via_quadraui_markdown_on_gtk() { + let mut engine = small_engine(); + engine.show_editor_hover( + 1, + 4, + "plain821 **bold821** and `code821` — see https://example.com/docs821", + crate::core::engine::EditorHoverSource::Lsp, + false, + false, + ); + let mut h = harness(engine, 1400, 900); + h.driver.render(); + + assert!( + h.driver.screen_contains("bold821"), + "the word inside **bold821** must still paint, syntax stripped; painted texts: {:?}", + h.driver.painted_texts() + ); + assert!( + !h.driver.screen_contains("**bold821**"), + "bold markdown delimiters must not leak into painted text; painted texts: {:?}", + h.driver.painted_texts() + ); + assert!( + !h.driver.screen_contains("`code821`"), + "inline-code backticks must not leak into painted text; painted texts: {:?}", + h.driver.painted_texts() + ); + + let link_rects = h.editor_hover_link_rects.borrow().clone(); + assert!( + link_rects + .iter() + .any(|(_, _, _, _, uri)| uri == "https://example.com/docs821"), + "the bare URL must be linkified into a real clickable link rect; got {link_rects:?}" + ); + } } /// Black-box paint proof for the four panel-region surfaces #670 ported from diff --git a/src/render.rs b/src/render.rs index 840a2434..72ea5348 100644 --- a/src/render.rs +++ b/src/render.rs @@ -1293,8 +1293,14 @@ pub struct HoverPopup { /// Data for rendering an editor hover popup with rich markdown content. #[derive(Debug, Clone)] pub struct EditorHoverPopupData { - /// Rendered markdown content. - pub rendered: crate::core::markdown::MdRendered, + /// Raw markdown source. Styled at paint time with the active theme — + /// see `markdown_hover_to_quadraui_lines` (#821: adopts + /// `quadraui::compose::markdown::render_markdown_to_styled`). + pub markdown: String, + /// Plain per-line text (markdown syntax stripped). + pub line_text: Vec, + /// Per-line tree-sitter highlights for fenced code-block lines. + pub code_highlights: Vec>, /// Clickable link regions: (line_idx, start_byte, end_byte, url). pub links: Vec<(usize, usize, usize, String)>, /// Buffer line where the hover is anchored (0-indexed). @@ -1335,44 +1341,90 @@ pub struct PopupScrollbarHit { pub total: usize, } -/// Flatten a `MdRendered` block into per-line `quadraui::StyledText` + -/// per-line heading font scale. Shared by every rich-text hover/popup -/// builder (editor hover, panel-item hover) so markdown → styled-span -/// conversion lives in exactly one place. -fn markdown_rendered_to_quadraui_lines( - rendered: &crate::core::markdown::MdRendered, +/// Render hover-popup markdown into per-line `quadraui::StyledText` + per-line +/// heading font scale, via quadraui's shared +/// `quadraui::compose::markdown::render_markdown_to_styled` (#821 — replaces +/// the previous hand-rolled `MdStyle`-to-color byte-position span walk). +/// Shared by every rich-text hover/popup builder (editor hover, panel-item +/// hover) so markdown → styled-span conversion lives in exactly one place. +/// +/// `code_highlights` are vimcode's own tree-sitter highlights for fenced +/// code-block lines (precomputed once at hover-show time by +/// `core::markdown::hover_markdown_structure`, since quadraui's renderer is +/// deliberately language-agnostic — see its own doc: "Tree-sitter-capable +/// callers opt into per-language highlighting"). They're overlaid onto the +/// relevant lines after quadraui's render. +fn markdown_hover_to_quadraui_lines( + markdown: &str, + code_highlights: &[Vec], theme: &Theme, ) -> (Vec, Vec) { - let mut q_lines: Vec = Vec::with_capacity(rendered.lines.len()); - let mut line_scales: Vec = Vec::with_capacity(rendered.lines.len()); - for (line_idx, line_text) in rendered.lines.iter().enumerate() { - let md_spans = rendered.spans.get(line_idx); - let code_hl = rendered.code_highlights.get(line_idx); - q_lines.push(hover_line_to_styled_text( - line_text, - md_spans.map(|v| v.as_slice()).unwrap_or(&[]), - code_hl.map(|v| v.as_slice()).unwrap_or(&[]), - theme, - )); - // Heading rows render at a larger font scale (matches the - // legacy `font_scale` on the render-side StyledSpan). - let heading_level = md_spans - .and_then(|spans| { - spans.iter().find_map(|s| match s.style { - crate::core::markdown::MdStyle::Heading(n) => Some(n), - _ => None, - }) - }) - .unwrap_or(0); - let scale = match heading_level { - 1 => 1.4, - 2 => 1.2, - 3..=6 => 1.1, - _ => 1.0, - }; - line_scales.push(scale); + let q_theme = to_quadraui_theme(theme); + let mut rendered = quadraui::compose::markdown::render_markdown_to_styled(markdown, &q_theme); + for (line_idx, highlights) in code_highlights.iter().enumerate() { + if highlights.is_empty() { + continue; + } + if let Some(line) = rendered.lines.get_mut(line_idx) { + overlay_code_highlights(line, highlights, theme); + } + } + (rendered.lines, rendered.line_scales) +} + +/// Recolor a fenced code-block content line's raw-code span with vimcode's +/// tree-sitter scope colors. Per quadraui's `render_code_content`, a +/// code-block content line always has its raw (unprefixed) code text as the +/// *last* span — the two before it are the code-rail indent + bar, which are +/// left untouched. `highlights`' byte offsets are relative to that last +/// span's text (see `core::markdown::hover_markdown_structure`'s doc). +fn overlay_code_highlights( + line: &mut quadraui::StyledText, + highlights: &[crate::core::markdown::MdCodeHighlight], + theme: &Theme, +) { + let Some(code_span) = line.spans.pop() else { + return; + }; + let default_fg = code_span.fg; + let bg = code_span.bg; + let scope_at = |byte_pos: usize| -> Option<&str> { + highlights + .iter() + .find(|h| byte_pos >= h.start_byte && byte_pos < h.end_byte) + .map(|h| h.scope.as_str()) + }; + + let mut byte_pos = 0usize; + let mut current_text = String::new(); + let mut current_scope: Option<&str> = None; + let flush = |text: &mut String, scope: Option<&str>, spans: &mut Vec| { + if text.is_empty() { + return; + } + let fg = scope + .map(|s| to_quadraui_color(theme.scope_color(s))) + .or(default_fg); + spans.push(quadraui::StyledSpan { + text: std::mem::take(text), + fg, + bg, + bold: false, + italic: false, + underline: false, + }); + }; + + for ch in code_span.text.chars() { + let scope = scope_at(byte_pos); + if scope != current_scope { + flush(&mut current_text, current_scope, &mut line.spans); + current_scope = scope; + } + current_text.push(ch); + byte_pos += ch.len_utf8(); } - (q_lines, line_scales) + flush(&mut current_text, current_scope, &mut line.spans); } /// Convert `(line, start_byte, end_byte, url)` link tuples (the shape @@ -1401,7 +1453,8 @@ pub fn editor_hover_to_quadraui_rich_text( eh: &EditorHoverPopupData, theme: &Theme, ) -> quadraui::RichTextPopup { - let (q_lines, line_scales) = markdown_rendered_to_quadraui_lines(&eh.rendered, theme); + let (q_lines, line_scales) = + markdown_hover_to_quadraui_lines(&eh.markdown, &eh.code_highlights, theme); let q_links = md_links_to_quadraui_rich_text_links(&eh.links); let q_selection = eh @@ -1416,7 +1469,7 @@ pub fn editor_hover_to_quadraui_rich_text( quadraui::RichTextPopup { id: quadraui::WidgetId::new("editor_hover"), lines: q_lines, - line_text: eh.rendered.lines.clone(), + line_text: eh.line_text.clone(), line_scales, scroll_top: eh.scroll_top, max_visible_rows: EDITOR_HOVER_MAX_ROWS, @@ -1465,7 +1518,7 @@ pub fn editor_hover_popup_paint( Option<(f32, f32, f32, f32)>, Option, ) { - if eh.rendered.lines.is_empty() { + if eh.line_text.is_empty() { return (vec![], None, None); } let popup = editor_hover_to_quadraui_rich_text(eh, theme); @@ -1522,101 +1575,6 @@ pub fn editor_hover_popup_paint( (link_rects, popup_rect, scrollbar_hit) } -/// Flatten one rendered hover line (text + markdown spans + tree-sitter -/// code highlights) into a `quadraui::StyledText` whose spans correspond -/// to contiguous runs sharing fg/bold/italic. -fn hover_line_to_styled_text( - line_text: &str, - md_spans: &[crate::core::markdown::MdSpan], - code_highlights: &[crate::core::markdown::MdCodeHighlight], - theme: &Theme, -) -> quadraui::StyledText { - use crate::core::markdown::MdStyle; - if line_text.is_empty() { - return quadraui::StyledText::default(); - } - - let default_fg = to_quadraui_color(theme.hover_fg); - let h1_fg = to_quadraui_color(theme.md_heading1); - let h2_fg = to_quadraui_color(theme.md_heading2); - let h3_fg = to_quadraui_color(theme.md_heading3); - let code_fg = to_quadraui_color(theme.md_code); - let link_fg = to_quadraui_color(theme.md_link); - - // Style at byte position. Code highlights take priority on lines - // that have any (matching the TUI rasteriser's behaviour). - let style_at = |byte_pos: usize| -> (quadraui::Color, bool, bool) { - if !code_highlights.is_empty() { - for h in code_highlights { - if byte_pos >= h.start_byte && byte_pos < h.end_byte { - return (to_quadraui_color(theme.scope_color(&h.scope)), false, false); - } - } - return (code_fg, false, false); - } - for span in md_spans { - if byte_pos >= span.start_byte && byte_pos < span.end_byte { - return match span.style { - MdStyle::Heading(1) => (h1_fg, true, false), - MdStyle::Heading(2) => (h2_fg, true, false), - MdStyle::Heading(_) => (h3_fg, true, false), - MdStyle::Bold => (default_fg, true, false), - MdStyle::Italic => (default_fg, false, true), - MdStyle::BoldItalic => (default_fg, true, true), - MdStyle::Code | MdStyle::CodeBlock => (code_fg, false, false), - MdStyle::Link | MdStyle::LinkUrl => (link_fg, false, false), - MdStyle::BlockQuote => (h3_fg, false, true), - MdStyle::ListBullet => (h1_fg, true, false), - MdStyle::HorizontalRule | MdStyle::Image => (link_fg, false, true), - }; - } - } - (default_fg, false, false) - }; - - let mut spans: Vec = Vec::new(); - let mut byte_pos: usize = 0; - let mut current_text = String::new(); - let mut current_style: Option<(quadraui::Color, bool, bool)> = None; - - for ch in line_text.chars() { - let s = style_at(byte_pos); - match current_style { - Some(prev) if prev == s => { - current_text.push(ch); - } - _ => { - if !current_text.is_empty() { - let st = current_style.unwrap(); - spans.push(quadraui::StyledSpan { - text: std::mem::take(&mut current_text), - fg: Some(st.0), - bg: None, - bold: st.1, - italic: st.2, - underline: false, - }); - } - current_text.push(ch); - current_style = Some(s); - } - } - byte_pos += ch.len_utf8(); - } - if !current_text.is_empty() { - let st = current_style.unwrap_or((default_fg, false, false)); - spans.push(quadraui::StyledSpan { - text: current_text, - fg: Some(st.0), - bg: None, - bold: st.1, - italic: st.2, - underline: false, - }); - } - quadraui::StyledText { spans } -} - // ─── SignatureHelp ──────────────────────────────────────────────────────────── /// Data needed to render the signature help popup (shown above cursor in insert mode). @@ -7974,8 +7932,13 @@ pub struct ExtPanelSectionData { /// Rendering data for a sidebar panel hover popup (rendered markdown). #[derive(Debug, Clone)] pub struct PanelHoverPopupData { - /// Rendered markdown content. - pub rendered: crate::core::markdown::MdRendered, + /// Raw markdown source. Styled at paint time with the active theme — + /// see `EditorHoverPopupData::markdown`'s doc (#821). + pub markdown: String, + /// Plain per-line text (markdown syntax stripped). + pub line_text: Vec, + /// Per-line tree-sitter highlights for fenced code-block lines. + pub code_highlights: Vec>, /// Clickable link regions: (line_idx, start_byte, end_byte, url). pub links: Vec<(usize, usize, usize, String)>, /// Flat item index being hovered (for positioning relative to panel). @@ -8001,13 +7964,14 @@ pub fn panel_hover_to_quadraui_rich_text( ph: &PanelHoverPopupData, theme: &Theme, ) -> quadraui::RichTextPopup { - let (q_lines, line_scales) = markdown_rendered_to_quadraui_lines(&ph.rendered, theme); + let (q_lines, line_scales) = + markdown_hover_to_quadraui_lines(&ph.markdown, &ph.code_highlights, theme); let q_links = md_links_to_quadraui_rich_text_links(&ph.links); quadraui::RichTextPopup { id: quadraui::WidgetId::new("panel_hover"), lines: q_lines, - line_text: ph.rendered.lines.clone(), + line_text: ph.line_text.clone(), line_scales, scroll_top: 0, max_visible_rows: PANEL_HOVER_MAX_ROWS, @@ -8139,7 +8103,7 @@ pub fn panel_hover_popup_paint( let Some(ref hover) = screen.panel_hover else { return (vec![], None); }; - if hover.rendered.lines.is_empty() { + if hover.line_text.is_empty() { return (vec![], None); } let is_native = hover.panel_name == "source_control"; @@ -12902,13 +12866,17 @@ pub fn build_screen_layout_with_breadcrumb_row( hunk_lines: dp.hunk_lines.clone(), }), panel_hover: engine.panel_hover.as_ref().map(|ph| PanelHoverPopupData { - rendered: ph.rendered.clone(), + markdown: ph.markdown.clone(), + line_text: ph.line_text.clone(), + code_highlights: ph.code_highlights.clone(), links: ph.links.clone(), item_index: ph.item_index, panel_name: ph.panel_name.clone(), }), editor_hover: engine.editor_hover.as_ref().map(|eh| EditorHoverPopupData { - rendered: eh.rendered.clone(), + markdown: eh.markdown.clone(), + line_text: eh.line_text.clone(), + code_highlights: eh.code_highlights.clone(), links: eh.links.clone(), anchor_line: eh.anchor_line, anchor_col: eh.anchor_col, @@ -23966,25 +23934,16 @@ mod tests { /// hit-rect even if the Pango measurement itself is accurate (#488). #[test] fn test_editor_hover_to_quadraui_rich_text_link_offsets() { - use crate::core::markdown::{MdRendered, MdSpan, MdStyle}; - let line = "See https://example.com for details"; // byte offsets of "https://example.com": starts at 4, ends at 23 let link_start = 4usize; let link_end = 23usize; assert_eq!(&line[link_start..link_end], "https://example.com"); - let rendered = MdRendered { - lines: vec![line.to_string()], - spans: vec![vec![MdSpan { - start_byte: link_start, - end_byte: link_end, - style: MdStyle::LinkUrl, - }]], - code_highlights: vec![vec![]], - }; let eh = EditorHoverPopupData { - rendered, + markdown: line.to_string(), + line_text: vec![line.to_string()], + code_highlights: vec![vec![]], links: vec![(0, link_start, link_end, "https://example.com".to_string())], anchor_line: 0, anchor_col: 0, @@ -24029,8 +23988,6 @@ mod tests { /// would cause the GTK closure to measure the wrong line or wrong span. #[test] fn test_editor_hover_to_quadraui_rich_text_multi_link() { - use crate::core::markdown::{MdRendered, MdSpan, MdStyle}; - let line0 = "Docs: https://docs.rs/foo"; let line1 = "Also see https://crates.io/crates/foo"; // "https://docs.rs/foo" starts at 6, ends at 25 @@ -24040,24 +23997,10 @@ mod tests { assert_eq!(&line0[s0..e0], "https://docs.rs/foo"); assert_eq!(&line1[s1..e1], "https://crates.io/crates/foo"); - let rendered = MdRendered { - lines: vec![line0.to_string(), line1.to_string()], - spans: vec![ - vec![MdSpan { - start_byte: s0, - end_byte: e0, - style: MdStyle::LinkUrl, - }], - vec![MdSpan { - start_byte: s1, - end_byte: e1, - style: MdStyle::LinkUrl, - }], - ], - code_highlights: vec![vec![], vec![]], - }; let eh = EditorHoverPopupData { - rendered, + markdown: format!("{line0}\n{line1}"), + line_text: vec![line0.to_string(), line1.to_string()], + code_highlights: vec![vec![], vec![]], links: vec![ (0, s0, e0, "https://docs.rs/foo".to_string()), (1, s1, e1, "https://crates.io/crates/foo".to_string()), diff --git a/src/tui_main/panels.rs b/src/tui_main/panels.rs index 01fbf725..284942a4 100644 --- a/src/tui_main/panels.rs +++ b/src/tui_main/panels.rs @@ -801,7 +801,7 @@ pub(super) fn render_panel_hover_popup( return (vec![], None); }; - let lines = &ph.rendered.lines; + let lines = &ph.line_text; if lines.is_empty() { return (vec![], None); } @@ -928,7 +928,7 @@ pub(super) fn render_editor_hover_popup( Option<(u16, u16, u16, u16)>, Option, ) { - if eh.rendered.lines.is_empty() { + if eh.line_text.is_empty() { return (vec![], None, None); } let popup = render::editor_hover_to_quadraui_rich_text(eh, theme); diff --git a/src/tui_main/shell_app.rs b/src/tui_main/shell_app.rs index 28d28614..b872a000 100644 --- a/src/tui_main/shell_app.rs +++ b/src/tui_main/shell_app.rs @@ -11466,6 +11466,116 @@ mod tests { ); } + /// #821: hover popups adopt `quadraui::compose::markdown::render_markdown_to_styled` + /// instead of vimcode's hand-rolled `MdStyle`-to-color span walk. Pins the + /// acceptance-criteria markdown features (inline code, links — bold isn't + /// separately checkable on TUI, see below) on both the engine's resolved + /// link list and painted TUI output, plus the one real feature gap the + /// adoption introduced: quadraui only recognizes `[text](url)` links, not + /// bare `http://`/`https://` autolinks. `core::markdown::linkify_bare_urls` + /// closes that gap by rewriting the source markdown before quadraui ever + /// parses it. + /// + /// Bold isn't asserted on the *painted* side: quadraui's TUI + /// `draw_rich_text_popup` rasterises `StyledSpan.fg`/`.bg` only — `.bold`/ + /// `.italic`/`.underline` are never consulted (confirmed against the + /// pinned rev; the sole exception is an unconditional underline overlay + /// for the keyboard-*focused* link, unrelated to markdown styling). Bold + /// markdown was equally invisible on TUI before #821 (the same rasteriser + /// painted the old hand-rolled `RichTextPopup` too), so asserting it here + /// would fail for a pre-existing quadraui-TUI reason, not a #821 + /// regression. + /// + /// **RED against an unfixed tree:** comment out the + /// `crate::core::markdown::linkify_bare_urls` call in + /// `Engine::show_editor_hover` and the bare-URL assertions below fail — + /// the engine's `links` list comes back empty (quadraui's renderer never + /// turns unbracketed text into a link) and `driver.find` still finds the + /// URL text, but its color is indistinguishable from body text. + #[test] + fn driver_editor_hover_renders_code_and_bare_url_link_via_quadraui_markdown() { + let mut app = TuiShellApp::new(None); + app.engine.settings.use_nerd_fonts = false; + crate::icons::set_nerd_fonts(false); + app.engine.session.explorer_visible = false; + app.engine.buffer_mut().insert(0, "fn main() {}\n"); + app.engine.show_editor_hover( + 0, + 3, + "plain821 **bold821** and `code821` — see https://example.com/docs821", + crate::core::engine::EditorHoverSource::Lsp, + false, + false, + ); + + // Engine-level: the bare URL must have resolved to a real clickable + // link — checked directly against `EditorHoverPopup::links`, with no + // dependency on paint at all. + let links = app + .engine + .editor_hover + .as_ref() + .expect("show_editor_hover must have set editor_hover") + .links + .clone(); + assert!( + links + .iter() + .any(|(_, _, _, url)| url == "https://example.com/docs821"), + "the bare URL must be linkified and resolved into a clickable link; got {links:?}" + ); + + // Wider than the other hover tests in this module: the popup is a + // fixed-width, non-wrapping box (horizontal scroll instead), and this + // test's line is long enough that a plain 80-col terminal clips the + // trailing URL text before `find` can locate it. + let driver = driver_with_shell(app, TuiShellApp::shell_config(false), 160, 24); + + // Markdown syntax must be stripped — proves quadraui's parser ran + // rather than the raw source being painted verbatim. + assert!( + !driver.screen_contains("**bold821**"), + "bold markdown delimiters must not leak into painted text; screen:\n{}", + driver.screen() + ); + assert!( + !driver.screen_contains("`code821`"), + "inline-code backticks must not leak into painted text; screen:\n{}", + driver.screen() + ); + assert!( + driver.screen_contains("bold821"), + "the word inside **bold821** must still paint, syntax stripped; screen:\n{}", + driver.screen() + ); + + let (px, py) = driver.find("plain821").expect("plain body word must paint"); + let plain_style = driver + .style_at(px as u16, py as u16) + .expect("plain word cell must have a style"); + + let (cx, cy) = driver.find("code821").expect("inline code word must paint"); + let code_style = driver + .style_at(cx as u16, cy as u16) + .expect("code word cell must have a style"); + assert_ne!( + code_style.fg, plain_style.fg, + "inline `code821` must render in a color distinct from body text" + ); + + // Bare URL (no `[]()` brackets) must still paint, in link color. + let (ux, uy) = driver + .find("https://example.com/docs821") + .expect("linkify_bare_urls must have preserved the bare URL text so it still paints"); + let url_style = driver + .style_at(ux as u16, uy as u16) + .expect("URL cell must have a style"); + assert_ne!( + url_style.fg, plain_style.fg, + "a linkified bare URL must render in link color, not body color" + ); + } + // ── #757 slice 2: the shared focus-owner keyboard rung ───────────── // // `render::route_focus_key` now states the activity-bar → sidebar-panel From 3779061e0c81615533b1863d1bf6c90eca83564d Mon Sep 17 00:00:00 2001 From: JDonaghy Date: Sat, 5 Sep 2026 17:14:27 +0000 Subject: [PATCH 2/5] fix(#821): repair tests/ext_panel.rs after EditorHoverPopup restructure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #821 commit replaced EditorHoverPopup's `rendered: MarkdownRendered` field with the theme-independent `line_text: Vec` (plus `links` / `code_highlights`), but tests/ext_panel.rs still read `hover.rendered.lines` in three places, so `cargo test` failed to compile with E0609 and the whole suite — including the new hover driver tests — never ran. Point the three assertions at `line_text`, which carries exactly the same markdown-stripped plain per-line text the old `rendered.lines` did, so the tests keep asserting the same thing (hover content contains the annotation / plugin text). Test-only fix; no behaviour change. `cargo build`, `cargo test --no-run`, `cargo fmt -- --check`, `cargo clippy -- -D warnings` and `cargo clippy --no-default-features -- -D warnings` are all clean. Co-Authored-By: Claude Opus 5 --- tests/ext_panel.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/ext_panel.rs b/tests/ext_panel.rs index 9345303d..b85540ba 100644 --- a/tests/ext_panel.rs +++ b/tests/ext_panel.rs @@ -470,9 +470,9 @@ fn editor_hover_shows_annotation() { e.trigger_editor_hover_at_cursor(); assert!(e.editor_hover.is_some()); let hover = e.editor_hover.as_ref().unwrap(); - assert!(!hover.rendered.lines.is_empty()); + assert!(!hover.line_text.is_empty()); // Content should include the annotation - let text = hover.rendered.lines.join("\n"); + let text = hover.line_text.join("\n"); assert!(text.contains("blame: John, 2h ago")); } @@ -484,7 +484,7 @@ fn editor_hover_shows_plugin_content() { .insert(0, "**Commit abc123**\n\nfeat: add hover".to_string()); e.trigger_editor_hover_at_cursor(); assert!(e.editor_hover.is_some()); - let text = e.editor_hover.as_ref().unwrap().rendered.lines.join("\n"); + let text = e.editor_hover.as_ref().unwrap().line_text.join("\n"); assert!(text.contains("Commit abc123")); } From be3687153ad0d22d613c18c31f823c01f0d71b57 Mon Sep 17 00:00:00 2001 From: JDonaghy Date: Sat, 5 Sep 2026 17:43:24 +0000 Subject: [PATCH 3/5] fix(#821 test): divider drag must not leave a phantom double divider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #753 driver test `group_divider_drag_moves_the_painted_divider_via_shell_app` was failing on `develop` (verified by running it on unmodified `origin/develop` in this worktree — the failure predates #821 and is unrelated to the hover-popup markdown adoption). Root cause: `RenderedWindow::rect` is an f64 *widening* of the f32 rect `quadraui::SplitTree::layout` produced, but the sibling divider's `position` is that layout's `bounds.x + first_w` added in **f32**. Dragging a group divider to column 48 of a 46-wide editor area stores `ratio = 14/46`, which f32 resolves to a left pane `13.999999046…` cells wide. quadraui's f32 `34.0 + 13.999999046` rounds back up to exactly `48.0` (the divider paints at cell 48), while vimcode's f64 sum stays `47.999999046…` and truncates to 47 — putting the left pane's own separator at column 46 instead of 47. `group_divider_cells`' #481 "the left pane already separates these two groups" guard then stopped matching and *both* lines painted, with a blank column wedged between them. Fix: one `window_right_edge_cell` helper that sums the edge in f32 and truncates with quadraui's own `SplitTreeDivider::cell_position` convention, used by both `vertical_separator_cells` and `group_divider_cells`' scrollbar check. Tests: the existing driver-tier test goes green; added `separator_column_tracks_the_divider_after_a_fractional_drag`, a pure-data pin on the exact post-drag geometry. Both were observed RED with the helper reverted to the f64 sum and green with it restored. Also fixes a clippy `--all-targets` warning introduced earlier on this branch (`line_text.get(0)` → `.first()`). Co-Authored-By: Claude Opus 5 --- src/render.rs | 2 +- src/tui_main/render_impl.rs | 113 +++++++++++++++++++++++++++++++++++- 2 files changed, 112 insertions(+), 3 deletions(-) diff --git a/src/render.rs b/src/render.rs index 72ea5348..29c31db9 100644 --- a/src/render.rs +++ b/src/render.rs @@ -23974,7 +23974,7 @@ mod tests { assert_eq!(link.url, "https://example.com"); // line_text[0] must equal the raw line so index_to_pos byte indices are valid. assert_eq!( - popup.line_text.get(0).map(String::as_str), + popup.line_text.first().map(String::as_str), Some(line), "line_text must carry the raw text unchanged" ); diff --git a/src/tui_main/render_impl.rs b/src/tui_main/render_impl.rs index 258d5a2b..588bf054 100644 --- a/src/tui_main/render_impl.rs +++ b/src/tui_main/render_impl.rs @@ -939,6 +939,32 @@ fn window_overflows_vertically(w: &RenderedWindow) -> bool { w.total_lines > text_rows } +/// Terminal column one past a window's right edge — i.e. the column the +/// *next* pane starts in, and therefore the column its group divider paints +/// in. +/// +/// The sum is deliberately taken in **f32**, then truncated with quadraui's +/// own cell convention (`as u16`, per `SplitTreeDivider::cell_position`). +/// `RenderedWindow::rect` is an f64 *widening* of the f32 rect +/// `quadraui::SplitTree::layout` produced, and the sibling divider's +/// `position` is that same layout's `bounds.x + first_w` added in f32 — so +/// adding the two f64 fields back together here is not the same arithmetic +/// and can land a whole cell away. +/// +/// Concretely (the drag this fixes, #753's `group_divider_drag_moves_the_ +/// painted_divider_via_shell_app`): dragging a group divider to column 48 of +/// a 46-wide editor area stores `ratio = 14/46`, which f32 resolves to a +/// left pane of `13.999999046…` cells. quadraui's f32 `34.0 + 13.999999046` +/// rounds back up to exactly `48.0`, so the divider paints at cell 48; the +/// f64 sum stays `47.999999046…` and truncates to **47**, putting the left +/// pane's own separator at 46 instead of 47. `group_divider_cells`' "the +/// left pane already separates these two groups" guard then stops matching +/// and both lines paint — the #481 phantom double divider, with a blank +/// column wedged between them. +fn window_right_edge_cell(rect: &WindowRect) -> u16 { + (rect.x as f32 + rect.width as f32).max(0.0) as u16 +} + /// Absolute `(x, y)` terminal cells where [`render_separators`] paints a /// vertical `'│'` window-divider glyph — the same geometry its own /// painting loop below walks, factored out as a pure data computation (no @@ -961,7 +987,9 @@ fn vertical_separator_cells(windows: &[RenderedWindow]) -> std::collections::Has if (a.rect.x + a.rect.width - b.rect.x).abs() < 1.0 && v_overlap { // #550: `a.rect`/`b.rect` are already absolute terminal-screen // coordinates, so no `editor_area` offset addition needed. - let sep_x = (a.rect.x + a.rect.width) as u16; + // See [`window_right_edge_cell`] for why the boundary column + // is not `(a.rect.x + a.rect.width) as u16`. + let sep_x = window_right_edge_cell(&a.rect); let y_start = a.rect.y.max(b.rect.y) as u16; let y_end = (a.rect.y + a.rect.height).min(b.rect.y + b.rect.height) as u16; @@ -1213,7 +1241,9 @@ pub(super) fn group_divider_cells( if div_x > editor_area.x { let left_col = div_x - 1; let left_has_scrollbar = windows.iter().any(|w| { - let last_col = (w.rect.x + w.rect.width) as u16; + // Same f32-precision boundary the separator pass uses — + // see [`window_right_edge_cell`]. + let last_col = window_right_edge_cell(&w.rect); // Bound the row range the same way `window_overflows_vertically` // bounds `text_rows`: `draw_editor` only paints the scrollbar // into the window's *text* rows, never the last row when that @@ -2531,4 +2561,83 @@ mod tests { ); } } + + /// Regression pin for [`window_right_edge_cell`]: the exact geometry a + /// group-divider drag produces (the `#753` + /// `group_divider_drag_moves_the_painted_divider_via_shell_app` driver + /// test's, reduced to pure data). + /// + /// Dragging the divider of a 46-cell-wide editor area to column 48 + /// stores `ratio = 14/46`, and `quadraui::SplitTree::layout` resolves + /// that to a left pane exactly `13.999999046325684` cells wide — while + /// the divider `position` it returns from the *same* `first_w`, summed + /// in f32, is exactly `48.0`. Summing the pane's f64 `x + width` here + /// instead yields `47.999999046…`, truncating to 47, so the separator + /// landed at 46: one column left of the divider's own cell, the #481 + /// "already separated" guard stopped matching, and both the separator + /// and the group divider painted with a blank column wedged between. + #[test] + fn separator_column_tracks_the_divider_after_a_fractional_drag() { + // `13.999999046325684` is not a typo — it is `(14f32 / 46f32 * 46f32)` + // widened to f64, i.e. what quadraui actually hands back. + let left_width = (14.0f32 / 46.0f32 * 46.0f32) as f64; + assert!( + left_width < 14.0, + "fixture precondition: the f32 round-trip must land just *under* \ + a whole cell (got {left_width})" + ); + let left = fixture_window( + WindowId(0), + WindowRect::new(34.0, 2.0, left_width, 21.0), + 1, + None, + ); + let right = fixture_window(WindowId(1), WindowRect::new(48.0, 2.0, 32.0, 21.0), 1, None); + let windows = [left, right]; + + // The separator must sit in column 47 — the cell immediately left of + // the divider's own cell (48), exactly as it does before any drag. + let cells = vertical_separator_cells(&windows); + for y in 2..23u16 { + assert!( + cells.contains(&(47, y)), + "row {y}: the left pane's separator must track the divider's \ + cell (48) at column 47; got {cells:?}" + ); + } + + // ...and because it does, the group divider must not paint a *second* + // line at 48 across the panes' rows (#481). + let divider = GroupDivider { + split_index: 0, + direction: SplitDirection::Vertical, + position: 48.0, + axis_start: 34.0, + axis_size: 46.0, + cross_start: 0.0, + cross_size: 24.0, + }; + let editor_area = Rect { + x: 34, + y: 0, + width: 46, + height: 24, + }; + let div_cells = group_divider_cells(&[divider], &windows, editor_area); + for y in 2..23u16 { + assert!( + !div_cells.contains(&(48, y)), + "row {y}: the left pane already separates the two groups, so \ + the group divider must not paint a second line beside it; \ + got {div_cells:?}" + ); + } + // Above the panes (the tab-bar rows) nothing else separates them, so + // the divider itself is still what paints there. + assert!( + div_cells.contains(&(48, 1)), + "the divider must still paint on the tab-bar row, where no pane \ + separator covers it; got {div_cells:?}" + ); + } } From 72464f0665b7e210a15e39e6a6eb4be77d2dfa88 Mon Sep 17 00:00:00 2001 From: JDonaghy Date: Sat, 5 Sep 2026 17:46:31 +0000 Subject: [PATCH 4/5] fix(test-hygiene): SC tests must not commit the checkout they run in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `make_sc_engine_with_files()` built its engine with `Engine::new()`, whose `cwd` is the process working directory — i.e. the vimcode checkout. The `sc_*` actions the tests around it drive shell out to real `git -C ` commands, so `test_sc_unified_dispatch_stage_all` ran `git add -A` over the developer's working tree and `test_sc_commit_ctrl_enter_commits` then committed it, under the message "test\nmultiline". Observed for real: a plain `cargo test` during the #821 work silently swept every uncommitted change in this worktree into a junk commit. The tests' own comments ("will fail silently since we're not in a real git repo") were simply untrue when run from a checkout. Point the fixture's `cwd` at an empty, pid-suffixed, deliberately non-git scratch dir under the system temp dir, so those git calls fail the way the tests already assume, and add `sc_fixture_cwd_is_never_inside_a_git_work_tree` as a standing guard (it fails against the pre-fix fixture). Co-Authored-By: Claude Opus 5 --- src/core/engine/tests.rs | 52 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/core/engine/tests.rs b/src/core/engine/tests.rs index 8c89a6a9..11fd0886 100644 --- a/src/core/engine/tests.rs +++ b/src/core/engine/tests.rs @@ -14082,9 +14082,37 @@ fn test_plugin_disabled_not_registered() { // ─── Source Control (Session 99) ───────────────────────────────────────── +/// An existing, empty, **non-git** directory for [`make_sc_engine_with_files`] +/// to use as `Engine::cwd` — see that fixture's doc comment for what running +/// the SC tests against a real checkout did instead. +/// +/// Per-process (pid-suffixed) rather than a shared constant so two concurrent +/// `cargo test` runs (a second worktree, CI matrix job, …) cannot collide, and +/// deliberately *not* `git init`-ed: every `git -C` call made from it must +/// fail, which is exactly what the tests below assert. +fn sc_scratch_cwd() -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!("vimcode-sc-tests-{}", std::process::id())); + let _ = std::fs::create_dir_all(&dir); + dir +} + /// Build an engine with synthetic SC file statuses for testing. +/// +/// `cwd` is re-pointed at an empty scratch directory that is deliberately +/// **not** a git repository, because the `sc_*` actions the tests below drive +/// (`sc_do_commit`, `sc_stage_all`, `sc_unstage_all`, …) shell out to real +/// `git -C ` commands. Several of those tests document their +/// expectations as "will fail silently since we're not in a real git repo" — +/// which was only true when the suite ran somewhere unversioned. Run from a +/// checkout (i.e. always), `Engine::new()`'s `cwd` *is* the vimcode repo, so +/// `test_sc_unified_dispatch_stage_all` ran `git add -A` over the developer's +/// working tree and `test_sc_commit_ctrl_enter_commits` then committed it +/// under the message `"test\nmultiline"` — silently swallowing every +/// uncommitted change in the checkout the suite was run from. Observed for +/// real while fixing #821. fn make_sc_engine_with_files() -> Engine { let mut engine = Engine::new(); + engine.cwd = sc_scratch_cwd(); engine.sc_file_statuses = vec![ git::FileStatus { path: "a.rs".to_string(), @@ -14101,6 +14129,30 @@ fn make_sc_engine_with_files() -> Engine { engine } +/// Guard for the hazard documented on [`make_sc_engine_with_files`]: the SC +/// fixture's `cwd` must never be inside a git work tree, or the `sc_*` tests +/// below stage and commit the checkout the suite is being run from. +/// +/// Fails (as intended) against the pre-fix fixture, whose `cwd` was +/// `Engine::new()`'s — the repo root. +#[test] +fn sc_fixture_cwd_is_never_inside_a_git_work_tree() { + let engine = make_sc_engine_with_files(); + let out = std::process::Command::new("git") + .arg("-C") + .arg(&engine.cwd) + .args(["rev-parse", "--is-inside-work-tree"]) + .output() + .expect("git must be runnable to check the fixture's cwd"); + assert!( + !out.status.success(), + "the SC fixture's cwd ({}) is inside a git work tree — the sc_* tests \ + run real `git add -A` / `git commit` against it and would swallow \ + every uncommitted change in that checkout", + engine.cwd.display() + ); +} + #[test] fn test_sc_commit_input_mode_toggle() { let mut engine = make_sc_engine_with_files(); From 3f2b645628bd73b8d34f5540f32962343a1a9cb3 Mon Sep 17 00:00:00 2001 From: JDonaghy Date: Sat, 5 Sep 2026 18:36:21 +0000 Subject: [PATCH 5/5] fix(#821 review): skip inline code spans in bare-URL linkifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit linkify_bare_urls_in_line tracked the fenced-code-block toggle but not single-backtick inline code spans. A hover line like `See \`http://x\` for docs.` got rewritten to wrap the URL in markdown link syntax even though it sits inside an inline code span — and quadraui's parse_inline treats backtick-delimited text as verbatim, so the rendered code span showed the literal `[http://x](http://x)` text instead of the URL, un-clickable. Track a backtick toggle alongside the fenced-block toggle and skip scanning while inside one. Adds regression tests for the inline-code case, a URL immediately after a closed code span, and end-to-end hover_markdown_structure coverage confirming no link is produced. Also documents two further quadraui-vs-local-renderer parity gaps the issue asked to have called out (no image support, single-level list support only) that were verified but not yet written down. --- src/core/markdown.rs | 59 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/src/core/markdown.rs b/src/core/markdown.rs index 770febe1..0ae97ba2 100644 --- a/src/core/markdown.rs +++ b/src/core/markdown.rs @@ -614,14 +614,29 @@ pub fn render_markdown(input: &str) -> MdRendered { // (`Theme::md_heading1/2/3`) are *not* preserved — quadraui renders every // heading level as bold + a larger `line_scales` factor, with no per-level // color; this is an accepted, documented divergence, not a bug. +// +// Two further parity gaps vs. the local `render_markdown`, both documented as +// intentional deferrals in quadraui's own `compose/markdown.rs` module doc +// (pinned rev `4ff2a645`) rather than bugs to work around here: quadraui's +// renderer has **no image support** (`![alt](src)` degrades silently — no +// alt-text fallback, unlike the local pipeline's `image_alt_text` handling), +// and **only single-level list support** (nested `- a\n - b` markdown does +// not indent the nested item). Low real-world impact for LSP hover/doc- +// comment text, which rarely nests lists or embeds images, but noted here so +// a future caller doesn't assume full parity. /// Wrap bare `http://` / `https://` URLs in `[url](url)` markdown link /// syntax so quadraui's `render_markdown_to_styled` (which only recognizes /// bracketed links) still makes them clickable. A URL immediately preceded /// by `(` is assumed to already be a link destination (`[text](url)`) and is /// left alone. Fenced code blocks are skipped entirely (their contents -/// should never be rewritten). Adapted from `scan_bare_urls` above, which -/// performs the equivalent scan for the local pulldown-cmark pipeline. +/// should never be rewritten), and so are inline single-backtick code spans +/// (`` `like this` ``) — quadraui's `parse_inline` treats backtick-delimited +/// text as verbatim and never re-parses markdown syntax inside it, so +/// rewriting a URL there would inject literal `[url](url)` text into the +/// rendered code span instead of producing a link. Adapted from +/// `scan_bare_urls` above, which performs the equivalent scan for the local +/// pulldown-cmark pipeline. pub fn linkify_bare_urls(input: &str) -> String { let mut out = String::with_capacity(input.len()); let mut in_code_block = false; @@ -647,7 +662,20 @@ fn linkify_bare_urls_in_line(line: &str, out: &mut String) { let bytes = line.as_bytes(); let len = bytes.len(); let mut i = 0usize; + let mut in_code_span = false; while i < len { + if bytes[i] == b'`' { + in_code_span = !in_code_span; + out.push('`'); + i += 1; + continue; + } + if in_code_span { + let ch = line[i..].chars().next().unwrap(); + out.push(ch); + i += ch.len_utf8(); + continue; + } let scheme_len = if bytes[i..].starts_with(b"https://") { Some(8usize) } else if bytes[i..].starts_with(b"http://") { @@ -1026,6 +1054,33 @@ mod tests { ); } + #[test] + fn linkify_bare_urls_skips_inline_code_spans() { + let out = linkify_bare_urls("See `https://example.com` for docs."); + assert_eq!(out, "See `https://example.com` for docs."); + } + + #[test] + fn linkify_bare_urls_still_wraps_url_after_inline_code_span_closes() { + let out = linkify_bare_urls("`code` then https://example.com after"); + assert_eq!( + out, + "`code` then [https://example.com](https://example.com) after" + ); + } + + #[test] + fn hover_markdown_structure_leaves_url_in_inline_code_as_plain_text() { + let markdown = linkify_bare_urls("See `https://example.com` for docs."); + let (line_text, links, _) = hover_markdown_structure(&markdown); + assert_eq!(line_text.len(), 1); + assert_eq!(line_text[0], "See https://example.com for docs."); + assert!( + links.is_empty(), + "URL inside inline code span must not become a clickable link: {links:?}" + ); + } + #[test] fn hover_markdown_structure_extracts_bold_code_and_link() { let (line_text, links, _) =