From 49d2a68ad107a3129b0214e66fb1e5009abd9337 Mon Sep 17 00:00:00 2001 From: default <216188+jdx@users.noreply.github.com> Date: Fri, 21 Aug 2026 13:23:28 +0000 Subject: [PATCH 1/3] feat(spec)!: lower the derive's flatten into a flagset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `#[usage(flatten)]` splices a struct's declarations into the command that holds it, and the emitted spec repeated them under every command that flattened the struct. mise gives one `Args` type to ten commands; the spec said it ten times. Now the struct becomes one `flagset`, named after it, and each command that flattens it gets a `use`. A struct that flattens another becomes a set that uses a set. Positionals stay on each command, since a set holds flags only. `CommandMeta` gains `flatten_groups`: the seam the expansion leaves behind, so `to_kdl` knows which run of a command's flags came from which struct. Nothing about parsing changes — the flags are in the flag table either way, and the hot path never reads metadata. Two decisions worth recording: - **One set per flattened struct**, not only for the ones with several users or the ones that save lines. A threshold would make how one command is written depend on what another command does, so adding a `flatten` elsewhere would restructure a command nobody touched. - **Two structs whose names end in the same word get no set.** One name cannot stand for both, so both are written inline — what every flatten did before, never wrong and only longer. Detected by comparing flag keys, which the derive hashes from the declaring type. usage-cli is the first case: its four shell commands flatten `Shell`, and its checked-in spec now says so. Its generated markdown, JSON reference and manpage are byte-identical across the change, because the reference parser resolves a `use` while it reads the file. Breaking only in that `CommandMeta` grows a field. It is constructed by generated code, which moves with this crate, so an adopter recompiles rather than edits. Co-Authored-By: Claude Opus 5 --- PLAN.md | 19 ++- argv/src/spec.rs | 252 ++++++++++++++++++++++++------ cli/src/cli/mod.rs | 5 + cli/usage.usage.kdl | 16 +- conformance/src/tables.rs | 3 + conformance/tests/flatten.rs | 24 ++- derive/src/codegen.rs | 55 ++++++- docs/rust/subcommands.md | 34 +++- docs/spec/reference/flagset.md | 12 ++ usage-rs/tests/facade.rs | 275 +++++++++++++++++++++++++++++++++ 10 files changed, 623 insertions(+), 72 deletions(-) diff --git a/PLAN.md b/PLAN.md index f54c8602a..93c1b1aa6 100644 --- a/PLAN.md +++ b/PLAN.md @@ -321,10 +321,21 @@ tasks --usage"`, so task names are meant to come from running that. usage-argv d different arguments would mean two different things. Sets travel through `include`, and each file resolves its own `use` nodes — a file cannot use a set declared only by a file that includes _it_, which would make a spec's meaning depend on who read it. - What this does **not** do yet is give the derive's `flatten` a lossless lowering: it still - emits the expanded copy, so the flags mise's spec repeats stay repeated in the generated - file. Emitting one flagset per flattened struct is the follow-up, and it is what would - turn this from an authoring convenience into a smaller generated spec. + The derive lowers `flatten` into it rather than emitting the expanded copy: one flagset + per flattened struct, named after it, and a `use` on every command that flattens it. A + struct that flattens another becomes a set that uses a set. One set per struct rather + than only for the ones with several users — a threshold would make how one command is + written depend on what another does, so adding a `flatten` elsewhere would restructure a + command nobody touched. Two flattened structs whose names end in the same word get no set: + one name cannot stand for both, so both are written inline, which is what every flatten + did before. usage-cli's own checked-in spec is the first case of it, and its generated + markdown, JSON and manpage are byte-identical across the change — the reference parser + resolves the set while it reads the file, so what a command accepts never moved. + Still owed: a way to name a set other than after the Rust type, which is what a collision + needs to be fixable rather than merely safe. And usage-cli declares + `min_usage_version "4.0"` while now emitting a node no 4.0 can read — the floor moves to + whatever release carries this, which is release-plz's to stamp rather than a number to + guess here. ### Then: what a CLI framework has to have diff --git a/argv/src/spec.rs b/argv/src/spec.rs index a71fbb7ff..f020c3e2b 100644 --- a/argv/src/spec.rs +++ b/argv/src/spec.rs @@ -851,6 +851,13 @@ pub struct CommandMeta<'a> { /// Cold like everything else here: a group is checked once the last token has been /// read, by code the derive generates, and a successful parse never reads this. pub groups: &'a [GroupMeta<'a>], + /// Which runs of [`Self::flags`] came from a flattened `Args` type. + /// + /// Only [`Spec::to_kdl`] reads this, and only to write a `flagset` once instead of the + /// same flags under every command that flattens the struct. It changes nothing about + /// parsing: the flags are in `flags` either way, which is why this is a description of + /// where they came from rather than a table anything binds against. + pub flatten_groups: &'a [FlattenGroup<'a>], } impl CommandMeta<'_> { @@ -886,9 +893,29 @@ impl CommandMeta<'_> { flags: &[], args: &[], subcommands: &[], + flatten_groups: &[], }; } +/// A run of one command's flags that arrived from a flattened `Args` type. +/// +/// `#[usage(flatten)]` splices a struct's declarations into the command that holds it, so a +/// set of flags shared by thirty commands is thirty identical copies in the emitted spec. +/// This records the seam the expansion leaves behind, so [`Spec::to_kdl`] can write the set +/// once as a `flagset` and a `use` in each command that has it. +#[derive(Debug, Clone, Copy)] +pub struct FlattenGroup<'a> { + /// The name the emitted `flagset` is given: the flattened type's, in kebab-case. + pub name: &'a str, + /// Where this group's flags begin in the flattening command's flag list. + pub start: usize, + /// The flattened type's own metadata. + /// + /// The flagset's body comes from here rather than from the parent's slice, so a struct + /// that flattens another can be written as a set that `use`s a set. + pub meta: &'a CommandMeta<'a>, +} + /// What a flag knows about itself beyond how it parses. #[derive(Debug, Clone, Copy)] pub struct FlagMeta<'a> { @@ -1454,18 +1481,173 @@ impl Spec<'_> { for example in self.root.examples { write_example(out, example, 0)?; } + // Before the flags, since that is where the first `use` of one appears. Which sets + // there are is a property of the whole tree, so it is settled before anything is + // written rather than discovered command by command. + let w = Writing { + bin: self.bin.unwrap_or(self.name), + overlays, + sets: Flagsets::collect(self.root), + }; + for entry in w.sets.written() { + writeln!(out, "flagset {} {{", quoted(entry.name))?; + write_flag_layout(out, entry.meta, 1, &w.sets)?; + out.push_str("}\n"); + } // Nothing above the root, so what it does not state is the default. let mut path = Vec::new(); - write_body( - out, - self.root, - 0, - UnknownFlags::Value, - self.bin.unwrap_or(self.name), - overlays, - &mut path, - ) + write_body(out, self.root, 0, UnknownFlags::Value, &w, &mut path) + } +} + +/// Whether two flattened groups are the same declarations. +/// +/// Keys are hashed from the type a declaration came from, so a matching key sequence means +/// a matching struct. Pointer equality is tried first because it is the usual case: the same +/// associated const, reached from every command that flattens the type. +fn same_group(a: &CommandMeta<'_>, b: &CommandMeta<'_>) -> bool { + core::ptr::eq(a, b) + || (a.flags.len() == b.flags.len() + && a.flags + .iter() + .zip(b.flags) + .all(|(x, y)| x.flag.key == y.flag.key)) +} + +/// The flagsets a spec is going to write, worked out before anything is written. +/// +/// One per flattened struct, whether it is flattened once or thirty times. A threshold — +/// only sets with several users, or only ones that save lines — would make how a command is +/// written depend on what some other command does, so adding a second `#[usage(flatten)]` +/// elsewhere would restructure a command nobody touched. A `flatten` is a shared declaration +/// wherever it appears, and this writes it as one. +struct Flagsets<'a> { + entries: Vec>, +} + +struct FlagsetEntry<'a> { + name: &'a str, + meta: &'a CommandMeta<'a>, + /// Another type claimed this name, so it cannot stand for either of them. + ambiguous: bool, +} + +impl<'a> Flagsets<'a> { + fn collect(root: &'a CommandMeta<'a>) -> Self { + let mut sets = Self { + entries: Vec::new(), + }; + sets.walk(root); + sets + } + + fn walk(&mut self, meta: &'a CommandMeta<'a>) { + for group in meta.flatten_groups { + self.record(group); + } + for sub in meta.subcommands { + self.walk(sub); + } + } + + fn record(&mut self, group: &'a FlattenGroup<'a>) { + if let Some(entry) = self.entries.iter_mut().find(|e| e.name == group.name) { + if !same_group(entry.meta, group.meta) { + // Two flattened types whose names end in the same word. Neither can have + // the name, because a `use` of it would put one struct's flags on the + // command that asked for the other's. Both are written inline instead, which + // is what every flatten did before sets existed. + entry.ambiguous = true; + } + return; + } + self.entries.push(FlagsetEntry { + name: group.name, + meta: group.meta, + ambiguous: false, + }); + // The sets this one composes. Recorded once, when the set is first met, because + // they are written inside its body rather than by everything that uses it. + for nested in group.meta.flatten_groups { + self.record(nested); + } + } + + /// The sets that get written, in the order they were met. + fn written(&self) -> impl Iterator> { + self.entries.iter().filter(|e| Self::worth_writing(e)) + } + + /// Whether a `use` may stand for this group, or its flags have to be written out. + fn covers(&self, group: &FlattenGroup<'_>) -> bool { + self.entries.iter().any(|e| { + e.name == group.name && Self::worth_writing(e) && same_group(e.meta, group.meta) + }) + } + + /// A set with no flags in it has nothing to declare and nothing to stand for: a + /// flattened struct may hold only positionals, which stay where they are. + fn worth_writing(entry: &FlagsetEntry<'_>) -> bool { + !entry.ambiguous && !entry.meta.flags.is_empty() + } +} + +/// What every command in one document is written against. +/// +/// Bundled because it is the same for all of them: the binary a completer would name, the +/// per-command overlays a view applies, and which flagsets exist. Passing them one by one had +/// `write_command` and `write_body` at eight parameters each, most of them pass-through. +struct Writing<'a, 'o> { + bin: &'a str, + overlays: &'o [CommandOverlay<'o>], + sets: Flagsets<'a>, +} + +/// Write one command's flags, as `use` for every set that stands for a run of them. +/// +/// Shared with the body of a `flagset`, which is the same question asked of a flattened +/// struct's own metadata — so a struct that flattens another is written as a set that uses a +/// set, and a group written inline still uses the sets it composes. +fn write_flag_layout( + out: &mut String, + meta: &CommandMeta<'_>, + depth: usize, + sets: &Flagsets<'_>, +) -> core::fmt::Result { + let mut i = 0; + while i < meta.flags.len() { + // A group that contributed no flags has no run to stand for, and matching it would + // advance nothing. + let group = meta + .flatten_groups + .iter() + .find(|g| g.start == i && !g.meta.flags.is_empty()); + match group { + Some(group) if sets.covers(group) => { + indent(out, depth)?; + writeln!(out, "use {}", quoted(group.name))?; + i += group.meta.flags.len(); + } + Some(group) => { + write_flag_layout(out, group.meta, depth, sets)?; + i += group.meta.flags.len(); + } + None => { + // The two tables are written in the same order by construction, so a + // mismatch means a table was edited without its metadata. + debug_assert!( + meta.cmd + .flags + .get(i) + .is_some_and(|f| core::ptr::eq(*f, meta.flags[i].flag)), + "flag metadata is out of step with the parse table" + ); + write_flag(out, &meta.flags[i], depth)?; + i += 1; + } + } } + Ok(()) } /// Write a command's contents: its flags, arguments, and subcommands. @@ -1477,8 +1659,7 @@ fn write_body<'a>( meta: &CommandMeta<'a>, depth: usize, inherited_unknown_flags: UnknownFlags, - bin: &str, - overlays: &[CommandOverlay<'_>], + w: &Writing<'_, '_>, path: &mut Vec<&'a str>, ) -> core::fmt::Result { // The effective setting for everything inside, which is this command's if it stated one @@ -1501,18 +1682,7 @@ fn write_body<'a>( meta.subcommands.len(), "every subcommand in the parse table needs metadata" ); - for (i, flag) in meta.flags.iter().enumerate() { - // The two tables are written in the same order by construction, so a - // mismatch means a table was edited without its metadata. - debug_assert!( - meta.cmd - .flags - .get(i) - .is_some_and(|f| core::ptr::eq(*f, flag.flag)), - "flag metadata is out of step with the parse table" - ); - write_flag(out, flag, depth)?; - } + write_flag_layout(out, meta, depth, &w.sets)?; for (i, arg) in meta.args.iter().enumerate() { debug_assert!( meta.cmd @@ -1530,17 +1700,9 @@ fn write_body<'a>( write_group(out, group, depth)?; } #[cfg(feature = "complete")] - write_completers(out, meta, bin, depth)?; + write_completers(out, meta, w.bin, depth)?; for sub in meta.subcommands { - write_command( - out, - sub, - depth, - enclosing_unknown_flags, - bin, - overlays, - path, - )?; + write_command(out, sub, depth, enclosing_unknown_flags, w, path)?; } Ok(()) } @@ -1599,8 +1761,7 @@ fn write_command<'a>( meta: &CommandMeta<'a>, depth: usize, inherited_unknown_flags: UnknownFlags, - bin: &str, - overlays: &[CommandOverlay<'_>], + w: &Writing<'_, '_>, path: &mut Vec<&'a str>, ) -> core::fmt::Result { path.push(meta.cmd.name); @@ -1624,7 +1785,8 @@ fn write_command<'a>( if let Some(heading) = meta.help_heading { write!(out, " help_heading={}", quoted(heading))?; } - let effect = overlays + let effect = w + .overlays .iter() .rev() .find(|overlay| overlay.command.matches(meta, path)) @@ -1745,15 +1907,7 @@ fn write_command<'a>( for example in meta.examples { write_example(out, example, inner)?; } - write_body( - out, - meta, - inner, - effective_unknown_flags, - bin, - overlays, - path, - )?; + write_body(out, meta, inner, effective_unknown_flags, w, path)?; indent(out, depth)?; out.push_str("}\n"); @@ -3603,13 +3757,17 @@ mod tests { }; let mut out = String::new(); + let w = Writing { + bin: "ex", + overlays: &[], + sets: Flagsets::collect(&ROOT_META), + }; write_body( &mut out, &ROOT_META, 0, UnknownFlags::Value, - "ex", - &[], + &w, &mut Vec::new(), ) .unwrap(); diff --git a/cli/src/cli/mod.rs b/cli/src/cli/mod.rs index 272b0e1f9..9344e8375 100644 --- a/cli/src/cli/mod.rs +++ b/cli/src/cli/mod.rs @@ -20,6 +20,11 @@ mod sponsors; // 3.6 added `effect=` and 4.0 added it on flags and args; older `usage` CLIs reject the spec // outright with "unsupported cmd prop effect", so this moves in lockstep with the fields the // spec actually carries. +// +// Owed a bump: the four shell commands flatten `Shell`, which now emits a `flagset`, and no +// 4.0 can read that node. The floor is whichever release carries flagsets, so the number waits +// for that release rather than being guessed at here — and this crate warning about its own +// spec until then is worse than the stale claim. #[derive(DeriveCli)] #[usage( bin = "usage", diff --git a/cli/usage.usage.kdl b/cli/usage.usage.kdl index 8522bcbb0..4f5513f71 100644 --- a/cli/usage.usage.kdl +++ b/cli/usage.usage.kdl @@ -9,14 +9,17 @@ about "CLI for working with usage-based CLIs" usage "Usage: usage \n usage --completions \n usage --usage-spec" unknown_flags error subcommand_required #true +flagset shell { + flag -h help="Show help" + flag --help help="Show help" +} flag --completions help="Outputs completions for the specified shell for completing the `usage` CLI itself" { arg } flag --usage-spec help="Outputs a `usage.kdl` spec for this CLI itself" cmd bash help="Execute a shell script using bash" unknown_flags=value { long_help "Execute a shell script with the specified shell\n\nTypically, this will be called by a script's shebang.\n\nIf using `var=#true` on args/flags, they will be joined with spaces using `shell_words::join()`\nto properly escape and quote values with spaces in them." - flag -h help="Show help" - flag --help help="Show help" + use shell arg