From fb5589e8c3bfc977562d4066cef02505736fe212 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3?= Date: Thu, 10 Sep 2026 15:30:05 +0200 Subject: [PATCH 1/6] refactor: share one case-insensitive fold between case list and search --- crates/openpapir-core/src/records/case.rs | 17 ++++++----- crates/openpapir-core/src/records/mod.rs | 21 +++++++++++++ crates/openpapir-core/src/records/search.rs | 34 ++++++++++++--------- 3 files changed, 50 insertions(+), 22 deletions(-) diff --git a/crates/openpapir-core/src/records/case.rs b/crates/openpapir-core/src/records/case.rs index 7f5a690..bf2abd5 100644 --- a/crates/openpapir-core/src/records/case.rs +++ b/crates/openpapir-core/src/records/case.rs @@ -35,7 +35,7 @@ use crate::records::association::{self, Association}; use crate::records::document::{self, Record, Rewritable}; use crate::records::receipt::Receipt; use crate::records::submission::Submission; -use crate::records::{CASES_DIR, checked_notes, checked_title}; +use crate::records::{CASES_DIR, checked_notes, checked_title, fold, folded_contains}; /// The value a case record carries in `record_kind`. pub const KIND: &str = "case"; @@ -231,17 +231,18 @@ impl<'a> Filter<'a> { Matcher { status: self.status, tags: self.tags, - query: self.query.map(str::to_lowercase), + query: self.query.map(fold), } } } /// One filter prepared for a scan. /// -/// The query is folded to lower case once, when the matcher is built, rather -/// than once per case: the filter is applied to every record the listing read, -/// and folding the same short string again for each of them is work the scan -/// does not need. +/// The query is folded once, when the matcher is built, rather than once per +/// case: the filter is applied to every record the listing read, and folding +/// the same short string again for each of them is work the scan does not +/// need. The fold is [`fold`], the one `search` uses, so the two commands +/// compare text the same way. struct Matcher<'a> { status: Option, tags: &'a [String], @@ -260,11 +261,11 @@ impl Matcher<'_> { match &self.query { None => true, Some(query) => { - case.title.to_lowercase().contains(query) + folded_contains(&case.title, query) || case .notes .as_deref() - .is_some_and(|notes| notes.to_lowercase().contains(query)) + .is_some_and(|notes| folded_contains(notes, query)) } } } diff --git a/crates/openpapir-core/src/records/mod.rs b/crates/openpapir-core/src/records/mod.rs index a50b430..58ffd61 100644 --- a/crates/openpapir-core/src/records/mod.rs +++ b/crates/openpapir-core/src/records/mod.rs @@ -244,6 +244,27 @@ pub fn checked_query(value: &str) -> Result { required("query", value, MAX_QUERY_BYTES, Shape::MultiLine) } +/// Fold one text for a comparison that disregards case. +/// +/// `case list --query` and `search` make the same comparison, so both fold +/// through this one function rather than each calling `str::to_lowercase` +/// where it happens to need it. The fold is the whole of the comparison: it +/// is Unicode-aware, and it strips no accent, collapses no whitespace, and +/// normalises nothing else, so a query matches the text the user typed and +/// not a variant of it. +pub(crate) fn fold(value: &str) -> String { + value.to_lowercase() +} + +/// Whether one field's text holds an already folded query. +/// +/// `folded_query` is what [`fold`] returned for the query. A query is folded +/// once, before a scan begins, and the field is folded here as it is read, so +/// each side of the comparison is folded exactly once. +pub(crate) fn folded_contains(value: &str, folded_query: &str) -> bool { + fold(value).contains(folded_query) +} + /// Whether a value is 64 lowercase hexadecimal characters. #[must_use] pub fn is_digest(value: &str) -> bool { diff --git a/crates/openpapir-core/src/records/search.rs b/crates/openpapir-core/src/records/search.rs index 84840c6..f648ba7 100644 --- a/crates/openpapir-core/src/records/search.rs +++ b/crates/openpapir-core/src/records/search.rs @@ -23,11 +23,11 @@ //! //! # How it matches //! -//! The comparison is the one `case list --query` makes: the query and the -//! field are folded with `str::to_lowercase` and the match is a substring of -//! the folded field. There is **no index**, so the search reads every record -//! of every kind it was asked for, in one linear scan per kind, and its cost -//! grows with what the archive holds. +//! The comparison is the one `case list --query` makes, through the same +//! fold: the query and the field are folded to lower case and the match is a +//! substring of the folded field. There is **no index**, so the search reads +//! every record of every kind it was asked for, in one linear scan per kind, +//! and its cost grows with what the archive holds. //! //! # What a hit carries //! @@ -44,10 +44,10 @@ use crate::archive::Archive; use crate::error::{Details, Diagnostic, Failure, Outcome, Result, Warning, codes}; use crate::records::association::{self, Association}; use crate::records::case::{self, Case}; -use crate::records::checked_query; use crate::records::document::{self, Record}; use crate::records::receipt::{self, Receipt}; use crate::records::submission::{self, Submission}; +use crate::records::{checked_query, fold, folded_contains}; /// One record kind a search may read. /// @@ -159,11 +159,6 @@ impl Subject<'_> { } } -/// Whether one field's text holds the already folded query. -fn holds(value: &str, folded_query: &str) -> bool { - value.to_lowercase().contains(folded_query) -} - /// Push one hit per named field whose text holds the query. fn collect( hits: &mut Vec, @@ -172,7 +167,7 @@ fn collect( fields: &[(&'static str, Option<&str>)], ) { for (field, value) in fields { - if value.is_some_and(|value| holds(value, folded_query)) { + if value.is_some_and(|value| folded_contains(value, folded_query)) { hits.push(subject.hit(field)); } } @@ -184,6 +179,13 @@ fn collect( /// another and never a partial one, so no lock is taken, exactly as a listing /// takes none. /// +/// A record directory that cannot be listed reads as empty, exactly as it +/// does for `case list`: a search reports the records that are there, and a +/// kind whose directory could not be listed contributes no hit rather than +/// refusing the whole search. An absent directory and an unlistable one are +/// therefore indistinguishable here, so no absence of hits may be read as +/// evidence that the archive holds no matching record of that kind. +/// /// # Errors /// /// Returns `input.cap.field_length` for a query over the cap and @@ -222,7 +224,7 @@ fn find( // The query is checked and folded once, before the archive is opened, so // an oversized query is refused before anything is read and the folding // is not repeated for every record of every kind. - let folded = checked_query(query)?.to_lowercase(); + let folded = fold(&checked_query(query)?); let mut archive = Archive::open(root)?; warnings.extend(archive.take_warnings()); let root = archive.root(); @@ -291,7 +293,11 @@ fn case_hits(case: &Case, folded_query: &str, hits: &mut Vec) { // The tags are one field of the record, so a case whose query is on two // of its tags is one hit rather than two. Each tag is tested on its own // rather than as one joined string, so nothing matches across the join. - if case.tags.iter().any(|tag| holds(tag, folded_query)) { + if case + .tags + .iter() + .any(|tag| folded_contains(tag, folded_query)) + { hits.push(subject.hit("tags")); } } From 179d3b75b31522473c8d4f2e0cfe129748325bf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3?= Date: Thu, 10 Sep 2026 15:30:12 +0200 Subject: [PATCH 2/6] test: key the bench assertions by name and guard against hidden arguments --- crates/openpapir-cli/tests/bench.rs | 37 ++++- crates/openpapir-cli/tests/property.rs | 212 ++++++++++++++++++++++--- 2 files changed, 222 insertions(+), 27 deletions(-) diff --git a/crates/openpapir-cli/tests/bench.rs b/crates/openpapir-cli/tests/bench.rs index 0d3dd9f..01f6eba 100644 --- a/crates/openpapir-cli/tests/bench.rs +++ b/crates/openpapir-cli/tests/bench.rs @@ -54,7 +54,12 @@ const DELETE_CEILING: Duration = Duration::from_secs(10); const IMPORT_CEILING: Duration = Duration::from_secs(10); /// Files in the timed import, which is the per-import file cap itself. +/// +/// A batch under the cap would time something other than the largest import +/// the binary accepts, so the value is asserted against the core constant at +/// compile time rather than trusted to stay in step by hand. const IMPORT_BATCH: usize = 1_000; +const _: () = assert!(IMPORT_BATCH as u64 == openpapir_core::archive::limits::MAX_IMPORT_FILES); /// A search reads every record of all four kinds rather than one kind, so it /// reads four times what a listing reads and is given its own ceiling. @@ -209,24 +214,28 @@ fn the_linear_scans_stay_under_their_documented_ceilings() { report(cases, setup, &measurements); // A scan that matched nothing would be fast for the wrong reason, so what - // each listing reported is checked before its time is believed. + // each listing reported is checked before its time is believed. Each one + // is found by the name it was measured under rather than by its position, + // so a measurement added or reordered above cannot silently move an + // assertion onto another invocation. + let matched = cases.div_ceil(bench_support::QUERY_ONE_IN) as u64; assert_eq!( - reported_count(&measurements[0]), + reported_count(named(&measurements, "case list")), cases as u64, "the listing did not report every case" ); assert_eq!( - reported_count(&measurements[1]), - cases.div_ceil(bench_support::QUERY_ONE_IN) as u64, + reported_count(named(&measurements, "case list --query")), + matched, "the query matched a different share of the archive than it filed" ); assert_eq!( - reported_count(&measurements[4]), - cases.div_ceil(bench_support::QUERY_ONE_IN) as u64, + reported_count(named(&measurements, "search")), + matched, "the search matched a different share of the archive than it filed" ); assert_eq!( - reported_receipts(&measurements[2]), + reported_receipts(named(&measurements, "case show")), 1, "the shown case named no receipt, so the section that reads the \ associations was not measured" @@ -398,6 +407,20 @@ fn timed(directory: Option<&Path>, arguments: &[&str]) -> (Duration, Output) { (started.elapsed(), output) } +/// The one measurement taken under `name`. +/// +/// The measurements are keyed by the name they were measured under, because +/// the list they live in is built in the order the invocations run and an +/// invocation added between two others would otherwise renumber every +/// assertion after it. A name that is not there is a mistake in this file +/// rather than a result about the binary, so it panics. +fn named<'a>(measurements: &'a [Measurement], name: &str) -> &'a Measurement { + measurements + .iter() + .find(|measurement| measurement.name == name) + .unwrap_or_else(|| panic!("{name} was measured")) +} + /// The `count` one listing envelope reports. fn reported_count(measurement: &Measurement) -> u64 { let envelope: serde_json::Value = diff --git a/crates/openpapir-cli/tests/property.rs b/crates/openpapir-cli/tests/property.rs index 02d16ce..305f548 100644 --- a/crates/openpapir-cli/tests/property.rs +++ b/crates/openpapir-cli/tests/property.rs @@ -24,7 +24,7 @@ //! The case count is bounded because every case is a process; `PROPTEST_CASES` //! raises it locally, and `docs/testing.md` says how. -use std::collections::BTreeSet; +use std::collections::{BTreeMap, BTreeSet}; use std::ffi::OsString; use std::fs; use std::path::Path; @@ -172,15 +172,38 @@ fn command_line() -> impl Strategy> { /// already. fn known_names() -> &'static BTreeSet { static NAMES: OnceLock> = OnceLock::new(); - NAMES.get_or_init(|| { - let directory = tempfile::tempdir().expect("a temporary working directory"); - let output = run(directory.path(), &["manpage".to_owned()]); - assert!(output.status.success(), "the man page is written"); - let manpage = String::from_utf8(output.stdout).expect("a generated man page is UTF-8"); - argument_names(&manpage) + NAMES.get_or_init(|| argument_names(manpage())) +} + +/// The whole man page stream, written once for the suite. +fn manpage() -> &'static str { + static MANPAGE: OnceLock = OnceLock::new(); + MANPAGE.get_or_init(|| generated(&["manpage".to_owned()], "a generated man page")) +} + +/// The bash completion script, written once for the suite. +/// +/// It is generated from the same command definition, by a generator that +/// renders every subcommand and every flag whether or not it is marked +/// hidden, so it is the second opinion the derivation guard compares against. +fn completions() -> &'static str { + static COMPLETIONS: OnceLock = OnceLock::new(); + COMPLETIONS.get_or_init(|| { + generated( + &["completions".to_owned(), "bash".to_owned()], + "a generated completion script", + ) }) } +/// Run one document-writing command and return what it wrote. +fn generated(arguments: &[String], document: &str) -> String { + let directory = tempfile::tempdir().expect("a temporary working directory"); + let output = run(directory.path(), arguments); + assert!(output.status.success(), "{document} is written"); + String::from_utf8(output.stdout).unwrap_or_else(|_| panic!("{document} is UTF-8")) +} + /// The argument names a rendered man page stream declares. fn argument_names(manpage: &str) -> BTreeSet { let mut names = BTreeSet::new(); @@ -207,21 +230,17 @@ fn argument_names(manpage: &str) -> BTreeSet { /// /// A man page writes an argument as `\fB\-\-archive\fR \fI\fR` or /// `[\fIFILE\fR]`, so the roff font escapes and the escaped dashes are undone -/// first. The token is then trimmed twice: once with exactly the characters -/// the binary trims from the name it is willing to echo, so a bracketed -/// value name such as `DIGEST[:ROLE]` keeps its closing bracket the way the -/// walker keeps it, and once more with the brackets and commas the man page -/// adds around an optional positional or between a short and a long flag. -/// Both forms are known; the walker reports one of them. +/// first. The token is then trimmed twice: once with the characters the binary +/// trims from the name it is willing to echo, plus the comma the man page +/// writes between a short and a long flag, so a bracketed value name such as +/// `DIGEST[:ROLE]` keeps its closing bracket the way the walker keeps it while +/// `\fB\-h\fR,` still reduces to `h` rather than to `h,`; and once more with +/// the brackets the man page adds around an optional positional. Both forms +/// are known; the walker reports one of them. fn bare_names(token: &str) -> [String; 2] { - let plain = token - .replace("\\fB", "") - .replace("\\fI", "") - .replace("\\fR", "") - .replace("\\-", "-") - .to_lowercase(); + let plain = plain(token); let walker = plain - .trim_matches(|character: char| matches!(character, '-' | '<' | '>' | '.' | '=')) + .trim_matches(|character: char| matches!(character, '-' | '<' | '>' | '.' | '=' | ',')) .to_owned(); let wide = plain .trim_matches(|character: char| { @@ -231,6 +250,88 @@ fn bare_names(token: &str) -> [String; 2] { [walker, wide] } +/// One rendered token with the roff font escapes and escaped dashes undone. +fn plain(token: &str) -> String { + token + .replace("\\fB", "") + .replace("\\fI", "") + .replace("\\fR", "") + .replace("\\-", "-") + .to_lowercase() +} + +/// The separator the completion script writes between command words. +/// +/// `openpapir archive init` is one `openpapir__subcmd__archive__subcmd__init` +/// there, and `openpapir-archive-init` in the man page, so one replacement +/// turns a declared command into the name its page is filed under. +const COMPLETION_SEPARATOR: &str = "__subcmd__"; + +/// Every command the completion script declares, keyed by the name a page for +/// it is filed under, with the long flags declared for each. +fn declared_commands(completions: &str) -> BTreeMap> { + let mut declared = BTreeMap::new(); + let mut current: Option = None; + for line in completions.lines() { + let line = line.trim(); + if let Some(label) = line.strip_suffix(')') + && label.starts_with("openpapir") + && !label.contains(',') + { + current = Some(label.replace(COMPLETION_SEPARATOR, "-")); + } else if let Some(list) = line + .strip_prefix("opts=\"") + .and_then(|list| list.strip_suffix('"')) + && let Some(name) = current.take() + { + declared.insert(name, long_flags(list.split_whitespace())); + } + } + declared +} + +/// Every page the man page stream renders, keyed by its own name, with the +/// long flags each page's `OPTIONS` section declares. +fn rendered_pages(manpage: &str) -> BTreeMap> { + let mut pages: BTreeMap> = BTreeMap::new(); + let mut page = String::new(); + let mut in_options = false; + let mut after_tag = false; + for line in manpage.lines() { + if let Some(title) = line.strip_prefix(".TH ") { + page = title + .split_whitespace() + .next() + .unwrap_or_default() + .to_owned(); + pages.entry(page.clone()).or_default(); + in_options = false; + after_tag = false; + } else if let Some(section) = line.strip_prefix(".SH ") { + in_options = section.trim() == "OPTIONS"; + after_tag = false; + } else if line.trim() == ".TP" { + after_tag = true; + } else if std::mem::take(&mut after_tag) && in_options { + let flags = long_flags(line.split_whitespace()); + pages.entry(page.clone()).or_default().extend(flags); + } + } + pages +} + +/// The long flags a run of rendered or declared tokens names. +/// +/// A positional, a value name, and a short flag are left out: a long flag is +/// the one form both the man page and the completion script spell the same +/// way once the roff escapes and the separating comma are gone. +fn long_flags<'a, I: Iterator>(tokens: I) -> BTreeSet { + tokens + .map(|token| plain(token).trim_end_matches(',').to_owned()) + .filter(|token| token.len() > 2 && token.starts_with("--")) + .collect() +} + /// Whether the command line asked for the JSON form. /// /// The rule is the binary's own: the scan stops at `--`, after which a token @@ -356,6 +457,77 @@ mod regression { ); } + /// The man page renders every command and every argument the definition + /// declares, so nothing this suite reads its names from can be hidden. + /// + /// The derivation skips what is marked hidden twice over: `manpage.rs` + /// leaves out a hidden subcommand, and `clap_mangen` leaves out a hidden + /// argument. A command or a flag marked that way would still be accepted + /// at the command line and would appear in no page, so the walk that + /// `known_names` reads would not know a name the usage walker can report + /// and this suite would call a real name an undefined one. + /// + /// The shell completions are the second opinion: they are generated from + /// the same definition by a generator that renders every subcommand and + /// every long flag whether or not it is hidden. Every command they + /// declare is required to have a page of its own, and every long flag + /// they declare for it is required to be in that page. + /// + /// Two differences between the two are the generators' own and are not + /// hidden anything. `help` is clap's own command: the completions declare + /// it and one mirror of it under every command, and the man page renders + /// no page for any of them, so a command path naming `help` is left out + /// of the comparison. `--version` is added to every page by the + /// derivation itself, so that a page read on its own names the build it + /// came from, and it is the one flag a page may carry that the definition + /// does not declare there. + #[test] + fn the_command_tree_declares_no_hidden_argument_or_subcommand() { + let declared = declared_commands(completions()); + let rendered = rendered_pages(manpage()); + assert!( + declared.len() > SUBCOMMANDS.len(), + "the completion script declares the whole command tree" + ); + + let mut compared = 0; + for (name, flags) in &declared { + if name.split('-').any(|word| word == "help") { + continue; + } + let page = rendered + .get(name) + .unwrap_or_else(|| panic!("the man page stream holds a page for {name}")); + let hidden: Vec<&String> = flags.difference(page).collect(); + assert!( + hidden.is_empty(), + "{name} declares an argument no page renders: {hidden:?}" + ); + let added: Vec<&String> = page + .difference(flags) + .filter(|flag| *flag != "--version") + .collect(); + assert!( + added.is_empty(), + "the page for {name} renders an argument the definition does \ + not declare there: {added:?}" + ); + compared += 1; + } + + for name in rendered.keys() { + assert!( + declared.contains_key(name), + "{name} has a page but is not a command the definition declares" + ); + } + assert_eq!( + compared, + rendered.len(), + "every page was compared against the command that owns it" + ); + } + /// `archive import` refuses a command line that names no source, and the /// argument it names is `--from`, which the parser defines. The property /// generates this line, and refused it while the known names were kept by From eb03407a144d5571cec255e822e11e12f21b5d8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3?= Date: Thu, 10 Sep 2026 15:30:12 +0200 Subject: [PATCH 3/6] docs: state the import index bound and the gap a failed unlink leaves --- crates/openpapir-core/src/archive/import.rs | 6 ++++++ docs/archive-layout.md | 6 +++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/openpapir-core/src/archive/import.rs b/crates/openpapir-core/src/archive/import.rs index 89a9b22..65bf480 100644 --- a/crates/openpapir-core/src/archive/import.rs +++ b/crates/openpapir-core/src/archive/import.rs @@ -415,6 +415,12 @@ impl History { /// the next reader to rebuild it. Nothing is written when the operation never /// held an index, because writing one would mean scanning for it, and an /// import of new files is not the place to pay for that. +/// +/// What it holds in memory is keyed rather than accumulated: the index is one +/// entry per distinct digest an import event names, each holding that digest's +/// own events, so the entry count is the number of distinct digests the +/// archive has an import event for and an event folded in during the operation +/// joins the entry that was already there. #[derive(Debug, Default)] struct Histories { index: Option, diff --git a/docs/archive-layout.md b/docs/archive-layout.md index fd6d45d..251f092 100644 --- a/docs/archive-layout.md +++ b/docs/archive-layout.md @@ -1060,7 +1060,11 @@ What goes with the case is fixed: go refuses the deletion before the first unlink, and a deletion that removes no object removes none of them. A purge that stops between the two passes therefore leaves a record about bytes that are gone; `archive check` counts - one under `derived_orphans` and the next `archive derive` discards it. + one under `derived_orphans` and the next `archive derive` discards it. The + other way round holds too: an object whose unlink fails after its derived + record was removed is left with no derived record until the next + `archive derive` computes one again, which loses nothing, because a derived + record is disposable and nothing in the archive references one. An association naming submissions in **two cases** is refused rather than resolved while the user still asserts it. It references a submission that From 9fdc2c04633a4367264e848718b644906d70cf68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3?= Date: Thu, 10 Sep 2026 15:30:12 +0200 Subject: [PATCH 4/6] ci: fold the described sentence and clear the packaging staging directory --- .github/workflows/ci.yml | 6 ++++++ docs/releasing.md | 12 ++++++++---- scripts/package-release.sh | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f1c0d0d..017980b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -107,6 +107,12 @@ jobs: run: | set -euo pipefail staging=staging/openpapir-ci + # The script refuses a staging directory that already holds files, + # so that one staging cannot mix into another. The tree a previous + # run of this step left is this run's own, so it is removed first + # and the step can be run again on a working copy that has already + # run it. + rm -rf "$staging" scripts/package-release.sh target/release/openpapir "$staging" name=openpapir if [ -f "${staging}/openpapir.exe" ]; then diff --git a/docs/releasing.md b/docs/releasing.md index c9634ba..caa8fbb 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -139,7 +139,10 @@ step by hand. Before it writes anything, the script refuses a staging path that is a symbolic link and one that is a directory already holding files, so a staging -run never writes through a link and never mixes into an earlier archive. +run never writes through a link and never mixes into an earlier archive. The +sentence `--describe` prints is folded to at most 76 columns as it is written, +so it reads at one width wherever it is pasted however far the tables it names +grow. The CI test job calls the same script on the release binary it already builds, on each of Linux, macOS, and Windows, so the layout is exercised on every pull @@ -147,9 +150,10 @@ request without a tag and without an archive. That job then diffs the staged tree against `--describe --paths`, so a file staged but not described, or described but not staged, fails the pull request. It stages into `staging/` under the checkout root, which `.gitignore` lists, so running the same step -locally leaves no untracked tree. The draft release and the dry-run summary -print what `--describe` says, so a release cannot name a layout other than the -one that was staged. +locally leaves no untracked tree. The step removes that directory before it +stages, so a second run is not refused as a staging that already holds files. +The draft release and the dry-run summary print what `--describe` says, so a +release cannot name a layout other than the one that was staged. The build job keeps its uploaded artefact for one day, because the same run's verify and draft release jobs are its only consumers, and the layout a pull diff --git a/scripts/package-release.sh b/scripts/package-release.sh index 58a00f7..92ae396 100755 --- a/scripts/package-release.sh +++ b/scripts/package-release.sh @@ -80,12 +80,44 @@ shell_list() { prose_list ", and " "${shells[@]}" } +# The width the described sentence is folded to. +# +# It is under the 80 columns the repository wraps prose at, so the sentence +# still fits once a reader or a release note indents it. +readonly DESCRIBE_WIDTH=76 + +# Fold prose read from standard input to at most DESCRIBE_WIDTH columns. +# +# The tables the sentence names grow, so the width of a line is settled here +# rather than by where the words happen to sit in the here-document: adding one +# shell or one document would otherwise push a line past the column the rest of +# the repository wraps at. The break is at a space and nowhere else, so a +# backticked path is never split, and a word longer than the width stands on +# its own line rather than being cut. +fold_prose() { + local word line="" + set -f + # shellcheck disable=SC2013 # the input is prose, so word splitting is wanted. + for word in $(cat); do + if [ -z "$line" ]; then + line="$word" + elif [ "$((${#line} + 1 + ${#word}))" -le "$DESCRIBE_WIDTH" ]; then + line="${line} ${word}" + else + printf '%s\n' "$line" + line="$word" + fi + done + set +f + [ -z "$line" ] || printf '%s\n' "$line" +} + describe() { local shells documents shells=$(shell_list) # shellcheck disable=SC2086 # the table is a deliberate word list. documents=$(prose_list ", " $DOCUMENTS) - cat < Date: Thu, 10 Sep 2026 15:44:18 +0200 Subject: [PATCH 5/6] test: require a help page to be a derivable mirror before skipping it --- crates/openpapir-cli/tests/property.rs | 61 +++++++++++++++++++++----- 1 file changed, 51 insertions(+), 10 deletions(-) diff --git a/crates/openpapir-cli/tests/property.rs b/crates/openpapir-cli/tests/property.rs index 305f548..f8a62bc 100644 --- a/crates/openpapir-cli/tests/property.rs +++ b/crates/openpapir-cli/tests/property.rs @@ -269,27 +269,65 @@ const COMPLETION_SEPARATOR: &str = "__subcmd__"; /// Every command the completion script declares, keyed by the name a page for /// it is filed under, with the long flags declared for each. +/// +/// The script holds one `case` block per command, labelled with that command, +/// and the block sets `opts` to every flag and subcommand name the command +/// takes. A command is entered into the map by its own label, as the label is +/// read, so a command declares itself whether or not a later line in its block +/// turns out to name a flag: a block whose `opts` were missed would otherwise +/// leave the command out of the comparison altogether. fn declared_commands(completions: &str) -> BTreeMap> { - let mut declared = BTreeMap::new(); - let mut current: Option = None; + let mut declared: BTreeMap> = BTreeMap::new(); + let mut current = String::new(); for line in completions.lines() { let line = line.trim(); if let Some(label) = line.strip_suffix(')') && label.starts_with("openpapir") && !label.contains(',') { - current = Some(label.replace(COMPLETION_SEPARATOR, "-")); + current = label.replace(COMPLETION_SEPARATOR, "-"); + declared.entry(current.clone()).or_default(); } else if let Some(list) = line .strip_prefix("opts=\"") .and_then(|list| list.strip_suffix('"')) - && let Some(name) = current.take() + && let Some(flags) = declared.get_mut(¤t) { - declared.insert(name, long_flags(list.split_whitespace())); + flags.extend(long_flags(list.split_whitespace())); } } declared } +/// Whether `name` is one of the pages clap's own `help` command mirrors. +/// +/// clap gives every command a `help` subcommand, and gives that one a mirror +/// of every command below its parent, so the completion script declares +/// `openpapir-help`, `openpapir-archive-help`, `openpapir-help-case-create` +/// and `openpapir-archive-help-check` while the man page renders a page for +/// none of them. A mirror is therefore left out of the comparison, and it has +/// to be shown to be one rather than assumed from the word alone: a hidden +/// subcommand a later change names `help-topics` would carry the word too, and +/// exempting it would hide exactly what this guard is for. +/// +/// A name qualifies only when it can be derived from commands that are +/// themselves declared: at the first `help` in the path, the part before it +/// has to be a declared command, and the part after it has to be empty, or +/// `help` again, or name a declared command under that same parent. A hidden +/// `openpapir help-topics` fails on the last of those, because the tree holds +/// no `openpapir topics`. +fn is_help_mirror(name: &str, declared: &BTreeMap>) -> bool { + let words: Vec<&str> = name.split('-').collect(); + let Some(at) = words.iter().position(|word| *word == "help") else { + return false; + }; + let parent = words[..at].join("-"); + if !declared.contains_key(&parent) { + return false; + } + let rest = words[at + 1..].join("-"); + rest.is_empty() || rest == "help" || declared.contains_key(&format!("{parent}-{rest}")) +} + /// Every page the man page stream renders, keyed by its own name, with the /// long flags each page's `OPTIONS` section declares. fn rendered_pages(manpage: &str) -> BTreeMap> { @@ -474,10 +512,13 @@ mod regression { /// they declare for it is required to be in that page. /// /// Two differences between the two are the generators' own and are not - /// hidden anything. `help` is clap's own command: the completions declare - /// it and one mirror of it under every command, and the man page renders - /// no page for any of them, so a command path naming `help` is left out - /// of the comparison. `--version` is added to every page by the + /// hidden anything. `help` is clap's own command, and the completions + /// declare it and one mirror of it under every command while the man page + /// renders no page for any of them, so a mirror is left out of the + /// comparison; `is_help_mirror` requires a name to be derivable from + /// commands that are themselves declared before it is left out, so a + /// hidden subcommand that merely carries the word, `help-topics` for one, + /// is compared like any other. `--version` is added to every page by the /// derivation itself, so that a page read on its own names the build it /// came from, and it is the one flag a page may carry that the definition /// does not declare there. @@ -492,7 +533,7 @@ mod regression { let mut compared = 0; for (name, flags) in &declared { - if name.split('-').any(|word| word == "help") { + if is_help_mirror(name, &declared) { continue; } let page = rendered From 7dc51d614a0b873f04fc736dc7bf884bcfcf25a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3?= Date: Thu, 10 Sep 2026 15:44:18 +0200 Subject: [PATCH 6/6] ci: restore the globbing flag the prose fold turns off --- scripts/package-release.sh | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/scripts/package-release.sh b/scripts/package-release.sh index 92ae396..355b176 100755 --- a/scripts/package-release.sh +++ b/scripts/package-release.sh @@ -94,8 +94,17 @@ readonly DESCRIBE_WIDTH=76 # the repository wraps at. The break is at a space and nowhere else, so a # backticked path is never split, and a word longer than the width stands on # its own line rather than being cut. +# +# Splitting the input into words is what asks for a glob character in it to be +# left alone, so pathname expansion is turned off around the loop and put back +# the way it was found rather than switched on unconditionally: this function +# is a detail of one sentence and settles nothing for the rest of the script. fold_prose() { - local word line="" + local word line="" globbing="" + case "$-" in + *f*) ;; + *) globbing="+f" ;; + esac set -f # shellcheck disable=SC2013 # the input is prose, so word splitting is wanted. for word in $(cat); do @@ -108,7 +117,7 @@ fold_prose() { line="$word" fi done - set +f + [ -z "$globbing" ] || set "$globbing" [ -z "$line" ] || printf '%s\n' "$line" }