From ef2036ee86c130b56b9958a36c20675c6dc2c300 Mon Sep 17 00:00:00 2001 From: Amin Borghei Date: Thu, 1 Oct 2026 14:43:59 +0200 Subject: [PATCH 01/10] fix(links): match #anchors against the heading text GitHub slugs Anchors were slugged from the TOC display text, which drops inline code and carries smart-punctuation dashes, so [x](#install-ink-cli) or a heading made only of code could not be reached. Layout now records each heading's anchor text (inline code, :shortcode:, -- as written) next to the display text, and #fragment links match against that. --- src/app.rs | 105 +++++++++++++++++++++++++++++++++++++++++++-- src/layout/html.rs | 2 +- src/layout/mod.rs | 87 ++++++++++++++++++++++++++++++++++++- 3 files changed, 188 insertions(+), 6 deletions(-) diff --git a/src/app.rs b/src/app.rs index 84d8d62..3617129 100644 --- a/src/app.rs +++ b/src/app.rs @@ -39,6 +39,10 @@ struct Tab { lowered: Vec, scroll_offset: usize, toc: TocState, + /// Every top-level heading's anchor text (`LayoutHeading::anchor`) and + /// display line, in order, for `#fragment` links. Unlike the TOC this + /// keeps headings whose display text is empty (``## `--width` ``). + anchors: Vec<(String, usize)>, word_count: usize, reading_time: usize, /// Terminal width + theme generation this tab was laid out for. Used to @@ -1637,10 +1641,10 @@ fn find_heading<'a>(texts: impl IntoIterator, fragment: &str) -> /// Scroll offset that puts the heading named by `fragment` at the top of the /// viewport, framed the way `n`/`N` heading jumps frame it. fn heading_scroll(tab: &Tab, fragment: &str, viewport: usize) -> Option { - let headings = &tab.toc.headings; - let i = find_heading(headings.iter().map(|h| h.text.as_str()), fragment)?; + let anchors = &tab.anchors; + let i = find_heading(anchors.iter().map(|(text, _)| text.as_str()), fragment)?; let max_scroll = tab.ratatui_lines.len().saturating_sub(viewport); - Some(headings[i].line_index.saturating_sub(1).min(max_scroll)) + Some(anchors[i].1.saturating_sub(1).min(max_scroll)) } /// Show a history entry: the same document just scrolls back; anything else @@ -1749,8 +1753,9 @@ fn build_tab( headings, images: image_specs, code_blocks, - } = layout::layout_document( + } = layout::layout_document_with_source( root, + Some(&content), &theme::resolve_theme(&args.theme), max_content_width, args.spacing, @@ -1793,6 +1798,11 @@ fn build_tab( let (word_count, reading_time) = stats::document_stats(&content); + let anchors: Vec<(String, usize)> = headings + .iter() + .map(|h| (h.anchor.clone(), h.line_index)) + .collect(); + // Headings carry their exact display-line index straight from layout — // no substring reverse-scan, and duplicate/split headings map correctly. let toc_entries: Vec = headings @@ -1823,6 +1833,7 @@ fn build_tab( lowered, scroll_offset: 0, toc, + anchors, word_count, reading_time, built_width: term_width, @@ -2019,6 +2030,10 @@ mod section_tests { ) .collect(); toc.update_selection(scroll); + let anchors = headings + .iter() + .map(|(_, text, line_index, _)| ((*text).to_string(), *line_index)) + .collect(); Tab { filename: "t.md".into(), source: content.into(), @@ -2030,6 +2045,7 @@ mod section_tests { lowered: Vec::new(), scroll_offset: scroll, toc, + anchors, word_count: 0, reading_time: 0, built_width: 80, @@ -2166,6 +2182,87 @@ mod section_tests { assert!(!is_followable_local("image.png#frag")); } + /// Arguments for laying a document out the way the reader does. + pub(super) fn test_args() -> Args { + Args { + inputs: Vec::new(), + theme: "dark".into(), + width: None, + slides: false, + plain: false, + watch: false, + toc: false, + images: crate::image::ImageMode::Off, + image_protocol: crate::graphics::ProtocolChoice::HalfBlocks, + frontmatter: false, + spacing: crate::Spacing::Normal, + mouse_capture: false, + clipboard: crate::clipboard::ClipboardMode::Auto, + } + } + + /// A tab built through the real pipeline: wikilinks, parse, layout. + pub(super) fn built(src: &str) -> Tab { + let graphics = crate::graphics::Graphics::halfblocks(); + build_tab(src.into(), "t.md", &test_args(), 80, 0, &graphics) + } + + /// The display line `#fragment` scrolls to, or `None`. + fn anchor_line(t: &Tab, fragment: &str) -> Option { + let i = find_heading(t.anchors.iter().map(|(a, _)| a.as_str()), fragment)?; + Some(t.anchors[i].1) + } + + #[test] + fn anchors_see_inline_code_dashes_shortcodes_and_links() { + let src = "# Install `ink` CLI\n\n### `--width`\n\n## a -- b\n\n\ + ## :rocket: Launch\n\n## See [the docs](https://e.com) now\n\n\ + ## Dash — literal\n\n## Usage\n\n## Usage\n\n## Usage\n"; + let t = built(src); + let lines: Vec = t.anchors.iter().map(|(_, l)| *l).collect(); + assert_eq!(lines.len(), 9, "{:?}", t.anchors); + for (fragment, i) in [ + ("install-ink-cli", 0), + ("--width", 1), + ("a----b", 2), + ("rocket-launch", 3), + ("see-the-docs-now", 4), + ("dash--literal", 5), + ("usage", 6), + ("usage-1", 7), + ("usage-2", 8), + ] { + assert_eq!(anchor_line(&t, fragment), Some(lines[i]), "#{fragment}"); + } + // The old, display-text-derived slugs no longer match. + assert_eq!(anchor_line(&t, "install--cli"), None); + // The TOC still shows what it showed: no code, no empty entries. + let toc: Vec<&str> = t.toc.headings.iter().map(|h| h.text.as_str()).collect(); + assert_eq!(toc[0], "Install CLI"); + assert!(!toc.contains(&"")); + assert!(toc.contains(&"a – b"), "{toc:?}"); + } + + #[test] + fn anchor_text_undoes_smart_dashes_without_the_source() { + use comrak::{parse_document, Arena}; + let arena = Arena::new(); + let root = parse_document(&arena, "## a -- b --- c\n", &crate::parser::options()); + let theme = crate::theme::resolve_theme("dark"); + let headings = crate::layout::layout_document( + root, + &theme, + 80, + crate::Spacing::Normal, + 0, + None, + crate::image::ImageMode::Off, + None, + ) + .headings; + assert_eq!(headings[0].anchor, "a -- b --- c"); + } + #[test] fn anchor_scroll_lands_like_heading_jumps() { let mut t = tab(DOC, HEADINGS, 0); diff --git a/src/layout/html.rs b/src/layout/html.rs index 302260d..c4296c3 100644 --- a/src/layout/html.rs +++ b/src/layout/html.rs @@ -553,7 +553,7 @@ impl HtmlBlock<'_, '_> { match heading { Some((level, source_line)) => { let text: String = spans.iter().map(|s| s.text.replace('\n', " ")).collect(); - layout_heading_spans(level, spans, &text, source_line, self.ctx, lines); + layout_heading_spans(level, spans, &text, &text, source_line, self.ctx, lines); } None => { let ctx = self.ctx; diff --git a/src/layout/mod.rs b/src/layout/mod.rs index 8d951e9..7a36a8b 100644 --- a/src/layout/mod.rs +++ b/src/layout/mod.rs @@ -96,6 +96,10 @@ fn coalesce_line(line: &mut StyledLine) { pub struct LayoutHeading { pub level: u8, pub text: String, + /// The heading's text as GitHub sees it when it builds the anchor: inline + /// code included, `--` not turned into a dash, `:rocket:` kept as written. + /// `text` is what the TOC shows; this is what `#fragment` links match. + pub anchor: String, pub line_index: usize, /// 1-based line in the (frontmatter-stripped) markdown source this heading /// was written on. Lets `Y` copy a section's *source* rather than its @@ -152,6 +156,35 @@ pub fn layout_document<'a>( base_dir: Option<&std::path::Path>, images: ImageMode, graphics_font: Option<(u16, u16)>, +) -> LayoutResult { + layout_document_with_source( + root, + None, + theme, + width, + spacing, + center_margin, + base_dir, + images, + graphics_font, + ) +} + +/// `layout_document`, given the markdown `root` was parsed from. Heading +/// anchors then see the text exactly as written where smart punctuation +/// rewrote it (`--` stays `--`, a literal `–` stays `–`); without the source +/// they fall back to mapping dashes back to hyphens. +#[allow(clippy::too_many_arguments)] +pub fn layout_document_with_source<'a>( + root: &'a AstNode<'a>, + source: Option<&str>, + theme: &Theme, + width: u16, + spacing: Spacing, + center_margin: usize, + base_dir: Option<&std::path::Path>, + images: ImageMode, + graphics_font: Option<(u16, u16)>, ) -> LayoutResult { let mut lines: Vec = Vec::new(); let headings = std::cell::RefCell::new(Vec::new()); @@ -174,6 +207,7 @@ pub fn layout_document<'a>( image_specs: &image_specs, code_blocks: &code_blocks, record_headings: true, + source, }; layout_node(root, &ctx, &mut lines); sanitize_lines(&mut lines); @@ -226,6 +260,8 @@ struct LayoutContext<'a> { // record correct absolute line indices. Nested walks (blockquotes, list // items) build into their own buffers, so they don't record headings. record_headings: bool, + /// The parsed markdown, when the caller has it; see `heading_anchor_text`. + source: Option<&'a str>, } impl<'a> LayoutContext<'a> { @@ -380,19 +416,65 @@ fn layout_heading<'a>( level, spans, &collect_child_text(node), + &heading_anchor_text(node, ctx.source), source_line, ctx, lines, ); } +/// A heading's text the way GitHub reads it to build the anchor: markup +/// removed, but inline code kept, emoji shortcodes kept as `:name:`, and +/// smart punctuation undone (`--` was turned into `–`). Only dashes need +/// undoing: curly quotes and ellipses are punctuation the slug drops anyway. +fn heading_anchor_text<'a>(node: &'a AstNode<'a>, source: Option<&str>) -> String { + let mut text = String::new(); + for inner in node.descendants().skip(1) { + let data = inner.data.borrow(); + match data.value { + NodeValue::Text(ref t) => { + if !t.contains(['\u{2013}', '\u{2014}']) { + text.push_str(t); + } else if let Some(raw) = source_slice(source, data.sourcepos) { + text.push_str(raw); + } else { + text.push_str(&t.replace('\u{2014}', "---").replace('\u{2013}', "--")); + } + } + NodeValue::Code(ref c) => text.push_str(&c.literal), + NodeValue::ShortCode(ref s) => { + text.push(':'); + text.push_str(&s.code); + text.push(':'); + } + _ => {} + } + } + text +} + +/// The source a single-line inline node was parsed from (comrak columns are +/// 1-based, inclusive, in bytes). `None` when it spans lines, falls outside +/// the source, or holds an entity reference (`&` would slug as "amp"). +fn source_slice(source: Option<&str>, pos: comrak::nodes::Sourcepos) -> Option<&str> { + if pos.start.line != pos.end.line || pos.start.column == 0 { + return None; + } + let line = source?.lines().nth(pos.start.line.checked_sub(1)?)?; + let raw = line.get(pos.start.column - 1..pos.end.column)?; + (!raw.contains('&')).then_some(raw) +} + /// Lay out a heading from its inline spans; `text` is its plain text for the -/// TOC and `source_line` the 1-based source line it was written on. Shared by +/// TOC, `anchor` its text for `#fragment` matching (see `LayoutHeading`), and +/// `source_line` the 1-based source line it was written on. Shared by /// markdown headings and HTML `

`..`

`. +#[allow(clippy::too_many_arguments)] fn layout_heading_spans( level: u8, mut spans: Vec, text: &str, + anchor: &str, source_line: usize, ctx: &LayoutContext, lines: &mut Vec, @@ -428,6 +510,7 @@ fn layout_heading_spans( // screen through the TOC sidebar and the copy status line, neither // of which is a `StyledLine`. text: crate::sanitize::sanitize_text(text).into_owned(), + anchor: anchor.to_string(), line_index: lines.len(), source_line, }); @@ -1115,6 +1198,7 @@ fn layout_blockquote<'a>( code_blocks: ctx.code_blocks, graphics_font: ctx.graphics_font, record_headings: false, + source: ctx.source, }; let mut inner_lines = Vec::new(); let mut skip_first = admonition.is_some(); @@ -1229,6 +1313,7 @@ fn layout_list<'a>( code_blocks: ctx.code_blocks, graphics_font: ctx.graphics_font, record_headings: false, + source: ctx.source, }; for (i, item) in node.children().enumerate() { From 2a1286e51bc9b873e9ccc5db5c49f1d805740ae3 Mon Sep 17 00:00:00 2001 From: Amin Borghei Date: Thu, 1 Oct 2026 14:48:35 +0200 Subject: [PATCH 02/10] fix(theme): keep a late OSC 11 reply out of the shell and the reader The background query gave up after 100 ms (400 ms over SSH), so on slow links not flagged as SSH the reply arrived later and was echoed at the prompt or read as keys. The wait is now 1 s for every terminal (the DA1 answer still ends it at once on a normal terminal), a partial reply at the deadline gets one 500 ms extension, and the reader drops an OSC 11 reply already queued as input when the query got nothing at all. The query is also no longer sent when its answer is unused: --list-themes, non-rendering subcommands, and colour turned off. --- src/app.rs | 5 ++ src/cli.rs | 17 +++++- src/input/mod.rs | 124 ++++++++++++++++++++++++++++++++++++++++++++ src/theme/detect.rs | 108 ++++++++++++++++++++++++++++++-------- tests/pty.rs | 39 ++++++++++++++ 5 files changed, 270 insertions(+), 23 deletions(-) diff --git a/src/app.rs b/src/app.rs index 3617129..ccdc203 100644 --- a/src/app.rs +++ b/src/app.rs @@ -299,6 +299,11 @@ pub fn run(source: String, args: Args) -> Result { install_panic_hook(); enable_raw_mode()?; + // A background-colour reply that missed the startup query must not be + // read as key presses. + if theme::detect::reply_may_arrive_late() { + input::discard_late_reply(); + } let mut stdout = io::stdout(); execute!(stdout, EnterAlternateScreen)?; if mouse_capture { diff --git a/src/cli.rs b/src/cli.rs index 9fb1257..792ba18 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -247,8 +247,7 @@ pub fn run() -> Result<()> { // `auto` asks the terminal for its background colour (OSC 11) once, now, // before anything owns the screen. Only with a terminal on both ends: // the reply arrives on stdin, and a pipe would never answer. - let wants_terminal = matches!(cli.command, None | Some(Commands::Doctor { .. })); - if theme == "auto" && wants_terminal { + if theme == "auto" && uses_background(&cli, color) { theme::detect::init_background(stdout_tty && std::io::stdin().is_terminal()); } @@ -564,6 +563,20 @@ pub enum ThemeOrigin { Auto, } +/// Will this run draw with the `auto` theme's colours? Only then is the +/// terminal worth asking for its background: `ink doctor` reports it, the +/// reader and `--plain` draw with it when colour is on. `--list-themes` and +/// the other subcommands never look at it. +fn uses_background(cli: &Cli, plain_color: bool) -> bool { + match cli.command { + Some(Commands::Doctor { .. }) => true, + Some(_) => false, + None if cli.list_themes => false, + None if cli.plain => plain_color, + None => theme::caps::color_enabled(cli.color, true), + } +} + /// The theme to use: an explicit `--theme` beats the config file, which beats /// `auto`. (`--theme auto` is the clap default, so it means "not given".) fn theme_choice(flag: &str, config: &Option) -> (String, ThemeOrigin) { diff --git a/src/input/mod.rs b/src/input/mod.rs index 28c8bc0..b766439 100644 --- a/src/input/mod.rs +++ b/src/input/mod.rs @@ -272,7 +272,87 @@ pub fn is_actionable_key(key: &KeyEvent) -> bool { key.kind != KeyEventKind::Release } +/// Events read ahead of the event loop by [`discard_late_reply`] that were +/// real input, served before anything new is read. +static PENDING: std::sync::Mutex> = + std::sync::Mutex::new(std::collections::VecDeque::new()); + +/// Drop a background-colour reply that reached the input queue after the +/// startup query gave up on it (see `theme::detect::reply_may_arrive_late`). +/// Reads only what is already buffered; anything that is not part of the +/// reply is kept and handed to the event loop in order. crossterm already +/// swallows the DA1 answer (`ESC [ ? … c`) itself; the OSC 11 answer it +/// would turn into key presses (`Alt-]`, `1`, `1`, `;`, …). +pub fn discard_late_reply() { + let mut events = Vec::new(); + while event::poll(std::time::Duration::ZERO).unwrap_or(false) { + match event::read() { + Ok(e) => events.push(e), + Err(_) => break, + } + } + let kept = strip_leading_osc_reply(events); + if let Ok(mut pending) = PENDING.lock() { + pending.extend(kept); + } +} + +/// Remove a leading OSC 11 reply (`ESC ] 11 ; … BEL` or `… ESC \`), as +/// crossterm decodes it into key events, from `events`. A reply cut off +/// before its terminator is removed too. Anything else is left alone. +fn strip_leading_osc_reply(events: Vec) -> Vec { + // Windows reports key releases too; they carry no information here. + let presses: Vec<(usize, KeyCode, KeyModifiers)> = events + .iter() + .enumerate() + .filter_map(|(i, e)| match e { + Event::Key(k) if k.kind != KeyEventKind::Release => Some((i, k.code, k.modifiers)), + _ => None, + }) + .collect(); + let starts_reply = matches!(presses.first(), Some((_, KeyCode::Char(']'), m)) if m.contains(KeyModifiers::ALT)); + if !starts_reply { + return events; + } + // `11;` must follow, as far as the input goes. + let prefix: Vec = presses[1..] + .iter() + .take(3) + .map(|(_, c, _)| match c { + KeyCode::Char(c) => *c, + _ => '\0', + }) + .collect(); + if !"11;".chars().zip(&prefix).all(|(a, b)| a == *b) { + return events; + } + let end = presses.iter().skip(1).find_map(|(i, c, m)| match c { + KeyCode::Char('g') if m.contains(KeyModifiers::CONTROL) => Some(*i), + KeyCode::Char('\\') if m.contains(KeyModifiers::ALT) => Some(*i), + _ => None, + }); + match end { + Some(i) => events.into_iter().skip(i + 1).collect(), + // Cut off: only the reply's own characters may follow. + None if presses[1..].iter().all(|(_, c, m)| { + matches!(c, KeyCode::Char(c) if c.is_ascii_alphanumeric() || ":/;#?".contains(*c)) + && !m.intersects(KeyModifiers::CONTROL | KeyModifiers::ALT) + }) => + { + events + .into_iter() + .filter(|e| !matches!(e, Event::Key(_))) + .collect() + } + None => events, + } +} + pub fn poll_action(timeout: std::time::Duration, mode: InputMode) -> Option { + let queued = PENDING.lock().ok().and_then(|mut p| p.pop_front()); + if let Some(event) = queued { + return Some(map_event(event, mode)); + } if event::poll(timeout).ok()? { let event = event::read().ok()?; if let Event::Key(key) = &event { @@ -480,4 +560,48 @@ mod tests { assert_eq!(map_event(ev, mode), Action::None); } } + + /// Bytes as crossterm decodes them into key events (`ESC x` → Alt-x, + /// BEL → Ctrl-g). + fn typed(bytes: &[u8]) -> Vec { + let mut out = Vec::new(); + let mut alt = false; + for &b in bytes { + let (code, mut mods) = match b { + 0x1b => { + alt = true; + continue; + } + 0x07 => (KeyCode::Char('g'), KeyModifiers::CONTROL), + b if b.is_ascii_uppercase() => (KeyCode::Char(b as char), KeyModifiers::SHIFT), + b => (KeyCode::Char(b as char), KeyModifiers::NONE), + }; + if std::mem::take(&mut alt) { + mods |= KeyModifiers::ALT; + } + out.push(Event::Key(KeyEvent::new(code, mods))); + } + out + } + + #[test] + fn a_late_osc_11_reply_is_dropped_and_real_keys_are_kept() { + // BEL-terminated, followed by a real `j`. + let kept = strip_leading_osc_reply(typed(b"\x1b]11;rgb:ffff/FFFF/ffff\x07j")); + assert_eq!(kept, typed(b"j")); + // ST-terminated. + let kept = strip_leading_osc_reply(typed(b"\x1b]11;#1e1e2e\x1b\\q")); + assert_eq!(kept, typed(b"q")); + // Cut off before the terminator: all of it is reply. + assert!(strip_leading_osc_reply(typed(b"\x1b]11;rgb:ff")).is_empty()); + assert!(strip_leading_osc_reply(typed(b"\x1b]1")).is_empty()); + // Ordinary input is left alone. + assert_eq!(strip_leading_osc_reply(typed(b"jjq")), typed(b"jjq")); + assert_eq!(strip_leading_osc_reply(typed(b"\x1b]x")), typed(b"\x1b]x")); + assert_eq!( + strip_leading_osc_reply(typed(b"q\x1b]11;")), + typed(b"q\x1b]11;") + ); + assert!(strip_leading_osc_reply(Vec::new()).is_empty()); + } } diff --git a/src/theme/detect.rs b/src/theme/detect.rs index 7e2b39c..331acb9 100644 --- a/src/theme/detect.rs +++ b/src/theme/detect.rs @@ -56,6 +56,15 @@ pub fn is_dark_background() -> bool { background().dark } +/// True when the OSC 11 query went out and nothing at all came back before +/// the timeout: a terminal on a very slow link may still deliver the reply +/// later, as input. The reader then discards it (`input::discard_late_reply`). +pub fn reply_may_arrive_late() -> bool { + BACKGROUND.get().is_some_and(|b| b.query == NO_REPLY) +} + +const NO_REPLY: &str = "no reply"; + fn detect(allow_query: bool) -> Background { let query = if allow_query { match query_skip_reason() { @@ -127,16 +136,33 @@ fn skip_reason_for(term: &str, wt_session: bool, windows: bool) -> Option<&'stat None } -/// How long to wait for a terminal that answers neither query. Responsive -/// terminals end the wait early through the DA1 reply, so this only costs -/// time on the rare terminal that answers nothing — but across SSH the round -/// trip itself can take longer. -fn query_timeout() -> Duration { - if crate::platform::is_ssh() { - Duration::from_millis(400) - } else { - Duration::from_millis(100) +/// How long to wait for the replies. Every real terminal answers the DA1 +/// request that follows the query, and the read loop stops the moment that +/// answer is in, so on a normal terminal this costs milliseconds; the full +/// second is only spent on a terminal that answers nothing. It is long so +/// that slow links nobody flags as remote (`docker exec`, `kubectl exec`, +/// mosh, serial consoles) still get their reply read here rather than +/// leaking into the shell or the reader afterwards. +const QUERY_TIMEOUT: Duration = Duration::from_secs(1); + +/// Extra time granted, once, when the deadline passes in the middle of the +/// replies (something arrived, the DA1 answer has not): the rest is in +/// flight, and leaving it unread would leak it. +const QUERY_GRACE: Duration = Duration::from_millis(500); + +/// Should the read loop stop now? `buf` is everything read so far. +fn query_done(buf: &[u8], past_deadline: bool, extended: &mut bool) -> bool { + if contains_da1_reply(buf) { + return true; + } + if !past_deadline { + return false; } + if buf.is_empty() || *extended { + return true; + } + *extended = true; + false } /// The background query, followed by a primary device attributes request @@ -192,13 +218,18 @@ fn query_background() -> Result { return Err("skipped (cannot write to terminal)".into()); } - let deadline = Instant::now() + query_timeout(); + let mut deadline = Instant::now() + QUERY_TIMEOUT; + let mut extended = false; let mut buf: Vec = Vec::new(); let mut chunk = [0u8; 256]; loop { - let left = deadline.saturating_duration_since(Instant::now()); + let mut left = deadline.saturating_duration_since(Instant::now()); if left.is_zero() { - break; + if query_done(&buf, true, &mut extended) { + break; + } + deadline = Instant::now() + QUERY_GRACE; + left = QUERY_GRACE; } let mut pfd = libc::pollfd { fd, @@ -214,7 +245,7 @@ fn query_background() -> Result { break; } if n == 0 { - break; + continue; // timed out: the deadline check above decides } // SAFETY: reading into a stack buffer of the stated length. let got = unsafe { libc::read(fd, chunk.as_mut_ptr().cast(), chunk.len()) }; @@ -222,12 +253,12 @@ fn query_background() -> Result { break; } buf.extend_from_slice(&chunk[..got as usize]); - if contains_da1_reply(&buf) { + if query_done(&buf, false, &mut extended) { break; } } if buf.is_empty() { - return Err("no reply".into()); + return Err(NO_REPLY.into()); } Ok(String::from_utf8_lossy(&buf).into_owned()) } @@ -259,12 +290,20 @@ fn query_background() -> Result { { return Err("skipped (cannot write to terminal)".into()); } - let deadline = Instant::now() + query_timeout(); + let mut deadline = Instant::now() + QUERY_TIMEOUT; + let mut extended = false; let mut text = String::new(); loop { - let left = deadline.saturating_duration_since(Instant::now()); - if left.is_zero() || !event::poll(left).unwrap_or(false) { - break; + let mut left = deadline.saturating_duration_since(Instant::now()); + if left.is_zero() { + if query_done(text.as_bytes(), true, &mut extended) { + break; + } + deadline = Instant::now() + QUERY_GRACE; + left = QUERY_GRACE; + } + if !event::poll(left).unwrap_or(false) { + continue; // timed out: the deadline check above decides } let Ok(Event::Key(k)) = event::read() else { continue; @@ -278,12 +317,12 @@ fn query_background() -> Result { KeyCode::Char(c) => text.push(c), _ => {} } - if contains_da1_reply(text.as_bytes()) { + if query_done(text.as_bytes(), false, &mut extended) { break; } } if text.is_empty() { - return Err("no reply".into()); + return Err(NO_REPLY.into()); } Ok(text) } @@ -466,6 +505,33 @@ mod tests { assert!(!contains_da1_reply(b"\x1b]11;rgb:0/0/0\x07")); } + #[test] + fn the_read_loop_stops_on_da1_and_extends_once_for_a_partial_reply() { + let mut ext = false; + // DA1 in: done at once, deadline or not. + assert!(query_done( + b"\x1b]11;rgb:0/0/0\x07\x1b[?62c", + false, + &mut ext + )); + // Before the deadline, a partial reply keeps reading. + assert!(!query_done(b"\x1b]11;rgb:00", false, &mut ext)); + // Nothing at all by the deadline: give up. + assert!(query_done(b"", true, &mut ext)); + assert!(!ext); + // Partial at the deadline: one extension… + assert!(!query_done(b"\x1b]11;rgb:00", true, &mut ext)); + assert!(ext); + // …and the partial OSC 11 is not mistaken for an answer. + assert_eq!(parse_osc11_reply("\x1b]11;rgb:00"), None); + // …then stop even if still incomplete. + assert!(query_done( + b"\x1b]11;rgb:0000/0000/0000\x07\x1b[?6", + true, + &mut ext + )); + } + #[test] fn colorfgbg_light_and_dark_indices() { assert_eq!(colorfgbg_is_dark("15;0"), Some(true)); diff --git a/tests/pty.rs b/tests/pty.rs index c034e84..6d34749 100644 --- a/tests/pty.rs +++ b/tests/pty.rs @@ -193,6 +193,45 @@ fn the_reader_uses_the_light_theme_and_leaves_no_reply_on_screen() { assert!(contains(&s.output, b"\x1b[?1049l"), "{tail:?}"); } +#[test] +fn a_reply_that_arrives_after_300_ms_is_still_consumed() { + // A slow link nobody flags as SSH (docker exec, mosh, serial): the + // answer comes 300 ms after the query. The query must still read it — + // the light theme proves it was parsed — so it is never drawn or typed. + let fixture = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/test.md"); + let mut asked_at: Option = None; + let mut answered = false; + let mut quit = quit_when_up(); + let s = run_in_pty( + &["--image-protocol", "halfblocks", fixture], + &[("COLORTERM", "truecolor")], + |out, t| { + if asked_at.is_none() && contains(out, b"\x1b]11;?") { + asked_at = Some(t); + } + match asked_at { + Some(at) if !answered && t >= at + Duration::from_millis(300) => { + answered = true; + Some(b"\x1b]11;rgb:ffff/ffff/ffff\x07\x1b[?62;22c".to_vec()) + } + _ => quit(out, t), + } + }, + ); + assert!(answered, "ink never asked"); + assert!(contains(&s.output, b"48;2;255;255;255"), "reply not used"); + assert!(!contains(&s.output, b"rgb:ffff"), "reply echoed"); + assert!(contains(&s.output, b"\x1b[?1049l"), "reader did not exit"); +} + +#[test] +fn list_themes_never_queries_the_terminal() { + let s = run_in_pty(&["--list-themes"], &[], |_, _| None); + assert!(String::from_utf8_lossy(&s.output).contains("dracula")); + assert!(!contains(&s.output, b"\x1b]11;?"), "OSC 11 query written"); + assert!(!contains(&s.output, b"\x1b[c"), "DA1 query written"); +} + /// Quits the reader (`q`) once it has entered the alternate screen, retrying /// in case a slow start swallowed the key. fn quit_when_up() -> impl FnMut(&[u8], Duration) -> Option> { From 77995bab78ac15a7bc96d71ce1049a07520c0c87 Mon Sep 17 00:00:00 2001 From: Amin Borghei Date: Thu, 1 Oct 2026 14:49:44 +0200 Subject: [PATCH 03/10] fix(render): show scroll progress in the top bar with colour off With --color=never or NO_COLOR the filled and remaining parts of the bar were the same glyph in the same default colour, so it always read 100%. When the two colours cannot be told apart and the glyphs are the same, the remainder is now drawn blank. --- src/render/mod.rs | 86 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 73 insertions(+), 13 deletions(-) diff --git a/src/render/mod.rs b/src/render/mod.rs index 3439de3..d92a299 100644 --- a/src/render/mod.rs +++ b/src/render/mod.rs @@ -93,32 +93,55 @@ pub fn render_top_bar( ) { let accent = theme::hex_to_color(&t.colors.heading2); let bg = theme::hex_to_color(&t.colors.status_bar_bg); - let width = area.width as usize; + let line = progress_line( + area.width as usize, + scroll_offset, + total_lines, + viewport_height, + accent, + bg, + crate::glyphs::current(), + ); + frame.render_widget(Paragraph::new(vec![line]), area); +} +/// The top bar's progress line, `width` columns wide. The remainder is drawn +/// in the bar's own background colour, which hides it; when the fill colour +/// cannot be told apart from that (no colour at all, or a 16-colour palette +/// that maps both to one entry) and the glyphs are the same too, the +/// remainder is blank instead — otherwise the bar would read 100% always. +fn progress_line( + width: usize, + scroll_offset: usize, + total_lines: usize, + viewport_height: usize, + accent: Color, + bg: Color, + glyphs: &crate::glyphs::Glyphs, +) -> Line<'static> { if total_lines == 0 || total_lines <= viewport_height { - let line = Line::from(Span::styled( - crate::glyphs::current().progress.repeat(width), + return Line::from(Span::styled( + glyphs.progress.repeat(width), Style::default().fg(accent).bg(bg), )); - frame.render_widget(Paragraph::new(vec![line]), area); - return; } let progress = (scroll_offset as f64 / (total_lines - viewport_height) as f64).min(1.0); let filled = ((progress * width as f64) as usize).max(1); let empty = width.saturating_sub(filled); + let rest = if accent == bg && glyphs.progress == glyphs.progress_rest { + " " + } else { + glyphs.progress_rest + }; - let line = Line::from(vec![ + Line::from(vec![ Span::styled( - crate::glyphs::current().progress.repeat(filled), + glyphs.progress.repeat(filled), Style::default().fg(accent).bg(bg), ), - Span::styled( - crate::glyphs::current().progress_rest.repeat(empty), - Style::default().fg(bg).bg(bg), - ), - ]); - frame.render_widget(Paragraph::new(vec![line]), area); + Span::styled(rest.repeat(empty), Style::default().fg(bg).bg(bg)), + ]) } /// Bottom bar: keybindings (left) + filename + stats (right) @@ -846,6 +869,43 @@ mod tests { highlight_query_in_line(&line, query, Color::Red, false) } + /// The top bar drawn on a 20-column `TestBackend`, as text. + fn drawn_bar( + scroll: usize, + accent: Color, + bg: Color, + glyphs: &crate::glyphs::Glyphs, + ) -> String { + let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(20, 1)).unwrap(); + terminal + .draw(|f| { + let line = progress_line(20, scroll, 110, 10, accent, bg, glyphs); + f.render_widget(Paragraph::new(vec![line]), f.area()); + }) + .unwrap(); + let buf = terminal.backend().buffer(); + (0..20).map(|x| buf[(x, 0)].symbol().to_string()).collect() + } + + #[test] + fn progress_bar_moves_with_colour_off() { + let g = &crate::glyphs::UNICODE; + // Colour off: both colours are the terminal default. + let (a, b) = (Color::Reset, Color::Reset); + assert_eq!(drawn_bar(0, a, b, g), format!("▔{}", " ".repeat(19))); + assert_eq!( + drawn_bar(50, a, b, g), + format!("{}{}", "▔".repeat(10), " ".repeat(10)) + ); + assert_eq!(drawn_bar(100, a, b, g), "▔".repeat(20)); + // 16 colours that differ keep the hidden remainder glyph. + let rest = drawn_bar(50, Color::Cyan, Color::Black, g); + assert_eq!(rest, "▔".repeat(20)); + // ASCII glyphs already differ: `=` filled, `-` remainder. + let ascii = drawn_bar(50, a, b, &crate::glyphs::ASCII); + assert_eq!(ascii, format!("{}{}", "=".repeat(10), "-".repeat(10))); + } + #[test] fn offsets_map_lowercase_bytes_back_to_source() { // 'İ' is 2 bytes and lowercases to 3, so the map must not be identity. From 06bd4636a16e2eb4315e2aec99e61bca4f4c4a2b Mon Sep 17 00:00:00 2001 From: Amin Borghei Date: Thu, 1 Oct 2026 14:54:26 +0200 Subject: [PATCH 04/10] fix(image): decode tall screenshots, panoramas and 61 MP photos again The 8192 px per-side limit refused real images that rendered in v0.8.0. The side limit is now 65535 (it only rejects nonsense headers); the 256 MB allocation cap, checked against the declared output size before any pixel exists, is what bounds a decompression bomb. A grey 11000x11000 bomb now decodes once and is cached at 2048 px a side; the same image as RGBA (~484 MB) is still refused. --- src/image.rs | 93 +++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 74 insertions(+), 19 deletions(-) diff --git a/src/image.rs b/src/image.rs index e4303c2..a28a3d3 100644 --- a/src/image.rs +++ b/src/image.rs @@ -42,13 +42,19 @@ const IMAGE_CACHE_CAP: usize = 64; /// decodes could pin gigabytes. const IMAGE_CACHE_MAX_BYTES: usize = 256 * 1024 * 1024; -/// Decode limits for raster images. Anything wider or taller than this is -/// refused before its pixels are allocated: a tiny, highly compressible PNG -/// can declare 11000×11000 and expand to hundreds of MB (a decompression -/// bomb), and inline `data:` URIs deliver one with no network at all. -/// 8192 px still admits every real photo (a 48 MP sensor is 8000×6000). -const RASTER_MAX_SIDE_PX: u32 = 8192; -/// Hard cap on the bytes a single raster decode may allocate (8192² RGBA8). +/// Per-side limit for raster images: far beyond any real image (full-page +/// screenshots run 1280×12000+, panoramas 30000 px wide), so it rejects only +/// nonsense headers. It is not what stops decompression bombs — a tiny, +/// highly compressible PNG can declare 11000×11000 and expand to hundreds of +/// MB — [`RASTER_MAX_ALLOC`] is. +const RASTER_MAX_SIDE_PX: u32 = 65_535; +/// Hard cap on the bytes a single raster decode may allocate, checked +/// against the decoder's declared output size before any pixels exist. It +/// admits a 61 MP photo (9504×6336: ≈180 MB as RGB8, ≈241 MB as RGBA8) and +/// refuses e.g. 11000×11000 RGBA (≈484 MB). The trade-off: a bomb under the +/// cap (11000×11000 grey, 121 MB) does decode, transiently, one image at a +/// time; it is downscaled to [`CACHED_MAX_SIDE_PX`] before caching, so what +/// stays in memory is a few MB. const RASTER_MAX_ALLOC: u64 = 256 * 1024 * 1024; /// Decoded rasters are downscaled to at most this many pixels per side /// before caching. Both consumers — half-block rendering (≤60 cells wide) and @@ -868,33 +874,82 @@ mod tests { out } + /// A PNG whose header declares `w`×`h` RGBA pixels, with no pixel data: + /// enough for the decoder to report its output size. + fn rgba_png_header(w: u32, h: u32) -> Vec { + let mut png = blank_png(1, 1); + // IHDR data starts after the 8-byte signature and 8-byte chunk head. + png[16..20].copy_from_slice(&w.to_be_bytes()); + png[20..24].copy_from_slice(&h.to_be_bytes()); + png[25] = 6; // RGBA + let crc = { + let mut c = flate2::Crc::new(); + c.update(&png[12..29]); + c.sum() + }; + png[29..33].copy_from_slice(&crc.to_be_bytes()); + png + } + // Regression: decode_raster set no limits, so a ~100 KB PNG declaring // 11000×11000 expanded to hundreds of MB (and 64 of them stayed cached). - // It must be refused before allocation, as a normal decode failure. + // The allocation cap bounds every decode; a grey 11000×11000 (121 MB) + // fits under it, so it decodes once and is cached at ≤ 2048 px a side. #[test] - fn decompression_bomb_png_is_rejected() { + fn decompression_bomb_png_stays_bounded() { let png = blank_png(11_000, 11_000); assert!(png.len() < 1024 * 1024, "bomb fixture should be tiny"); - assert!( - decode_raster(&png).is_none(), - "over-limit PNG must not decode" - ); - // Through the public path (inline data: URI, no flag needed) it is - // the ordinary "could not be decoded" failure, not a panic. + // Through the public path (inline data: URI, no flag needed): what + // the cache keeps is ≤ 2048 px a side, so six copies cost six small + // entries, not 6 × 121 MB. use base64::Engine; let uri = format!( "data:image/png;base64,{}", base64::engine::general_purpose::STANDARD.encode(&png) ); - assert_eq!( - load_decoded(&uri, None, ImageMode::LocalOnly, None).err(), - Some(ImageUnavailable::Failed) + let img = load_decoded(&uri, None, ImageMode::LocalOnly, None) + .expect("a 121 MB grey decode is under the cap"); + assert_eq!((img.width(), img.height()), (2048, 2048)); + assert!( + img.as_bytes().len() <= 2048 * 2048 * 4, + "cached size is small" ); + // Past the allocation cap it is refused before any pixel exists: the + // same 11000×11000 as RGBA is ≈484 MB. + assert!(decode_raster(&rgba_png_header(11_000, 11_000)).is_none()); + assert!(decode_raster(&rgba_png_header(65_535, 65_535)).is_none()); + // Beyond the per-side limit, too. + assert!(decode_raster(&rgba_png_header(70_000, 1)).is_none()); + } + + #[test] + fn tall_screenshots_and_wide_panoramas_decode() { + // Regression: an 8192 px side limit refused full-page screenshots. + let tall = decode_raster(&blank_png(1280, 12_000)).expect("1280×12000 decodes"); + assert_eq!(tall.height(), CACHED_MAX_SIDE_PX); + assert_eq!(tall.width(), 218, "aspect ratio preserved"); + let wide = decode_raster(&blank_png(20_000, 1_000)).expect("panorama decodes"); + assert_eq!(wide.width(), CACHED_MAX_SIDE_PX); + // A 61 MP RGB photo declares ≈180 MB: under the cap. (Checked on the + // header alone; encoding a real one here would be slow.) + let mut photo = rgba_png_header(9504, 6336); + photo[25] = 2; // RGB + let mut c = flate2::Crc::new(); + c.update(&photo[12..29]); + photo[29..33].copy_from_slice(&c.sum().to_be_bytes()); + let reader = image::ImageReader::new(std::io::Cursor::new(&photo)) + .with_guessed_format() + .unwrap(); + use image::ImageDecoder; + let decoder = reader.into_decoder().unwrap(); + assert!(decoder.total_bytes() <= RASTER_MAX_ALLOC); + let rgba_61mp = 9504u64 * 6336 * 4; + assert!(rgba_61mp <= RASTER_MAX_ALLOC, "even as RGBA8"); } #[test] fn image_at_the_limit_decodes_and_is_downscaled_for_the_cache() { - let png = blank_png(RASTER_MAX_SIDE_PX, 16); + let png = blank_png(16_384, 16); let img = decode_raster(&png).expect("an image within the limits decodes"); assert_eq!(img.width(), CACHED_MAX_SIDE_PX, "wide image downscaled"); assert!(img.height() >= 1); From e1ea0890f0689f32bb7db46f21726b834608ee90 Mon Sep 17 00:00:00 2001 From: Amin Borghei Date: Thu, 1 Oct 2026 14:55:32 +0200 Subject: [PATCH 05/10] fix(clipboard): find clip.exe and PowerShell on WSL without the Windows PATH The PowerShell fallback was looked up on PATH, but it exists for appendWindowsPath=false, where neither clip.exe nor powershell.exe is on PATH. On WSL ink now also probes the C: drive mount (/mnt/c, or /c with automount root=/) for clip.exe, then for powershell.exe. --- src/clipboard.rs | 105 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 94 insertions(+), 11 deletions(-) diff --git a/src/clipboard.rs b/src/clipboard.rs index 5d2df08..6153a48 100644 --- a/src/clipboard.rs +++ b/src/clipboard.rs @@ -237,20 +237,48 @@ const CLIP_EXE: Helper = Helper { args: &[], encoding: Encoding::Utf16LeBom, }; -/// WSL with `clip.exe` missing from PATH (Windows PATH appending turned -/// off): PowerShell, told to read stdin as UTF-8, sets the clipboard. +/// WSL with `clip.exe` missing (Windows PATH appending turned off and no +/// `clip.exe` at the usual mount points): PowerShell, told to read stdin as +/// UTF-8, sets the clipboard. const POWERSHELL: Helper = Helper { program: "powershell.exe", - args: &[ - "-NoProfile", - "-NonInteractive", - "-Command", - "[Console]::InputEncoding = [Text.Encoding]::UTF8; Set-Clipboard -Value ([Console]::In.ReadToEnd())", - ], + args: POWERSHELL_ARGS, encoding: Encoding::Utf8, }; - -/// Pick the clipboard helper, given the session and which programs exist. +const POWERSHELL_ARGS: &[&str] = &[ + "-NoProfile", + "-NonInteractive", + "-Command", + "[Console]::InputEncoding = [Text.Encoding]::UTF8; Set-Clipboard -Value ([Console]::In.ReadToEnd())", +]; + +/// `appendWindowsPath = false` in `/etc/wsl.conf` takes the Windows +/// directories off PATH, so neither `clip.exe` nor `powershell.exe` is found +/// by name. WSL still mounts the Windows drive: at `/mnt/c` by default, at +/// `/c` with `[automount] root = /`. +const WSL_CLIP_EXE: [Helper; 2] = [ + Helper { + program: "/mnt/c/Windows/System32/clip.exe", + ..CLIP_EXE + }, + Helper { + program: "/c/Windows/System32/clip.exe", + ..CLIP_EXE + }, +]; +const WSL_POWERSHELL: [Helper; 2] = [ + Helper { + program: "/mnt/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe", + ..POWERSHELL + }, + Helper { + program: "/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe", + ..POWERSHELL + }, +]; + +/// Pick the clipboard helper, given the session and which programs exist +/// (`available` gets a bare name to look up on PATH, or an absolute path). /// /// Ordered by how specific the signal is: a Wayland session that also exports /// `DISPLAY` (XWayland) should still get `wl-copy`, and Termux (which may run @@ -263,7 +291,11 @@ pub fn choose_helper(env: HelperEnv, available: &dyn Fn(&str) -> bool) -> Option (XCLIP, env.x11), (XSEL, env.x11), (CLIP_EXE, true), + (WSL_CLIP_EXE[0], env.wsl), + (WSL_CLIP_EXE[1], env.wsl), (POWERSHELL, env.wsl), + (WSL_POWERSHELL[0], env.wsl), + (WSL_POWERSHELL[1], env.wsl), ]; candidates .into_iter() @@ -273,7 +305,16 @@ pub fn choose_helper(env: HelperEnv, available: &dyn Fn(&str) -> bool) -> Option /// The clipboard helper to use on this machine. pub fn native_helper() -> Option { - choose_helper(HelperEnv::detect(), &on_path) + choose_helper(HelperEnv::detect(), &program_exists) +} + +/// A bare program name is looked up on PATH; a path is checked directly. +fn program_exists(program: &str) -> bool { + if program.contains('/') { + std::path::Path::new(program).is_file() + } else { + on_path(program) + } } fn on_path(program: &str) -> bool { @@ -485,6 +526,48 @@ mod tests { assert_eq!(pick(HelperEnv::default(), &["clip.exe"]), Some("clip.exe")); } + #[test] + fn wsl_without_the_windows_path_finds_the_helpers_on_the_c_drive() { + let wsl = HelperEnv { + wsl: true, + ..Default::default() + }; + // appendWindowsPath=false: nothing by name, the drive still mounted. + let mounted = &[ + "/mnt/c/Windows/System32/clip.exe", + "/mnt/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe", + ]; + let clip = choose_helper(wsl, &only(mounted)).unwrap(); + assert_eq!(clip.program, "/mnt/c/Windows/System32/clip.exe"); + assert_eq!(clip.encoding, Encoding::Utf16LeBom); + // No clip.exe at all: PowerShell by its full path, same arguments. + let ps = pick( + wsl, + &["/mnt/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe"], + ); + assert_eq!( + ps, + Some("/mnt/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe") + ); + let ps = choose_helper( + wsl, + &only(&["/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe"]), + ) + .unwrap(); + assert_eq!(ps.args, POWERSHELL.args); + // `[automount] root = /` mounts the drive at /c. + assert_eq!( + pick(wsl, &["/c/Windows/System32/clip.exe"]), + Some("/c/Windows/System32/clip.exe") + ); + // On PATH still wins; outside WSL the mount paths are never probed. + assert_eq!( + pick(wsl, &["clip.exe", "/mnt/c/Windows/System32/clip.exe"]), + Some("clip.exe") + ); + assert_eq!(pick(HelperEnv::default(), mounted), None); + } + #[test] fn helper_input_bytes_match_the_helper() { let wsl = HelperEnv { From f12fb046972fed04d155c1637323273677ce751a Mon Sep 17 00:00:00 2001 From: Amin Borghei Date: Thu, 1 Oct 2026 14:56:41 +0200 Subject: [PATCH 06/10] fix(selection): treat # as decoration only as an ASCII heading marker In ASCII mode every leading # was skipped as decoration, so in a code box a line like '| # install deps' or '| #include' put the visual-mode cursor past the #, and a yank from line start dropped it. A # run now counts only as the line's first mark and when a space follows it. --- src/selection.rs | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/src/selection.rs b/src/selection.rs index 564f3a3..57b04ec 100644 --- a/src/selection.rs +++ b/src/selection.rs @@ -231,22 +231,33 @@ const DECORATION: &[char] = &[ /// Used to place the visual-mode cursor: a selection that starts on the `█` of /// a heading copies a glyph the document never contained. pub fn content_start_col(text: &str) -> usize { + content_start_col_in(text, crate::glyphs::current().ascii) +} + +/// [`content_start_col`] for an explicit glyph set. ASCII mode draws the +/// same structure with `#` (heading levels, as a run: `###`), `|` (bars, +/// borders), `*` (bullets) and `+` (corners). +fn content_start_col_in(text: &str, ascii: bool) -> usize { let mut col = 0usize; let mut seen_decoration = false; - // ASCII mode draws the same structure with `#` (heading levels, as a - // run: `###`), `|` (bars, borders), `*` (bullets) and `+` (corners). - let ascii = crate::glyphs::current().ascii; - for g in text.graphemes(true) { + for (at, g) in text.grapheme_indices(true) { let is_space = g.chars().all(char::is_whitespace); - let is_decoration = g.chars().all(|c| DECORATION.contains(&c)) - || (ascii && matches!(g, "#" | "|" | "*" | "+")); + let is_decoration = + g.chars().all(|c| DECORATION.contains(&c)) || (ascii && matches!(g, "|" | "*" | "+")); if is_space { col += g.width().max(1); continue; } if ascii && g == "#" { - col += 1; - continue; + // A heading marker only as the line's first mark and followed by + // a space (`## Title`); `| # comment` and `#include` in a code + // box are content. + let rest = &text[at..]; + let run = rest.len() - rest.trim_start_matches('#').len(); + if !seen_decoration && rest[run..].starts_with(' ') { + return col + run + 1; + } + break; } if is_decoration && !seen_decoration { seen_decoration = true; @@ -477,6 +488,22 @@ mod tests { assert_eq!(content_start_col(""), 0); } + #[test] + fn ascii_hash_is_decoration_only_as_a_heading_marker() { + let col = |t| content_start_col_in(t, true); + // Headings: past the `#` run and its space, onto the `T`. + assert_eq!(col(" # Title"), 4); + assert_eq!(col(" ### Third"), 6); + // Code-box lines keep their `#`: the box rule is the decoration. + assert_eq!(col(" | # install deps"), 4); + assert_eq!(col(" | #include "), 4); + // A bare `#word` is not a heading marker either. + assert_eq!(col(" #hashtag"), 2); + // The other ASCII decorations still count once. + assert_eq!(col(" * item"), 4); + assert_eq!(col(" | | nested"), 4); + } + #[test] fn snap_col_lands_on_grapheme_starts() { assert_eq!(snap_col("世界a", 0), 0); From 37847f323e22b46963b225efc5d98ff1ccc222cb Mon Sep 17 00:00:00 2001 From: Amin Borghei Date: Thu, 1 Oct 2026 14:58:55 +0200 Subject: [PATCH 07/10] fix(links): follow files with # in the name, and wikilink sections Links were split at the first # before resolving, so [notes](c#-notes.md) or [x](issue#12.md), followed in v0.8.0, silently did nothing. The whole link is now tried as a file name first (%23 decoded), then split at the last #, then the first. [[page#Section]] resolves to page.md with the heading text as fragment (matched by slug, as in Obsidian), and [[#Section]] jumps within the document. A link that cannot be followed now flashes a message instead of doing nothing. --- src/app.rs | 135 ++++++++++++++++++++++++++++++++++++++++-------- src/sanitize.rs | 7 +++ src/wikilink.rs | 33 ++++++++++-- 3 files changed, 149 insertions(+), 26 deletions(-) diff --git a/src/app.rs b/src/app.rs index ccdc203..4aa5f3b 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1529,14 +1529,12 @@ fn open_link( } } if !is_followable_local(url) { - return None; + return Some(format!("cannot follow {url}")); } - let (path, fragment) = split_fragment(url); let size = terminal.size().ok(); let viewport = size.map(|s| s.height.saturating_sub(3)).unwrap_or(24) as usize; - if path.is_empty() { - let fragment = fragment.unwrap_or(""); + if let Some(fragment) = url.strip_prefix('#') { let tab = &mut tabs[active_tab]; let Some(line) = heading_scroll(tab, fragment, viewport) else { return Some(format!("no heading #{fragment}")); @@ -1550,10 +1548,9 @@ fn open_link( .parent() .unwrap_or(std::path::Path::new(".")) .to_path_buf(); - let src = crate::sanitize::resolve_local(&base, path) - .and_then(|target| Some((std::fs::read_to_string(&target).ok()?, target))); - let Some((src, target)) = src else { - return Some(format!("cannot open {path}")); + let Some((src, target, path, fragment)) = read_link_target(&base, url) else { + let shown = link_targets(url).last().map_or(url, |(path, _)| *path); + return Some(format!("cannot open {shown}")); }; nav.record(NavEntry::of(&tabs[active_tab])); let width = effective_width(size.map(|s| s.width).unwrap_or(80), args.toc); @@ -1579,20 +1576,44 @@ fn open_link( /// Links ink follows itself: a local markdown file, optionally with a /// `#heading` suffix, or a bare `#heading` in the current document. fn is_followable_local(url: &str) -> bool { - let (path, fragment) = split_fragment(url); - if path.is_empty() { - return fragment.is_some_and(|f| !f.is_empty()); + match url.strip_prefix('#') { + Some(fragment) => !fragment.is_empty(), + None => !link_targets(url).is_empty(), } - let lower = path.to_ascii_lowercase(); - !lower.contains("://") && (lower.ends_with(".md") || lower.ends_with(".markdown")) } -/// `other.md#intro` -> (`other.md`, Some(`intro`)); `#intro` -> (``, Some(`intro`)). -fn split_fragment(url: &str) -> (&str, Option<&str>) { - match url.split_once('#') { - Some((path, frag)) => (path, Some(frag)), - None => (url, None), +/// Read the file a local link points at, relative to `base`: the first of +/// [`link_targets`] whose file exists wins, so a `#` that is part of the +/// name (`c#-notes.md`) is not taken for a fragment. Returns the source, the +/// resolved path, the link's file part and its fragment. +fn read_link_target<'u>( + base: &std::path::Path, + url: &'u str, +) -> Option<(String, std::path::PathBuf, &'u str, Option<&'u str>)> { + link_targets(url).into_iter().find_map(|(path, fragment)| { + let target = crate::sanitize::resolve_local_exact(base, path)?; + let src = std::fs::read_to_string(&target).ok()?; + Some((src, target, path, fragment)) + }) +} + +/// The ways to read a local link as (markdown file, fragment), most literal +/// first: the whole link as a file name (`#` is legal in names, and `%23` +/// is decoded when resolving), then split at the last `#`, then at the +/// first. Readings whose file part is not markdown are dropped. +fn link_targets(url: &str) -> Vec<(&str, Option<&str>)> { + let mut out = vec![(url, None)]; + for i in [url.rfind('#'), url.find('#')].into_iter().flatten() { + let reading = (&url[..i], Some(&url[i + 1..])); + if !out.contains(&reading) { + out.push(reading); + } } + out.retain(|(path, _)| { + let lower = path.to_ascii_lowercase(); + !lower.contains("://") && (lower.ends_with(".md") || lower.ends_with(".markdown")) + }); + out } /// GitHub-style anchor for a heading: lowercased, whitespace to `-`, every @@ -2176,10 +2197,22 @@ mod section_tests { #[test] fn fragment_splitting_and_followable_links() { - assert_eq!(split_fragment("#a"), ("", Some("a"))); - assert_eq!(split_fragment("doc.md#a"), ("doc.md", Some("a"))); - assert_eq!(split_fragment("doc.md"), ("doc.md", None)); + assert_eq!(link_targets("doc.md#a"), [("doc.md", Some("a"))]); + assert_eq!(link_targets("doc.md"), [("doc.md", None)]); + // Literal name first, then the last `#`, then the first. + assert_eq!( + link_targets("c#-notes.md#intro"), + [("c#-notes.md", Some("intro"))] + ); + assert_eq!(link_targets("issue#12.md"), [("issue#12.md", None)]); + assert_eq!( + link_targets("a#b.md#c.md"), + [("a#b.md#c.md", None), ("a#b.md", Some("c.md"))] + ); + assert!(link_targets("image.png#frag").is_empty()); assert!(is_followable_local("#section")); + assert!(is_followable_local("c#-notes.md")); + assert!(is_followable_local("issue#12.md")); assert!(is_followable_local("docs/x.md#section")); assert!(is_followable_local("x.MARKDOWN")); assert!(!is_followable_local("#")); @@ -2268,6 +2301,66 @@ mod section_tests { assert_eq!(headings[0].anchor, "a -- b --- c"); } + #[test] + fn links_to_files_with_hash_in_the_name_resolve() { + let dir = tempfile::tempdir().unwrap(); + let base = dir.path(); + std::fs::write(base.join("c#-notes.md"), "# C sharp\n").unwrap(); + std::fs::write(base.join("issue#12.md"), "# Twelve\n").unwrap(); + std::fs::write(base.join("other.md"), "# Other\n\n## Intro\n").unwrap(); + let read = |url| { + read_link_target(base, url) + .map(|(src, _, path, frag)| (src.lines().next().unwrap().to_string(), path, frag)) + }; + assert_eq!( + read("c#-notes.md"), + Some(("# C sharp".into(), "c#-notes.md", None)) + ); + assert_eq!( + read("issue#12.md"), + Some(("# Twelve".into(), "issue#12.md", None)) + ); + // `%23` is a `#` in the name. + assert_eq!( + read("issue%2312.md"), + Some(("# Twelve".into(), "issue%2312.md", None)) + ); + // A `#` after the file name is still a fragment. + assert_eq!( + read("other.md#intro"), + Some(("# Other".into(), "other.md", Some("intro"))) + ); + assert_eq!( + read("c#-notes.md#top"), + Some(("# C sharp".into(), "c#-notes.md", Some("top"))) + ); + assert_eq!(read("missing.md#x"), None); + } + + #[test] + fn wikilink_sections_jump_to_the_heading_text() { + // `[[page#Section Name]]` → `page.md` + the heading text, matched by + // slugging it like a heading; `[[#Section]]` stays in the document. + let t = built("# Top\n\nSee [[#My Section]].\n\n## My Section\n\nbody\n"); + let urls: Vec = t + .styled_lines + .iter() + .flat_map(|l| &l.spans) + .filter_map(|s| s.style.link_url.clone()) + .collect(); + assert_eq!(urls, ["#My Section"]); + assert!(is_followable_local(&urls[0])); + let i = find_heading( + t.anchors.iter().map(|(a, _)| a.as_str()), + urls[0].trim_start_matches('#'), + ); + assert_eq!(i, Some(1)); + assert_eq!( + crate::wikilink::resolve_target("notes#Install `ink` CLI"), + "notes.md#Install `ink` CLI" + ); + } + #[test] fn anchor_scroll_lands_like_heading_jumps() { let mut t = tab(DOC, HEADINGS, 0); diff --git a/src/sanitize.rs b/src/sanitize.rs index b2a8cc6..beda583 100644 --- a/src/sanitize.rs +++ b/src/sanitize.rs @@ -94,6 +94,13 @@ pub fn resolve_local(base: &Path, target: &str) -> Option { }) } +/// [`resolve_local`] without the `?`/`#` suffix fallback: the target names +/// the file exactly (or percent-encoded). For callers that split a fragment +/// off themselves and must know whether it was part of the name. +pub fn resolve_local_exact(base: &Path, target: &str) -> Option { + resolve_variants(base, target) +} + fn resolve_variants(base: &Path, target: &str) -> Option { try_resolve(base, target).or_else(|| { percent_decode(target) diff --git a/src/wikilink.rs b/src/wikilink.rs index 98fea24..8e241ad 100644 --- a/src/wikilink.rs +++ b/src/wikilink.rs @@ -16,14 +16,25 @@ pub fn process_wikilinks(source: &str) -> String { source.to_string() } -/// The file a wikilink target points at: the target itself when it already -/// has an extension (`doc.pdf`), else `target.md`. +/// The link a wikilink target points at: the target itself when it already +/// has an extension (`doc.pdf`), else `target.md`. As in Obsidian, a `#` +/// starts a section: `[[page#Section]]` → `page.md#Section`, and +/// `[[#Section]]` is a jump within the document. The section is heading +/// text; anchors are matched by slugging it the way headings are slugged. pub fn resolve_target(target: &str) -> String { let target = target.trim(); - if target.contains('.') { - target.to_string() + let (page, section) = match target.split_once('#') { + Some((page, section)) => (page.trim_end(), Some(section.trim())), + None => (target, None), + }; + let file = if page.is_empty() || page.contains('.') { + page.to_string() } else { - format!("{target}.md") + format!("{page}.md") + }; + match section { + Some(section) => format!("{file}#{section}"), + None => file, } } @@ -96,6 +107,18 @@ mod tests { ); } + #[test] + fn wikilink_sections() { + assert_eq!( + links("See [[page#Install it]] and [[#Usage|usage]]"), + vec![ + pair("page#Install it", "page.md#Install it"), + pair("usage", "#Usage") + ] + ); + assert_eq!(resolve_target("doc.md#a"), "doc.md#a"); + } + #[test] fn wikilink_with_extension() { assert_eq!(links("See [[doc.pdf]]"), vec![pair("doc.pdf", "doc.pdf")]); From 9e0a6dd4ca8411883dca3740be5e454e5cc1073b Mon Sep 17 00:00:00 2001 From: Amin Borghei Date: Thu, 1 Oct 2026 15:13:29 +0200 Subject: [PATCH 08/10] fix(parser): stop switching footnotes off for ordinary documents The deep-nesting guard counted every * _ ~ [ on a line, inside fenced code and tables too, so one long minified line or a wide table row of links turned every [^1] into literal text. Footnotes are now disabled only when the measured AST depth (from a probe parse without footnotes, which comrak runs iteratively) exceeds 1200; comrak's recursive footnote pass overflows a 1 MB debug stack at about 1940. The probe runs only when the document uses [^ and has enough nesting-capable characters to reach the limit. --- src/layout/mod.rs | 56 ++++++++++++++++++ src/parser/mod.rs | 145 ++++++++++++++++++++++++++++++++++------------ 2 files changed, 164 insertions(+), 37 deletions(-) diff --git a/src/layout/mod.rs b/src/layout/mod.rs index 7a36a8b..6eda69d 100644 --- a/src/layout/mod.rs +++ b/src/layout/mod.rs @@ -2519,6 +2519,62 @@ mod deep_nesting { assert!(lines.iter().any(|l| l.contains("deep")), "content dropped"); } + /// Lay `src` out on a 1 MB stack and report whether its `[^1]` footnote + /// was parsed as one: the definition label is drawn bold, while literal + /// `[^1]: …` text would be a plain paragraph. + fn footnote_rendered(src: String) -> bool { + std::thread::Builder::new() + .stack_size(1 << 20) + .spawn(move || { + let arena = Arena::new(); + let root = parse_document(&arena, &src, &crate::parser::options_for(&src)); + let theme = crate::theme::resolve_theme("dark"); + let result = layout_document( + root, + &theme, + 60, + Spacing::Normal, + 0, + None, + ImageMode::Off, + None, + ); + result + .lines + .iter() + .flat_map(|l| &l.spans) + .any(|s| s.text == "[^1]: " && s.style.bold) + }) + .unwrap() + .join() + .expect("layout overflowed or panicked") + } + + const NOTE: &str = "\n\nText[^1].\n\n[^1]: the note\n"; + + #[test] + fn footnotes_render_beside_long_lines_that_nest_nothing() { + // Control: an ordinary document. + assert!(footnote_rendered(format!("Hello{NOTE}"))); + // A fenced block holding a 5000-character line of `_[*~`. + let code = format!("```\n{}\n```{NOTE}", "_[*~".repeat(1250)); + assert!( + footnote_rendered(code), + "fenced code switched footnotes off" + ); + // A wide table row with 300 links. + let row: String = (0..300) + .map(|i| format!("[l{i}](https://e.com/{i}) ")) + .collect(); + let table = format!("| a | b |\n|---|---|\n| {row} | x |{NOTE}"); + assert!(footnote_rendered(table), "table row switched footnotes off"); + // Hostile nesting still turns them off, and still renders. + assert!(!footnote_rendered(format!( + "{} deep{NOTE}", + ">".repeat(20000) + ))); + } + #[test] fn nesting_below_the_cap_is_unchanged() { let lines = render_text_lines("> a\n> > b\n> > > c\n", 60); diff --git a/src/parser/mod.rs b/src/parser/mod.rs index bd97ffc..b2e60df 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -50,51 +50,65 @@ pub fn options() -> Options<'static> { opts } -/// Block nesting beyond which `options_for` turns footnotes off. No real -/// document comes near it; a hostile one (20 KB of `>`) is 20000 deep. -const FOOTNOTE_NESTING_LIMIT: usize = 1000; +/// AST depth beyond which `options_for` turns footnotes off. comrak 0.36 +/// resolves footnotes with a recursive walk over the whole AST, one stack +/// frame per nesting level; everything else it does is iterative. Measured +/// on a 1 MB stack in a debug build, that walk overflows at a depth of about +/// 1940 — whether the levels are blockquotes, list items or emphasis — so +/// this keeps a margin of over a third. No real document comes near it; a +/// hostile one (20 KB of `>`) is 20000 deep. +const FOOTNOTE_DEPTH_LIMIT: usize = 1200; /// Parse options for a specific source: `options()`, except that footnotes -/// are disabled for absurdly deep block nesting. comrak 0.36 resolves -/// footnotes with a recursive walk over the whole AST (one stack frame per -/// nesting level), which overflows the stack on a few KB of `>`; ink's own -/// layout caps its recursion, but cannot reach into the parser. +/// are disabled where comrak's footnote pass could overflow the stack (see +/// [`FOOTNOTE_DEPTH_LIMIT`]); ink's own layout caps its recursion, but cannot +/// reach into the parser. +/// +/// The depth is measured, not guessed from characters: what nests is +/// container blocks and emphasis, and emphasis openers need not be adjacent +/// (`*a _*a _…`) or on one line, while long lines in code blocks or tables +/// full of links nest nothing. Without footnotes the parse is iterative at +/// any depth, so a probe parse without them finds the real depth. It only +/// runs when footnotes could matter (`[^` occurs) and the document has +/// enough nesting-capable characters to possibly reach the limit. pub fn options_for(source: &str) -> Options<'static> { let mut opts = options(); - if max_block_nesting(source) > FOOTNOTE_NESTING_LIMIT { + if !source.contains("[^") { + // No footnote syntax: the extension changes nothing but the pass. opts.extension.footnotes = false; + } else if nesting_bound(source) > FOOTNOTE_DEPTH_LIMIT { + let mut probe = opts.clone(); + probe.extension.footnotes = false; + let arena = Arena::new(); + if ast_depth(parse_document(&arena, source, &probe)) > FOOTNOTE_DEPTH_LIMIT { + opts.extension.footnotes = false; + } } opts } -/// Cheap upper bound on AST nesting, per line: the blockquote and list -/// markers that open it, one level per two columns of indentation, and every -/// character that can open an inline container (emphasis, strikethrough, -/// link/image brackets). -fn max_block_nesting(source: &str) -> usize { - source - .lines() - .map(|line| { - let bytes = line.as_bytes(); - let (mut depth, mut indent) = (0, 0); - for (i, &b) in bytes.iter().enumerate() { - match b { - b' ' => indent += 1, - b'\t' => indent += 4, - b'>' => depth += 1, - b'-' | b'*' | b'+' if bytes.get(i + 1).is_none_or(|b| *b == b' ') => depth += 1, - b'0'..=b'9' | b'.' | b')' => {} - _ => break, - } +/// Upper bound on AST depth: every nesting level is opened by at least one +/// of these bytes (`>` blockquotes; `-` `*` `+` `.` `)` list markers; `*` +/// `_` `~` emphasis and strikethrough; `[` links, images, footnotes), plus +/// the few fixed levels (document, paragraph, text). +fn nesting_bound(source: &str) -> usize { + 8 + source.bytes().filter(|b| b"<>*_~[-+.)".contains(b)).count() +} + +/// Depth of the deepest node, walked without recursion. +fn ast_depth<'a>(root: &'a AstNode<'a>) -> usize { + use comrak::arena_tree::NodeEdge; + let (mut depth, mut max) = (0usize, 0usize); + for edge in root.traverse() { + match edge { + NodeEdge::Start(_) => { + depth += 1; + max = max.max(depth); } - let inline = bytes - .iter() - .filter(|b| matches!(b, b'*' | b'_' | b'~' | b'[')) - .count(); - depth + indent / 2 + inline - }) - .max() - .unwrap_or(0) + NodeEdge::End(_) => depth -= 1, + } + } + max } pub fn extract_headings_from_ast<'a>(root: &'a AstNode<'a>) -> Vec { @@ -139,6 +153,8 @@ fn collect_text<'a>(node: &'a AstNode<'a>) -> String { mod tests { use super::*; + const NOTE: &str = "\n\nText[^1].\n\n[^1]: note\n"; + #[test] fn footnotes_stay_on_for_ordinary_documents() { let src = "> quote\n> > nested\n\n- a\n - b\n 1. c\n\nText[^1].\n\n[^1]: note\n"; @@ -150,10 +166,65 @@ mod tests { for src in [ format!("{} x", ">".repeat(20000)), format!("{}x", "> ".repeat(20000)), - format!("{}x", "- ".repeat(5000)), format!("{}x{}", "*".repeat(20000), "*".repeat(20000)), + // Openers need not be adjacent to nest. + format!("{}x{}", "*a _".repeat(2000), "_ a*".repeat(2000)), + ] { + let src = src + NOTE; + let arena = Arena::new(); + let mut probe = options(); + probe.extension.footnotes = false; + let depth = ast_depth(parse_document(&arena, &src, &probe)); + assert!( + !options_for(&src).extension.footnotes, + "{:?}… depth {depth}", + &src[..12] + ); + } + } + + #[test] + fn footnotes_survive_long_lines_that_nest_nothing() { + let code = format!("```\n{}\n```{NOTE}", "_[*~".repeat(1250)); + assert!(options_for(&code).extension.footnotes, "fenced code"); + let row: String = (0..300).map(|i| format!("[l{i}](u{i}) ")).collect(); + let table = format!("| a | b |\n|---|---|\n| {row} | x |{NOTE}"); + assert!(options_for(&table).extension.footnotes, "wide table row"); + let prose = format!("{}{NOTE}", "*a* _b_ ~c~ [d](e). ".repeat(400)); + assert!(options_for(&prose).extension.footnotes, "long prose"); + // comrak stops list nesting at a fixed depth, far below the limit. + let lists = format!("{}x{NOTE}", "- ".repeat(5000)); + assert!( + options_for(&lists).extension.footnotes, + "capped list nesting" + ); + } + + /// Just under the limit, footnotes stay on and comrak's recursive + /// footnote pass still fits a 1 MB stack (debug build) with room left. + #[test] + fn the_limit_leaves_a_margin_on_a_small_stack() { + for src in [ + format!("{} deep", ">".repeat(FOOTNOTE_DEPTH_LIMIT - 10)), + format!( + "{}deep{}", + "*".repeat(2 * FOOTNOTE_DEPTH_LIMIT - 20), + "*".repeat(2 * FOOTNOTE_DEPTH_LIMIT - 20) + ), ] { - assert!(!options_for(&src).extension.footnotes); + let src = src + NOTE; + std::thread::Builder::new() + .stack_size(1 << 20) + .spawn(move || { + let opts = options_for(&src); + assert!(opts.extension.footnotes); + let arena = Arena::new(); + let root = parse_document(&arena, &src, &opts); + assert!(ast_depth(root) > FOOTNOTE_DEPTH_LIMIT - 20); + }) + .unwrap() + .join() + .expect("footnote pass overflowed below the limit"); } } } From 24f53aab3e812cc79311a593f12ba0faa21bb4c3 Mon Sep 17 00:00:00 2001 From: Amin Borghei Date: Thu, 1 Oct 2026 15:15:25 +0200 Subject: [PATCH 09/10] fix(history): share the document with history entries and cap them Every NavEntry held its own copy of the tab source, pushed for each in-page anchor jump and cloned again on every back/forward; history was unbounded, so a 5 MB document and 200 jumps held about 1 GB. The source is now an Arc shared by the tab and its entries, history keeps the last 256 places, and going back to a local file that changed on disk since re-reads it, as v0.8.0 did (stdin and URL documents still use the snapshot). --- src/app.rs | 164 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 144 insertions(+), 20 deletions(-) diff --git a/src/app.rs b/src/app.rs index 4aa5f3b..9277a01 100644 --- a/src/app.rs +++ b/src/app.rs @@ -19,12 +19,18 @@ use crossterm::terminal::{ use ratatui::prelude::*; use ratatui::widgets::Paragraph; use std::io; -use std::time::{Duration, Instant}; +use std::sync::Arc; +use std::time::{Duration, Instant, SystemTime}; struct Tab { filename: String, - #[allow(dead_code)] - source: String, + /// The document as loaded. Shared, not copied, with every history entry + /// made from this tab: an in-page jump records a pointer. + source: Arc, + /// Modification time of `filename` when `source` was read from it; `None` + /// for stdin and URL documents. Going back to a file that has changed + /// since re-reads it. + mtime: Option, /// The markdown actually parsed: frontmatter stripped, wikilinks expanded. /// Heading `source_line`s index into this, so `Y` slices sections from it. content: String, @@ -70,13 +76,15 @@ struct ImagePlacement { /// A place to return to with `[` / `]`. /// -/// Carries the source that was on screen, so going back to stdin or a URL -/// document (or a file that has since moved) rebuilds what was shown instead -/// of trying to re-read the name as a local path. +/// Carries the source that was on screen (shared with the tab, not copied), +/// so going back to stdin or a URL document (or a file that has since moved) +/// rebuilds what was shown instead of trying to re-read the name as a local +/// path. A local file that has changed since is re-read instead. #[derive(Debug, Clone, PartialEq)] struct NavEntry { filename: String, - source: String, + source: Arc, + mtime: Option, scroll_offset: usize, } @@ -84,12 +92,16 @@ impl NavEntry { fn of(tab: &Tab) -> Self { Self { filename: tab.filename.clone(), - source: tab.source.clone(), + source: Arc::clone(&tab.source), + mtime: tab.mtime, scroll_offset: tab.scroll_offset, } } } +/// How many places `[` can go back to; the oldest are dropped beyond it. +const NAV_HISTORY_CAP: usize = 256; + /// Back/forward stacks shared by every way of following a link. #[derive(Debug, Default)] struct NavHistory { @@ -101,7 +113,7 @@ impl NavHistory { /// Remember `here` before moving somewhere new; a new branch drops the /// forward stack, as in a browser. fn record(&mut self, here: NavEntry) { - self.back.push(here); + self.push_back(here); self.forward.clear(); } @@ -115,9 +127,16 @@ impl NavHistory { /// Step forward from `here`, returning where to go. fn go_forward(&mut self, here: NavEntry) -> Option { let to = self.forward.pop()?; - self.back.push(here); + self.push_back(here); Some(to) } + + fn push_back(&mut self, here: NavEntry) { + if self.back.len() >= NAV_HISTORY_CAP { + self.back.remove(0); + } + self.back.push(here); + } } /// A labeled link in the current viewport for hint-mode selection. @@ -1674,17 +1693,39 @@ fn heading_scroll(tab: &Tab, fragment: &str, viewport: usize) -> Option { } /// Show a history entry: the same document just scrolls back; anything else -/// is rebuilt from the source the entry carries. +/// is rebuilt from the source the entry carries — or, for a local file that +/// has changed on disk since, from the file (as v0.8.0 always did). fn restore_nav_entry( tab: &mut Tab, - entry: NavEntry, + mut entry: NavEntry, args: &Args, width: u16, gen: u32, graphics: &crate::graphics::Graphics, ) { - if tab.filename != entry.filename || tab.source != entry.source { - *tab = build_tab(entry.source, &entry.filename, args, width, gen, graphics); + // A slide's source is one slide, not the file: never swap in the file. + if !args.slides { + let now = std::fs::metadata(&entry.filename) + .and_then(|m| m.modified()) + .ok(); + if file_changed(entry.mtime, now) { + if let Ok(fresh) = std::fs::read_to_string(&entry.filename) { + entry.source = fresh.into(); + entry.mtime = now; + } + } + } + let same_source = Arc::ptr_eq(&tab.source, &entry.source) || tab.source == entry.source; + if tab.filename != entry.filename || !same_source { + *tab = build_tab( + Arc::clone(&entry.source), + &entry.filename, + args, + width, + gen, + graphics, + ); + tab.mtime = entry.mtime; } tab.scroll_offset = entry .scroll_offset @@ -1714,12 +1755,15 @@ fn rebuild_tab( gen: u32, graphics: &crate::graphics::Graphics, ) { - let source = tab.source.clone(); + let source = Arc::clone(&tab.source); let filename = tab.filename.clone(); let scroll = tab.scroll_offset; let toc_visible = tab.toc.visible; + let mtime = tab.mtime; let width = effective_width(term_width, toc_visible); *tab = build_tab(source, &filename, args, width, gen, graphics); + // Same source as before, so the same snapshot time. + tab.mtime = mtime; // Clamp: the new layout may have far fewer lines (e.g. after widening) — // an unclamped stale offset blanked the viewport and panicked link-mode. tab.scroll_offset = scroll.min(tab.ratatui_lines.len().saturating_sub(1)); @@ -1739,16 +1783,29 @@ fn ensure_tab_current( } } +/// Has a file changed since a snapshot of it was taken? Only a local file +/// has a snapshot time (`Some`); a file that is gone (`now == None`) keeps +/// its snapshot. +fn file_changed(snapshot: Option, now: Option) -> bool { + matches!((snapshot, now), (Some(then), Some(now)) if then != now) +} + fn build_tab( - source: String, + source: impl Into>, filename: &str, args: &Args, term_width: u16, gen: u32, graphics: &crate::graphics::Graphics, ) -> Tab { + let source: Arc = source.into(); + let mtime = if is_local_file(filename) { + std::fs::metadata(filename).and_then(|m| m.modified()).ok() + } else { + None + }; let (_, content) = if args.frontmatter { - (None, source.clone()) + (None, source.to_string()) } else { frontmatter::strip_frontmatter(&source) }; @@ -1851,6 +1908,7 @@ fn build_tab( Tab { filename: filename.to_string(), source, + mtime, content, styled_lines, ratatui_lines, @@ -2063,6 +2121,7 @@ mod section_tests { Tab { filename: "t.md".into(), source: content.into(), + mtime: None, content: content.into(), styled_lines: Vec::new(), ratatui_lines: Vec::new(), @@ -2242,7 +2301,7 @@ mod section_tests { /// A tab built through the real pipeline: wikilinks, parse, layout. pub(super) fn built(src: &str) -> Tab { let graphics = crate::graphics::Graphics::halfblocks(); - build_tab(src.into(), "t.md", &test_args(), 80, 0, &graphics) + build_tab(src, "t.md", &test_args(), 80, 0, &graphics) } /// The display line `#fragment` scrolls to, or `None`. @@ -2375,11 +2434,76 @@ mod section_tests { fn entry(name: &str, scroll: usize) -> NavEntry { NavEntry { filename: name.into(), - source: format!("# {name}"), + source: format!("# {name}").into(), + mtime: None, scroll_offset: scroll, } } + #[test] + fn an_in_page_jump_records_a_pointer_not_a_copy() { + let t = built(DOC); + let mut nav = NavHistory::default(); + nav.record(NavEntry::of(&t)); + nav.record(NavEntry::of(&t)); + assert!(nav.back.iter().all(|e| Arc::ptr_eq(&e.source, &t.source))); + // Back and forward hand the same allocation around too. + let back = nav.go_back(NavEntry::of(&t)).unwrap(); + assert!(Arc::ptr_eq(&back.source, &t.source)); + assert!(Arc::ptr_eq(&nav.forward[0].source, &t.source)); + } + + #[test] + fn history_is_capped_dropping_the_oldest() { + let mut nav = NavHistory::default(); + for i in 0..NAV_HISTORY_CAP + 44 { + nav.record(entry("a.md", i)); + } + assert_eq!(nav.back.len(), NAV_HISTORY_CAP); + assert_eq!(nav.back[0].scroll_offset, 44); + // Forward steps that refill the back stack respect the cap too. + let here = nav.go_back(entry("b.md", 0)).unwrap(); + nav.go_forward(here).unwrap(); + assert_eq!(nav.back.len(), NAV_HISTORY_CAP); + } + + #[test] + fn going_back_rereads_a_file_only_when_it_changed() { + let t0 = SystemTime::UNIX_EPOCH + Duration::from_secs(1_000); + let t1 = t0 + Duration::from_secs(5); + assert!(file_changed(Some(t0), Some(t1)), "edited since"); + assert!(!file_changed(Some(t0), Some(t0)), "unchanged"); + assert!(!file_changed(Some(t0), None), "deleted: keep the snapshot"); + assert!(!file_changed(None, Some(t1)), "stdin / URL: no file"); + assert!(!file_changed(None, None)); + } + + #[test] + fn local_files_carry_their_mtime_and_stdin_does_not() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("doc.md"); + std::fs::write(&path, "# Doc\n").unwrap(); + let graphics = crate::graphics::Graphics::halfblocks(); + let file = build_tab( + "# Doc\n".to_string(), + path.to_str().unwrap(), + &test_args(), + 80, + 0, + &graphics, + ); + assert!(file.mtime.is_some()); + let stdin = build_tab( + "# Doc\n".to_string(), + "stdin", + &test_args(), + 80, + 0, + &graphics, + ); + assert_eq!(stdin.mtime, None); + } + #[test] fn history_returns_to_the_recorded_position() { let mut nav = NavHistory::default(); @@ -2412,7 +2536,7 @@ mod section_tests { t.filename = "stdin".into(); let e = NavEntry::of(&t); assert_eq!(e.filename, "stdin"); - assert_eq!(e.source, DOC); + assert_eq!(&*e.source, DOC); assert_eq!(e.scroll_offset, 7); } } From 3886b73f84549a33c920a40c26067546caaccc19 Mon Sep 17 00:00:00 2001 From: Amin Borghei Date: Thu, 1 Oct 2026 15:16:23 +0200 Subject: [PATCH 10/10] fix(table): drop blank lines from leading, trailing and doubled cell breaks A cell ending in
was drawn one line taller with an empty last line, and

left a blank line inside the cell. Breaks in a cell now only separate content: leading, trailing and repeated ones are dropped, as a trailing break in a paragraph already is. --- src/layout/table.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/layout/table.rs b/src/layout/table.rs index 4ac1358..7b282ac 100644 --- a/src/layout/table.rs +++ b/src/layout/table.rs @@ -531,6 +531,16 @@ fn collect_cell_text<'a>(node: &'a AstNode<'a>) -> String { _ => {} } } + if buf.contains('\n') { + // A break opens a new line only between content: a trailing or + // leading `
`, or `

`, must not add an empty row line + // (paragraphs drop a trailing break the same way). + buf = buf + .split('\n') + .filter(|seg| !seg.trim().is_empty()) + .collect::>() + .join("\n"); + } buf } @@ -542,6 +552,36 @@ mod tests { // input — the transposed layout aligns values on that guarantee. The // emoji inputs are the regression: per-char width accounting returned a // 41-column label for a 27-column slot. + /// The body cells of the first table in `src`. + fn body_cells(src: &str) -> Vec> { + let arena = comrak::Arena::new(); + let root = comrak::parse_document(&arena, src, &crate::parser::options()); + let table = root + .descendants() + .find(|n| matches!(n.data.borrow().value, NodeValue::Table(_))) + .unwrap(); + extract_table_data(table).1 + } + + #[test] + fn breaks_at_a_cell_edge_or_doubled_add_no_empty_lines() { + let head = "| a | b |\n|---|---|\n"; + for (row, want) in [ + ("| step one
step two
| ok |", "step one\nstep two"), + ("|
step one
step two | ok |", "step one\nstep two"), + ("| one

two | ok |", "one\ntwo"), + ("| one


two

| ok |", "one\ntwo"), + ("|
| ok |", ""), + ("| one
two | ok |", "one\ntwo"), + ] { + let cells = body_cells(&format!("{head}{row}\n")); + assert_eq!(cells[0][0], want, "{row}"); + // The row is exactly as tall as its content. + let lines = &wrap_row(&cells[0], &[20, 4])[0]; + assert_eq!(lines.len(), want.split('\n').count().max(1), "{row}"); + } + } + #[test] fn fit_label_is_always_exact_width() { let inputs = [