From d7f9f894eb4b0aa40fb32af15ca279f8f5d62a2e Mon Sep 17 00:00:00 2001 From: ldm0 Date: Wed, 16 Sep 2026 20:45:38 +0800 Subject: [PATCH] feat(markdown): render simple HTML tables as GFM Synthesize empty headings for simple headerless tables and keep complex tables readable as blocks. Use owned output fragments to avoid repeated copying through nested fallback tables. Cover escaping, alignment, layouts, CLI output and linear copy growth. Validated with cargo fmt, workspace Clippy, and 18,114 passing nextest tests (13 skipped). --- moli-html2md/README.md | 38 ++- moli-html2md/src/copy_tests.rs | 72 ++++ moli-html2md/src/lib.rs | 8 + moli-html2md/src/machine.rs | 76 ++++- moli-html2md/src/output.rs | 136 ++++++++ moli-html2md/src/table.rs | 214 ++++++++++++ moli-html2md/src/writer.rs | 61 +++- moli-html2md/tests/conversion.rs | 37 ++- moli-html2md/tests/fixtures/data-table.html | 8 + moli-html2md/tests/fixtures/data-table.md | 4 + .../tests/fixtures/hacker-news-layout.md | 8 +- .../tests/fixtures/headerless-data-table.html | 7 + .../tests/fixtures/headerless-data-table.md | 4 + moli-html2md/tests/regressions.rs | 11 +- moli-html2md/tests/tables.rs | 313 +++++++++++++++++- moli-html2md/tests/turndown/README.md | 6 +- moli-renderer-v8/src/runtime/page_dump.rs | 12 +- moli/tests/fetch_cli/markdown.rs | 32 +- 18 files changed, 991 insertions(+), 56 deletions(-) create mode 100644 moli-html2md/src/copy_tests.rs create mode 100644 moli-html2md/src/output.rs create mode 100644 moli-html2md/src/table.rs create mode 100644 moli-html2md/tests/fixtures/data-table.html create mode 100644 moli-html2md/tests/fixtures/data-table.md create mode 100644 moli-html2md/tests/fixtures/headerless-data-table.html create mode 100644 moli-html2md/tests/fixtures/headerless-data-table.md diff --git a/moli-html2md/README.md b/moli-html2md/README.md index 9767df8d83..67584234bb 100644 --- a/moli-html2md/README.md +++ b/moli-html2md/README.md @@ -34,8 +34,15 @@ their children as ordinary content. Comments and doctypes produce no output. - An inline writer tracks desired and emitted styles, pending whitespace, and block boundaries. It collapses HTML whitespace across node boundaries, keeps Unicode spaces, and coalesces adjacent identical emphasis without changing nodes. -- Lists, quotes, and headings have output buffers finalized by exit +- Lists, quotes, headings, and table cells have output buffers finalized by exit tasks. Raw code text uses the same iterative traversal with formatting disabled. +- Tables inspect their direct sections, rows and cells before conversion. Nested + content uses the task stack; eligibility checks never rescan cell descendants. +- Finished table cells form an owned fragment tree. Fallback joins and writes to + parent buffers move fragment roots without copying descendant text; the final + output is flattened in one iterative pass. Fragment destruction is iterative + too. Quotes, headings and list items still materialize their captured text + when rewriting its lines. - List spacing follows block boundaries recorded during conversion. Nested lists keep their own spacing; list items need no preliminary DOM scan. - Ordered lists use `start` followed by sequential numbers, like Turndown core. @@ -44,10 +51,26 @@ their children as ordinary content. Comments and doctypes produce no output. punctuation and Unicode; a name containing backticks uses a tilde fence. Fallback language hints end at the first line ending. -Tables, sections, rows, and cells follow Turndown core's ordinary block behavior: -their content is emitted in DOM order, separated by blank lines. Nested tables -use the same rule. The converter does not classify layout tables or generate -GFM table syntax; headers, roles, borders, and spans do not select another path. +Simple tables produce GFM pipe tables by default. A first row inside `thead` or +consisting entirely of `th` cells is used as the heading, following the heading +recognition used by Turndown's GFM tables plugin. Without a heading, the converter +adds an empty heading as wide as the widest data row and retains every data row. +Whitespace, comments and empty spacer rows do not affect recognition. The first +cell in each column supplies its `align` attribute; a preceding caption remains +a separate Markdown block. + +Cells retain inline formatting, links, images and code. Literal pipes are escaped, +and cell line breaks and paragraph boundaries use inline `
` tags. Attribute +newlines are encoded or normalized so they cannot break a table row. GFM supplies +missing trailing cells in short rows; the converter does not allocate a padded grid. + +Complex tables retain Turndown core's ordinary block expansion, in DOM order with +blank lines between cells. This includes non-unit spans, multiple `thead` rows, +rows wider than an explicit header, or cells containing nested tables, lists, +headings, quotes or preformatted blocks. Inner simple tables can still produce +GFM when their containing layout table is expanded. Roles, borders and CSS do +not select another conversion path, so structurally simple layout tables also +produce GFM. Complex layouts remain readable blocks instead of raw HTML tables. Supported output includes headings, paragraphs, emphasis, strikethrough, links, images, lists, blockquotes, hard breaks, and fenced code. Inline HTML is used when @@ -77,6 +100,11 @@ checks DOM immutability, and explicitly asserts 12 documented differences that preserve Moli's handling of literal text, code, emphasis, and list boundaries. Targeted regressions also cover empty elements, links, attributes in headings and tables, code whitespace and language names, list numbering, and loose lists. +An operation-count regression checks nested table fallback at 64, 128 and 256 +levels with 256 text bytes per level, both with and without headers. It bounds +bytes copied during fragment materialization and writes to parent buffers by +output size, so a repeated flatten/copy regression fails without timing thresholds. +Dropping 20,000 nested fragments is also tested on a 64 KiB thread stack. Emphasis tests cover adjacent links that can use Markdown delimiters and intraword link boundaries that still require inline HTML. diff --git a/moli-html2md/src/copy_tests.rs b/moli-html2md/src/copy_tests.rs new file mode 100644 index 0000000000..e7fed121b5 --- /dev/null +++ b/moli-html2md/src/copy_tests.rs @@ -0,0 +1,72 @@ +#[path = "../tests/support/mod.rs"] +mod support; + +use crate::{ + Converter, Options, + output::{Output, measure_copies}, +}; + +#[test] +fn nested_table_fallback_copies_text_linearly() { + let before = "x".repeat(128); + let after = "y".repeat(128); + let converter = Converter::new(Options { + max_depth: usize::MAX, + ..Options::default() + }); + for heading in ["", "Header"] { + let mut previous_copies = None; + for depth in [64, 128, 256] { + let html = format!( + "{}leaf{}", + format!("{heading}
{before}").repeat(depth), + format!("{after}
").repeat(depth) + ); + let dom = support::Tree::parse(&html); + let (actual, copied) = measure_copies(|| converter.convert(&dom, dom.root)); + assert_eq!(actual.matches(&before).count(), depth); + assert_eq!(actual.matches(&after).count(), depth); + assert_eq!( + support::rendered_html(&actual).matches("").count(), + 1 + ); + // Count actual bytes copied when materializing fragments or writing + // strings back to a parent, not elapsed time or recursion depth. + assert!( + copied <= 2 * actual.len(), + "depth={depth}, output={}, copied={copied}", + actual.len() + ); + if let Some(previous) = previous_copies { + assert!( + copied <= previous * 2 + 128, + "copy volume must grow linearly" + ); + } + previous_copies = Some(copied); + println!( + "header={}, depth={depth}, output={}, copied={copied}", + !heading.is_empty(), + actual.len() + ); + } + } +} + +#[test] +fn dropping_nested_fragments_does_not_recurse() { + std::thread::Builder::new() + .stack_size(64 * 1024) + .spawn(|| { + let mut output = Output::from("leaf".to_owned()); + for _ in 0..20_000 { + let mut parent = Output::default(); + parent.append(output); + output = parent; + } + drop(output); + }) + .expect("spawn small-stack test") + .join() + .expect("dropping fragments should be iterative"); +} diff --git a/moli-html2md/src/lib.rs b/moli-html2md/src/lib.rs index dadb292b10..c14d26876e 100644 --- a/moli-html2md/src/lib.rs +++ b/moli-html2md/src/lib.rs @@ -2,8 +2,16 @@ mod converter; mod dom; mod machine; mod options; +mod output; +mod table; mod writer; pub use converter::{Converter, convert}; pub use dom::{Dom, NodeKind}; pub use options::Options; + +#[cfg(test)] +extern crate self as moli_html2md; + +#[cfg(test)] +mod copy_tests; diff --git a/moli-html2md/src/machine.rs b/moli-html2md/src/machine.rs index af25d44886..903f042b54 100644 --- a/moli-html2md/src/machine.rs +++ b/moli-html2md/src/machine.rs @@ -1,3 +1,5 @@ +use crate::output::Output; +use crate::table::Table; use crate::writer::{Style, Writer, longest_run}; use crate::{Dom, NodeKind, Options}; @@ -14,6 +16,8 @@ enum Task<'a, Id> { RawChildren(Option, usize, bool), EndCode, EndPre(Option<&'a str>), + TableCell, + EndTableCell, } struct List { @@ -29,6 +33,7 @@ struct Machine<'a, D: Dom + ?Sized> { tasks: Vec>, writers: Vec>, lists: Vec, + tables: Vec>, raw: String, serial: usize, } @@ -40,11 +45,12 @@ pub(crate) fn convert(dom: &D, root: D::NodeId, options: &Optio tasks: vec![Task::Visit(root, 0)], writers: vec![Writer::default()], lists: Vec::new(), + tables: Vec::new(), raw: String::new(), serial: 0, }; machine.run(); - machine.take_writer() + machine.take_writer().into_string() } impl<'a, D: Dom + ?Sized> Machine<'a, D> { @@ -64,7 +70,7 @@ impl<'a, D: Dom + ?Sized> Machine<'a, D> { Task::PopStyle => self.writer().pop_style(), Task::EndLink(serial) => self.writer().end_link(serial), Task::EndQuote => { - let content = self.take_writer(); + let content = self.take_writer().into_string(); let mut quote = String::new(); for line in content.lines() { if !quote.is_empty() { @@ -79,7 +85,7 @@ impl<'a, D: Dom + ?Sized> Machine<'a, D> { self.writer().block("e, 2, 2); } Task::EndHeading(level) => { - let content = self.take_writer(); + let content = self.take_writer().into_string(); if !content.is_empty() { let heading = format!("{} {}", "#".repeat(level), content.replace('\n', " ")); @@ -92,7 +98,7 @@ impl<'a, D: Dom + ?Sized> Machine<'a, D> { self.lists.pop(); let content = self.take_writer(); let has_blocks = self.writer().has_blocks; - self.writer().block(&content, before, 2); + self.writer().block_output(content, before, 2); if before == 1 { // A nested list alone does not make its parent item loose. self.writer().has_blocks = has_blocks; @@ -100,7 +106,7 @@ impl<'a, D: Dom + ?Sized> Machine<'a, D> { } Task::EndItem(marker) => { let loose = self.writer().has_blocks; - let content = self.take_writer(); + let content = self.take_writer().into_string(); if content.is_empty() { continue; } @@ -130,6 +136,13 @@ impl<'a, D: Dom + ?Sized> Machine<'a, D> { self.writer().code_with_edges(&text, preformatted); } Task::EndPre(language) => self.end_pre(language), + Task::TableCell => self.table_cell(), + Task::EndTableCell => { + let content = self.take_writer(); + let table = self.tables.last_mut().expect("cell belongs to a table"); + table.content.push(content); + table.in_cell = false; + } } } } @@ -143,7 +156,7 @@ impl<'a, D: Dom + ?Sized> Machine<'a, D> { self.writers.push(writer); } - fn take_writer(&mut self) -> String { + fn take_writer(&mut self) -> Output { let writer = self.writers.pop().expect("capture has an output"); if let Some(parent) = self.writers.last_mut() { parent.last_link = parent.last_link.max(writer.last_link); @@ -180,6 +193,27 @@ impl<'a, D: Dom + ?Sized> Machine<'a, D> { NodeKind::Other => return, NodeKind::Element(tag) => tag, }; + if let Some(table) = self.tables.last_mut() + && table.in_cell + && matches!( + tag, + "table" + | "pre" + | "blockquote" + | "ul" + | "ol" + | "li" + | "hr" + | "h1" + | "h2" + | "h3" + | "h4" + | "h5" + | "h6" + ) + { + table.has_complex_content = true; + } match tag { "head" | "script" | "style" | "noscript" | "template" => return, "br" => { @@ -252,6 +286,21 @@ impl<'a, D: Dom + ?Sized> Machine<'a, D> { } "ul" | "ol" => self.start_list(node, tag == "ol"), "li" => self.start_item(), + "table" => { + if let Some(mut table) = + Table::from_dom(self.dom, node, depth, self.options.max_depth) + { + let captions = std::mem::take(&mut table.captions); + self.tables.push(table); + self.tasks.push(Task::TableCell); + for caption in captions.into_iter().rev() { + self.tasks.push(Task::Visit(caption.node, caption.depth)); + } + return; + } + self.writer().boundary(2); + self.tasks.push(Task::Boundary); + } _ if is_block(tag) => { self.writer().boundary(2); self.tasks.push(Task::Boundary); @@ -261,6 +310,21 @@ impl<'a, D: Dom + ?Sized> Machine<'a, D> { self.children(node, depth + 1); } + fn table_cell(&mut self) { + let table = self.tables.last_mut().expect("table conversion is active"); + if let Some(cell) = table.cells.next() { + table.in_cell = true; + self.capture(); + self.writer().table_cell(); + self.tasks.push(Task::TableCell); + self.tasks.push(Task::EndTableCell); + self.children(cell.node, cell.depth + 1); + } else { + let table = self.tables.pop().expect("table conversion is active"); + self.writer().block_output(table.finish(), 2, 2); + } + } + fn start_list(&mut self, node: D::NodeId, ordered: bool) { let before = if self .lists diff --git a/moli-html2md/src/output.rs b/moli-html2md/src/output.rs new file mode 100644 index 0000000000..4419a4b090 --- /dev/null +++ b/moli-html2md/src/output.rs @@ -0,0 +1,136 @@ +/// Finished Markdown fragments. Joining outputs moves their roots, not their +/// text or descendant vectors. Both flattening and destruction are iterative. +#[derive(Default)] +pub(crate) struct Output { + parts: Vec, + len: usize, + last: Option, + trailing_newlines: usize, +} + +enum Part { + Text(String), + Literal(&'static str), + Group(Output), +} + +impl Output { + pub(crate) fn is_empty(&self) -> bool { + self.len == 0 + } + + pub(crate) fn last_char(&self) -> Option { + self.last + } + + pub(crate) fn trailing_newlines(&self) -> usize { + self.trailing_newlines + } + + fn extend_tail(&mut self, len: usize, last: char, trailing_newlines: usize) { + self.len += len; + self.last = Some(last); + self.trailing_newlines = if trailing_newlines == len { + self.trailing_newlines + trailing_newlines + } else { + trailing_newlines + }; + } + + pub(crate) fn push_text(&mut self, text: String) { + if let Some(last) = text.chars().next_back() { + self.extend_tail( + text.len(), + last, + text.len() - text.trim_end_matches('\n').len(), + ); + self.parts.push(Part::Text(text)); + } + } + + pub(crate) fn push_literal(&mut self, text: &'static str) { + if let Some(last) = text.chars().next_back() { + self.extend_tail( + text.len(), + last, + text.len() - text.trim_end_matches('\n').len(), + ); + self.parts.push(Part::Literal(text)); + } + } + + pub(crate) fn append(&mut self, output: Self) { + if let Some(last) = output.last { + self.extend_tail(output.len, last, output.trailing_newlines); + self.parts.push(Part::Group(output)); + } + } + + pub(crate) fn into_string(mut self) -> String { + // Ordinary inline output already has a contiguous buffer. Keep that + // allocation instead of adding a copy to the common, non-table path. + if self.parts.len() == 1 { + match self.parts.pop().expect("one output fragment") { + Part::Text(text) => return text, + part => self.parts.push(part), + } + } + let mut text = String::with_capacity(self.len); + let mut pending = std::mem::take(&mut self.parts); + pending.reverse(); + while let Some(part) = pending.pop() { + match part { + Part::Text(fragment) => { + #[cfg(test)] + record_copy(fragment.len()); + text.push_str(&fragment); + } + Part::Literal(fragment) => { + #[cfg(test)] + record_copy(fragment.len()); + text.push_str(fragment); + } + Part::Group(mut output) => pending.extend(output.parts.drain(..).rev()), + } + } + text + } +} + +impl From for Output { + fn from(text: String) -> Self { + let mut output = Self::default(); + output.push_text(text); + output + } +} + +impl Drop for Output { + fn drop(&mut self) { + // Dropping nested Vec normally recurses. Empty each group's + // children before dropping it, including outputs never flattened. + let mut pending = std::mem::take(&mut self.parts); + while let Some(part) = pending.pop() { + if let Part::Group(mut output) = part { + pending.append(&mut output.parts); + } + } + } +} + +#[cfg(test)] +thread_local! { + static COPIED_BYTES: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +#[cfg(test)] +pub(crate) fn record_copy(bytes: usize) { + COPIED_BYTES.with(|count| count.set(count.get() + bytes)); +} + +#[cfg(test)] +pub(crate) fn measure_copies(convert: impl FnOnce() -> T) -> (T, usize) { + COPIED_BYTES.with(|count| count.set(0)); + let output = convert(); + (output, COPIED_BYTES.with(std::cell::Cell::get)) +} diff --git a/moli-html2md/src/table.rs b/moli-html2md/src/table.rs new file mode 100644 index 0000000000..7a682d17ad --- /dev/null +++ b/moli-html2md/src/table.rs @@ -0,0 +1,214 @@ +use crate::{Dom, NodeKind, output::Output, writer::is_space}; + +pub(crate) struct Cell { + pub(crate) node: Id, + pub(crate) depth: usize, +} + +pub(crate) struct Table { + pub(crate) captions: Vec>, + pub(crate) cells: std::vec::IntoIter>, + pub(crate) content: Vec, + pub(crate) in_cell: bool, + pub(crate) has_complex_content: bool, + rows: Vec, + separators: Vec<&'static str>, + has_header: bool, +} + +impl Table { + /// Inspect only the table's sections, rows and cells. Cell descendants are + /// converted by the main task stack, so nested tables never recurse or + /// repeatedly scan their descendants to decide how to render an ancestor. + pub(crate) fn from_dom + ?Sized>( + dom: &D, + node: Id, + depth: usize, + max_depth: usize, + ) -> Option { + let mut table = Self { + captions: Vec::new(), + cells: Vec::new().into_iter(), + content: Vec::new(), + in_cell: false, + has_complex_content: false, + rows: Vec::new(), + separators: Vec::new(), + has_header: false, + }; + let mut cells = Vec::new(); + let mut child = dom.first_child(node); + while let Some(node) = child.filter(|_| depth + 1 < max_depth) { + child = dom.next_sibling(node); + match dom.node_kind(node) { + NodeKind::Element("caption") if table.rows.is_empty() => { + table.captions.push(Cell { + node, + depth: depth + 1, + }); + } + NodeKind::Element("tr") => { + table.add_row(dom, node, depth + 1, max_depth, "table", &mut cells)?; + } + NodeKind::Element(tag @ ("thead" | "tbody" | "tfoot")) => { + let mut row = dom.first_child(node); + while let Some(node) = row.filter(|_| depth + 2 < max_depth) { + row = dom.next_sibling(node); + match dom.node_kind(node) { + NodeKind::Element("tr") => { + table.add_row(dom, node, depth + 2, max_depth, tag, &mut cells)? + } + kind if ignorable(kind) => {} + _ => return None, + } + } + } + NodeKind::Element("colgroup" | "col") => {} + kind if ignorable(kind) => {} + _ => return None, + } + } + if table.rows.is_empty() { + return None; + } + table.cells = cells.into_iter(); + Some(table) + } + + fn add_row + ?Sized>( + &mut self, + dom: &D, + node: Id, + depth: usize, + max_depth: usize, + section: &str, + cells: &mut Vec>, + ) -> Option<()> { + let start = cells.len(); + let mut all_headers = true; + let mut child = dom.first_child(node); + while let Some(node) = child.filter(|_| depth + 1 < max_depth) { + child = dom.next_sibling(node); + match dom.node_kind(node) { + NodeKind::Element(tag @ ("th" | "td")) => { + // GFM has no cell spans. Never use their values to allocate + // a grid or silently shift the following cells left. + if ["colspan", "rowspan"].iter().any(|name| { + dom.attribute(node, name) + .is_some_and(|span| span.trim().parse::() != Ok(1)) + }) { + return None; + } + all_headers &= tag == "th"; + cells.push(Cell { + node, + depth: depth + 1, + }); + // The first cell encountered in each column supplies its + // alignment. A synthetic header can grow to fit later rows. + if !self.has_header && cells.len() - start > self.separators.len() { + let align = dom.attribute(node, "align").unwrap_or_default().trim(); + self.separators.push(if align.eq_ignore_ascii_case("left") { + ":---" + } else if align.eq_ignore_ascii_case("right") { + "---:" + } else if align.eq_ignore_ascii_case("center") { + ":---:" + } else { + "---" + }); + } + } + kind if ignorable(kind) => {} + _ => return None, + } + } + let count = cells.len() - start; + if count == 0 { + return Some(()); + } + if self.rows.is_empty() { + // Recognize explicit headings as in Turndown's GFM plugin. When + // absent, synthesize empty headings without consuming a data row. + self.has_header = section == "thead" || (section != "tfoot" && all_headers); + } else if section == "thead" || (self.has_header && count > self.separators.len()) { + // GFM supports a single heading row. + // Markdown parsers discard cells beyond the header's width. + return None; + } + self.rows.push(count); + Some(()) + } + + pub(crate) fn finish(self) -> Output { + if self.has_complex_content { + let mut output = Output::default(); + for cell in self.content.into_iter().filter(|cell| !cell.is_empty()) { + if !output.is_empty() { + output.push_literal("\n\n"); + } + output.append(cell); + } + return output; + } + let mut output = String::new(); + if !self.has_header { + output.push('|'); + for _ in &self.separators { + output.push_str(" |"); + } + separator_row(&mut output, &self.separators); + } + let mut content = self.content.into_iter(); + for (row, count) in self.rows.into_iter().enumerate() { + if !output.is_empty() { + output.push('\n'); + } + output.push('|'); + // GFM supplies missing trailing cells. Keeping short rows short + // avoids expanding sparse input to rows * header width bytes. + for _ in 0..count { + output.push(' '); + let cell = content + .next() + .expect("every table cell was converted") + .into_string(); + for (line, text) in cell.split('\n').enumerate() { + if line > 0 { + output.push_str("
"); + } + for ch in text.trim_end_matches(is_space).chars() { + if ch == '|' { + // GFM removes this escape before parsing inline + // content, including code spans and link URLs. + output.push('\\'); + } + output.push(ch); + } + } + output.push_str(" |"); + } + if row == 0 && self.has_header { + separator_row(&mut output, &self.separators); + } + } + output.into() + } +} + +fn separator_row(output: &mut String, separators: &[&str]) { + output.push_str("\n|"); + for separator in separators { + output.push(' '); + output.push_str(separator); + output.push_str(" |"); + } +} + +fn ignorable(kind: NodeKind<'_>) -> bool { + match kind { + NodeKind::Other | NodeKind::Element("script" | "style" | "noscript" | "template") => true, + NodeKind::Text(text) => text.chars().all(is_space), + _ => false, + } +} diff --git a/moli-html2md/src/writer.rs b/moli-html2md/src/writer.rs index fcb3da3b80..c32bb6acb6 100644 --- a/moli-html2md/src/writer.rs +++ b/moli-html2md/src/writer.rs @@ -1,3 +1,5 @@ +use crate::output::Output; + // Formatting is delayed until visible content arrives. This keeps whitespace // outside newly opened/closed marks and coalesces adjacent identical styles. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -22,6 +24,8 @@ struct OpenStyle<'a> { #[derive(Default)] pub(crate) struct Writer<'a> { + // Blocks are immutable fragments; inline edits only touch the local tail. + prefix: Output, output: String, desired: Vec>, emitted: Vec>, @@ -55,6 +59,11 @@ impl<'a> Writer<'a> { self.single_line_attributes = true; } + pub(crate) fn table_cell(&mut self) { + // Literal attribute newlines must not split a Markdown table row. + self.single_line_attributes = true; + } + pub(crate) fn push_style(&mut self, style: Style<'a>) -> bool { // Repeated emphasis has the same visible meaning. Nested links cannot // be represented in Markdown, so retain the outer destination. @@ -102,7 +111,7 @@ impl<'a> Writer<'a> { continue; } self.prepare_inline(ch); - let line_start = self.output.is_empty() || self.output.ends_with('\n'); + let line_start = self.is_empty() || self.last_char() == Some('\n'); if line_start { self.line_digits = Some(0); } @@ -242,18 +251,44 @@ impl<'a> Writer<'a> { } self.boundary(before); self.flush_breaks(); + #[cfg(test)] + crate::output::record_copy(text.len()); self.output.push_str(text); self.line_digits = None; self.boundary(after); } - pub(crate) fn finish(mut self) -> String { + pub(crate) fn block_output(&mut self, output: Output, before: usize, after: usize) { + if output.is_empty() { + self.boundary(before.max(after)); + return; + } + // Closing styles before detaching the tail keeps all inline edit + // offsets local to the current String, never inside a finished block. + self.boundary(before); + self.flush_breaks(); + self.prefix.push_text(std::mem::take(&mut self.output)); + self.prefix.append(output); + self.line_digits = None; + self.boundary(after); + } + + pub(crate) fn finish(mut self) -> Output { self.flush_code(); self.close_to(0, None); self.flush_spaces(false); let end = self.output.trim_end_matches(is_space).len(); self.output.truncate(end); - self.output + self.prefix.push_text(self.output); + self.prefix + } + + fn is_empty(&self) -> bool { + self.output.is_empty() && self.prefix.is_empty() + } + + fn last_char(&self) -> Option { + self.output.chars().next_back().or(self.prefix.last_char()) } fn prepare_inline(&mut self, next: char) { @@ -289,12 +324,7 @@ impl<'a> Writer<'a> { } else { next }; - let html = is_punctuation(first) - && self - .output - .chars() - .next_back() - .is_some_and(char::is_alphanumeric); + let html = is_punctuation(first) && self.last_char().is_some_and(char::is_alphanumeric); match style { Style::Strong => self.output.push_str(if html { "" } else { "**" }), Style::Emphasis => self.output.push_str(if html { "" } else { "*" }), @@ -320,7 +350,7 @@ impl<'a> Writer<'a> { if !self.preserved_spaces.is_empty() { // Materialize any block boundary before preserving Unicode space. self.flush_breaks(); - let text = if self.output.is_empty() || self.output.ends_with('\n') { + let text = if self.is_empty() || self.last_char() == Some('\n') { self.preserved_spaces.trim_start_matches(' ') } else { &self.preserved_spaces @@ -329,7 +359,7 @@ impl<'a> Writer<'a> { self.preserved_spaces.clear(); self.line_digits = None; } - if trailing_ascii && self.space && !self.output.is_empty() && !self.output.ends_with('\n') { + if trailing_ascii && self.space && !self.is_empty() && self.last_char() != Some('\n') { self.output.push(' '); self.line_digits = None; } @@ -365,7 +395,7 @@ impl<'a> Writer<'a> { { preceding } - _ => self.output.chars().next_back(), + _ => self.last_char(), }; let closing_needs_html = next.is_some_and(char::is_alphanumeric) && preceding.is_some_and(is_punctuation); @@ -397,8 +427,11 @@ impl<'a> Writer<'a> { fn flush_breaks(&mut self) { if self.breaks > 0 { - if !self.output.is_empty() { - let existing = self.output.len() - self.output.trim_end_matches('\n').len(); + if !self.is_empty() { + let mut existing = self.output.len() - self.output.trim_end_matches('\n').len(); + if existing == self.output.len() { + existing += self.prefix.trailing_newlines(); + } for _ in existing..self.breaks { self.output.push('\n'); } diff --git a/moli-html2md/tests/conversion.rs b/moli-html2md/tests/conversion.rs index ed76b79b0b..7dc4cfcd90 100644 --- a/moli-html2md/tests/conversion.rs +++ b/moli-html2md/tests/conversion.rs @@ -205,13 +205,13 @@ fn supports_headings_images_and_non_content_elements() { } #[test] -fn expands_headerless_table_cells_as_blocks() { +fn adds_empty_headers_to_headerless_tables() { let mut dom = Tree::new(); let table = dom.element(0, "table"); let row = dom.element(table, "tr"); dom.leaf(row, "td", "one"); dom.leaf(row, "td", "two"); - assert_eq!(convert(&dom, 0), "one\n\ntwo"); + assert_eq!(convert(&dom, 0), "| | |\n| --- | --- |\n| one | two |"); } #[test] @@ -226,7 +226,7 @@ fn preserves_table_cell_boundaries_hard_breaks_and_literal_pipes() { dom.text(cell, "a|b"); dom.element(cell, "br"); dom.text(cell, "c"); - assert_eq!(convert(&dom, 0), "Key\n\na|b \nc"); + assert_eq!(convert(&dom, 0), "| Key |\n| ---: |\n| a\\|b
c |"); } #[test] @@ -317,13 +317,42 @@ fn converts_deep_tables_on_a_small_thread_stack() { max_depth: usize::MAX, ..Options::default() }); - assert_eq!(converter.convert(&dom, 0), "deep"); + assert_eq!(converter.convert(&dom, 0), "| |\n| --- |\n| deep |"); }) .expect("spawn small-stack test") .join() .expect("nested tables should not recurse"); } +#[test] +fn converts_nested_header_tables_on_a_small_thread_stack() { + std::thread::Builder::new() + .stack_size(64 * 1024) + .spawn(|| { + let mut dom = Tree::new(); + let mut node = 0; + for _ in 0..1_000 { + let table = dom.element(node, "table"); + let row = dom.element(table, "tr"); + dom.leaf(row, "th", "Header"); + let row = dom.element(table, "tr"); + node = dom.element(row, "td"); + } + dom.text(node, "deep"); + let converter = Converter::new(Options { + max_depth: usize::MAX, + ..Options::default() + }); + let actual = converter.convert(&dom, 0); + assert_eq!(actual.matches("Header").count(), 1_000); + assert_eq!(actual.matches("| --- |").count(), 1); + assert!(actual.ends_with("| deep |")); + }) + .expect("spawn small-stack test") + .join() + .expect("header table conversion should not recurse"); +} + #[test] fn conversion_is_limited_to_the_supplied_subtree() { let mut dom = Tree::new(); diff --git a/moli-html2md/tests/fixtures/data-table.html b/moli-html2md/tests/fixtures/data-table.html new file mode 100644 index 0000000000..cea27ca719 --- /dev/null +++ b/moli-html2md/tests/fixtures/data-table.html @@ -0,0 +1,8 @@ + + +
+ + + +
NameAge
Alice30
Bob_under12
+ diff --git a/moli-html2md/tests/fixtures/data-table.md b/moli-html2md/tests/fixtures/data-table.md new file mode 100644 index 0000000000..999226fb0e --- /dev/null +++ b/moli-html2md/tests/fixtures/data-table.md @@ -0,0 +1,4 @@ +| Name | Age | +| --- | --- | +| Alice | 30 | +| Bob\_under | 12 | diff --git a/moli-html2md/tests/fixtures/hacker-news-layout.md b/moli-html2md/tests/fixtures/hacker-news-layout.md index 7abae5463d..5dd23b2c76 100644 --- a/moli-html2md/tests/fixtures/hacker-news-layout.md +++ b/moli-html2md/tests/fixtures/hacker-news-layout.md @@ -1,8 +1,6 @@ -**[News](news)** - -[new](newest) | [past](front) - -[login](login?goto=news) +| | | | +| --- | --- | --- | +| **[News](news)** | [new](newest) \| [past](front) | [login](login?goto=news) | 1\. diff --git a/moli-html2md/tests/fixtures/headerless-data-table.html b/moli-html2md/tests/fixtures/headerless-data-table.html new file mode 100644 index 0000000000..99a4520c11 --- /dev/null +++ b/moli-html2md/tests/fixtures/headerless-data-table.html @@ -0,0 +1,7 @@ + + + + + +
苹果5 元
香蕉3 元
+ diff --git a/moli-html2md/tests/fixtures/headerless-data-table.md b/moli-html2md/tests/fixtures/headerless-data-table.md new file mode 100644 index 0000000000..d4eee65d24 --- /dev/null +++ b/moli-html2md/tests/fixtures/headerless-data-table.md @@ -0,0 +1,4 @@ +| | | +| --- | --- | +| 苹果 | 5 元 | +| 香蕉 | 3 元 | diff --git a/moli-html2md/tests/regressions.rs b/moli-html2md/tests/regressions.rs index b36ea76f1b..1eae5b2994 100644 --- a/moli-html2md/tests/regressions.rs +++ b/moli-html2md/tests/regressions.rs @@ -181,7 +181,16 @@ fn table_attributes_keep_the_same_meaning_as_attributes_outside_tables() { ] { let inline = rendered_html(&markdown(content, false)); let html = format!("
{content}
"); - assert_eq!(rendered_html(&markdown(&html, false)), inline, "{html}",); + let inline = inline + .strip_prefix("

") + .unwrap() + .strip_suffix("

\n") + .unwrap(); + assert_eq!( + rendered_html(&markdown(&html, false)), + format!("\n
{inline}
\n"), + "{html}" + ); } } diff --git a/moli-html2md/tests/tables.rs b/moli-html2md/tests/tables.rs index d323863704..5e2bf3ac65 100644 --- a/moli-html2md/tests/tables.rs +++ b/moli-html2md/tests/tables.rs @@ -19,19 +19,24 @@ fn hacker_news_layout_tables_preserve_story_and_metadata_order() { include_str!("fixtures/hacker-news-layout.md").trim_end() ); let html = rendered_html(&result); - assert!(!html.contains("")); + // The simple navigation row becomes a table; nested/spanning story layout + // still expands into blocks in the original content order. + assert_eq!(html.matches("
").count(), 1); assert!(!html.contains("
")); assert_eq!(html.matches("before

\n

Key

\n

Value

\n

a|b

\n

c|d

\n

after

\n" + "

before

\n
\n\n
KeyValue
a|bc|d
\n

after

\n" ); } @@ -43,7 +48,10 @@ fn wrapping_a_table_preserves_its_content() { markdown(&format!("
{data}
")), markdown(data) ); - assert_eq!(markdown(data), "one\n\ntwo\n\nthree\n\nfour"); + assert_eq!( + markdown(data), + "| | |\n| --- | --- |\n| one | two |\n| three | four |" + ); } #[test] @@ -59,7 +67,10 @@ fn table_roles_do_not_change_conversion() { let html = format!( "
NameValue
onetwo
" ); - assert_eq!(markdown(&html), "Name\n\nValue\n\none\n\ntwo"); + assert_eq!( + markdown(&html), + "| Name | Value |\n| --- | --- |\n| one | two |" + ); } } @@ -77,25 +88,293 @@ fn captions_sections_headers_and_spacer_rows_preserve_content_order() { for (html, expected) in [ ( "
ab
cd
", - "a\n\nb\n\nc\n\nd", + "| | |\n| --- | --- |\n| a | b |\n| c | d |", ), ( "
Data
ab
cd
", - "Data\n\na\n\nb\n\nc\n\nd", + "Data\n\n| | |\n| --- | --- |\n| a | b |\n| c | d |", ), ( "
ab
cd
end
", - "a\n\nb\n\nc\n\nd\n\nend", + "| a | b |\n| --- | --- |\n| c | d |\n| end |", ), ( "
ab
cd
", - "a\n\nb\n\nc\n\nd", + "| a | b |\n| --- | --- |\n| c | d |", ), ] { assert_eq!(markdown(html), expected, "{html}"); } } +#[test] +fn data_table_outputs_gfm_and_preserves_literal_underscores() { + let actual = markdown(include_str!("fixtures/data-table.html")); + assert_eq!(actual, include_str!("fixtures/data-table.md").trim_end()); + assert_eq!( + rendered_html(&actual), + "\n\n\n
NameAge
Alice30
Bob_under12
\n" + ); +} + +#[test] +fn headerless_data_table_keeps_every_data_row_under_empty_headers() { + let actual = markdown(include_str!("fixtures/headerless-data-table.html")); + assert_eq!( + actual, + include_str!("fixtures/headerless-data-table.md").trim_end() + ); + assert_eq!( + rendered_html(&actual), + "\n\n\n
苹果5 元
香蕉3 元
\n" + ); +} + +#[test] +fn headerless_tables_use_the_widest_row_without_discarding_cells() { + let actual = markdown( + "
one
twothree
four
", + ); + assert_eq!( + actual, + "| | |\n| ---: | :---: |\n| one |\n| two | three |\n| four |" + ); + let rendered = rendered_html(&actual); + assert_eq!(rendered.matches("three")); +} + +#[test] +fn recognizes_headers_through_sections_whitespace_and_comments() { + for header in [ + "\nNameAge\n", + "\nNameAge\n", + " \n NameAge", + ] { + let html = + format!("{header}
Alice30
"); + assert_eq!( + markdown(&html), + "| Name | Age |\n| --- | --- |\n| Alice | 30 |", + "{html}" + ); + } + assert_eq!( + markdown( + "
NameAge
Alice30
" + ), + "| | |\n| --- | --- |\n| Name | Age |\n| Alice | 30 |" + ); +} + +#[test] +fn preserves_caption_alignment_empty_cells_and_short_rows() { + let html = "
People
NameRoleAge
Alice30
Bob
"; + let actual = markdown(html); + assert_eq!( + actual, + "**People**\n\n| Name | Role | Age |\n| :--- | :---: | ---: |\n| Alice | | 30 |\n| Bob |\n| | | |" + ); + let rendered = rendered_html(&actual); + assert!( + rendered.contains("Name"), + "{rendered}" + ); + assert!( + rendered.contains("Role"), + "{rendered}" + ); + assert!( + rendered.contains("Age"), + "{rendered}" + ); + assert_eq!(rendered.matches("second

*third*

fourth |" + ); + assert!( + rendered_html(&actual) + .contains("first
second

third

fourth") + ); +} + +#[test] +fn literal_pipes_survive_in_text_code_links_and_image_attributes() { + for slashes in 0..=3 { + let content = format!("a{}|b", "\\".repeat(slashes)); + let html = format!( + "
TextCodeLinkImage
{content}{content}{content}{content}
" + ); + let actual = markdown(&html); + let rendered = rendered_html(&actual); + assert_eq!(rendered.matches("").count(), 4, "{actual}"); + assert_eq!(rendered.matches("").count(), 4, "{actual}"); + assert!( + rendered.contains(&format!("{content}")), + "{rendered}" + ); + assert!( + rendered.contains(&format!("{content}")), + "{rendered}" + ); + assert!( + rendered.contains("href=\"/a%7Cb\" title=\"a|b\""), + "{rendered}" + ); + assert!( + rendered.contains(&format!("alt=\"{content}\"")), + "{rendered}" + ); + } +} + +#[test] +fn unrepresentable_tables_expand_without_losing_or_duplicating_content() { + for row in [ + "onetwo", + "onetwo", + "onetwo", + "onetwo", + "onetwoextra", + ] { + let actual = markdown(&format!( + "{row}
AB
" + )); + let extra = if row.contains("extra") { + "\n\nextra" + } else { + "" + }; + assert_eq!(actual, format!("A\n\nB\n\none\n\ntwo{extra}")); + } + for content in [ + "
  code\nnext\n
", + "
  • item
", + "

Title

", + "
quote
", + "
nested
", + ] { + for cell in ["th", "td"] { + let actual = markdown(&format!( + "<{cell}>A<{cell}>B
one{content}
" + )); + assert_eq!( + actual, + format!("A\n\nB\n\none\n\n{}", markdown(content)), + "{cell}: {content}" + ); + } + } +} + +#[test] +fn table_blocks_work_inside_lists_and_blockquotes() { + let table = "
AB
onetwo
"; + for wrapper in [ + format!("
{table}
"), + format!("
  • {table}
"), + ] { + let actual = markdown(&wrapper); + let rendered = rendered_html(&actual); + assert_eq!(rendered.matches("").count(), 1, "{actual}"); + assert_eq!(rendered.matches("")); + assert_eq!(markdown("
").count(), 2, "{actual}"); + } +} + +#[test] +fn table_blocks_preserve_surrounding_inline_styles() { + for (tag, open, close) in [ + ("strong", "**", "**"), + ("a href='/outer'", "[", "](/outer)"), + ] { + let end_tag = tag.split(' ').next().unwrap(); + let actual = markdown(&format!( + "<{tag}>before
cell
after" + )); + assert_eq!( + actual, + format!( + "{open}before{close}\n\n| |\n| --- |\n| {open}cell{close} |\n\n{open}after{close}" + ) + ); + } +} + +#[test] +fn multiple_heading_rows_fall_back_to_blocks() { + assert_eq!( + markdown( + "
AB
onetwo
threefour
" + ), + "A\n\nB\n\none\n\ntwo\n\nthree\n\nfour" + ); +} + +#[test] +fn footer_only_tables_keep_the_footer_as_data() { + assert_eq!( + markdown("
Footer
"), + "| |\n| --- |\n| Footer |" + ); +} + +#[test] +fn empty_headers_and_unit_spans_are_representable() { + let actual = markdown( + "
onetwo
", + ); + assert_eq!(actual, "| | |\n| --- | --- |\n| one | two |"); + assert!(rendered_html(&actual).contains("
onetwo
"), ""); + assert_eq!(markdown("
"), ""); +} + +#[test] +fn short_rows_do_not_expand_to_a_large_grid() { + for cell in ["th", "td"] { + let html = format!( + "{}{}
", + format!("<{cell}>Header").repeat(64), + "cell".repeat(64) + ); + let actual = markdown(&html); + assert!( + actual.len() < html.len(), + "short rows must not be padded to the header width" + ); + let rendered = rendered_html(&actual); + assert_eq!(rendered.matches("cell").count(), 64); + let data_rows = if cell == "th" { 64 } else { 65 }; + assert_eq!(rendered.matches("").count(), data_rows * 64); + } +} + +#[test] +fn table_cells_respect_the_depth_limit() { + let dom = Tree::parse( + "
Heading
visiblehidden
", + ); + let converter = Converter::new(Options { + max_depth: 6, + ..Options::default() + }); + assert_eq!( + converter.convert(&dom, dom.root), + "| Heading |\n| --- |\n| visible |" + ); + assert_eq!( + convert(&dom, dom.root), + "| Heading |\n| --- |\n| visible*hidden* |" + ); +} + #[test] fn table_presentation_attributes_do_not_change_conversion() { for attributes in [ @@ -107,7 +386,10 @@ fn table_presentation_attributes_do_not_change_conversion() { let html = format!( "
HomeHelp
" ); - assert_eq!(markdown(&html), "[Home](/)\n\n[Help](/help)"); + assert_eq!( + markdown(&html), + "| | |\n| --- | --- |\n| [Home](/) | [Help](/help) |" + ); } } @@ -116,12 +398,15 @@ fn nested_tables_obey_the_conversion_depth_limit() { let dom = Tree::parse( "
visible
too deep
", ); - for max_depth in [6, 7] { + for (max_depth, expected) in [(6, "| |\n| --- |\n| visible |"), (7, "visible")] { let converter = Converter::new(Options { max_depth, ..Options::default() }); - assert_eq!(converter.convert(&dom, dom.root), "visible"); + assert_eq!(converter.convert(&dom, dom.root), expected); } - assert_eq!(convert(&dom, dom.root), "visible\n\ntoo deep"); + assert_eq!( + convert(&dom, dom.root), + "visible\n\n| |\n| --- |\n| too deep |" + ); } diff --git a/moli-html2md/tests/turndown/README.md b/moli-html2md/tests/turndown/README.md index 3e9dd8595b..703252d8d2 100644 --- a/moli-html2md/tests/turndown/README.md +++ b/moli-html2md/tests/turndown/README.md @@ -29,8 +29,10 @@ There are also exact Markdown assertions in the original and regression suites. These preserve bare pre blocks and avoid upstream losses of literal characters, emphasis, whitespace, and list boundaries. They remain active assertions; no case is skipped. A reference change that makes a difference obsolete also fails -the test. Separate converter tests cover table expansion using Turndown core's -ordinary block behavior and strikethrough as a Moli extension. +the test. Separate converter tests cover GFM tables and strikethrough as Moli +extensions. Explicit table headers follow Turndown's GFM plugin; simple headerless +tables receive empty headings. Complex tables retain the core's block expansion +rather than the plugin's raw HTML fallback. ## Findings tracked outside the reference corpus diff --git a/moli-renderer-v8/src/runtime/page_dump.rs b/moli-renderer-v8/src/runtime/page_dump.rs index fac217816b..5cf4cf192c 100644 --- a/moli-renderer-v8/src/runtime/page_dump.rs +++ b/moli-renderer-v8/src/runtime/page_dump.rs @@ -373,17 +373,23 @@ mod tests { } #[test] - fn markdown_renderer_expands_table_cells() { + fn markdown_renderer_outputs_gfm_tables() { assert_eq!( markdown_from_html( "
NameCount
moli2
" ), - "Name\n\nCount\n\nmoli\n\n2" + "| Name | Count |\n| --- | --- |\n| moli | 2 |" + ); + assert_eq!( + markdown_from_html( + "
苹果5 元
香蕉3 元
" + ), + "| | |\n| --- | --- |\n| 苹果 | 5 元 |\n| 香蕉 | 3 元 |" ); } #[test] - fn markdown_renderer_flattens_hacker_news_layout_tables() { + fn markdown_renderer_preserves_hacker_news_layout_content() { let dom = HtmlParser::SCRIPTING_DISABLED.parse( test_url(), include_str!("../../../moli-html2md/tests/fixtures/hacker-news-layout.html").to_owned(), diff --git a/moli/tests/fetch_cli/markdown.rs b/moli/tests/fetch_cli/markdown.rs index 3eadba80a2..36711ea604 100644 --- a/moli/tests/fetch_cli/markdown.rs +++ b/moli/tests/fetch_cli/markdown.rs @@ -6,7 +6,11 @@ const HTML: &str = include_str!("../../../moli-html2md/tests/fixtures/hacker-new const MARKDOWN: &str = include_str!("../../../moli-html2md/tests/fixtures/hacker-news-layout.md"); fn assert_layout_table_dump(args: &[&str]) -> Result<()> { - let url = format!("data:text/html;base64,{}", STANDARD.encode(HTML)); + assert_markdown_dump(HTML, MARKDOWN, args) +} + +fn assert_markdown_dump(html: &str, markdown: &str, args: &[&str]) -> Result<()> { + let url = format!("data:text/html;base64,{}", STANDARD.encode(html)); let output = Command::new(env!("CARGO_BIN_EXE_moli")) .args([ "fetch", @@ -25,7 +29,7 @@ fn assert_layout_table_dump(args: &[&str]) -> Result<()> { "{}", String::from_utf8_lossy(&output.stderr) ); - assert_eq!(String::from_utf8(output.stdout)?, MARKDOWN.trim_end()); + assert_eq!(String::from_utf8(output.stdout)?, markdown.trim_end()); Ok(()) } @@ -38,3 +42,27 @@ fn hacker_news_layout_tables_dump_markdown() -> Result<()> { fn hacker_news_layout_tables_dump_markdown_with_layout() -> Result<()> { assert_layout_table_dump(&["--layout"]) } + +#[test] +fn data_tables_dump_gfm_markdown_with_and_without_layout() -> Result<()> { + for args in [&[][..], &["--layout"][..]] { + assert_markdown_dump( + include_str!("../../../moli-html2md/tests/fixtures/data-table.html"), + include_str!("../../../moli-html2md/tests/fixtures/data-table.md"), + args, + )?; + } + Ok(()) +} + +#[test] +fn headerless_tables_dump_gfm_markdown_with_and_without_layout() -> Result<()> { + for args in [&[][..], &["--layout"][..]] { + assert_markdown_dump( + include_str!("../../../moli-html2md/tests/fixtures/headerless-data-table.html"), + include_str!("../../../moli-html2md/tests/fixtures/headerless-data-table.md"), + args, + )?; + } + Ok(()) +}