diff --git a/src/shared/header.rs b/src/shared/header.rs index 0bd455c6..99287a87 100644 --- a/src/shared/header.rs +++ b/src/shared/header.rs @@ -94,6 +94,11 @@ pub fn resolve_header_rows(table: &Table, declared: usize) -> usize { } let (mut header_votes, mut data_votes) = (0usize, 0usize); + // Columns whose first-row value recurs below. Kept apart from the type + // votes: one body cell that happens to repeat its label (a `region` + // column holding "region") is weaker evidence than a column of numbers + // under a text label. + let mut repeat_votes = 0usize; for (c, label) in head.iter().enumerate().take(width) { let values: Vec<&str> = body .iter() @@ -119,20 +124,25 @@ pub fn resolve_header_rows(table: &Table, declared: usize) -> usize { // recurs below is plainly a value of the column, not its label. _ => { if values.iter().any(|v| fold(v) == fold(label)) { - data_votes += 1; + repeat_votes += 1; } } } } - if header_votes == 0 && data_votes == 0 { - // Text above text, with no value repeated: only the shape of the row - // is left to go on. Short single-line entries read as labels, and the - // alternative is the empty header row every such table renders today. - let labelled = head.iter().take(width).all(|v| v.chars().count() <= MAX_LABEL); - return usize::from(labelled); + // Type evidence decides whenever there is any; a repeated value only + // settles tables that have none. + if header_votes > 0 || data_votes > 0 { + return usize::from(header_votes > data_votes); + } + if repeat_votes > 0 { + return 0; } - usize::from(header_votes > data_votes) + // Text above text, with no value repeated: only the shape of the row is + // left to go on. Short single-line entries read as labels, and the + // alternative is the empty header row every such table renders today. + let labelled = head.iter().take(width).all(|v| v.chars().count() <= MAX_LABEL); + usize::from(labelled) } /// The field count most sampled body rows agree on. Frequency ties break diff --git a/tests/header_collision.rs b/tests/header_collision.rs new file mode 100644 index 00000000..3923af6d --- /dev/null +++ b/tests/header_collision.rs @@ -0,0 +1,178 @@ +//! Header detection when a body cell repeats its own column's label. One +//! value equal to the header (`region` under `region`) must not outweigh a +//! typed column under a text label, in any grid format that has no declared +//! header row. + +use anydoc::{Format, to_markdown_bytes}; +use std::io::{Cursor, Write}; + +const HEADER: [&str; 3] = ["city", "region", "pop"]; +const HEADER_LINE: &str = "| city | region | pop |"; + +type Row = [&'static str; 3]; +type Build = fn(&Row, &[Row]) -> Vec; + +/// (name, header, body rows). Every case has a text label over a numeric +/// `pop` column, so the first row is the header. +const CASES: &[(&str, Row, &[Row])] = &[ + ( + "collision in col A", + HEADER, + &[["Paris", "north", "100"], ["city", "south", "200"], ["Nice", "east", "300"]], + ), + ( + "collision in col B", + HEADER, + &[["Paris", "north", "100"], ["Lyon", "region", "200"], ["Nice", "east", "300"]], + ), + ( + "upper case", + HEADER, + &[["Paris", "north", "100"], ["Lyon", "REGION", "200"], ["Nice", "east", "300"]], + ), + ( + "mixed case", + HEADER, + &[["Paris", "north", "100"], ["Lyon", "ReGiOn", "200"], ["Nice", "east", "300"]], + ), + ( + "padded", + HEADER, + &[["Paris", "north", "100"], ["Lyon", " region ", "200"], ["Nice", "east", "300"]], + ), + ( + "collision on the first data row", + HEADER, + &[["Paris", "region", "100"], ["Lyon", "south", "200"], ["Nice", "east", "300"]], + ), + ( + "Status column containing status", + ["city", "Status", "pop"], + &[["Paris", "active", "100"], ["Lyon", "status", "200"], ["Nice", "closed", "300"]], + ), + ( + "no collision", + HEADER, + &[["Paris", "north", "100"], ["Lyon", "south", "200"], ["Nice", "east", "300"]], + ), +]; + +fn rows<'a>(header: &'a [&'a str; 3], body: &'a [[&'a str; 3]]) -> Vec<&'a [&'a str; 3]> { + std::iter::once(header).chain(body.iter()).collect() +} + +fn csv(header: &[&str; 3], body: &[[&str; 3]]) -> Vec { + let mut out = String::new(); + for row in rows(header, body) { + out.push_str(&row.join(",")); + out.push('\n'); + } + out.into_bytes() +} + +fn zip(parts: &[(&str, &str)]) -> Vec { + let mut zip = zip::ZipWriter::new(Cursor::new(Vec::new())); + let opts = zip::write::SimpleFileOptions::default(); + for (name, body) in parts { + zip.start_file(*name, opts).unwrap(); + zip.write_all(body.as_bytes()).unwrap(); + } + zip.finish().unwrap().into_inner() +} + +fn xlsx(header: &[&str; 3], body: &[[&str; 3]]) -> Vec { + const SML: &str = "http://schemas.openxmlformats.org/spreadsheetml/2006/main"; + const R: &str = "http://schemas.openxmlformats.org/officeDocument/2006/relationships"; + let mut data = String::new(); + for (r, row) in rows(header, body).into_iter().enumerate() { + data.push_str(&format!(r#""#, r + 1)); + for (c, value) in row.iter().enumerate() { + let at = format!("{}{}", (b'A' + c as u8) as char, r + 1); + if value.parse::().is_ok() { + data.push_str(&format!(r#"{value}"#)); + } else { + data.push_str(&format!( + r#"{value}"# + )); + } + } + data.push_str(""); + } + let workbook = format!( + r#""# + ); + let rels = format!( + r#""# + ); + let sheet = format!( + r#"{data}"# + ); + zip(&[ + ("xl/workbook.xml", &workbook), + ("xl/_rels/workbook.xml.rels", &rels), + ("xl/worksheets/sheet1.xml", &sheet), + ]) +} + +fn ods(header: &[&str; 3], body: &[[&str; 3]]) -> Vec { + let mut data = String::new(); + for row in rows(header, body) { + data.push_str(""); + for value in row { + if value.parse::().is_ok() { + data.push_str(&format!( + r#"{value}"# + )); + } else { + data.push_str(&format!( + r#"{value}"# + )); + } + } + data.push_str(""); + } + let content = format!( + r#" + {data} + "# + ); + zip(&[ + ("mimetype", "application/vnd.oasis.opendocument.spreadsheet"), + ("content.xml", &content), + ]) +} + +/// Every case in every format: the real labels are the header line, and the +/// table holds the header, the separator and each body row exactly once. +#[test] +fn a_label_repeated_in_its_column_keeps_the_header() { + let builders: [(Format, Build); 3] = + [(Format::Csv, csv), (Format::Excel, xlsx), (Format::Ods, ods)]; + for (format, build) in builders { + for (name, header, body) in CASES { + let md = to_markdown_bytes(&build(header, body), format) + .unwrap_or_else(|e| panic!("{format:?} {name}: {e}")); + let expected = format!("| {} |", header.join(" | ")); + if *header == HEADER { + assert_eq!(expected, HEADER_LINE); + } + let table: Vec<&str> = md.lines().filter(|l| l.starts_with('|')).collect(); + assert_eq!(md.lines().next(), Some(expected.as_str()), "{format:?} {name}:\n{md}"); + assert_eq!(table.len(), body.len() + 2, "{format:?} {name}:\n{md}"); + } + } +} + +/// The table without any collision renders exactly as it did before the fix. +#[test] +fn no_collision_is_unchanged() { + let (_, header, body) = CASES.last().unwrap(); + let md = to_markdown_bytes(&csv(header, body), Format::Csv).unwrap(); + assert_eq!( + md.trim_end(), + "| city | region | pop |\n| --- | --- | --- |\n| Paris | north | 100 |\n| Lyon | south | 200 |\n| Nice | east | 300 |" + ); +}