diff --git a/Cargo.lock b/Cargo.lock index 5a86089..1e5adbd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -218,6 +218,16 @@ version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" +[[package]] +name = "mdscrub" +version = "0.1.0" +dependencies = [ + "clap", + "corpus-core", + "regex", + "textkit", +] + [[package]] name = "memchr" version = "2.8.3" @@ -236,6 +246,7 @@ version = "0.1.0" dependencies = [ "chunkmd", "corpus-core", + "mdscrub", "serde_json", "textkit", ] diff --git a/Cargo.toml b/Cargo.toml index f5c045f..52d0c9e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,7 @@ repository = "https://git.pumpin.online/funkpower/catena" corpus-core = { path = "crates/corpus-core", version = "0.1.0" } textkit = { path = "crates/textkit", version = "0.1.0" } chunkmd = { path = "crates/chunkmd", version = "0.1.0" } +mdscrub = { path = "crates/mdscrub", version = "0.1.0" } clap = { version = "4", features = ["derive"] } fancy-regex = "0.16" diff --git a/crates/chunkmd/src/pages.rs b/crates/chunkmd/src/pages.rs index f853e07..6e90f0c 100644 --- a/crates/chunkmd/src/pages.rs +++ b/crates/chunkmd/src/pages.rs @@ -76,7 +76,7 @@ pub fn split_by_page(markdown: &str) -> Vec> { let text = trim(&markdown[start..end]); if !text.is_empty() { pages.push(Page { - number: Some(page_number(&markdown[marker.start()..marker.end()])), + number: Some(textkit::decimal(&markdown[marker.start()..marker.end()])), text, offset: start, }); @@ -138,7 +138,7 @@ impl PageIndex { pub fn new(markdown: &str) -> Self { Self( PAGE.find_iter(markdown) - .map(|marker| (marker.start(), page_number(marker.as_str()))) + .map(|marker| (marker.start(), textkit::decimal(marker.as_str()))) .collect(), ) } @@ -193,20 +193,6 @@ pub fn page_chapters(chapters: &[Chapter]) -> HashMap { out } -/// Read the digits out of a page marker. -/// -/// Digits are matched by the Unicode class, so a marker could in principle -/// carry non-ASCII decimal digits; those count the same way here as they -/// would in Python. A number too large for the field saturates rather than -/// wrapping — see `parity/KNOWN.md`. -fn page_number(marker: &str) -> u32 { - let mut number: u32 = 0; - for digit in marker.chars().filter_map(|c| c.to_digit(10)) { - number = number.saturating_mul(10).saturating_add(digit); - } - number -} - fn trim(text: &str) -> &str { text.trim_matches(textkit::is_space) } diff --git a/crates/mdscrub/Cargo.toml b/crates/mdscrub/Cargo.toml new file mode 100644 index 0000000..82a9406 --- /dev/null +++ b/crates/mdscrub/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "mdscrub" +description = "Cleans converted markdown: residual HTML, control characters, headings, page markers." +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +repository.workspace = true + +[dependencies] +clap.workspace = true +corpus-core.workspace = true +regex.workspace = true +textkit.workspace = true + +[lints] +workspace = true diff --git a/crates/mdscrub/src/heading.rs b/crates/mdscrub/src/heading.rs new file mode 100644 index 0000000..d9af8fc --- /dev/null +++ b/crates/mdscrub/src/heading.rs @@ -0,0 +1,149 @@ +//! Making a heading line readable again. +//! +//! A converted heading arrives carrying whatever the source used to style it: +//! link wrappers around the whole title, attribute blocks, anchor spans, bold +//! markers around a title that was set in capitals. The heading text is what +//! every chunk in the chapter will be labelled with, so it is worth cleaning +//! properly rather than passing through. + +use std::sync::LazyLock; + +use regex::Regex; + +fn pattern(source: &str) -> Regex { + Regex::new(source).expect("a valid pattern") +} + +/// `[text](url)` — a heading wrapped in a link to its own anchor. +static LINK: LazyLock = LazyLock::new(|| pattern(r"\[([^\]]*)\]\([^)]*\)")); + +/// `[text]{.cls}` — pandoc's bracketed inline span. +static BRACKETED: LazyLock = LazyLock::new(|| pattern(r"\[([^\]]+)\]\{[^}]*\}")); + +/// A trailing `{.attr}` block. +static ATTRIBUTE_TAIL: LazyLock = LazyLock::new(|| pattern(r"\s*\{[^}]*\}\s*$")); + +/// Any inline tag. Broader than the one used on body text, because a heading +/// has no code fences to protect. +static TAG: LazyLock = LazyLock::new(|| pattern(r"<[^>]+>")); + +static SPACE_RUN: LazyLock = LazyLock::new(|| pattern(r"\s+")); + +/// A heading line, and the text after its hashes. +static HEADING: LazyLock = LazyLock::new(|| pattern(r"^(#{1,6})\s+(.*)$")); + +/// A page anchor: ``, ``, and the rest of +/// the shapes an EPUB uses to say where a printed page began. +pub static PAGE_ANCHOR: LazyLock = LazyLock::new(|| { + pattern( + r#"(?i)<(?:span|a|div)\b[^>]*\bid\s*=\s*["'][^"']*?page[-_]?(\d+)[^"']*["'][^>]*>(?:)?"#, + ) +}); + +/// Clean one heading's text. +pub fn clean(raw: &str) -> String { + let text = LINK.replace_all(raw, "$1"); + let text = BRACKETED.replace_all(&text, "$1"); + let text = ATTRIBUTE_TAIL.replace_all(&text, ""); + let text = TAG.replace_all(&text, ""); + let text = SPACE_RUN.replace_all(&text, " "); + let text = text.trim_matches(textkit::is_space); + // Emphasis markers around a title that was set in capitals say nothing + // once the title is a label on a search result. + text.replace("**", "").replace("__", "") +} + +/// Clean every heading in a document, moving any page anchor found in a +/// heading to a marker on its own line just after it. +/// +/// After the heading rather than before, so that the heading keeps its offset +/// relative to the body text that follows, and the chapter title stays whole. +pub fn clean_all(markdown: &str) -> String { + let mut out: Vec = Vec::new(); + for line in markdown.split('\n') { + let Some(found) = HEADING.captures(line) else { + out.push(line.to_string()); + continue; + }; + let hashes = &found[1]; + let raw = &found[2]; + let pages: Vec = PAGE_ANCHOR + .captures_iter(raw) + .map(|anchor| textkit::decimal(&anchor[1])) + .collect(); + let cleaned = clean(&PAGE_ANCHOR.replace_all(raw, "")); + if !cleaned.is_empty() { + out.push(format!("{hashes} {cleaned}")); + } + for page in pages { + out.push(String::new()); + out.push(format!("")); + } + } + out.join("\n") +} + +/// Turn the page anchors left in the body into page markers. +pub fn anchors_to_markers(markdown: &str) -> String { + PAGE_ANCHOR + .replace_all(markdown, |found: ®ex::Captures| { + format!("\n\n\n\n", textkit::decimal(&found[1])) + }) + .into_owned() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_link_wrapper_comes_off() { + assert_eq!(clean("[I. The Setting](#ch1)"), "I. The Setting"); + } + + #[test] + fn a_bracketed_span_comes_off() { + assert_eq!(clean("[The Quarrel]{.chapter-title}"), "The Quarrel"); + } + + #[test] + fn a_trailing_attribute_block_comes_off() { + assert_eq!(clean("The Break {#ch3 .unnumbered}"), "The Break"); + } + + #[test] + fn tags_come_off_and_whitespace_collapses() { + assert_eq!(clean("The Break "), "The Break"); + } + + #[test] + fn emphasis_markers_come_off() { + assert_eq!(clean("**THE SETTLEMENT**"), "THE SETTLEMENT"); + } + + #[test] + fn a_page_anchor_in_a_heading_becomes_a_marker_after_it() { + let cleaned = clean_all("# The Quarrel"); + assert_eq!(cleaned, "# The Quarrel\n\n"); + } + + #[test] + fn a_heading_that_was_only_an_anchor_leaves_only_the_marker() { + let cleaned = clean_all("## "); + assert_eq!(cleaned, "\n"); + } + + #[test] + fn body_anchors_become_markers() { + let cleaned = anchors_to_markers("text more"); + assert!(cleaned.contains("")); + } + + #[test] + fn a_line_that_is_not_a_heading_is_untouched() { + assert_eq!( + clean_all("#no space is not a heading"), + "#no space is not a heading" + ); + } +} diff --git a/crates/mdscrub/src/html.rs b/crates/mdscrub/src/html.rs new file mode 100644 index 0000000..37524f6 --- /dev/null +++ b/crates/mdscrub/src/html.rs @@ -0,0 +1,122 @@ +//! Removing the HTML that survives conversion to markdown. + +use std::sync::LazyLock; + +use regex::Regex; + +/// An opening or closing inline tag. +/// +/// Converted markdown is full of them: pandoc's commonmark output keeps an +/// EPUB's styling spans and anchors, and a PDF converter emits anchor spans +/// and superscript note references. None of it means anything once the page +/// anchors have been pulled out, and all of it pollutes an embedding, a +/// full-text index, and every result a reader sees. +/// +/// Comments are deliberately not matched: `` has to survive. +static TAG: LazyLock = LazyLock::new(|| Regex::new(r"]*>").expect("valid")); + +/// A tag that stands for a line break rather than for styling. +static BREAK: LazyLock = LazyLock::new(|| Regex::new(r"(?i)").expect("valid")); + +/// The start of a fenced code block. +static FENCE: LazyLock = LazyLock::new(|| Regex::new(r"^\s*(```|~~~)").expect("valid")); + +/// Control characters that must never reach a text column. +/// +/// A database will reject `0x00` outright. The other C0 controls — tab, +/// newline and carriage return excepted — are storable and are extraction +/// garbage: bad scans and malformed PDFs emit them routinely. +static CONTROL: LazyLock = + LazyLock::new(|| Regex::new(r"[\x00-\x08\x0b\x0c\x0e-\x1f]").expect("valid")); + +/// Three or more newlines, left behind wherever a tag-only line was removed. +static BLANK_RUN: LazyLock = LazyLock::new(|| Regex::new(r"\n{3,}").expect("valid")); + +/// Strip inline HTML, keeping the text inside it and the HTML comments. +/// +/// Fenced code blocks are left alone, so a book that quotes HTML keeps its +/// examples. `
` becomes a line break rather than nothing, because +/// deleting it welds the words on either side into one — which is a spelling +/// mistake in the index that no reader can search for. +pub fn strip_tags(markdown: &str) -> String { + let mut out = Vec::new(); + let mut in_fence = false; + for line in markdown.split('\n') { + if FENCE.is_match(line) { + in_fence = !in_fence; + out.push(line.to_string()); + continue; + } + if in_fence { + out.push(line.to_string()); + } else { + let line = BREAK.replace_all(line, "\n"); + out.push(TAG.replace_all(&line, "").into_owned()); + } + } + out.join("\n") +} + +/// Remove the control characters a text column cannot hold. +pub fn scrub_control(text: &str) -> String { + CONTROL.replace_all(text, "").into_owned() +} + +/// Collapse runs of blank lines to a single blank line. +pub fn collapse_blank_runs(text: &str) -> String { + BLANK_RUN.replace_all(text, "\n\n").into_owned() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn styling_tags_go_and_their_text_stays() { + assert_eq!( + strip_tags("The second estate"), + "The second estate" + ); + } + + #[test] + fn html_comments_survive() { + assert_eq!(strip_tags(""), ""); + } + + #[test] + fn a_break_tag_does_not_weld_the_words_either_side_of_it() { + assert_eq!(strip_tags("first
second"), "first\nsecond"); + assert_eq!(strip_tags("first
second"), "first\nsecond"); + assert_eq!(strip_tags("first
second"), "first\nsecond"); + } + + #[test] + fn fenced_code_keeps_its_tags() { + let markdown = "before x\n\n```html\nkept\n```\n\nafter y"; + let cleaned = strip_tags(markdown); + assert!(cleaned.contains("kept")); + assert!(cleaned.contains("before x")); + assert!(cleaned.contains("after y")); + } + + #[test] + fn a_tag_split_across_two_lines_is_left_alone() { + // Matching per line is what keeps a stray `<` in prose from eating + // the rest of the document. + let markdown = "a < b and\nc > d"; + assert_eq!(strip_tags(markdown), markdown); + } + + #[test] + fn control_characters_go_but_tabs_and_newlines_stay() { + assert_eq!(scrub_control("a\u{0}b\u{1}c"), "abc"); + assert_eq!(scrub_control("a\tb\nc\r\n"), "a\tb\nc\r\n"); + } + + #[test] + fn blank_line_runs_collapse_to_one() { + assert_eq!(collapse_blank_runs("a\n\n\n\n\nb"), "a\n\nb"); + assert_eq!(collapse_blank_runs("a\n\nb"), "a\n\nb"); + } +} diff --git a/crates/mdscrub/src/lib.rs b/crates/mdscrub/src/lib.rs new file mode 100644 index 0000000..42a5f2e --- /dev/null +++ b/crates/mdscrub/src/lib.rs @@ -0,0 +1,69 @@ +//! Cleans converted markdown. +//! +//! Conversion from a PDF or an EPUB leaves markdown that is readable but not +//! clean: styling tags that survived, control characters a text column will +//! reject, headings wrapped in links to their own anchors, and page +//! boundaries spelled three different ways. None of it means anything +//! downstream, and all of it ends up in an index. +//! +//! The order of the passes matters. Page boundaries are normalised first, +//! because one of the things that marks a page boundary — a form feed — is +//! also a control character that the later pass deletes. + +pub mod heading; +pub mod html; +pub mod pages; + +/// Clean a converted markdown document. +/// +/// Safe to run twice: the second run changes nothing. +pub fn scrub(markdown: &str) -> String { + let text = pages::normalize(markdown); + let text = heading::clean_all(&text); + let text = heading::anchors_to_markers(&text); + let text = html::strip_tags(&text); + let text = html::scrub_control(&text); + let text = html::collapse_blank_runs(&text); + format!("{}\n", text.trim_matches(textkit::is_space)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_converted_heading_comes_out_readable() { + let markdown = "# [I. The Setting](#ch1) {.unnumbered}"; + assert_eq!(scrub(markdown), "# I. The Setting\n\n\n"); + } + + #[test] + fn running_it_twice_changes_nothing() { + let markdown = "\n\n# The Quarrel\n\n\n\nA mill\u{0} \ + stood here.
It still does.\n\n\n\nOn."; + let once = scrub(markdown); + assert_eq!(scrub(&once), once); + } + + #[test] + fn a_page_marker_survives_every_pass() { + assert!(scrub("\n\ntext").contains("")); + } + + #[test] + fn the_result_always_ends_in_exactly_one_newline() { + for markdown in ["", "text", "text\n\n\n", " \n"] { + let cleaned = scrub(markdown); + assert!(cleaned.ends_with('\n'), "{markdown:?}"); + assert!(!cleaned.ends_with("\n\n"), "{markdown:?}"); + } + } + + #[test] + fn form_feeds_become_pages_rather_than_disappearing() { + // The control-character pass would delete a form feed, so page + // normalisation has to happen first or the boundary is lost. + let cleaned = scrub("one\u{c}two"); + assert!(cleaned.contains(""), "{cleaned:?}"); + } +} diff --git a/crates/mdscrub/src/main.rs b/crates/mdscrub/src/main.rs new file mode 100644 index 0000000..5630d89 --- /dev/null +++ b/crates/mdscrub/src/main.rs @@ -0,0 +1,62 @@ +//! Cleans converted markdown. + +use std::io::{Read, Write}; +use std::path::{Path, PathBuf}; +use std::process::ExitCode; + +use clap::Parser; +use corpus_core::exit; + +/// Clean converted markdown: drop residual HTML and control characters, +/// make headings readable, and spell every page boundary the same way. +/// +/// Reads markdown on stdin, or from a path, and writes it to stdout. Safe to +/// run twice. +#[derive(Parser)] +#[command( + version, + about, + long_about = None, + after_help = "\ +Example: + + pandoc -t commonmark book.epub | mdscrub | chunkmd > chunks.ndjson" +)] +struct Args { + /// Markdown file to read. Reads stdin when absent. + path: Option, +} + +fn main() -> ExitCode { + let args = Args::parse(); + + let markdown = match read(args.path.as_deref()) { + Ok(markdown) => markdown, + Err(e) => { + eprintln!("mdscrub: {e}"); + return ExitCode::from(exit::IO); + } + }; + + let cleaned = mdscrub::scrub(&markdown); + match std::io::stdout().write_all(cleaned.as_bytes()) { + Ok(()) => ExitCode::from(exit::OK), + // A consumer that stopped reading is not a failure. + Err(e) if e.kind() == std::io::ErrorKind::BrokenPipe => ExitCode::from(exit::OK), + Err(e) => { + eprintln!("mdscrub: {e}"); + ExitCode::from(exit::IO) + } + } +} + +fn read(path: Option<&Path>) -> std::io::Result { + match path { + Some(path) => std::fs::read_to_string(path), + None => { + let mut markdown = String::new(); + std::io::stdin().read_to_string(&mut markdown)?; + Ok(markdown) + } + } +} diff --git a/crates/mdscrub/src/pages.rs b/crates/mdscrub/src/pages.rs new file mode 100644 index 0000000..43f9b7e --- /dev/null +++ b/crates/mdscrub/src/pages.rs @@ -0,0 +1,112 @@ +//! Turning whatever a converter used for page boundaries into one marker. + +use std::sync::LazyLock; + +use regex::Regex; + +/// The separator a paginating PDF converter writes between pages, like +/// `{0}------------------------------`. Its numbering is zero-based. +static SEPARATOR: LazyLock = + LazyLock::new(|| Regex::new(r"\{(\d+)\}-{20,}[^\S\n]*\n").expect("a valid pattern")); + +/// A form feed, which is what a plain text extractor leaves between pages. +const FORM_FEED: char = '\u{c}'; + +/// Rewrite page boundaries as `` markers. +/// +/// Separators first, then form feeds, and text with neither is returned +/// unchanged — a cleanup pass has to be safe to run twice, and inventing a +/// page marker for a document that has none would not be. +pub fn normalize(text: &str) -> String { + if SEPARATOR.is_match(text) { + return from_separators(text); + } + if text.contains(FORM_FEED) { + return from_form_feeds(text); + } + text.to_string() +} + +fn from_separators(text: &str) -> String { + let mut pages: Vec = Vec::new(); + let mut at = 0; + let mut number = 1; + + for found in SEPARATOR.captures_iter(text) { + let whole = found.get(0).expect("group 0"); + let before = trim(&text[at..whole.start()]); + if !before.is_empty() { + pages.push(format!("\n\n{before}")); + } + // The separator carries the number of the page it *ends*, counted + // from zero, so the page that follows it is that number plus one. + number = textkit::decimal(&found[1]).saturating_add(1); + at = whole.end(); + } + + let last = trim(&text[at..]); + if !last.is_empty() { + pages.push(format!("\n\n{last}")); + } + pages.join("\n\n") +} + +fn from_form_feeds(text: &str) -> String { + text.split(FORM_FEED) + .map(trim) + .enumerate() + .filter(|(_, page)| !page.is_empty()) + .map(|(at, page)| format!("\n\n{page}", at + 1)) + .collect::>() + .join("\n\n") +} + +fn trim(text: &str) -> &str { + text.trim_matches(textkit::is_space) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn separators_become_markers_numbered_from_one() { + let text = "first page\n{0}------------------------------\nsecond page\n\ + {1}------------------------------\nthird page"; + assert_eq!( + normalize(text), + "\n\nfirst page\n\n\ + \n\nsecond page\n\n\ + \n\nthird page" + ); + } + + #[test] + fn form_feeds_become_markers() { + let text = "one\u{c}two\u{c}three"; + assert_eq!( + normalize(text), + "\n\none\n\n\n\ntwo\n\n\n\nthree" + ); + } + + #[test] + fn an_empty_page_is_dropped_and_the_numbering_carries_on() { + assert_eq!( + normalize("one\u{c}\u{c}three"), + "\n\none\n\n\n\nthree" + ); + } + + #[test] + fn text_with_no_page_boundaries_is_left_exactly_as_it_was() { + let text = "# A Heading\n\nSome prose, with no pagination at all.\n"; + assert_eq!(normalize(text), text); + } + + #[test] + fn a_short_dash_run_is_not_a_separator() { + let text = "before\n{0}-----\nafter"; + assert_eq!(normalize(text), text); + } +} diff --git a/crates/textkit/src/lib.rs b/crates/textkit/src/lib.rs index dbcbe9f..21cffcf 100644 --- a/crates/textkit/src/lib.rs +++ b/crates/textkit/src/lib.rs @@ -10,8 +10,10 @@ //! chunk ends and a chunk boundary that moves is a chunk that no longer //! matches what is already in an index. +pub mod number; pub mod segment; pub mod tokens; +pub use number::decimal; pub use segment::{is_space, paragraphs, sentences}; pub use tokens::{count, decode_lossy, encode, split_at_token_limit}; diff --git a/crates/textkit/src/number.rs b/crates/textkit/src/number.rs new file mode 100644 index 0000000..7711f91 --- /dev/null +++ b/crates/textkit/src/number.rs @@ -0,0 +1,91 @@ +//! Reading numbers out of text the way Python reads them. + +/// The value of a run of decimal digits, Unicode included. +/// +/// Digits are matched by the Unicode class in the patterns these tools port, +/// and Python's `int()` accepts every decimal digit, not just the ASCII ones +/// — so `page٤٢` is page forty-two there and has to be here too. Non-digits +/// are skipped, and a number too large for the field saturates rather than +/// wrapping. +pub fn decimal(text: &str) -> u32 { + let mut value: u32 = 0; + for digit in text.chars().filter_map(digit_value) { + value = value.saturating_mul(10).saturating_add(digit); + } + value +} + +/// What one decimal digit is worth, in any script. +/// +/// [`char::to_digit`] knows only the ASCII digits. Every Unicode decimal +/// block, though, runs zero to nine in consecutive codepoints, so counting +/// back to the first non-digit gives the value without a lookup table. +fn digit_value(c: char) -> Option { + if let Some(digit) = c.to_digit(10) { + return Some(digit); + } + if !c.is_numeric() { + return None; + } + let code = c as u32; + let mut value = 9; + for back in 1..=9 { + let previous = code.checked_sub(back).and_then(char::from_u32); + match previous { + Some(previous) if previous.is_numeric() => continue, + _ => { + value = back - 1; + break; + } + } + } + // Confirm it really is a run of ten. Fractions and the other numeric + // characters sit in short runs, and would otherwise be read as digits: + // `½` follows `¼`, which would make it a one. + let zero = code - value; + let ten = (zero..zero + 10).all(|code| char::from_u32(code).is_some_and(char::is_numeric)); + ten.then_some(value) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ascii_digits_read_as_themselves() { + assert_eq!(decimal(""), 412); + } + + #[test] + fn other_decimal_digits_read_too() { + assert_eq!(decimal("٤٢"), 42); + assert_eq!(decimal("42"), 42); + } + + #[test] + fn every_digit_of_a_script_is_worth_what_it_looks_like() { + for (zero, name) in [('\u{660}', "arabic-indic"), ('\u{ff10}', "fullwidth")] { + for offset in 0..10u32 { + let digit = char::from_u32(zero as u32 + offset).expect("a digit"); + assert_eq!(digit_value(digit), Some(offset), "{name} {digit}"); + } + } + } + + #[test] + fn a_character_that_is_not_a_digit_is_worth_nothing() { + for c in ['x', ' ', '-', '½'] { + assert_eq!(digit_value(c), None, "{c}"); + } + } + + #[test] + fn nothing_reads_as_zero() { + assert_eq!(decimal("page"), 0); + } + + #[test] + fn an_absurd_number_saturates_rather_than_wrapping() { + assert_eq!(decimal(&"9".repeat(40)), u32::MAX); + } +} diff --git a/parity/Cargo.toml b/parity/Cargo.toml index a5a534f..88ba7bc 100644 --- a/parity/Cargo.toml +++ b/parity/Cargo.toml @@ -15,6 +15,7 @@ python = [] [dependencies] chunkmd.workspace = true +mdscrub.workspace = true corpus-core.workspace = true serde_json.workspace = true textkit.workspace = true diff --git a/parity/KNOWN.md b/parity/KNOWN.md index aea0ade..4f2d642 100644 --- a/parity/KNOWN.md +++ b/parity/KNOWN.md @@ -18,4 +18,65 @@ affected and how often it occurs in practice. --- -None recorded yet. +## mdscrub — a line-break tag becomes a line break, not nothing + +**Python:** `strip_residual_html` removes every inline tag, `
` included, +so `the
northern end` becomes `thenorthern end`. + +**Rust:** `
`, `
` and `
` become a newline; every other tag is +still removed. + +**Why this is acceptable:** the welded word is not a formatting difference, +it is a word that does not exist. It cannot be searched for, it embeds as a +token sequence no query will produce, and it is invisible in a result because +the surrounding sentence reads normally. The divergence only affects +documents that use `
` at all — mostly verse, addresses and tables of +contents converted out of HTML — and within those, only the joins. + +**Measured:** `parity/cases/mdscrub/breaks.md`; both answers are committed as +`golden/mdscrub/breaks.md` and `golden/mdscrub/breaks.rust.md`. Three welds +in eight lines. + +## mdscrub — un-paginated text is not given a page 1 marker + +**Python:** `_normalize_marker_pages` ends by wrapping any text it found no +pagination in as `\n\n…`, so that downstream can assume a +marker exists. + +**Rust:** text with no page separators and no form feeds is returned +unchanged. + +**Why this is acceptable:** a cleanup filter has to be safe to run twice, and +the Python's version is not — a second pass wraps the first pass's marker in +another one. The assumption it exists to serve belongs to the converter that +knows it just read a PDF, not to a filter that is handed markdown of unknown +provenance. The chunker already handles a document with no page markers by +emitting chunks with no page number, which is the honest answer. + +**Measured:** not exercised by any parity case, since the Python function is +only reachable from the PDF conversion path. Covered by +`mdscrub::pages::tests::text_with_no_page_boundaries_is_left_exactly_as_it_was`. + +## chunkmd, role and footnote patterns — `\s` excludes four control characters + +**Python:** `\s` in a `str` pattern matches the Unicode `White_Space` +property *and* `U+001C`–`U+001F`, the four ASCII information separators. + +**Rust:** `regex` and `fancy-regex` match the `White_Space` property only, so +those four characters are not whitespace to a ported pattern. + +**Why this is acceptable:** every one of those codepoints is deleted by +`mdscrub` before a chunker sees it — they fall inside the control-character +class that a text column rejects. For the pattern to behave differently, a +document would have to reach `chunkmd` without having been cleaned, and carry +an information separator in exactly the position where a pattern expects +whitespace. + +Note that this does **not** apply to the sentence and paragraph splitters in +`textkit`, which match Python's definition exactly — see +`textkit::segment::is_space`. Those decide chunk boundaries, so the four +characters are handled there rather than argued about. + +**Measured:** zero differences over the parity corpus; the sample in +`parity/src/sample.rs` includes `U+001C` deliberately and the token counts +agree over all ten thousand lines. diff --git a/parity/cases/mdscrub/anchors.md b/parity/cases/mdscrub/anchors.md new file mode 100644 index 0000000..fba6111 --- /dev/null +++ b/parity/cases/mdscrub/anchors.md @@ -0,0 +1,11 @@ +# The Quarrel + +The mill stood at the northern end of the valley. + + + +Two miles downstream the abbey had built its own. + +## + +Text under a heading that was nothing but an anchor. diff --git a/parity/cases/mdscrub/blanks.md b/parity/cases/mdscrub/blanks.md new file mode 100644 index 0000000..14396fc --- /dev/null +++ b/parity/cases/mdscrub/blanks.md @@ -0,0 +1,10 @@ +First paragraph. + + + + +Second paragraph after four blank lines. + + + +Third paragraph after a line that was nothing but a tag. diff --git a/parity/cases/mdscrub/breaks.md b/parity/cases/mdscrub/breaks.md new file mode 100644 index 0000000..613cad4 --- /dev/null +++ b/parity/cases/mdscrub/breaks.md @@ -0,0 +1,5 @@ +The address was set with explicit line breaks:
Twelve Mill Lane
The +Valley
and the county beyond. + +A second paragraph, where the break sits mid sentence — the mill stood at +the
northern end of the valley, where the fall was steepest. diff --git a/parity/cases/mdscrub/control.md b/parity/cases/mdscrub/control.md new file mode 100644 index 0000000..e1629d7 Binary files /dev/null and b/parity/cases/mdscrub/control.md differ diff --git a/parity/cases/mdscrub/fenced.md b/parity/cases/mdscrub/fenced.md new file mode 100644 index 0000000..b6773f2 --- /dev/null +++ b/parity/cases/mdscrub/fenced.md @@ -0,0 +1,14 @@ +Ordinary prose before the fence. + +```html +This is an example and must survive. +So must this. +``` + +Ordinary prose after the fence. + +~~~xml +Also kept. +~~~ + +The end of it. diff --git a/parity/cases/mdscrub/headings.md b/parity/cases/mdscrub/headings.md new file mode 100644 index 0000000..91f0aa1 --- /dev/null +++ b/parity/cases/mdscrub/headings.md @@ -0,0 +1,11 @@ +# [I. The Setting](#ch1) {.unnumbered} + +## [The Quarrel]{.chapter-title} + +### **THE SETTLEMENT** + +#### A Spaced Heading + +#not a heading + +Body text follows, and is left alone apart from its tags. diff --git a/parity/cases/mdscrub/mixed.md b/parity/cases/mdscrub/mixed.md new file mode 100644 index 0000000..11834d8 --- /dev/null +++ b/parity/cases/mdscrub/mixed.md @@ -0,0 +1,12 @@ +# [III. The Second Estate](#ch3) + +The obligation ran both ways, and contemporaries said so plainly +whenever they were asked.4 + + + +What the rolls do not say is how often the obligation was tested. + +## [Notes]{.section-title} + +4. Bloch, *Feudal Society*, vol. 2, p. 17. diff --git a/parity/cases/mdscrub/tags.md b/parity/cases/mdscrub/tags.md new file mode 100644 index 0000000..23a199d --- /dev/null +++ b/parity/cases/mdscrub/tags.md @@ -0,0 +1,11 @@ +The second estate held its land in +exchange for service, and the service was +1 military. + +
+ +A paragraph inside a div that means nothing once the styling is gone. + +
+ +Text with an angle bracket in it: a < b and c > d, which is not a tag. diff --git a/parity/golden/mdscrub/anchors.md b/parity/golden/mdscrub/anchors.md new file mode 100644 index 0000000..f29f5cf --- /dev/null +++ b/parity/golden/mdscrub/anchors.md @@ -0,0 +1,13 @@ +# The Quarrel + + + +The mill stood at the northern end of the valley. + + + +Two miles downstream the abbey had built its own. + + + +Text under a heading that was nothing but an anchor. diff --git a/parity/golden/mdscrub/blanks.md b/parity/golden/mdscrub/blanks.md new file mode 100644 index 0000000..7ca6888 --- /dev/null +++ b/parity/golden/mdscrub/blanks.md @@ -0,0 +1,5 @@ +First paragraph. + +Second paragraph after four blank lines. + +Third paragraph after a line that was nothing but a tag. diff --git a/parity/golden/mdscrub/breaks.md b/parity/golden/mdscrub/breaks.md new file mode 100644 index 0000000..a56d84c --- /dev/null +++ b/parity/golden/mdscrub/breaks.md @@ -0,0 +1,5 @@ +The address was set with explicit line breaks:Twelve Mill LaneThe +Valleyand the county beyond. + +A second paragraph, where the break sits mid sentence — the mill stood at +thenorthern end of the valley, where the fall was steepest. diff --git a/parity/golden/mdscrub/breaks.rust.md b/parity/golden/mdscrub/breaks.rust.md new file mode 100644 index 0000000..8aad06f --- /dev/null +++ b/parity/golden/mdscrub/breaks.rust.md @@ -0,0 +1,9 @@ +The address was set with explicit line breaks: +Twelve Mill Lane +The +Valley +and the county beyond. + +A second paragraph, where the break sits mid sentence — the mill stood at +the +northern end of the valley, where the fall was steepest. diff --git a/parity/golden/mdscrub/control.md b/parity/golden/mdscrub/control.md new file mode 100644 index 0000000..0d8859b --- /dev/null +++ b/parity/golden/mdscrub/control.md @@ -0,0 +1,3 @@ +Text with a null and a vertical tab and a shift-out in it. + +A second paragraph, clean. diff --git a/parity/golden/mdscrub/fenced.md b/parity/golden/mdscrub/fenced.md new file mode 100644 index 0000000..dbc3a18 --- /dev/null +++ b/parity/golden/mdscrub/fenced.md @@ -0,0 +1,14 @@ +Ordinary prose before the fence. + +```html +This is an example and must survive. +So must this. +``` + +Ordinary prose after the fence. + +~~~xml +Also kept. +~~~ + +The end of it. diff --git a/parity/golden/mdscrub/headings.md b/parity/golden/mdscrub/headings.md new file mode 100644 index 0000000..8107005 --- /dev/null +++ b/parity/golden/mdscrub/headings.md @@ -0,0 +1,11 @@ +# I. The Setting + +## The Quarrel + +### THE SETTLEMENT + +#### A Spaced Heading + +#not a heading + +Body text follows, and is left alone apart from its tags. diff --git a/parity/golden/mdscrub/mixed.md b/parity/golden/mdscrub/mixed.md new file mode 100644 index 0000000..7ffac53 --- /dev/null +++ b/parity/golden/mdscrub/mixed.md @@ -0,0 +1,14 @@ +# III. The Second Estate + + + +The obligation ran both ways, and contemporaries said so plainly +whenever they were asked.4 + + + +What the rolls do not say is how often the obligation was tested. + +## Notes + +4. Bloch, *Feudal Society*, vol. 2, p. 17. diff --git a/parity/golden/mdscrub/tags.md b/parity/golden/mdscrub/tags.md new file mode 100644 index 0000000..9a2e256 --- /dev/null +++ b/parity/golden/mdscrub/tags.md @@ -0,0 +1,7 @@ +The second estate held its land in +exchange for service, and the service was +1 military. + +A paragraph inside a div that means nothing once the styling is gone. + +Text with an angle bracket in it: a < b and c > d, which is not a tag. diff --git a/parity/oracle/mdscrub.py b/parity/oracle/mdscrub.py new file mode 100644 index 0000000..0a058c7 --- /dev/null +++ b/parity/oracle/mdscrub.py @@ -0,0 +1,26 @@ +"""Clean converted markdown the way the Python pipeline does. + +Reads markdown on stdin, writes the cleaned markdown on stdout. + + CATENA_PARITY_PYTHONPATH=/path/to/book-pipeline/src \ + python3 oracle/mdscrub.py < case.md +""" + +import os +import sys + +for path in os.environ.get("CATENA_PARITY_PYTHONPATH", "").split(os.pathsep): + if path: + sys.path.insert(0, path) + +from book_pipeline.convert import _epub_postprocess # noqa: E402 + + +def main() -> int: + markdown = sys.stdin.buffer.read().decode("utf-8") + sys.stdout.write(_epub_postprocess(markdown)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/parity/src/bin/regen.rs b/parity/src/bin/regen.rs index 40f0a63..014e5bd 100644 --- a/parity/src/bin/regen.rs +++ b/parity/src/bin/regen.rs @@ -19,7 +19,11 @@ type Written = Result, String>; type Job = (&'static str, fn() -> Written); fn main() -> ExitCode { - let jobs: [Job; 2] = [("tokens", tokens), ("chunkmd", chunkmd)]; + let jobs: [Job; 3] = [ + ("tokens", tokens), + ("chunkmd", chunkmd), + ("mdscrub", mdscrub), + ]; for (name, job) in jobs { match job() { @@ -50,6 +54,31 @@ fn tokens() -> Written { Ok(vec![path.display().to_string()]) } +fn mdscrub() -> Written { + let dir = parity::golden("mdscrub"); + std::fs::create_dir_all(&dir).map_err(|e| e.to_string())?; + + let mut written = Vec::new(); + for case in parity::cases("mdscrub") { + let markdown = std::fs::read_to_string(&case).map_err(|e| e.to_string())?; + let name = parity::case_name(&case); + let cleaned = parity::run_oracle("mdscrub.py", &markdown)?; + let path = dir.join(format!("{name}.md")); + std::fs::write(&path, cleaned).map_err(|e| e.to_string())?; + written.push(path.display().to_string()); + + // Where the Rust deliberately differs, its own answer is committed + // too, so the difference is visible in a review rather than only in + // a test failure. See parity/KNOWN.md. + if parity::DIVERGENT_MDSCRUB.contains(&name.as_str()) { + let path = dir.join(format!("{name}.rust.md")); + std::fs::write(&path, ::mdscrub::scrub(&markdown)).map_err(|e| e.to_string())?; + written.push(path.display().to_string()); + } + } + Ok(written) +} + fn chunkmd() -> Written { let dir = parity::golden("chunkmd"); std::fs::create_dir_all(&dir).map_err(|e| e.to_string())?; diff --git a/parity/src/lib.rs b/parity/src/lib.rs index a7bace8..2016033 100644 --- a/parity/src/lib.rs +++ b/parity/src/lib.rs @@ -83,6 +83,10 @@ pub fn run_oracle_with(script: &str, args: &[String], input: &str) -> Result Vec { let dir = dir().join("cases").join(tool); diff --git a/parity/tests/mdscrub.rs b/parity/tests/mdscrub.rs new file mode 100644 index 0000000..39483ed --- /dev/null +++ b/parity/tests/mdscrub.rs @@ -0,0 +1,59 @@ +//! Cleaned markdown must match the Python byte for byte, except where the +//! Rust is meant to differ — and there, the difference must be exactly the +//! one that was argued for. + +fn case_text(case: &std::path::Path) -> String { + std::fs::read_to_string(case).expect("a readable case") +} + +#[test] +fn every_case_cleans_the_same_as_python() { + let cases = parity::cases("mdscrub"); + assert!(!cases.is_empty(), "no cases found"); + + for case in cases { + let name = parity::case_name(&case); + if parity::DIVERGENT_MDSCRUB.contains(&name.as_str()) { + continue; + } + let ours = mdscrub::scrub(&case_text(&case)); + let theirs = parity::read_golden_text(&format!("mdscrub/{name}.md")); + assert_eq!(ours, theirs, "{name} differs from the python"); + } +} + +#[test] +fn the_documented_divergences_are_exactly_what_was_argued_for() { + for name in parity::DIVERGENT_MDSCRUB { + let case = parity::dir() + .join("cases/mdscrub") + .join(format!("{name}.md")); + let ours = mdscrub::scrub(&case_text(&case)); + let theirs = parity::read_golden_text(&format!("mdscrub/{name}.md")); + assert_ne!(ours, theirs, "{name} no longer diverges — update KNOWN.md"); + assert_eq!( + ours, + parity::read_golden_text(&format!("mdscrub/{name}.rust.md")), + "{name} diverges differently than recorded" + ); + } +} + +/// The one thing the divergence exists to prevent. +#[test] +fn a_line_break_tag_never_welds_two_words_together() { + let cleaned = mdscrub::scrub("the mill stood at the
northern end"); + assert!(!cleaned.contains("thenorthern"), "{cleaned:?}"); +} + +/// Catches a golden file drifting away from a Python that has moved on. +#[test] +#[cfg(feature = "python")] +fn the_golden_files_still_match_live_python() { + for case in parity::cases("mdscrub") { + let name = parity::case_name(&case); + let live = parity::run_oracle("mdscrub.py", &case_text(&case)).expect("the oracle runs"); + let golden = parity::read_golden_text(&format!("mdscrub/{name}.md")); + assert_eq!(live, golden, "golden file for {name} is stale"); + } +}