From 58d7c6f70b7cb19386685c7b1a3affc93f8bf254 Mon Sep 17 00:00:00 2001 From: Steve Morin Date: Sat, 30 May 2026 13:10:52 -0700 Subject: [PATCH 1/6] feat(cli): add ergonomic subcommands as additive layer over flat flags Add `toggle`/`scan`/`check`/`list`/`insert`/`remove` subcommands, each exposing only the flags that apply to that operation. The layer is purely additive: the legacy flat-flag interface is unchanged, and each subcommand translates itself to the equivalent legacy argv (`Commands::to_legacy_argv`) which is re-parsed through the same `build_command()` path. clap therefore stays the single source of truth for defaults, validation, and binary-name aliasing, making subcommand/flat-flag drift structurally impossible. The legacy flat-flag form emits a one-line, TTY-only deprecation nudge (suppressed under --json, non-TTY stderr, and meta/no-op invocations) so scripts and pipes are unaffected. Adds 15 parity tests asserting each subcommand is behavior-identical to its flat-flag equivalent, including a defaulted `--remove-mode` case that guards against silent default drift, plus scoping and help/man-render checks. Refs P10-T02..T04, P10-TS01. --- PROJECTS.md | 35 +++ crates/togl-cli/src/cli.rs | 436 ++++++++++++++++++++++++++- crates/togl-cli/src/main.rs | 47 ++- crates/togl-cli/tests/subcommands.rs | 251 +++++++++++++++ 4 files changed, 765 insertions(+), 4 deletions(-) create mode 100644 crates/togl-cli/tests/subcommands.rs diff --git a/PROJECTS.md b/PROJECTS.md index fcb497b..db97052 100644 --- a/PROJECTS.md +++ b/PROJECTS.md @@ -227,3 +227,38 @@ See `docs/superpowers/specs/2026-05-29-release-please-commitlint-design.md`. ### Manual Steps (maintainer) - Add `RELEASE_PLEASE_APP_ID` + `RELEASE_PLEASE_PRIVATE_KEY` (GitHub App) or `RELEASE_PLEASE_APP_TOKEN` (PAT) secrets. + +--- + +## [~] Project P10: CLI ergonomics refactor (v0.5.0) +**Goal**: Make the CLI more ergonomic without breaking the existing flat-flag +interface. Three additive layers, shipped in order: +1. **Declarative clap constraints** — replace hand-rolled flag-combination + checks with clap `group`/`requires`, so impossible combinations fail at parse + time with consistent messages. +2. **Subcommands** — `toggle`/`scan`/`check`/`list`/`insert`/`remove` expose + only the flags that apply to each operation. *Additive*: the legacy flat + flags remain; each subcommand translates to the equivalent legacy argv and is + re-parsed through the same pipeline, so the two forms cannot drift. The legacy + form emits a TTY-only deprecation nudge. +3. **stdin/stdout filter** — accept `-`/stdin and write to stdout so `togl` + composes as a Unix filter. + +**Out of Scope** +- Removing the flat-flag interface (deprecate only; removal is a future major). +- A `recover` subcommand (the rare `--recover` path stays flat-flag only). + +### Tests & Tasks +- [x] [P10-T01] Option 2: `operation` arg-group + `requires` constraints in `cli.rs` +- [x] [P10-T02] Option 1: `Commands` subcommand enum + `GlobalArgs` flatten struct +- [x] [P10-T03] Option 1: `Commands::to_legacy_argv` translation (clap owns defaults) +- [x] [P10-T04] Option 1: `main()` subcommand bridge + TTY-only deprecation notice +- [x] [P10-TS01] Parity tests: each subcommand ≡ its flat-flag form (incl. a + defaulted `--remove-mode` case to guard default drift); scoping + help/man render +- [ ] [P10-T05] Option 3: stdin/stdout filter support +- [ ] [P10-TS02] Option 3: filter round-trip + `-`-as-path tests + +### Automated Verification +- `cargo test` green (all parity + existing suites) +- `cargo clippy --all-targets -- -D warnings` clean +- `togl --help` / `--man` / `--completions` render the subcommands under the aliased bin name diff --git a/crates/togl-cli/src/cli.rs b/crates/togl-cli/src/cli.rs index cb42995..9df8a8f 100644 --- a/crates/togl-cli/src/cli.rs +++ b/crates/togl-cli/src/cli.rs @@ -27,8 +27,15 @@ pub enum RemoveMode { } #[derive(Parser)] -#[command(author, version, about)] +#[command(author, version, about, args_conflicts_with_subcommands = true)] pub struct Cli { + /// Subcommand form (e.g. `togl scan PATH`). Optional: when omitted, the + /// legacy flat flags below are used. Subcommands are translated to the + /// equivalent flat flags and run through the same pipeline (see + /// `Commands::to_legacy_argv`), so behavior is identical by construction. + #[command(subcommand)] + pub command: Option, + /// File or directory paths to process pub paths: Vec, @@ -181,3 +188,430 @@ pub struct Cli { #[arg(long = "man")] pub man: bool, } + +// ── Subcommand front-end (additive over the legacy flat flags) ── +// +// Each subcommand offers a scoped, ergonomic surface (only the flags that apply +// to that operation). Rather than duplicate the run-pipeline logic, every +// subcommand translates itself into the equivalent *legacy* argv via +// `to_legacy_argv`, which `main()` re-parses through the very same +// `build_command()` path it already uses. clap therefore remains the single +// source of truth for defaults, validation, and binary-name aliasing — so the +// subcommand and flat-flag forms cannot drift apart. + +use std::ffi::OsString; + +/// Flags shared by every subcommand. Flattened into each variant so the legacy +/// definitions on `Cli` stay untouched (additive, not a restructure). +#[derive(clap::Args, Debug)] +pub struct GlobalArgs { + /// Human-readable log lines to stderr + #[arg(short = 'v', long = "verbose")] + pub verbose: bool, + + /// Machine-readable single-line JSON to stdout + #[arg(long = "json")] + pub json: bool, + + /// Comment mode (auto/single/multi) + #[arg(short = 'm', long = "mode", default_value = "auto")] + pub mode: String, + + /// Override file codec (UTF-8 only in Phase 0) + #[arg(short = 'e', long = "encoding", default_value = "utf-8")] + pub encoding: String, + + /// EOL normalization: preserve, lf, or crlf + #[arg(long = "eol", default_value = "preserve")] + pub eol: String, + + /// Map exit codes to sysexits.h values + #[arg(short = 'x', long = "posix-exit")] + pub posix_exit: bool, + + /// Operate on symlink itself instead of target + #[arg(short = 'N', long = "no-dereference")] + pub no_dereference: bool, + + /// Error if target is not .py + #[arg(long = "strict-ext")] + pub strict_ext: bool, + + /// Prompt before modifying each file + #[arg(short = 'i', long = "interactive")] + pub interactive: bool, + + /// Show diff of changes without writing files + #[arg(long = "dry-run")] + pub dry_run: bool, + + /// Extension for atomic temp file + #[arg(short = 't', long = "temp-suffix")] + pub temp_suffix: Option, + + /// Create backup with given extension before modifying (e.g. --backup .bak) + #[arg(long = "backup")] + pub backup: Option, + + /// Path to .toggleConfig TOML file + #[arg(long = "config")] + pub config: Option, + + /// Override comment style: SINGLE [MULTI_START MULTI_END] + #[arg(long = "comment-style", num_args = 1..=3, value_names = ["SINGLE", "MULTI_START", "MULTI_END"])] + pub comment_style: Vec, +} + +impl GlobalArgs { + /// Emit the legacy flags for the globals the user effectively chose. + /// Defaulted string values are emitted only when non-default, keeping the + /// synthesized argv minimal (and behavior-identical either way). + fn push_argv(&self, out: &mut Vec) { + if self.verbose { + out.push("--verbose".into()); + } + if self.json { + out.push("--json".into()); + } + if self.mode != "auto" { + out.push("--mode".into()); + out.push((&self.mode).into()); + } + if self.encoding != "utf-8" { + out.push("--encoding".into()); + out.push((&self.encoding).into()); + } + if self.eol != "preserve" { + out.push("--eol".into()); + out.push((&self.eol).into()); + } + if self.posix_exit { + out.push("--posix-exit".into()); + } + if self.no_dereference { + out.push("--no-dereference".into()); + } + if self.strict_ext { + out.push("--strict-ext".into()); + } + if self.interactive { + out.push("--interactive".into()); + } + if self.dry_run { + out.push("--dry-run".into()); + } + if let Some(t) = &self.temp_suffix { + out.push("--temp-suffix".into()); + out.push(t.into()); + } + if let Some(b) = &self.backup { + out.push("--backup".into()); + out.push(b.into()); + } + if let Some(c) = &self.config { + out.push("--config".into()); + out.push(c.into()); + } + if !self.comment_style.is_empty() { + out.push("--comment-style".into()); + for v in &self.comment_style { + out.push(v.into()); + } + } + } +} + +/// Ergonomic subcommand surface. Each maps to a legacy operation mode. +#[derive(clap::Subcommand, Debug)] +pub enum Commands { + /// Toggle comments on sections or line ranges (default operation). + Toggle { + /// File or directory paths to process + paths: Vec, + /// Section ID(s) to toggle (repeatable). Use `group:variant` for variants. + #[arg(short = 'S', long = "section", action = clap::ArgAction::Append)] + sections: Vec, + /// Line range : or :+ (repeatable) + #[arg(short = 'l', long = "line", action = clap::ArgAction::Append)] + lines: Vec, + /// Recursively walk directories + #[arg(short = 'R', long = "recursive")] + recursive: bool, + /// Force toggle state (on/off/invert) + #[arg(short = 'f', long = "force", visible_short_alias = 'F')] + force: Option, + /// Extend the last --line range to the end of file + #[arg(long = "to-end")] + to_end: bool, + /// Require exactly 2 variants in the targeted group; error otherwise. + #[arg(long = "pair")] + pair: bool, + /// Enable atomic multi-file mode: all files succeed or none are modified. + #[arg(long = "atomic")] + atomic: bool, + /// Disable backup creation in atomic mode (only valid with --atomic). + #[arg(long = "no-backup")] + no_backup: bool, + #[command(flatten)] + global: GlobalArgs, + }, + /// Scan for section IDs without modifying files (read-only). + Scan { + /// File or directory paths to scan + paths: Vec, + /// Limit/expand to specific section ID(s) for the detailed group view. + #[arg(short = 'S', long = "section", action = clap::ArgAction::Append)] + sections: Vec, + /// Use the recursive summary view. + #[arg(short = 'R', long = "recursive")] + recursive: bool, + /// Require exactly 2 variants in the targeted group. + #[arg(long = "pair")] + pair: bool, + #[command(flatten)] + global: GlobalArgs, + }, + /// Validate section integrity without modifying files (scan + check). + Check { + /// File or directory paths to validate + paths: Vec, + /// Limit validation to specific section ID(s). + #[arg(short = 'S', long = "section", action = clap::ArgAction::Append)] + sections: Vec, + /// Enforce exactly 2 variants in each targeted group. + #[arg(long = "pair")] + pair: bool, + #[command(flatten)] + global: GlobalArgs, + }, + /// List all section IDs found in files (discovery mode). + List { + /// File or directory paths to list + paths: Vec, + /// Recursively walk directories + #[arg(short = 'R', long = "recursive")] + recursive: bool, + /// Detail level: ids | files | lines + #[arg(long = "fields", default_value = "lines")] + fields: ListFields, + #[command(flatten)] + global: GlobalArgs, + }, + /// Insert a toggle:start/end marker pair around a single line range. + Insert { + /// Single file to modify + path: PathBuf, + /// Section ID for the inserted marker (exactly one) + #[arg(short = 'S', long = "section")] + section: String, + /// Line range to wrap (exactly one) + #[arg(short = 'l', long = "line")] + line: String, + /// Description for the inserted section marker + #[arg(long = "desc")] + desc: Option, + /// Extend the range to the end of file + #[arg(long = "to-end")] + to_end: bool, + #[command(flatten)] + global: GlobalArgs, + }, + /// Remove a named section (markers and/or body) from files. + Remove { + /// File or directory paths to process + paths: Vec, + /// Section ID to remove (exactly one) + #[arg(short = 'S', long = "section")] + section: String, + /// Recursively walk directories + #[arg(short = 'R', long = "recursive")] + recursive: bool, + /// What to strip: markers | commented | all + #[arg(long = "remove-mode", default_value = "commented")] + remove_mode: RemoveMode, + /// Exit non-zero if -S matched no sections. + #[arg(long = "require-match")] + require_match: bool, + #[command(flatten)] + global: GlobalArgs, + }, +} + +/// Canonical kebab-case name for a `ValueEnum` value (e.g. `Lines` -> "lines"). +fn enum_name(v: &E) -> OsString { + v.to_possible_value() + .expect("ValueEnum variant is not skipped") + .get_name() + .into() +} + +impl Commands { + /// Translate this subcommand into the equivalent legacy argv, with `bin` as + /// argv[0]. The result is fed back through `build_command()` so clap owns + /// defaults/validation/aliasing — making subcommand/flat-flag drift + /// structurally impossible. + pub fn to_legacy_argv(&self, bin: OsString) -> Vec { + let mut out: Vec = vec![bin]; + match self { + Commands::Toggle { + paths, + sections, + lines, + recursive, + force, + to_end, + pair, + atomic, + no_backup, + global, + } => { + for s in sections { + out.push("-S".into()); + out.push(s.into()); + } + for l in lines { + out.push("-l".into()); + out.push(l.into()); + } + if *recursive { + out.push("--recursive".into()); + } + if let Some(f) = force { + out.push("--force".into()); + out.push(f.into()); + } + if *to_end { + out.push("--to-end".into()); + } + if *pair { + out.push("--pair".into()); + } + if *atomic { + out.push("--atomic".into()); + } + if *no_backup { + out.push("--no-backup".into()); + } + global.push_argv(&mut out); + push_paths(&mut out, paths); + } + Commands::Scan { + paths, + sections, + recursive, + pair, + global, + } => { + out.push("--scan".into()); + for s in sections { + out.push("-S".into()); + out.push(s.into()); + } + if *recursive { + out.push("--recursive".into()); + } + if *pair { + out.push("--pair".into()); + } + global.push_argv(&mut out); + push_paths(&mut out, paths); + } + Commands::Check { + paths, + sections, + pair, + global, + } => { + out.push("--scan".into()); + out.push("--check".into()); + for s in sections { + out.push("-S".into()); + out.push(s.into()); + } + if *pair { + out.push("--pair".into()); + } + global.push_argv(&mut out); + push_paths(&mut out, paths); + } + Commands::List { + paths, + recursive, + fields, + global, + } => { + out.push("--list-sections".into()); + if *recursive { + out.push("--recursive".into()); + } + if *fields != ListFields::Lines { + out.push("--fields".into()); + out.push(enum_name(fields)); + } + global.push_argv(&mut out); + push_paths(&mut out, paths); + } + Commands::Insert { + path, + section, + line, + desc, + to_end, + global, + } => { + out.push("--insert".into()); + out.push("-S".into()); + out.push(section.into()); + out.push("-l".into()); + out.push(line.into()); + if let Some(d) = desc { + out.push("--desc".into()); + out.push(d.into()); + } + if *to_end { + out.push("--to-end".into()); + } + global.push_argv(&mut out); + out.push("--".into()); + out.push(path.into()); + } + Commands::Remove { + paths, + section, + recursive, + remove_mode, + require_match, + global, + } => { + out.push("--remove".into()); + out.push("-S".into()); + out.push(section.into()); + if *recursive { + out.push("--recursive".into()); + } + if *remove_mode != RemoveMode::Commented { + out.push("--remove-mode".into()); + out.push(enum_name(remove_mode)); + } + if *require_match { + out.push("--require-match".into()); + } + global.push_argv(&mut out); + push_paths(&mut out, paths); + } + } + out + } +} + +/// Push positional paths after a `--` guard so a path starting with `-` (or a +/// path literally named like a flag) is never misread as an option. +fn push_paths(out: &mut Vec, paths: &[PathBuf]) { + if paths.is_empty() { + return; + } + out.push("--".into()); + for p in paths { + out.push(p.into()); + } +} diff --git a/crates/togl-cli/src/main.rs b/crates/togl-cli/src/main.rs index 4ceaa20..4e18a26 100644 --- a/crates/togl-cli/src/main.rs +++ b/crates/togl-cli/src/main.rs @@ -84,9 +84,16 @@ fn build_command() -> clap::Command { .bin_name(bin_name) } -fn main() { +/// Parse argv through the canonical clap command, exiting the process on a +/// parse error (mirroring clap's default behavior, but routed through our +/// custom `build_command()` so binary-name aliasing is preserved). +fn parse_cli(argv: I) -> Cli +where + I: IntoIterator, + T: Into + Clone, +{ let mut cmd = build_command(); - let matches = match cmd.try_get_matches_from_mut(std::env::args_os()) { + let matches = match cmd.try_get_matches_from_mut(argv) { Ok(m) => m, Err(e) => { let kind = e.kind(); @@ -105,12 +112,31 @@ fn main() { }); } }; - let cli = match ::from_arg_matches(&matches) { + match ::from_arg_matches(&matches) { Ok(c) => c, Err(e) => { let _ = e.print(); std::process::exit(ExitCode::Usage.code()); } + } +} + +fn main() { + let raw: Vec = std::env::args_os().collect(); + let parsed = parse_cli(raw.iter().cloned()); + + // ── Subcommand bridge ── + // A subcommand is an ergonomic front-end: translate it to the equivalent + // legacy argv and re-parse through the same path, yielding a flat `Cli` + // (command = None) that the existing pipeline handles unchanged. The legacy + // flat-flag form emits a one-line deprecation nudge (humans only). + let cli = if let Some(command) = &parsed.command { + let bin = raw.first().cloned().unwrap_or_else(|| "togl".into()); + let legacy_argv = command.to_legacy_argv(bin); + parse_cli(legacy_argv) + } else { + maybe_warn_legacy(&parsed); + parsed }; let result = run(&cli); @@ -133,6 +159,21 @@ fn main() { std::process::exit(exit_val); } +/// Emit a one-line deprecation nudge toward the subcommand form when the legacy +/// flat-flag interface is used interactively. Suppressed in machine contexts +/// (`--json`, non-TTY stderr) and for meta/no-op invocations so it never +/// pollutes scripts, pipes, or generated output. +fn maybe_warn_legacy(cli: &Cli) { + let meta = cli.completions.is_some() || cli.man; + if cli.json || meta || cli.paths.is_empty() || !std::io::stderr().is_terminal() { + return; + } + eprintln!( + "note: flat-flag usage is deprecated; prefer subcommands \ + (toggle/scan/check/list/insert/remove). See `--help`." + ); +} + fn classify_error(err: &anyhow::Error) -> ExitCode { // Walk the error chain looking for specific typed errors for cause in err.chain() { diff --git a/crates/togl-cli/tests/subcommands.rs b/crates/togl-cli/tests/subcommands.rs new file mode 100644 index 0000000..ff6611e --- /dev/null +++ b/crates/togl-cli/tests/subcommands.rs @@ -0,0 +1,251 @@ +//! Parity tests for the additive subcommand front-end. +//! +//! The subcommand form (`togl scan PATH`) is translated to the legacy flat +//! flags (`togl --scan PATH`) and run through the same pipeline, so the two +//! forms must be behavior-identical. These tests pin that invariant: for +//! read-only ops they compare stdout; for write ops they compare resulting +//! file bytes. At least one case (`remove` without `--remove-mode`) exercises a +//! *defaulted* field, guarding against silent default drift between the two +//! front-ends. + +use assert_cmd::Command; +use predicates::prelude::*; +use std::fs; +use tempfile::TempDir; + +#[allow(deprecated)] +fn cmd() -> Command { + Command::cargo_bin("toggle").unwrap() +} + +const SECTION_FILE: &str = "# toggle:start ID=feat\nprint(\"hi\")\n# toggle:end ID=feat\nafter\n"; + +/// Run `args` and return captured stdout as a String. +fn stdout_of(args: &[&str]) -> String { + let out = cmd().args(args).assert().get_output().stdout.clone(); + String::from_utf8(out).unwrap() +} + +/// Assert that a read-only subcommand and its legacy equivalent print the same +/// stdout for identical input files. +fn assert_stdout_parity(content: &str, sub: &[&str], legacy: &[&str]) { + let dir = TempDir::new().unwrap(); + let a = dir.path().join("a.py"); + let b = dir.path().join("b.py"); + fs::write(&a, content).unwrap(); + fs::write(&b, content).unwrap(); + + let mut sub_args: Vec<&str> = sub.to_vec(); + sub_args.push(a.to_str().unwrap()); + let mut legacy_args: Vec<&str> = legacy.to_vec(); + legacy_args.push(b.to_str().unwrap()); + + // Normalize the differing temp paths out of the comparison. + let sub_out = stdout_of(&sub_args).replace(a.to_str().unwrap(), "

"); + let legacy_out = stdout_of(&legacy_args).replace(b.to_str().unwrap(), "

"); + assert_eq!(sub_out, legacy_out, "stdout parity: {sub:?} vs {legacy:?}"); +} + +/// Assert that a write subcommand and its legacy equivalent produce +/// byte-identical files. `sub`/`legacy` are the args *before* the path; the +/// path is appended (subcommand) / handled per `path_first`. +fn assert_write_parity(content: &str, filename: &str, sub: &[&str], legacy: &[&str]) { + let dir = TempDir::new().unwrap(); + let a = dir.path().join(format!("a_{filename}")); + let b = dir.path().join(format!("b_{filename}")); + fs::write(&a, content).unwrap(); + fs::write(&b, content).unwrap(); + + let mut sub_args: Vec<&str> = sub.to_vec(); + sub_args.push(a.to_str().unwrap()); + let mut legacy_args: Vec<&str> = legacy.to_vec(); + legacy_args.push(b.to_str().unwrap()); + + cmd().args(&sub_args).assert().success(); + cmd().args(&legacy_args).assert().success(); + + let after_a = fs::read(&a).unwrap(); + let after_b = fs::read(&b).unwrap(); + assert_eq!( + after_a, + after_b, + "write parity: {sub:?} vs {legacy:?}\n sub={:?}\n leg={:?}", + String::from_utf8_lossy(&after_a), + String::from_utf8_lossy(&after_b) + ); +} + +// ── Read-only parity ── + +#[test] +fn scan_parity() { + assert_stdout_parity(SECTION_FILE, &["scan"], &["--scan"]); +} + +#[test] +fn scan_with_recursive_parity() { + assert_stdout_parity(SECTION_FILE, &["scan", "-R"], &["--scan", "--recursive"]); +} + +#[test] +fn check_parity() { + assert_stdout_parity(SECTION_FILE, &["check"], &["--scan", "--check"]); +} + +#[test] +fn list_default_parity() { + assert_stdout_parity(SECTION_FILE, &["list"], &["--list-sections"]); +} + +#[test] +fn list_fields_ids_parity() { + assert_stdout_parity( + SECTION_FILE, + &["list", "--fields", "ids"], + &["--list-sections", "--fields", "ids"], + ); +} + +#[test] +fn toggle_json_stdout_parity() { + // --json is a GlobalArgs flag flattened into the subcommand. + assert_stdout_parity( + SECTION_FILE, + &["toggle", "-S", "feat", "--dry-run", "--json"], + &["-S", "feat", "--dry-run", "--json"], + ); +} + +// ── Write parity ── + +#[test] +fn toggle_write_parity() { + assert_write_parity( + SECTION_FILE, + "t.py", + &["toggle", "-S", "feat"], + &["-S", "feat"], + ); +} + +#[test] +fn remove_default_mode_write_parity() { + // Exercises the DEFAULTED --remove-mode (commented): if the subcommand and + // legacy front-ends disagreed on the default, these files would diverge. + assert_write_parity( + SECTION_FILE, + "r.py", + &["remove", "-S", "feat"], + &["--remove", "-S", "feat"], + ); +} + +#[test] +fn remove_explicit_mode_write_parity() { + assert_write_parity( + SECTION_FILE, + "r.py", + &["remove", "-S", "feat", "--remove-mode", "all"], + &["--remove", "-S", "feat", "--remove-mode", "all"], + ); +} + +#[test] +fn insert_write_parity() { + // insert takes the path as a leading positional in both forms. + let dir = TempDir::new().unwrap(); + let a = dir.path().join("a.py"); + let b = dir.path().join("b.py"); + fs::write(&a, "x\ny\nz\n").unwrap(); + fs::write(&b, "x\ny\nz\n").unwrap(); + + cmd() + .args([ + "insert", + a.to_str().unwrap(), + "-S", + "new", + "-l", + "2:2", + "--desc", + "d", + ]) + .assert() + .success(); + cmd() + .args([ + "--insert", + b.to_str().unwrap(), + "-S", + "new", + "-l", + "2:2", + "--desc", + "d", + ]) + .assert() + .success(); + + assert_eq!(fs::read(&a).unwrap(), fs::read(&b).unwrap()); +} + +// ── Subcommand-specific ergonomics & guards ── + +#[test] +fn subcommand_rejects_out_of_scope_flag() { + // The ergonomic win: each subcommand exposes only its own flags. A flag + // that belongs to a different operation is rejected at parse time. + cmd().args(["scan", "--insert"]).assert().failure(); + cmd() + .args(["list", "--remove-mode", "all"]) + .assert() + .failure(); +} + +#[test] +fn insert_requires_section_and_line() { + // The insert subcommand enforces required -S/-l at parse time. + let dir = TempDir::new().unwrap(); + let a = dir.path().join("a.py"); + fs::write(&a, "x\n").unwrap(); + cmd() + .args(["insert", a.to_str().unwrap()]) + .assert() + .failure(); +} + +#[test] +fn remove_requires_section() { + let dir = TempDir::new().unwrap(); + let a = dir.path().join("a.py"); + fs::write(&a, "x\n").unwrap(); + cmd() + .args(["remove", a.to_str().unwrap()]) + .assert() + .failure(); +} + +#[test] +fn legacy_form_emits_no_notice_when_piped() { + // The deprecation nudge is TTY-only; under a captured (non-TTY) stderr it + // must stay silent so scripts/pipes are unaffected. + let dir = TempDir::new().unwrap(); + let a = dir.path().join("a.py"); + fs::write(&a, SECTION_FILE).unwrap(); + cmd() + .args([a.to_str().unwrap(), "-S", "feat", "--dry-run"]) + .assert() + .success() + .stderr(predicate::str::contains("deprecated").not()); +} + +#[test] +fn subcommands_listed_in_help() { + cmd() + .arg("--help") + .assert() + .success() + .stdout(predicate::str::contains("scan")) + .stdout(predicate::str::contains("insert")) + .stdout(predicate::str::contains("remove")); +} From 1c936bd4bfc9328ba721e494591b2668b0697986 Mon Sep 17 00:00:00 2001 From: Steve Morin Date: Sat, 30 May 2026 13:50:38 -0700 Subject: [PATCH 2/6] feat(cli): add stdin/stdout filter mode for writer operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a single stdin->stdout filter mode with three spellings — a `-` path, `--stdin`, or `--stdout` — for the writer operations (toggle/insert/remove). Input is read from stdin and the transformed result is written to stdout; the file is never touched. Per the design, this is one mode (stdin in, stdout out), not an input x output matrix: a real file path combined with --stdin/--stdout is rejected. Comment style for the pathless stream resolves from --comment-style if given, else defaults to Python `#` (a synthetic `.py` path); pipe other languages with --comment-style. Filter mode rejects flags that collide with stdout output (--json, --atomic, --backup, --interactive, --dry-run, -R) and the read-only operations. Detection runs after the meta/recover/journal short-circuits and before the file-oriented arity checks, so a stdin stream is never rejected as a missing file path. Adds 14 tests: stdin-equals-file byte parity for each writer op, no-op byte-identity preserving exact trailing-newline handling both ways, spelling equivalence, and the full rejection set. Insert's subcommand path becomes optional so `insert --stdin` needs no positional. New io helpers `read_stdin_encoded` / `write_stdout_encoded` mirror the file-encoded variants. Refs P10-T05, P10-TS02. Completes P10. --- PROJECTS.md | 8 +- crates/togl-cli/src/cli.rs | 57 ++++++++++- crates/togl-cli/src/main.rs | 153 +++++++++++++++++++++++++---- crates/togl-cli/tests/filter.rs | 168 ++++++++++++++++++++++++++++++++ crates/togl-lib/src/io.rs | 26 +++++ 5 files changed, 387 insertions(+), 25 deletions(-) create mode 100644 crates/togl-cli/tests/filter.rs diff --git a/PROJECTS.md b/PROJECTS.md index db97052..ed21822 100644 --- a/PROJECTS.md +++ b/PROJECTS.md @@ -230,7 +230,7 @@ See `docs/superpowers/specs/2026-05-29-release-please-commitlint-design.md`. --- -## [~] Project P10: CLI ergonomics refactor (v0.5.0) +## [x] Project P10: CLI ergonomics refactor (v0.5.0) **Goal**: Make the CLI more ergonomic without breaking the existing flat-flag interface. Three additive layers, shipped in order: 1. **Declarative clap constraints** — replace hand-rolled flag-combination @@ -255,8 +255,10 @@ interface. Three additive layers, shipped in order: - [x] [P10-T04] Option 1: `main()` subcommand bridge + TTY-only deprecation notice - [x] [P10-TS01] Parity tests: each subcommand ≡ its flat-flag form (incl. a defaulted `--remove-mode` case to guard default drift); scoping + help/man render -- [ ] [P10-T05] Option 3: stdin/stdout filter support -- [ ] [P10-TS02] Option 3: filter round-trip + `-`-as-path tests +- [x] [P10-T05] Option 3: stdin/stdout filter mode (`-`/`--stdin`/`--stdout`, + writer ops only; rejection set for `--json`/`--atomic`/`--backup`/etc.) +- [x] [P10-TS02] Option 3: stdin≡file parity + no-op byte-identity (trailing + newline both ways) + rejection-set tests ### Automated Verification - `cargo test` green (all parity + existing suites) diff --git a/crates/togl-cli/src/cli.rs b/crates/togl-cli/src/cli.rs index 9df8a8f..e5a9c62 100644 --- a/crates/togl-cli/src/cli.rs +++ b/crates/togl-cli/src/cli.rs @@ -138,6 +138,16 @@ pub struct Cli { #[arg(long = "backup")] pub backup: Option, + /// Read input from stdin instead of a file (filter mode; writes to stdout). + /// Equivalent to passing `-` as the path. Only valid for toggle/insert/remove. + #[arg(long = "stdin")] + pub stdin: bool, + + /// Write the transformed result to stdout instead of modifying files + /// (filter mode). A spelling of stdin filter mode; reads from stdin. + #[arg(long = "stdout")] + pub stdout: bool, + /// Extend the last --line range to the end of file #[arg(long = "to-end", requires = "lines")] pub to_end: bool, @@ -321,6 +331,31 @@ impl GlobalArgs { } } +/// Filter-mode flags, shared by the writer subcommands (toggle/insert/remove). +/// Both `--stdin` and `--stdout` are spellings of the same stdin→stdout filter +/// mode (the classic `-` path is the third spelling). +#[derive(clap::Args, Debug)] +pub struct FilterArgs { + /// Read input from stdin and write to stdout (filter mode). Same as `-`. + #[arg(long = "stdin")] + pub stdin: bool, + + /// Write the transformed result to stdout instead of modifying files. + #[arg(long = "stdout")] + pub stdout: bool, +} + +impl FilterArgs { + fn push_argv(&self, out: &mut Vec) { + if self.stdin { + out.push("--stdin".into()); + } + if self.stdout { + out.push("--stdout".into()); + } + } +} + /// Ergonomic subcommand surface. Each maps to a legacy operation mode. #[derive(clap::Subcommand, Debug)] pub enum Commands { @@ -353,6 +388,8 @@ pub enum Commands { #[arg(long = "no-backup")] no_backup: bool, #[command(flatten)] + filter: FilterArgs, + #[command(flatten)] global: GlobalArgs, }, /// Scan for section IDs without modifying files (read-only). @@ -399,8 +436,8 @@ pub enum Commands { }, /// Insert a toggle:start/end marker pair around a single line range. Insert { - /// Single file to modify - path: PathBuf, + /// Single file to modify. Omit (with --stdin) or pass `-` to read stdin. + path: Option, /// Section ID for the inserted marker (exactly one) #[arg(short = 'S', long = "section")] section: String, @@ -414,6 +451,8 @@ pub enum Commands { #[arg(long = "to-end")] to_end: bool, #[command(flatten)] + filter: FilterArgs, + #[command(flatten)] global: GlobalArgs, }, /// Remove a named section (markers and/or body) from files. @@ -433,6 +472,8 @@ pub enum Commands { #[arg(long = "require-match")] require_match: bool, #[command(flatten)] + filter: FilterArgs, + #[command(flatten)] global: GlobalArgs, }, } @@ -463,6 +504,7 @@ impl Commands { pair, atomic, no_backup, + filter, global, } => { for s in sections { @@ -492,6 +534,7 @@ impl Commands { if *no_backup { out.push("--no-backup".into()); } + filter.push_argv(&mut out); global.push_argv(&mut out); push_paths(&mut out, paths); } @@ -557,6 +600,7 @@ impl Commands { line, desc, to_end, + filter, global, } => { out.push("--insert".into()); @@ -571,9 +615,12 @@ impl Commands { if *to_end { out.push("--to-end".into()); } + filter.push_argv(&mut out); global.push_argv(&mut out); - out.push("--".into()); - out.push(path.into()); + if let Some(p) = path { + out.push("--".into()); + out.push(p.into()); + } } Commands::Remove { paths, @@ -581,6 +628,7 @@ impl Commands { recursive, remove_mode, require_match, + filter, global, } => { out.push("--remove".into()); @@ -596,6 +644,7 @@ impl Commands { if *require_match { out.push("--require-match".into()); } + filter.push_argv(&mut out); global.push_argv(&mut out); push_paths(&mut out, paths); } diff --git a/crates/togl-cli/src/main.rs b/crates/togl-cli/src/main.rs index 4e18a26..9b9821d 100644 --- a/crates/togl-cli/src/main.rs +++ b/crates/togl-cli/src/main.rs @@ -204,8 +204,13 @@ fn run(cli: &Cli) -> Result<()> { return Ok(()); } + // Filter mode (stdin → stdout) has three spellings: a `-` path, `--stdin`, + // or `--stdout`. In filter mode the input comes from stdin, so an empty + // path list is not an error. + let filter_mode = cli.stdin || cli.stdout || cli.paths.iter().any(|p| p.as_os_str() == "-"); + // Path is required for everything else - if cli.paths.is_empty() { + if !filter_mode && cli.paths.is_empty() { return Err(UsageError("at least one file or directory path is required".into()).into()); } @@ -283,6 +288,31 @@ fn run(cli: &Cli) -> Result<()> { return Err(UsageError(format!("Unsupported encoding: '{}'", cli.encoding)).into()); } + let opts = ToggleOptions { + force: &effective_force, + mode: &effective_mode, + temp_suffix: cli.temp_suffix.as_deref(), + dry_run: cli.dry_run, + backup: cli.backup.as_deref(), + config: config.as_ref(), + verbose: cli.verbose && !cli.json, // suppress verbose in JSON mode + eol: &cli.eol, + no_dereference: cli.no_dereference, + encoding: &cli.encoding, + json: cli.json, + to_end: cli.to_end, + comment_style_override: &cli.comment_style, + interactive: cli.interactive, + }; + + // ── Filter mode (stdin → stdout) ── + // Detected before the file-oriented validations below so a stdin stream is + // never rejected by "requires exactly one file path"-style checks. Handles + // the writer operations (toggle/insert/remove) only. + if filter_mode { + return run_filter(cli, &opts); + } + // Handle --scan mode early (read-only, no toggle options needed). // Per PRD §0.14.2, --scan -S is the detailed group view, so --section is allowed here. if cli.scan { @@ -383,23 +413,6 @@ fn run(cli: &Cli) -> Result<()> { } // --desc requires --insert: enforced declaratively in cli.rs via clap `requires`. - let opts = ToggleOptions { - force: &effective_force, - mode: &effective_mode, - temp_suffix: cli.temp_suffix.as_deref(), - dry_run: cli.dry_run, - backup: cli.backup.as_deref(), - config: config.as_ref(), - verbose: cli.verbose && !cli.json, // suppress verbose in JSON mode - eol: &cli.eol, - no_dereference: cli.no_dereference, - encoding: &cli.encoding, - json: cli.json, - to_end: cli.to_end, - comment_style_override: &cli.comment_style, - interactive: cli.interactive, - }; - if cli.insert { run_insert(cli, &opts) } else if cli.remove { @@ -415,6 +428,110 @@ fn run(cli: &Cli) -> Result<()> { } } +/// Filter mode: read the single input stream from stdin, apply the requested +/// writer operation (toggle / insert / remove), and write the result to stdout. +/// The file is never touched. Comment style is resolved from `--comment-style` +/// if given, else defaults to Python `#` (a synthetic `.py` path) — pipe +/// other languages with `--comment-style`. +fn run_filter(cli: &Cli, opts: &ToggleOptions) -> Result<()> { + // Filter mode is for the writer operations only. + if cli.scan || cli.list_sections { + return Err(UsageError( + "stdin/stdout filter mode is only supported for toggle, insert, and remove".into(), + ) + .into()); + } + // Reject flags that have no meaning when the result goes to stdout. + let reject = |cond: bool, flag: &str| -> Result<()> { + if cond { + return Err(UsageError(format!( + "{flag} cannot be combined with stdin/stdout filter mode" + )) + .into()); + } + Ok(()) + }; + reject(cli.json, "--json")?; + reject(cli.atomic, "--atomic")?; + reject(cli.backup.is_some(), "--backup")?; + reject(cli.interactive, "--interactive")?; + reject(cli.recursive, "--recursive")?; + if cli.dry_run { + return Err(UsageError( + "--dry-run is redundant in filter mode; stdout already shows the result".into(), + ) + .into()); + } + // Only the `-` placeholder may appear in paths; no real files. + if cli.paths.iter().any(|p| p.as_os_str() != "-") { + return Err(UsageError( + "filter mode reads from stdin; do not pass file paths (use `-`, --stdin, or --stdout)" + .into(), + ) + .into()); + } + + let vpath = PathBuf::from(".py"); + let input = io::read_stdin_encoded(opts.encoding).context("Failed to read input from stdin")?; + + let output = if cli.insert { + if cli.force.is_some() { + return Err(UsageError( + "--insert does not take --force (the body is left uncommented)".into(), + ) + .into()); + } + if cli.sections.len() != 1 { + return Err(UsageError("--insert requires exactly one -S ".into()).into()); + } + if cli.lines.len() != 1 { + return Err(UsageError("--insert requires exactly one -l ".into()).into()); + } + let prefix = resolve_comment_style(&vpath, opts)?.single_line; + let (start, mut end) = core::parse_line_range(&cli.lines[0])?; + if opts.to_end { + end = input.lines().count(); + } + let modified = core::insert_section( + &input, + &cli.sections[0], + cli.desc.as_deref(), + start, + end, + &prefix, + )?; + io::normalize_eol(&modified, opts.eol) + } else if cli.remove { + if cli.sections.len() != 1 { + return Err(UsageError("--remove requires exactly one -S ".into()).into()); + } + let mode = match cli.remove_mode { + RemoveMode::Markers => core::RemoveMode::Markers, + RemoveMode::Commented => core::RemoveMode::Commented, + RemoveMode::All => core::RemoveMode::All, + }; + let style = resolve_comment_style(&vpath, opts)?; + let (modified, removed) = core::remove_section(&input, &cli.sections[0], mode, &style); + if removed == 0 { + eprintln!("Warning: -S {} matched no sections", cli.sections[0]); + if cli.require_match { + return Err(UsageError(format!( + "--require-match: -S {} matched no sections", + cli.sections[0] + )) + .into()); + } + } + io::normalize_eol(&modified, opts.eol) + } else { + // Default operation: toggle line ranges and/or sections. + compute_file_changes(&vpath, cli, opts, &input)? + }; + + io::write_stdout_encoded(&output, opts.encoding).context("Failed to write to stdout")?; + Ok(()) +} + /// Per PRD §0.13.4: error if any targeted group does not contain exactly 2 variants /// in any input file. Runs before file mutation; failure leaves all files untouched. fn validate_pair_groups(cli: &Cli) -> Result<()> { diff --git a/crates/togl-cli/tests/filter.rs b/crates/togl-cli/tests/filter.rs new file mode 100644 index 0000000..83dcc11 --- /dev/null +++ b/crates/togl-cli/tests/filter.rs @@ -0,0 +1,168 @@ +//! Tests for stdin/stdout filter mode (Option 3). +//! +//! Filter mode is a single stdin→stdout transform with three spellings: a `-` +//! path, `--stdin`, or `--stdout`. It applies to the writer operations +//! (toggle/insert/remove) only. The core guarantees: +//! * stdin≡file: the filter output equals the in-place file result, byte for byte. +//! * no-op byte identity: an input that triggers no change is emitted verbatim, +//! including exact trailing-newline handling. +//! * the rejection set: flags that collide with stdout output are refused. + +use assert_cmd::Command; +use std::fs; +use tempfile::TempDir; + +#[allow(deprecated)] +fn cmd() -> Command { + Command::cargo_bin("toggle").unwrap() +} + +const SECTION_FILE: &str = "# toggle:start ID=feat\nprint(\"hi\")\n# toggle:end ID=feat\nafter\n"; + +/// Run a filter-mode command feeding `input` on stdin; return stdout bytes. +fn filter_stdout(args: &[&str], input: &str) -> Vec { + cmd() + .args(args) + .write_stdin(input) + .assert() + .success() + .get_output() + .stdout + .clone() +} + +/// Apply `legacy_args` (a flat-flag write op) to a file containing `input`, +/// returning the file's bytes afterward. +fn file_result(input: &str, legacy_args: &[&str]) -> Vec { + let dir = TempDir::new().unwrap(); + let p = dir.path().join("f.py"); + fs::write(&p, input).unwrap(); + let mut args: Vec<&str> = vec![p.to_str().unwrap()]; + args.extend_from_slice(legacy_args); + cmd().args(&args).assert().success(); + fs::read(&p).unwrap() +} + +// ── stdin≡file parity (filter stdout == in-place file bytes) ── + +#[test] +fn toggle_filter_equals_file() { + let via_stdin = filter_stdout(&["toggle", "-", "-S", "feat"], SECTION_FILE); + let via_file = file_result(SECTION_FILE, &["-S", "feat"]); + assert_eq!(via_stdin, via_file); +} + +#[test] +fn remove_filter_equals_file() { + let via_stdin = filter_stdout(&["remove", "-", "-S", "feat"], SECTION_FILE); + let via_file = file_result(SECTION_FILE, &["--remove", "-S", "feat"]); + assert_eq!(via_stdin, via_file); +} + +#[test] +fn insert_filter_equals_file() { + let input = "a\nb\nc\n"; + let via_stdin = filter_stdout( + &["insert", "-", "-S", "new", "-l", "2:2", "--desc", "d"], + input, + ); + let via_file = file_result( + input, + &["--insert", "-S", "new", "-l", "2:2", "--desc", "d"], + ); + assert_eq!(via_stdin, via_file); +} + +// ── no-op byte identity (verbatim passthrough, exact trailing newline) ── + +#[test] +fn noop_preserves_trailing_newline() { + let input = "x\ny\nz\n"; + let out = filter_stdout(&["toggle", "-", "-S", "absent"], input); + assert_eq!(out, input.as_bytes()); +} + +#[test] +fn noop_preserves_missing_trailing_newline() { + let input = "x\ny\nz"; // no trailing newline + let out = filter_stdout(&["toggle", "-", "-S", "absent"], input); + assert_eq!(out, input.as_bytes()); +} + +// ── spelling equivalence: `-`, --stdin, --stdout ── + +#[test] +fn stdin_and_stdout_aliases_match_dash() { + let dash = filter_stdout(&["toggle", "-", "-S", "feat"], SECTION_FILE); + let stdin = filter_stdout(&["toggle", "--stdin", "-S", "feat"], SECTION_FILE); + let stdout = filter_stdout(&["toggle", "--stdout", "-S", "feat"], SECTION_FILE); + assert_eq!(dash, stdin); + assert_eq!(dash, stdout); +} + +#[test] +fn insert_via_stdin_flag_without_path() { + // --stdin lets insert omit the positional path entirely. + let out = filter_stdout( + &["insert", "--stdin", "-S", "new", "-l", "2:2"], + "a\nb\nc\n", + ); + let s = String::from_utf8(out).unwrap(); + assert!(s.contains("toggle:start ID=new")); + assert!(s.contains("toggle:end ID=new")); +} + +// ── rejection set ── + +fn assert_filter_rejected(args: &[&str]) { + cmd() + .args(args) + .write_stdin(SECTION_FILE) + .assert() + .failure(); +} + +#[test] +fn rejects_json_in_filter_mode() { + assert_filter_rejected(&["toggle", "-", "-S", "feat", "--json"]); +} + +#[test] +fn rejects_dry_run_in_filter_mode() { + assert_filter_rejected(&["toggle", "-", "-S", "feat", "--dry-run"]); +} + +#[test] +fn rejects_atomic_in_filter_mode() { + assert_filter_rejected(&["toggle", "-", "-S", "feat", "--atomic"]); +} + +#[test] +fn rejects_backup_in_filter_mode() { + assert_filter_rejected(&["toggle", "-", "-S", "feat", "--backup", ".bak"]); +} + +#[test] +fn rejects_recursive_in_filter_mode() { + assert_filter_rejected(&["toggle", "-", "-S", "feat", "-R"]); +} + +#[test] +fn rejects_real_path_with_stdout() { + // A real file path plus --stdout is the matrix the design declined: error. + cmd() + .args(["toggle", "somefile.py", "--stdout", "-S", "feat"]) + .write_stdin(SECTION_FILE) + .assert() + .failure(); +} + +#[test] +fn scan_rejects_stdin_flag() { + // Read-only ops are not filter-mode writers. + cmd() + .args(["--scan", "--stdin"]) + .write_stdin(SECTION_FILE) + .assert() + .failure(); +} diff --git a/crates/togl-lib/src/io.rs b/crates/togl-lib/src/io.rs index 6771d6c..0c46ce3 100644 --- a/crates/togl-lib/src/io.rs +++ b/crates/togl-lib/src/io.rs @@ -37,6 +37,32 @@ pub fn read_file_encoded(path: &Path, encoding: &str) -> io::Result { Ok(decoded.into_owned()) } +/// Read all of stdin and decode it with the specified encoding. +/// The filter-mode analog of [`read_file_encoded`]. +pub fn read_stdin_encoded(encoding: &str) -> io::Result { + let mut bytes = Vec::new(); + io::stdin().read_to_end(&mut bytes)?; + if encoding.eq_ignore_ascii_case("utf-8") { + return String::from_utf8(bytes).map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e)); + } + let enc = resolve_encoding(encoding)?; + let (decoded, _, had_errors) = enc.decode(&bytes); + if had_errors { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("Failed to decode stdin as {}", encoding), + )); + } + Ok(decoded.into_owned()) +} + +/// Encode `content` with the specified encoding and write it to stdout. +/// The filter-mode analog of [`write_file_encoded`]. +pub fn write_stdout_encoded(content: &str, encoding: &str) -> io::Result<()> { + let bytes = encode_string(content, encoding)?; + io::stdout().write_all(&bytes) +} + /// Resolve an encoding label to an encoding_rs::Encoding. /// Handles common aliases like "latin-1" that encoding_rs doesn't directly recognize. fn resolve_encoding(label: &str) -> io::Result<&'static encoding_rs::Encoding> { From 4cba97f48b44c8d4a56380e23450b0d088907bc0 Mon Sep 17 00:00:00 2001 From: Steve Morin Date: Sat, 30 May 2026 13:51:37 -0700 Subject: [PATCH 3/6] docs(readme): document subcommands and stdin/stdout filter mode --- README.md | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/README.md b/README.md index 974a782..8da54eb 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,24 @@ toggle -S debug --force on -R src/ toggle --scan -R src/ ``` +## Subcommands + +Every operation is also available as a subcommand that exposes only the flags +relevant to it. The subcommands are equivalent to the flat flags below (they run +through the same engine), but are easier to discover and harder to misuse: + +| Subcommand | Flat-flag equivalent | +|---|---| +| `toggle -S id` | `toggle -S id` | +| `toggle scan -R src/` | `toggle --scan -R src/` | +| `toggle check -R src/` | `toggle --scan --check -R src/` | +| `toggle list src/` | `toggle --list-sections src/` | +| `toggle insert main.py -S id -l 10:20` | `toggle --insert -S id -l 10:20 main.py` | +| `toggle remove main.py -S id` | `toggle --remove -S id main.py` | + +Run `toggle --help` to see its scoped flags. The flat-flag form +still works and is supported, but is deprecated in favor of the subcommands. + ## Section markers Wrap any block in a paired marker comment that the tool can find: @@ -106,6 +124,30 @@ toggle --scan -R src/ --json `--check` exits non-zero on any error finding (unclosed markers, duplicate IDs); warnings (variant gaps, pair-count mismatches) do not fail the run. +## Filter mode (stdin → stdout) + +The writer operations (`toggle`, `insert`, `remove`) can read from stdin and +write the transformed result to stdout, leaving any file untouched — so `toggle` +composes in a pipeline. Use a `-` path, or the `--stdin` / `--stdout` aliases: + +```bash +# Toggle a section, reading stdin and writing stdout +cat main.py | toggle - -S featureX + +# Equivalent spellings +toggle --stdin -S featureX < main.py +toggle --stdout -S featureX < main.py + +# Works with the subcommands too +toggle remove --stdin -S featureX < main.py +toggle insert --stdin -S featureX -l 10:20 < main.py > wrapped.py +``` + +Piped input has no file extension, so the comment style defaults to `#` +(Python); pass `--comment-style` for other languages. Filter mode is a single +stdin→stdout transform: it does not accept file paths, `--json`, `--atomic`, +`--backup`, `--dry-run`, `--interactive`, or `-R`. + ## Atomic multi-file mode ```bash From 946cfb9b776496b6863af1701849ec48018a9342 Mon Sep 17 00:00:00 2001 From: Steve Morin Date: Sat, 30 May 2026 13:55:08 -0700 Subject: [PATCH 4/6] test(cli): add atomic-recursive parity test through subcommand bridge --- crates/togl-cli/tests/subcommands.rs | 36 ++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/crates/togl-cli/tests/subcommands.rs b/crates/togl-cli/tests/subcommands.rs index ff6611e..6074838 100644 --- a/crates/togl-cli/tests/subcommands.rs +++ b/crates/togl-cli/tests/subcommands.rs @@ -249,3 +249,39 @@ fn subcommands_listed_in_help() { .stdout(predicate::str::contains("insert")) .stdout(predicate::str::contains("remove")); } + +#[test] +fn toggle_atomic_recursive_write_parity() { + // Atomic multi-file mode reaches the real file-mutation + journal + backup + // path through the subcommand bridge. Each run is isolated in its own cwd + // (the journal lives in cwd) so they don't collide. + fn build_dir() -> TempDir { + let dir = TempDir::new().unwrap(); + for name in ["a.py", "b.py"] { + fs::write(dir.path().join(name), SECTION_FILE).unwrap(); + } + dir + } + + let sub_dir = build_dir(); + cmd() + .current_dir(sub_dir.path()) + .args(["toggle", ".", "-S", "feat", "--atomic", "-R"]) + .assert() + .success(); + + let legacy_dir = build_dir(); + cmd() + .current_dir(legacy_dir.path()) + .args([".", "-S", "feat", "--atomic", "-R"]) + .assert() + .success(); + + for name in ["a.py", "b.py"] { + assert_eq!( + fs::read(sub_dir.path().join(name)).unwrap(), + fs::read(legacy_dir.path().join(name)).unwrap(), + "atomic parity differs for {name}" + ); + } +} From f1f7633b535724ee53bedb944294b3e557acbadf Mon Sep 17 00:00:00 2001 From: Steve Morin Date: Sat, 30 May 2026 14:28:21 -0700 Subject: [PATCH 5/6] =?UTF-8?q?feat(cli):=20promote=20--stdout=20to=20a=20?= =?UTF-8?q?file=E2=86=92stdout=20capability?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously `--stdout` was a pure alias of stdin filter mode. It now also accepts a real file path: `toggle file.py --stdout -S feat` reads that file, applies the transform, and writes the result to stdout without modifying the file — the prettier/clang-format model, useful for editor integration and previewing. Because a real file has a real extension, its comment style resolves normally (e.g. a `.js` file uses `//`), unlike piped stdin which falls back to the synthetic `.py` Python default. Filter mode remains a single stream to stdout: mixing `-`/`--stdin` with a file path, multiple files, or a directory is rejected. Adds 7 tests: file→stdout byte parity with the in-place result, file-unmodified, real-extension comment style (.js → //), and the new rejection cases. Updates README and --stdout help text. Refs P10-T05. --- README.md | 20 +++-- crates/togl-cli/src/cli.rs | 10 ++- crates/togl-cli/src/main.rs | 44 +++++++++-- crates/togl-cli/tests/filter.rs | 127 +++++++++++++++++++++++++++++--- 4 files changed, 174 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 8da54eb..0f5a53a 100644 --- a/README.md +++ b/README.md @@ -131,22 +131,28 @@ write the transformed result to stdout, leaving any file untouched — so `toggl composes in a pipeline. Use a `-` path, or the `--stdin` / `--stdout` aliases: ```bash -# Toggle a section, reading stdin and writing stdout +# stdin → stdout: read stdin, write the result to stdout cat main.py | toggle - -S featureX -# Equivalent spellings +# Equivalent spellings for stdin → stdout toggle --stdin -S featureX < main.py toggle --stdout -S featureX < main.py +# file → stdout: transform a real file, print the result, leave the file on disk +# untouched (the prettier / clang-format model; great for editor integration). +toggle main.py --stdout -S featureX + # Works with the subcommands too toggle remove --stdin -S featureX < main.py -toggle insert --stdin -S featureX -l 10:20 < main.py > wrapped.py +toggle insert main.py --stdout -S featureX -l 10:20 > wrapped.py ``` -Piped input has no file extension, so the comment style defaults to `#` -(Python); pass `--comment-style` for other languages. Filter mode is a single -stdin→stdout transform: it does not accept file paths, `--json`, `--atomic`, -`--backup`, `--dry-run`, `--interactive`, or `-R`. +With `file --stdout`, the comment style is resolved from the file's real +extension. Piped (stdin) input has no extension, so it defaults to `#` (Python); +pass `--comment-style` for other languages. Filter mode is always a single +stream to stdout and never modifies a file, so it does not accept multiple +files, directories, `--json`, `--atomic`, `--backup`, `--dry-run`, +`--interactive`, or `-R`. ## Atomic multi-file mode diff --git a/crates/togl-cli/src/cli.rs b/crates/togl-cli/src/cli.rs index e5a9c62..e105462 100644 --- a/crates/togl-cli/src/cli.rs +++ b/crates/togl-cli/src/cli.rs @@ -143,8 +143,9 @@ pub struct Cli { #[arg(long = "stdin")] pub stdin: bool, - /// Write the transformed result to stdout instead of modifying files - /// (filter mode). A spelling of stdin filter mode; reads from stdin. + /// Write the transformed result to stdout instead of modifying files. + /// With a file path, reads that file (real extension → comment style); + /// with no path, reads stdin. Only valid for toggle/insert/remove. #[arg(long = "stdout")] pub stdout: bool, @@ -332,8 +333,9 @@ impl GlobalArgs { } /// Filter-mode flags, shared by the writer subcommands (toggle/insert/remove). -/// Both `--stdin` and `--stdout` are spellings of the same stdin→stdout filter -/// mode (the classic `-` path is the third spelling). +/// Filter-mode flags. `--stdin` (= the classic `-` path) reads stdin and writes +/// stdout. `--stdout` writes stdout but reads a file path when one is given +/// (so the comment style comes from its real extension), else reads stdin. #[derive(clap::Args, Debug)] pub struct FilterArgs { /// Read input from stdin and write to stdout (filter mode). Same as `-`. diff --git a/crates/togl-cli/src/main.rs b/crates/togl-cli/src/main.rs index 9b9821d..e3ab55c 100644 --- a/crates/togl-cli/src/main.rs +++ b/crates/togl-cli/src/main.rs @@ -462,17 +462,49 @@ fn run_filter(cli: &Cli, opts: &ToggleOptions) -> Result<()> { ) .into()); } - // Only the `-` placeholder may appear in paths; no real files. - if cli.paths.iter().any(|p| p.as_os_str() != "-") { + // Resolve the single input source. Filter mode always writes to stdout and + // never modifies a file; the input is either stdin or one real file: + // * stdin — a `-` path, `--stdin`, or `--stdout` with no file path. + // * file — a real file path with `--stdout` (read it, emit to stdout). + // A real file gives a real extension, so its comment style is resolved + // normally; stdin has no extension and falls back to Python `#` (synthetic + // `.py`), overridable with `--comment-style`. + let real_paths: Vec<&PathBuf> = cli.paths.iter().filter(|p| p.as_os_str() != "-").collect(); + let has_dash = cli.paths.iter().any(|p| p.as_os_str() == "-"); + + if has_dash && !real_paths.is_empty() { + return Err( + UsageError("cannot mix `-` (stdin) with a file path in filter mode".into()).into(), + ); + } + if cli.stdin && !real_paths.is_empty() { + return Err( + UsageError("--stdin reads from stdin; do not also pass a file path".into()).into(), + ); + } + if real_paths.len() > 1 { return Err(UsageError( - "filter mode reads from stdin; do not pass file paths (use `-`, --stdin, or --stdout)" - .into(), + "filter mode writes a single stream; pass one file (or read stdin)".into(), ) .into()); } - let vpath = PathBuf::from(".py"); - let input = io::read_stdin_encoded(opts.encoding).context("Failed to read input from stdin")?; + let (input, vpath) = if let Some(path) = real_paths.first() { + if path.is_dir() { + return Err(UsageError(format!( + "filter mode operates on a single file; '{}' is a directory", + path.display() + )) + .into()); + } + let content = io::read_file_encoded(path, opts.encoding) + .with_context(|| format!("Failed to read {}", path.display()))?; + (content, (*path).clone()) + } else { + let content = + io::read_stdin_encoded(opts.encoding).context("Failed to read input from stdin")?; + (content, PathBuf::from(".py")) + }; let output = if cli.insert { if cli.force.is_some() { diff --git a/crates/togl-cli/tests/filter.rs b/crates/togl-cli/tests/filter.rs index 83dcc11..8136a46 100644 --- a/crates/togl-cli/tests/filter.rs +++ b/crates/togl-cli/tests/filter.rs @@ -1,12 +1,16 @@ //! Tests for stdin/stdout filter mode (Option 3). //! -//! Filter mode is a single stdin→stdout transform with three spellings: a `-` -//! path, `--stdin`, or `--stdout`. It applies to the writer operations -//! (toggle/insert/remove) only. The core guarantees: +//! Filter mode writes a single transformed stream to stdout and never modifies a +//! file, for the writer operations (toggle/insert/remove) only. The input is +//! either stdin (`-`, `--stdin`, or `--stdout` with no path) or one real file +//! (`file --stdout`, whose real extension drives the comment style). The core +//! guarantees: //! * stdin≡file: the filter output equals the in-place file result, byte for byte. +//! * file→stdout: `file --stdout` emits that result and leaves the file untouched. //! * no-op byte identity: an input that triggers no change is emitted verbatim, //! including exact trailing-newline handling. -//! * the rejection set: flags that collide with stdout output are refused. +//! * the rejection set: flags/inputs that collide with single-stream stdout output +//! are refused. use assert_cmd::Command; use std::fs; @@ -148,21 +152,124 @@ fn rejects_recursive_in_filter_mode() { } #[test] -fn rejects_real_path_with_stdout() { - // A real file path plus --stdout is the matrix the design declined: error. +fn scan_rejects_stdin_flag() { + // Read-only ops are not filter-mode writers. cmd() - .args(["toggle", "somefile.py", "--stdout", "-S", "feat"]) + .args(["--scan", "--stdin"]) .write_stdin(SECTION_FILE) .assert() .failure(); } +// ── file → stdout (`--stdout` with a real file path) ── + #[test] -fn scan_rejects_stdin_flag() { - // Read-only ops are not filter-mode writers. +fn file_to_stdout_equals_in_place_result() { + // `file --stdout` emits the same bytes the in-place transform would write. + let dir = TempDir::new().unwrap(); + let read_only = dir.path().join("ro.py"); + fs::write(&read_only, SECTION_FILE).unwrap(); + let out = cmd() + .args([read_only.to_str().unwrap(), "--stdout", "-S", "feat"]) + .assert() + .success() + .get_output() + .stdout + .clone(); + + let in_place = file_result(SECTION_FILE, &["-S", "feat"]); + assert_eq!(out, in_place); +} + +#[test] +fn file_to_stdout_leaves_file_unmodified() { + let dir = TempDir::new().unwrap(); + let p = dir.path().join("keep.py"); + fs::write(&p, SECTION_FILE).unwrap(); cmd() - .args(["--scan", "--stdin"]) + .args([p.to_str().unwrap(), "--stdout", "-S", "feat"]) + .assert() + .success(); + assert_eq!(fs::read_to_string(&p).unwrap(), SECTION_FILE); +} + +#[test] +fn file_to_stdout_uses_real_extension_comment_style() { + // The point of file→stdout: comment style comes from the real extension. + // A `.js` file must use `//`, not the synthetic-stdin Python `#` default. + let dir = TempDir::new().unwrap(); + let js = dir.path().join("app.js"); + fs::write( + &js, + "// toggle:start ID=feat\nconsole.log(1)\n// toggle:end ID=feat\n", + ) + .unwrap(); + let out = cmd() + .args([js.to_str().unwrap(), "--stdout", "-S", "feat"]) + .assert() + .success() + .get_output() + .stdout + .clone(); + let s = String::from_utf8(out).unwrap(); + assert!( + s.contains("// console.log(1)"), + "expected // comment, got:\n{s}" + ); +} + +#[test] +fn rejects_dash_mixed_with_file_path() { + cmd() + .args(["toggle", "-", "somefile.py", "--stdout", "-S", "feat"]) + .write_stdin(SECTION_FILE) + .assert() + .failure(); +} + +#[test] +fn rejects_stdin_flag_with_file_path() { + let dir = TempDir::new().unwrap(); + let p = dir.path().join("f.py"); + fs::write(&p, SECTION_FILE).unwrap(); + cmd() + .args(["toggle", p.to_str().unwrap(), "--stdin", "-S", "feat"]) .write_stdin(SECTION_FILE) .assert() .failure(); } + +#[test] +fn rejects_multiple_files_with_stdout() { + let dir = TempDir::new().unwrap(); + let a = dir.path().join("a.py"); + let b = dir.path().join("b.py"); + fs::write(&a, SECTION_FILE).unwrap(); + fs::write(&b, SECTION_FILE).unwrap(); + cmd() + .args([ + "toggle", + a.to_str().unwrap(), + b.to_str().unwrap(), + "--stdout", + "-S", + "feat", + ]) + .assert() + .failure(); +} + +#[test] +fn rejects_directory_with_stdout() { + let dir = TempDir::new().unwrap(); + cmd() + .args([ + "toggle", + dir.path().to_str().unwrap(), + "--stdout", + "-S", + "feat", + ]) + .assert() + .failure(); +} From 5338e8e6468c98a17abdca518b6849db7abca798 Mon Sep 17 00:00:00 2001 From: Steve Morin Date: Sat, 30 May 2026 14:39:43 -0700 Subject: [PATCH 6/6] test(cli): fix subcommand stdout parity on Windows (JSON escapes backslash) Run both forms against the same file instead of normalizing two temp paths textually; JSON escapes \ on Windows so the raw-path replace never matched. All callers are non-mutating (scan/check/list/--dry-run), so sharing one file is safe and makes paths byte-identical. --- crates/togl-cli/tests/subcommands.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/crates/togl-cli/tests/subcommands.rs b/crates/togl-cli/tests/subcommands.rs index 6074838..4964c93 100644 --- a/crates/togl-cli/tests/subcommands.rs +++ b/crates/togl-cli/tests/subcommands.rs @@ -26,23 +26,24 @@ fn stdout_of(args: &[&str]) -> String { String::from_utf8(out).unwrap() } -/// Assert that a read-only subcommand and its legacy equivalent print the same -/// stdout for identical input files. +/// Assert that a read-only / dry-run subcommand and its legacy equivalent print +/// identical stdout. Both forms run against the *same* file, so any path in the +/// output (e.g. the `file` field in `--json`) is byte-identical without fragile +/// textual normalization (which broke on Windows, where JSON escapes `\`). +/// Safe because every caller is non-mutating (scan/check/list/`--dry-run`). fn assert_stdout_parity(content: &str, sub: &[&str], legacy: &[&str]) { let dir = TempDir::new().unwrap(); - let a = dir.path().join("a.py"); - let b = dir.path().join("b.py"); - fs::write(&a, content).unwrap(); - fs::write(&b, content).unwrap(); + let p = dir.path().join("a.py"); + fs::write(&p, content).unwrap(); + let path = p.to_str().unwrap(); let mut sub_args: Vec<&str> = sub.to_vec(); - sub_args.push(a.to_str().unwrap()); + sub_args.push(path); let mut legacy_args: Vec<&str> = legacy.to_vec(); - legacy_args.push(b.to_str().unwrap()); + legacy_args.push(path); - // Normalize the differing temp paths out of the comparison. - let sub_out = stdout_of(&sub_args).replace(a.to_str().unwrap(), "

"); - let legacy_out = stdout_of(&legacy_args).replace(b.to_str().unwrap(), "

"); + let sub_out = stdout_of(&sub_args); + let legacy_out = stdout_of(&legacy_args); assert_eq!(sub_out, legacy_out, "stdout parity: {sub:?} vs {legacy:?}"); }