diff --git a/PROJECTS.md b/PROJECTS.md index fcb497b..ed21822 100644 --- a/PROJECTS.md +++ b/PROJECTS.md @@ -227,3 +227,40 @@ 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. + +--- + +## [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 + 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 +- [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) +- `cargo clippy --all-targets -- -D warnings` clean +- `togl --help` / `--man` / `--completions` render the subcommands under the aliased bin name diff --git a/README.md b/README.md index 974a782..0f5a53a 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,36 @@ 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 +# stdin → stdout: read stdin, write the result to stdout +cat main.py | toggle - -S featureX + +# 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 main.py --stdout -S featureX -l 10:20 > wrapped.py +``` + +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 ```bash diff --git a/crates/togl-cli/src/cli.rs b/crates/togl-cli/src/cli.rs index cb42995..e105462 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, @@ -131,6 +138,17 @@ 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. + /// 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, + /// Extend the last --line range to the end of file #[arg(long = "to-end", requires = "lines")] pub to_end: bool, @@ -181,3 +199,470 @@ 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()); + } + } + } +} + +/// Filter-mode flags, shared by the writer subcommands (toggle/insert/remove). +/// 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 `-`. + #[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 { + /// 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)] + filter: FilterArgs, + #[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. 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, + /// 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)] + filter: FilterArgs, + #[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)] + filter: FilterArgs, + #[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, + filter, + 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()); + } + filter.push_argv(&mut out); + 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, + filter, + 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()); + } + filter.push_argv(&mut out); + global.push_argv(&mut out); + if let Some(p) = path { + out.push("--".into()); + out.push(p.into()); + } + } + Commands::Remove { + paths, + section, + recursive, + remove_mode, + require_match, + filter, + 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()); + } + filter.push_argv(&mut out); + 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..e3ab55c 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() { @@ -163,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()); } @@ -242,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 { @@ -342,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 { @@ -374,6 +428,142 @@ 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()); + } + // 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 writes a single stream; pass one file (or read stdin)".into(), + ) + .into()); + } + + 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() { + 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..8136a46 --- /dev/null +++ b/crates/togl-cli/tests/filter.rs @@ -0,0 +1,275 @@ +//! Tests for stdin/stdout filter mode (Option 3). +//! +//! 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/inputs that collide with single-stream 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 scan_rejects_stdin_flag() { + // Read-only ops are not filter-mode writers. + cmd() + .args(["--scan", "--stdin"]) + .write_stdin(SECTION_FILE) + .assert() + .failure(); +} + +// ── file → stdout (`--stdout` with a real file path) ── + +#[test] +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([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(); +} diff --git a/crates/togl-cli/tests/subcommands.rs b/crates/togl-cli/tests/subcommands.rs new file mode 100644 index 0000000..4964c93 --- /dev/null +++ b/crates/togl-cli/tests/subcommands.rs @@ -0,0 +1,288 @@ +//! 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 / 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 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(path); + let mut legacy_args: Vec<&str> = legacy.to_vec(); + legacy_args.push(path); + + 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:?}"); +} + +/// 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")); +} + +#[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}" + ); + } +} 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> {