From e6059ab1b05700d7fd158946c55b0fdd0d90c7ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Sat, 19 Sep 2026 20:22:05 +0300 Subject: [PATCH] help, output, connect: bring the tool up to the house guidelines --- README.md | 2 +- connect/src/emulator/registry.rs | 34 ++++- connect/src/lib.rs | 2 +- src/args.rs | 15 +- src/data/paths.rs | 2 +- src/error.rs | 7 +- src/firmware/mod.rs | 1 - src/help.rs | 255 +++++++++++++++++++------------ src/help/devices.md | 5 +- src/help/output.md | 17 ++- src/main.rs | 6 +- src/output.rs | 19 ++- src/pairing.rs | 2 +- src/style.rs | 34 ++++- tests/palette.rs | 101 +++++++++++- 15 files changed, 354 insertions(+), 148 deletions(-) diff --git a/README.md b/README.md index 9dda9a8..0df9292 100644 --- a/README.md +++ b/README.md @@ -91,7 +91,7 @@ One Ark is picked automatically. With several, select one with `-d` by locator, ## Output and automation -Default output is formatted for reading. Use `--json` for complete, exact values, with an indented result document on stdout and one JSON event per line on stderr. App reports pass through raw by default. Colour requires a terminal; `NO_COLOR` or `CLICOLOR=0` disables it. +Default output is formatted for reading. Use `--json` for complete, exact values, with an indented result document on stdout and one JSON event per line on stderr. App reports pass through raw by default. Color requires a terminal; `NO_COLOR` or `CLICOLOR=0` disables it. Scripts and AI agents should read `ark help agents` first. Nothing prompts without a terminal or with `--json`, and the exit code says what happened. `ark help output` defines the streams, JSON fields and error codes. diff --git a/connect/src/emulator/registry.rs b/connect/src/emulator/registry.rs index a1f3fc8..9fb4251 100644 --- a/connect/src/emulator/registry.rs +++ b/connect/src/emulator/registry.rs @@ -51,9 +51,15 @@ impl Instance { } } +/// The listing version this build understands. A breaking change to the +/// registry bumps it, so a newer registry is refused rather than misread. +const VERSION: u64 = 1; + /// Registry response with entries retained for individual decoding. #[derive(Deserialize)] struct Listing { + #[serde(default)] + version: Option, // Absent on a registry older than the contract #[serde(default)] instances: Vec, // Entries decoded independently for compatibility } @@ -73,6 +79,15 @@ fn list_at(addr: SocketAddr) -> Result, Error> { }; let listing: Listing = serde_json::from_slice(&body) .map_err(|err| Error::Registry(io::Error::new(io::ErrorKind::InvalidData, err)))?; + if listing.version != Some(VERSION) { + return Err(Error::Registry(io::Error::new( + io::ErrorKind::InvalidData, + match listing.version { + Some(version) => format!("emulator registry version {version} is not supported"), + None => "emulator registry listing carries no version".to_string(), + }, + ))); + } // Skip entries this build cannot decode without losing compatible entries // from the same registry response. @@ -196,7 +211,7 @@ mod tests { #[test] fn test_chunked_listing() { let (listener, addr) = bind(); - let parts = ["{\"instances\":[", "{\"port\":18181}]}"]; + let parts = ["{\"version\":1,\"instances\":[", "{\"port\":18181}]}"]; let mut response = String::from("HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n"); for part in parts { response.push_str(&format!("{:x}\r\n{part}\r\n", part.len())); @@ -223,6 +238,23 @@ mod tests { ); } + // Tests that a listing of a version this build does not know, or of no + // version at all, is refused rather than read. + #[test] + fn test_unknown_version_is_refused() { + for body in [ + "{\"version\":2,\"instances\":[{\"port\":18181}]}", + "{\"instances\":[{\"port\":18181}]}", + ] { + let (listener, addr) = bind(); + serve(listener, format!("HTTP/1.1 200 OK\r\n\r\n{body}")); + assert!( + matches!(list_at(addr), Err(Error::Registry(error)) if error.kind() == io::ErrorKind::InvalidData), + "{body}" + ); + } + } + // Tests that a service answering with anything but a listing fails the // lookup, a refusal, a body of another shape or no answer at all. #[test] diff --git a/connect/src/lib.rs b/connect/src/lib.rs index 5732ece..2d85e90 100644 --- a/connect/src/lib.rs +++ b/connect/src/lib.rs @@ -62,7 +62,7 @@ //! Firmware preparation may require approval, so a rejected proof refreshes cloud //! keys and returns an error for the caller to retry explicitly. //! -//! Downloads, package catalogues, version selection, caches, prompts, signal +//! Downloads, package catalogs, version selection, caches, prompts, signal //! handling and reboot waits belong to callers. Connect accepts readers, checks //! declared sizes and optional dataset hashes, and does not retry a failed //! transfer. Progress supplies upload session and execution task IDs for explicit diff --git a/src/args.rs b/src/args.rs index 7f0a443..2eee6e1 100644 --- a/src/args.rs +++ b/src/args.rs @@ -129,13 +129,13 @@ pub(crate) enum Command { Unlock, /// Give the Ark its attested identity Enroll(Enroll), - /// Datasets: read with list/show/paths; change with upload/fetch/delete/repair + /// Read and change the datasets on the Ark #[command(subcommand)] Data(Data), - /// Apps: run, cancel + /// Run an app on the Ark, or cancel one #[command(subcommand)] App(App), - /// Firmware: list, update + /// List and install Ark firmware #[command(subcommand)] Firmware(Firmware), /// Check this computer, the Ark and the cloud; suggest fixes @@ -150,7 +150,7 @@ pub(crate) enum Command { /// Command path or topic name #[arg(num_args = 0.., value_name = "COMMAND_OR_TOPIC")] path: Vec, - // Print all command help and embedded topics as one manual. + /// Print the whole manual: every command page and every topic #[arg(long, conflicts_with = "path")] all: bool, }, @@ -267,10 +267,7 @@ pub(crate) enum Firmware { /// Plan the update without approval or installation #[arg(long, conflicts_with = "unlock")] dry_run: bool, - /// Verify the Ark returns running the target build (default) - #[arg(long, conflicts_with = "no_wait")] - wait: bool, - /// Return when installation is acknowledged + /// Return when installation is acknowledged, before the reboot is verified #[arg(long)] no_wait: bool, }, @@ -402,7 +399,7 @@ mod tests { "x", "--no-cache", ], - vec!["ark", "firmware", "update", "--wait", "--no-wait"], + vec!["ark", "firmware", "update", "--dry-run", "--unlock"], vec!["ark", "--quiet", "status", "-v"], ] { assert!( diff --git a/src/data/paths.rs b/src/data/paths.rs index 71a9d09..c0847fe 100644 --- a/src/data/paths.rs +++ b/src/data/paths.rs @@ -165,7 +165,7 @@ mod tests { assert_eq!( text, format!( - " / directory, + grantable, ! unavailable; details with --json\n v1/sample/ +\n groups// !{}alpha, beta\n value{}A/G, T|T, A, ./., A/., ., AT/A, \n{}T/*, A/\n summary\n v2/sample/a-long-example-directory-name/ !", + " / directory, + grantable, ! unavailable; details with --json\n v1/sample/ +\n groups// !{}alpha, beta\n value{}A/G, T|T, A, ./., A/., ., AT/A,\n{}T/*, A/\n summary\n v2/sample/a-long-example-directory-name/ !", " ".repeat(26), " ".repeat(35), " ".repeat(46) diff --git a/src/error.rs b/src/error.rs index d36a0c5..a9c05e0 100644 --- a/src/error.rs +++ b/src/error.rs @@ -41,11 +41,12 @@ impl Error { self.hints.push(hint.into()); self } - /// Encodes the result error; hints travel as stderr events instead. + /// Encodes the result error; hints travel as stderr events instead. The + /// Ark's number is a decimal string, as every 64-bit value is in JSON. pub fn json(&self) -> Value { let mut value = json!({"code": self.code, "message": self.message}); if let Some(remote) = &self.remote { - value["remote"] = json!({"code": remote.code, "message": remote.msg}); + value["remote"] = json!({"code": remote.code.to_string(), "message": remote.msg}); } value } @@ -214,7 +215,7 @@ mod tests { assert_eq!(error.class, 5); assert_eq!(error.code, "ark"); assert!(error.hints.is_empty()); - assert_eq!(error.json()["remote"]["code"], code); + assert_eq!(error.json()["remote"]["code"], code.to_string()); assert_eq!(error.json()["remote"]["message"], "owner's verdict"); } } diff --git a/src/firmware/mod.rs b/src/firmware/mod.rs index b9451f5..7aa8fd1 100644 --- a/src/firmware/mod.rs +++ b/src/firmware/mod.rs @@ -70,7 +70,6 @@ pub(crate) fn run(context: &Context, command: args::Firmware) -> Result<(), Erro let args::Firmware::Update { version, dry_run, - wait: _, no_wait, } = command else { diff --git a/src/help.rs b/src/help.rs index 67c89ab..b92e328 100644 --- a/src/help.rs +++ b/src/help.rs @@ -4,7 +4,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//! Help is generated from the commands this build actually serves. +//! Help is generated from the commands this build actually serves. A page has +//! one shape on a terminal and in a pipe; only color and glyphs differ. use crate::{ args::Cli, @@ -13,11 +14,19 @@ use crate::{ }; use clap::CommandFactory; -/// Help stays plain in pipes and gains terminal styling independently of --json. -pub(crate) fn theme(stderr: bool) -> Theme { - let mut theme = Theme::new(false, stderr); - theme.human = theme.interactive; - theme +/// Options every command accepts, listed once on the root page and hidden on +/// every other page, since an agent reads the manual from the root. +const GLOBAL: [&str; 10] = [ + "device", "json", "timeout", "unlock", "yes", "no_input", "env", "quiet", "verbose", "log", +]; + +/// Help topics in the order the manual prints them. +const TOPICS: [&str; 6] = ["agents", "states", "output", "devices", "datasets", "apps"]; + +/// Help is written for reading even when the invocation selects JSON, so the +/// theme never follows that flag; a pipe still loses color and glyphs. +pub(crate) fn theme() -> Theme { + Theme::new(false, false) } /// Builds the executable command tree with shared styling and command-specific contracts. @@ -25,31 +34,35 @@ pub(crate) fn command(theme: &Theme) -> clap::Command { let mut command = Cli::command(); decorate(&mut command, "", theme); command.build(); - compact(&mut command, theme); + compact(&mut command, theme, true); command } /// Clap normally expands long help onto two lines per option. Render its short -/// layout once, then let the help action select the short or long footer. -fn compact(command: &mut clap::Command, theme: &Theme) { +/// layout once, wrapped to the width, then let the help action select the +/// short or long footer. The shared options drop off every page but the root. +fn compact(command: &mut clap::Command, theme: &Theme, root: bool) { + if !root { + for name in GLOBAL { + *command = command + .clone() + .mut_arg(name, |argument| argument.hide(true)); + } + } let mut display = command.clone().after_help(None).after_long_help(None); - let rendered = display.render_help(); - let scan = if theme.human { - rendered - .ansi() - .to_string() - .lines() - .map(|line| style::wrap(&theme.inline(line), theme.width, 6)) - .collect::>() - .join("\n") - } else { - rendered.to_string() - }; + let scan = display + .render_help() + .ansi() + .to_string() + .lines() + .map(|line| style::wrap(&theme.inline(line), theme.width, 6)) + .collect::>() + .join("\n"); *command = command .clone() .help_template(format!("{}{{after-help}}", scan.trim_end())); for child in command.get_subcommands_mut() { - compact(child, theme); + compact(child, theme, false); } } @@ -104,7 +117,7 @@ fn decorate(command: &mut clap::Command, parent: &str, theme: &Theme) { ), "pair" => ( "an unpaired Ark and cloud access", - "scan in Ark Companion and confirm colours", + "scan in Ark Companion and confirm colors", "up to 10 minutes to scan; then approval and storage setup", "serial, paired; pairing URL on stderr", "ark pair\nark pair --json", @@ -118,7 +131,7 @@ fn decorate(command: &mut clap::Command, parent: &str, theme: &Theme) { ), "enroll" => ( "one Ark; --cwt accepts an existing attestation", - "online enrollment uses the Hub; none with --cwt", + "online enrollment uses Ark Hub; none with --cwt", "seconds for --cwt and reconnection", "enrolled, url for online enrollment; status fields then enrolled with --cwt", "ark enroll\nark enroll --cwt attestation.cwt", @@ -158,12 +171,19 @@ fn decorate(command: &mut clap::Command, parent: &str, theme: &Theme) { "slot, size, cached, outcome; JSON fetched: slot, id, url, size_bytes, sha256, cached, outcome, error", "ark data fetch --all\nark data fetch reference-genome --dry-run --json", ), - "data delete" | "data repair" => ( + "data delete" => ( "a paired, unlocked Ark (--unlock only if status reports locked)", "on your phone; never under --dry-run", "up to a minute for approval", "slot, id, state, changed; required_by under --dry-run", - "ark data delete snp-indel-calls --dry-run\nark data repair snp-indel-calls", + "ark data delete snp-indel-calls\nark data delete snp-indel-calls --dry-run", + ), + "data repair" => ( + "a paired, unlocked Ark (--unlock only if status reports locked)", + "on your phone; never under --dry-run", + "up to a minute for approval", + "slot, id, state, changed; required_by under --dry-run", + "ark data repair snp-indel-calls\nark data repair snp-indel-calls --dry-run", ), "app run" => ( "a local WASM file and a paired, unlocked Ark (--unlock only if status reports locked)", @@ -233,25 +253,6 @@ fn decorate(command: &mut clap::Command, parent: &str, theme: &Theme) { } _ => "0 done; 1 local; 2 usage", }; - let help = format!( - "Requires: {requires}\nApproval: {approval}\nTime: {time}\nPrints: {prints}\nExit: {exits}; 130/143 interrupted\n\nExamples:\n {}", - examples.replace('\n', "\n ") - ); - let help = if theme.human { - footer( - theme, - &[ - ("Requires", requires), - ("Approval", approval), - ("Time", time), - ("Prints", prints), - ("Exit", &format!("{exits}; 130/143 interrupted")), - ], - examples, - ) - } else { - help - }; let help = if parent.is_empty() { "Output is formatted for reading; --json keeps complete, exact values. Scripts and AI agents: read `ark help agents` first. @@ -262,6 +263,8 @@ Topics: agents, states, output, devices, datasets, apps." "Each subcommand has its own requirements, approvals and output.\nRead `ark {key} COMMAND --help` for its contract." ) } else { + // Clap keeps an option hidden from the short page out of the rendered + // scan too, so the long page lists it by hand ahead of the contract. let advanced = if matches!(key, "status" | "enroll") { "Advanced: --pubkey Pin an xDSA public key instead of verifying the attestation @@ -270,16 +273,26 @@ Topics: agents, states, output, devices, datasets, apps." } else { "" }; - format!("{advanced}{help}") - }; - let help = if theme.human { - help.lines() - .map(|line| style::wrap(&theme.inline(line), theme.width, 0)) - .collect::>() - .join("\n") - } else { - help + format!( + "{advanced}{}", + footer( + theme, + &[ + ("Requires", requires), + ("Approval", approval), + ("Time", time), + ("Prints", prints), + ("Exit", &format!("{exits}; 130/143 interrupted")), + ], + examples, + ) + ) }; + let help = help + .lines() + .map(|line| style::wrap(&theme.inline(line), theme.width, 0)) + .collect::>() + .join("\n"); let mut decorated = command.clone().after_long_help(format!("{help}\n")); if parent.is_empty() { decorated = decorated.after_help(format!("{help}\n")); @@ -293,42 +306,25 @@ Topics: agents, states, output, devices, datasets, apps." /// Prints a command page, an embedded topic or the full manual without discovery. /// Help remains readable text even when the invocation selects JSON. pub(crate) fn run(path: &[String], all: bool) -> Result<(), Error> { - let theme = theme(false); + let theme = theme(); let mut root = command(&theme); if all { - if theme.human { - let mut pages = Vec::new(); - collect_help(&mut root, &mut pages); - pages.extend( - ["agents", "states", "output", "devices", "datasets", "apps"] - .map(|name| markdown(&theme, topic(name).unwrap())), - ); - println!( - "{}", - pages.join(&format!( - "\n\n{}\n\n", - theme.paint(Role::Muted, "-".repeat(theme.width.min(80))) - )) - ); - return Ok(()); - } - print_command(&mut root)?; - for name in ["agents", "states", "output", "devices", "datasets", "apps"] { - println!("\n{}", topic(name).unwrap().trim_end()); - } + let mut pages = Vec::new(); + collect_help(&mut root, &mut pages); + pages.extend(TOPICS.map(|name| markdown(&theme, topic(name).unwrap()))); + println!( + "{}", + pages.join(&format!( + "\n\n{}\n\n", + theme.paint(Role::Muted, "-".repeat(theme.width.min(80))) + )) + ); return Ok(()); } if path.len() == 1 && let Some(topic) = topic(&path[0]) { - println!( - "{}", - if theme.human { - markdown(&theme, topic) - } else { - topic.trim_end().to_string() - } - ); + println!("{}", markdown(&theme, topic)); return Ok(()); } let mut command = &mut root; @@ -345,29 +341,36 @@ pub(crate) fn run(path: &[String], all: bool) -> Result<(), Error> { Ok(()) } -/// Aligns short contract labels and shell examples within the human terminal width. +/// Lays out the contract: labels with a colon padded to one column, values +/// wrapped under themselves, then the examples as bare commands, since a pasted +/// prompt breaks in a shell. fn footer(theme: &Theme, fields: &[(&str, &str)], examples: &str) -> String { + let column = fields + .iter() + .map(|(label, _)| label.len() + 2) + .max() + .unwrap_or(0); let mut lines = fields .iter() .map(|(label, text)| { style::wrap( &format!( "{}{}{}", - theme.paint(Role::Muted, label), - " ".repeat(11 - label.len()), + theme.paint(Role::Muted, format!("{label}:")), + " ".repeat(column - label.len() - 1), theme.inline(text) ), theme.width, - 11, + column, ) }) .collect::>(); - lines.push(format!("\n{}", theme.paint(Role::Heading, "Examples"))); + lines.push(format!("\n{}", theme.paint(Role::Heading, "Examples:"))); lines.extend(examples.lines().map(|line| { style::wrap( - &format!(" $ {}", theme.paint(Role::Accent, line)), + &format!(" {}", theme.paint(Role::Accent, line)), theme.width, - 4, + 2, ) })); lines.join("\n") @@ -421,7 +424,7 @@ fn markdown(theme: &Theme, text: &str) -> String { lines.join("\n").trim_end().to_string() } -/// Collects human command pages in command-tree order for the complete manual. +/// Collects command pages in command-tree order for the complete manual. fn collect_help(command: &mut clap::Command, pages: &mut Vec) { pages.push( command @@ -435,15 +438,7 @@ fn collect_help(command: &mut clap::Command, pages: &mut Vec) { collect_help(child, pages); } } -/// Prints this command and its descendants as plain long-help pages. -fn print_command(command: &mut clap::Command) -> Result<(), Error> { - command.print_long_help()?; - println!("\n"); - for child in command.get_subcommands_mut() { - print_command(child)?; - } - Ok(()) -} + /// Returns a compiled-in help topic by its public name. fn topic(name: &str) -> Option<&'static str> { Some(match name { @@ -470,7 +465,7 @@ mod tests { &[("Requires", "a paired Ark"), ("Approval", "only if locked")], "ark unlock" ), - "Requires a paired Ark\nApproval only if locked\n\n\x1b[1mExamples\x1b[0m\n $ \x1b[1mark unlock\x1b[0m" + "Requires: a paired Ark\nApproval: only if locked\n\n\x1b[1mExamples:\x1b[0m\n \x1b[1mark unlock\x1b[0m" ); assert_eq!( markdown( @@ -481,6 +476,34 @@ mod tests { ); } + /// A terminal and a pipe print the same page; only color differs. + #[test] + fn pages_have_one_shape_with_and_without_color() { + let colored = Theme::test(80, Color::True, true); + let plain = Theme { + interactive: false, + unicode: false, + color: Color::Off, + ..colored.clone() + }; + let mut styled = command(&colored); + let mut bare = command(&plain); + for path in [vec!["data", "upload"], vec!["status"], vec![]] { + let (mut styled, mut bare) = (&mut styled, &mut bare); + for name in &path { + styled = styled.find_subcommand_mut(name).unwrap(); + bare = bare.find_subcommand_mut(name).unwrap(); + } + let rendered = styled.render_long_help().ansi().to_string(); + assert!(rendered.contains("\x1b["), "{path:?}"); + assert_eq!( + console::strip_ansi_codes(&rendered), + bare.render_long_help().to_string(), + "{path:?}" + ); + } + } + #[test] fn help_fits_narrow_terminals_without_losing_commands() { let theme = Theme::test(60, Color::True, true); @@ -500,6 +523,36 @@ mod tests { assert!(rendered.contains("\x1b[")); } + #[test] + fn shared_options_are_listed_on_the_root_page_only() { + let theme = Theme::test(80, Color::Off, false); + let mut root = command(&theme); + // Examples mention the flags too, so the listing is told by the text + // clap prints beside each option. + let listed = [ + "--timeout ", + "Print the complete result as JSON", + "Never prompt; fail with", + "Diagnostic logs: debug for connect", + "Which Ark: locator", + ]; + let page = root.render_help().to_string(); + for option in listed { + assert!(page.contains(option), "{option}"); + } + for path in [vec!["data"], vec!["data", "upload"], vec!["doctor"]] { + let mut command = &mut root; + for name in &path { + command = command.find_subcommand_mut(name).unwrap(); + } + let page = command.render_long_help().to_string(); + for option in listed { + assert!(!page.contains(option), "{path:?}: {option}"); + } + assert!(page.contains("-h, --help"), "{path:?}"); + } + } + #[test] fn groups_point_to_child_contracts() { let theme = Theme::test(80, Color::Off, false); diff --git a/src/help/devices.md b/src/help/devices.md index 67a9ec8..72a2053 100644 --- a/src/help/devices.md +++ b/src/help/devices.md @@ -2,8 +2,9 @@ `ark devices` discovers hardware over USB and running emulators through their local launcher registry. It does not connect or authenticate. A missing emulator -registry is normal when no emulator is running. A failed discovery source does -not hide devices found through another source. +registry is normal when no emulator is running. A registry whose listing +version this tool does not know is a failed source, reported as a warning. A +failed discovery source does not hide devices found through another source. Emulators come from the desktop app at https://github.com/dark-bio/emulator, which boots the real firmware on this computer. It exists for development and diff --git a/src/help/output.md b/src/help/output.md index 7045185..be757e8 100644 --- a/src/help/output.md +++ b/src/help/output.md @@ -6,7 +6,7 @@ instructions, hints and errors. Keep the streams separate when parsing output. ## Reading and parsing Default output is formatted for reading, including when redirected. It may use -tables, scale units, localise timestamps and add status marks. Labels describe +tables, scale units, localize timestamps and add status marks. Labels describe what is shown, so Size carries its unit in the value. Byte columns share one unit so sizes can be compared down the column. A mark before a state value is decoration, not part of the value. Absent values appear as -, empty lists as none and @@ -20,13 +20,14 @@ differ. --json prints one complete, exact result document on stdout, indented by two spaces. Keys are snake_case, absent values null, enums strings, times ISO 8601 -UTC, byte counts suffixed _bytes and durations _seconds. Task IDs are decimal -strings so every u64 is exact. JSON field additions are allowed; renames require -a major version. Scripts should pin the tool version. Reading layouts and -labels may change. Help and completions always print text. +UTC, byte counts suffixed _bytes and durations _seconds. Task IDs and the Ark's +error numbers are decimal strings so every 64-bit value is exact. JSON field +additions are allowed; renames require a major version. Scripts should pin the +tool version. Reading layouts and labels may change. Help and completions always +print text. -Colour and live progress require a terminal. NO_COLOR or CLICOLOR=0 disables -colour; neither can force it on in a pipe. +Color and live progress require a terminal. NO_COLOR or CLICOLOR=0 disables +color; neither can force it on in a pipe. ## Payloads and failures @@ -70,7 +71,7 @@ structured progress API. The code in error[code]: is stable and its exit code is the class below. This prefix also applies to argument errors. hint: lines name a next step where the tool knows one. JSON errors carry code, message and, for the Ark's own verdicts, -remote code and message. +remote code and message, the code as a decimal string. Exit 1, local input or confirmation: - `file-not-found`, `file-unreadable`, `file-empty`: the named path diff --git a/src/main.rs b/src/main.rs index 14c6798..6fd18c7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -40,7 +40,7 @@ fn main() -> ExitCode { .skip(1) .take_while(|arg| *arg != "--") .any(|arg| arg == "--json"); - let mut command = help::command(&help::theme(false)); + let mut command = help::command(&help::theme()); let matches = match command.try_get_matches_from_mut(&arguments) { Ok(matches) => matches, Err(error) => { @@ -117,7 +117,7 @@ fn main() -> ExitCode { fn run(context: &Context, command: Option) -> Result<(), Error> { match command { None => { - help::command(&help::theme(false)).print_help()?; + help::command(&help::theme()).print_help()?; Ok(()) } Some(Command::Devices) => device::devices(context), @@ -134,7 +134,7 @@ fn run(context: &Context, command: Option) -> Result<(), Error> { Some(Command::Completions { shell }) => { clap_complete::generate( shell, - &mut help::command(&help::theme(false)), + &mut help::command(&help::theme()), "ark", &mut std::io::stdout(), ); diff --git a/src/output.rs b/src/output.rs index e308079..958ce22 100644 --- a/src/output.rs +++ b/src/output.rs @@ -216,14 +216,17 @@ impl Output { return; } let message = message.as_ref(); + // JSON escapes on its own; the reading streams get a printable copy, + // since a device name or verdict must not drive the terminal. + let shown = style::printable(message); if kind == "progress" && self.terminal() { let theme = &self.0.err; let line = format!( "{} {}", theme.paint(Role::Muted, "progress:"), - theme.inline(message) + theme.inline(&shown) ); - self.progress_line(message.split(':').next().unwrap_or(message), &line); + self.progress_line(shown.split(':').next().unwrap_or(&shown), &line); return; } { @@ -237,9 +240,9 @@ impl Output { let _ = writeln!(stderr, "{}", json!({"event":kind,"message":message})); } else if self.terminal() { separate_result(&mut terminal, &mut stderr); - let _ = writeln!(stderr, "{}", event_line(&self.0.err, kind, message)); + let _ = writeln!(stderr, "{}", event_line(&self.0.err, kind, &shown)); } else { - let _ = writeln!(stderr, "{kind}: {message}"); + let _ = writeln!(stderr, "{kind}: {shown}"); } terminal.err_printed = true; let _ = stderr.flush(); @@ -474,6 +477,7 @@ impl Output { .as_ref() .map(|remote| format!(" (code 0x{:x})", remote.code)) .unwrap_or_default(); + let message = style::printable(&error.message); let mut terminal = self.0.terminal.lock().expect("output not poisoned"); let mut stderr = io::stderr().lock(); if self.terminal() { @@ -482,12 +486,12 @@ impl Output { let line = format!( "{} {}{}", theme.paint(Role::Failure, format!("error[{}]:", error.code)), - theme.inline(&error.message), + theme.inline(&message), theme.paint(Role::Muted, &remote) ); let _ = writeln!(stderr, "{}", style::wrap(&line, theme.width, 2)); } else { - let _ = writeln!(stderr, "error[{}]: {}{remote}", error.code, error.message); + let _ = writeln!(stderr, "error[{}]: {message}{remote}", error.code); } terminal.err_printed = true; } @@ -583,12 +587,13 @@ fn tick(theme: &Theme, terminal: &mut Terminal, output: &mut impl Write, now: In } /// Formats scalar values and lists without terminal styling or field-specific units. +/// Text is made printable here, since every reading layout passes through it. pub(crate) fn scalar(value: &Value) -> String { match value { Value::Null => "-".into(), Value::Bool(true) => "yes".into(), Value::Bool(false) => "no".into(), - Value::String(value) => value.clone(), + Value::String(value) => style::printable(value), Value::Array(values) => values.iter().map(scalar).collect::>().join(", "), value => value.to_string(), } diff --git a/src/pairing.rs b/src/pairing.rs index 3b3c799..db1d6fe 100644 --- a/src/pairing.rs +++ b/src/pairing.rs @@ -93,7 +93,7 @@ pub(crate) fn run(context: &Context) -> Result<(), Error> { } context .output - .event("approve", "confirm the colours on the Ark and your phone"); + .event("approve", "confirm the colors on the Ark and your phone"); if context.output.terminal() { previous = Some("approval"); } diff --git a/src/style.rs b/src/style.rs index 8bd7e71..6511483 100644 --- a/src/style.rs +++ b/src/style.rs @@ -48,8 +48,6 @@ pub(crate) enum Color { /// Presentation capabilities resolved once for one output stream. #[derive(Clone, Debug)] pub(crate) struct Theme { - /// Whether this stream uses human layouts. - pub human: bool, /// Whether this stream permits cursor control and live line updates. pub interactive: bool, /// Whether terminal and locale permit decorative Unicode glyphs. @@ -120,7 +118,6 @@ impl Theme { .size_checked() .map_or(80, |(_, width)| usize::from(width).max(1)); Self { - human, interactive, unicode, color, @@ -234,6 +231,23 @@ impl Theme { /// Terminal control sequences belong to the writer, never to result content. pub(crate) const CLEAR_LINE: &str = "\r\x1b[2K"; +/// Escapes control characters in text that came from a device, a file or a +/// remote, so it can neither drive the terminal nor start a line of its own. +pub(crate) fn printable(text: &str) -> String { + if !text.chars().any(char::is_control) { + return text.to_string(); + } + text.chars() + .flat_map(|ch| { + if ch.is_control() { + ch.escape_default().collect::>() + } else { + vec![ch] + } + }) + .collect() +} + /// Formats a byte count in binary units up to GiB with one decimal place. pub(crate) fn bytes(bytes: u64) -> String { for (unit, divisor) in [("GiB", 1_u64 << 30), ("MiB", 1 << 20), ("KiB", 1 << 10)] { @@ -253,6 +267,10 @@ pub(crate) fn wrap(text: &str, width: usize, indent: usize) -> String { let mut append = |word: &str| { let size = console::measure_text_width(word.trim_end()); if column > indent && column + size > width && size <= width - indent { + // The space that ended the previous word is not part of the line. + while result.ends_with(' ') { + result.pop(); + } result.push('\n'); result.push_str(&" ".repeat(indent)); column = indent; @@ -305,7 +323,6 @@ pub(crate) fn wrap(text: &str, width: usize, indent: usize) -> String { impl Theme { pub fn test(width: usize, color: Color, unicode: bool) -> Self { Self { - human: true, interactive: true, unicode, color, @@ -468,6 +485,15 @@ mod tests { assert_eq!(theme.truncate("abcdef", 6), "abcdef"); } + #[test] + fn control_characters_cannot_reach_the_terminal() { + assert_eq!(printable("plain name"), "plain name"); + assert_eq!( + printable("evil\x1b[2Jname\nsecond line\t"), + "evil\\u{1b}[2Jname\\nsecond line\\t" + ); + } + #[test] fn inline_code_keeps_unmatched_backticks() { let theme = Theme::test(80, Color::Basic, false); diff --git a/tests/palette.rs b/tests/palette.rs index 2e20c42..db8f878 100644 --- a/tests/palette.rs +++ b/tests/palette.rs @@ -84,9 +84,13 @@ fn conformance(args: &[&str]) { fn command_tree_output_conforms() { for (path, page) in commands() { let args: Vec<_> = path.iter().map(String::as_str).collect(); - assert!(page.contains("--json"), "{path:?}"); + // The shared options are listed once, on the root page. Examples + // mention the flags too, so the listing is told by its description. + for option in ["--timeout ", "Print the complete result as JSON"] { + assert_eq!(page.contains(option), path.is_empty(), "{path:?}: {option}"); + } assert!(!page.contains("--format"), "{path:?}"); - assert!(page.contains("--log"), "{path:?}"); + assert!(page.contains("-h, --help"), "{path:?}"); let mut rejected = args.clone(); rejected.extend(["--json", "--format", "json"]); let output = ark(&rejected); @@ -149,8 +153,10 @@ fn help_differs_exactly_where_it_promises_more() { #[test] fn documented_usage_errors_keep_the_text_prefix() { + // Topics render in a pipe as on a terminal, so code spans lose their + // backtick markers there and keep only their text. let help = String::from_utf8(ark(&["help", "output"]).stdout).unwrap(); - assert!(help.contains("Exit 2, `usage`")); + assert!(help.contains("Exit 2, usage")); for args in [ vec!["bogus"], vec!["--timeout", "0", "status"], @@ -169,7 +175,7 @@ fn documented_usage_errors_keep_the_text_prefix() { } let output = ark(&["status", "--device", "hardware:palette-no-device"]); assert_eq!(output.status.code(), Some(3)); - assert!(help.contains("`no-device`")); + assert!(help.contains("no-device: no Ark found")); assert!( String::from_utf8(output.stderr) .unwrap() @@ -301,7 +307,13 @@ fn help_matches_the_supported_palette() { ] { assert!(long.contains(field), "{path}: {long}"); } - assert!(long.contains("--timeout ")); + // The contract block keeps one shape in a pipe: colon labels, wrapped + // values, and examples as bare commands with no prompt. + for line in long.lines() { + assert!(line.chars().count() <= 80, "{path}: {line}"); + assert!(!line.trim_start().starts_with("$ "), "{path}: {line}"); + } + assert!(!long.contains("--timeout "), "{path}"); let mut command = vec!["help"]; command.extend(path.split(' ')); assert_eq!(ark(&command).stdout, long.as_bytes()); @@ -328,6 +340,85 @@ fn help_matches_the_supported_palette() { ); } +/// The manual names the example apps and the emulator, the two other corners +/// of the loop a reader arrives in, and the root page lists the shared options. +#[test] +fn manual_carries_the_cross_references() { + let manual = String::from_utf8(ark(&["help", "--all"]).stdout).unwrap(); + for link in [ + "https://github.com/dark-bio/examples", + "https://github.com/dark-bio/emulator", + ] { + assert!(manual.contains(link), "{link}"); + } + let root = String::from_utf8(ark(&["--help"]).stdout).unwrap(); + assert!(root.contains("--timeout ")); + assert!(root.contains("--json")); +} + +/// Every error code the source can emit, read from the source itself, so the +/// output topic is checked against what the tool does and not a second list. +fn emitted_codes() -> std::collections::BTreeSet { + fn visit(dir: &std::path::Path, codes: &mut std::collections::BTreeSet) { + for entry in std::fs::read_dir(dir).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + visit(&path, codes); + continue; + } + if path.extension().is_none_or(|extension| extension != "rs") { + continue; + } + let text = std::fs::read_to_string(&path).unwrap(); + for prefix in ["Error::new(", "Self::new("] { + for (index, _) in text.match_indices(prefix) { + let rest = text[index + prefix.len()..] + .trim_start() + .trim_start_matches(|c: char| c.is_ascii_digit()) + .trim_start() + .trim_start_matches(',') + .trim_start(); + if let Some(rest) = rest.strip_prefix('"') + && let Some(end) = rest.find('"') + { + codes.insert(rest[..end].to_string()); + } + } + } + // The Ark's reserved verdicts map to codes in match arms. + if path.file_name().is_some_and(|name| name == "error.rs") { + for (index, _) in text.match_indices("=> \"") { + let rest = &text[index + 4..]; + if let Some(end) = rest.find('"') { + codes.insert(rest[..end].to_string()); + } + } + } + } + } + let mut codes = std::collections::BTreeSet::new(); + visit( + &std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"), + &mut codes, + ); + assert!(codes.len() > 20, "{codes:?}"); + codes +} + +#[test] +fn every_error_code_is_documented() { + let topic = std::fs::read_to_string( + std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src/help/output.md"), + ) + .unwrap(); + for code in emitted_codes() { + assert!( + topic.contains(&format!("`{code}`")), + "{code} is not in `ark help output`" + ); + } +} + #[test] fn completions_are_generated_for_the_binary_name() { for shell in ["bash", "zsh", "fish", "powershell", "elvish"] {