Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -567,9 +567,10 @@ Groups are the opposite case: `Command::get_groups`, `ArgGroup::get_args` and
while `long_version` can provide extended `--version` build information.
Typed declarations, portable KDL, usage-lib, generated Go, and the clap
bridge preserve both; computed values pair with explicit spec literals.
- [ ] **Explicit `display_order`.** Declaration order covers the common case,
but cannot preserve an adopter that deliberately presents fields or
subcommands in a different order.
- [x] **Explicit `display_order`.** Fields and subcommands retain deliberate
presentation order through typed metadata, portable KDL, the clap bridge,
usage-lib, and generated Rust and Go help. Positional parsing still follows
declaration order; the setting changes presentation only.

**API surface**

Expand Down
96 changes: 80 additions & 16 deletions argv/src/help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ fn help_structure(
!arg.hide_short_help
}
};
let args: Vec<_> = meta.args.iter().filter(visible_arg).collect();
let mut args: Vec<_> = meta.args.iter().filter(visible_arg).collect();
order_args(&mut args, meta.args);
if args.iter().any(|arg| arg.help_heading.is_none()) {
headings.push("Arguments".to_string());
}
Expand All @@ -151,7 +152,8 @@ fn help_structure(
!flag.hide_short_help
}
};
let own: Vec<_> = own.into_iter().filter(visible_flag).collect();
let mut own: Vec<_> = own.into_iter().filter(visible_flag).collect();
order_flags(&mut own, meta.flags);
let inherited: Vec<_> = inherited
.into_iter()
.filter(|(flag, _)| {
Expand Down Expand Up @@ -186,7 +188,7 @@ fn help_structure(

fn flat_help_headings(path: &[&str], meta: &CommandMeta<'_>, headings: &mut Vec<String>) {
let mut visible: Vec<_> = meta.subcommands.iter().filter(|sub| !sub.hide).collect();
visible.sort_by_key(|sub| sub.cmd.name);
order_commands(&mut visible);
for sub in visible {
let mut sub_path = path.to_vec();
sub_path.push(sub.cmd.name);
Expand Down Expand Up @@ -666,11 +668,12 @@ pub fn short_help(spec: &Spec<'_>, path: &[&str], chain: &[&CommandMeta<'_>]) ->
// after the name it belonged to, so nothing in `-h` lined up with anything — and `-h` is
// the form most people type. One column per section over its visible entries, which is
// the rule the long page already follows.
let args: Vec<&ArgMeta<'_>> = meta
let mut args: Vec<&ArgMeta<'_>> = meta
.args
.iter()
.filter(|a| !a.hide && !a.hide_short_help)
.collect();
order_args(&mut args, meta.args);
let arg_col = args
.iter()
.map(|a| arg_usage(a).chars().count())
Expand Down Expand Up @@ -801,7 +804,8 @@ pub fn short_help(spec: &Spec<'_>, path: &[&str], chain: &[&CommandMeta<'_>]) ->

/// The list of subcommands, and the `help` command every CLI with subcommands has.
fn commands_section(out: &mut String, path: &[&str], meta: &CommandMeta<'_>) {
let visible: Vec<&&CommandMeta<'_>> = meta.subcommands.iter().filter(|c| !c.hide).collect();
let mut visible: Vec<&&CommandMeta<'_>> = meta.subcommands.iter().filter(|c| !c.hide).collect();
order_commands(&mut visible);
// Nothing visible, no section — `mise direnv` and `mise dotfiles` have subcommands and
// every one of them is hidden. The usage *line* still says `<SUBCOMMAND>`, because
// usage-lib computes it before filtering and stores it; matching the reference means
Expand All @@ -823,7 +827,12 @@ fn commands_section(out: &mut String, path: &[&str], meta: &CommandMeta<'_>) {
(usage_line(&sub_path, sub), *sub)
})
.collect();
lines.sort_by(|a, b| a.0.cmp(&b.0));
lines.sort_by(|a, b| {
a.1.display_order
.unwrap_or(999)
.cmp(&b.1.display_order.unwrap_or(999))
.then_with(|| a.0.cmp(&b.0))
});

for (usage, sub) in &lines {
let _ = write!(out, " {usage}");
Expand Down Expand Up @@ -866,7 +875,7 @@ fn commands_section(out: &mut String, path: &[&str], meta: &CommandMeta<'_>) {

fn flat_commands_short(out: &mut String, path: &[&str], meta: &CommandMeta<'_>) {
let mut visible: Vec<_> = meta.subcommands.iter().filter(|sub| !sub.hide).collect();
visible.sort_by_key(|sub| sub.cmd.name);
order_commands(&mut visible);
for sub in visible {
let mut sub_path = path.to_vec();
sub_path.push(sub.cmd.name);
Expand All @@ -875,16 +884,18 @@ fn flat_commands_short(out: &mut String, path: &[&str], meta: &CommandMeta<'_>)
let _ = writeln!(out, "{}", about.trim_end());
}

let args: Vec<_> = sub
let mut args: Vec<_> = sub
.args
.iter()
.filter(|arg| !arg.hide && !arg.hide_short_help)
.collect();
let flags: Vec<&FlagMeta<'_>> = sub
order_args(&mut args, sub.args);
let mut flags: Vec<&FlagMeta<'_>> = sub
.flags
.iter()
.filter(|flag| !flag.flag.global && !flag.hide && !flag.hide_short_help)
.collect();
order_flags(&mut flags, sub.flags);
let arg_col = args
.iter()
.map(|arg| arg_usage(arg).chars().count())
Expand Down Expand Up @@ -983,6 +994,37 @@ fn groups_section<'m, T: 'm>(
}
}

fn order_args<'a>(items: &mut Vec<&'a ArgMeta<'a>>, declared: &'a [ArgMeta<'a>]) {
items.sort_by_key(|item| {
item.display_order.unwrap_or_else(|| {
declared
.iter()
.position(|candidate| core::ptr::eq(candidate, *item))
.unwrap_or(usize::MAX)
})
});
}

fn order_flags<'a>(items: &mut Vec<&'a FlagMeta<'a>>, declared: &'a [FlagMeta<'a>]) {
items.sort_by_key(|item| {
item.display_order.unwrap_or_else(|| {
declared
.iter()
.position(|candidate| core::ptr::eq(candidate, *item))
.unwrap_or(usize::MAX)
})
});
Comment thread
cursor[bot] marked this conversation as resolved.
}

fn order_commands(items: &mut Vec<&&CommandMeta<'_>>) {
items.sort_by(|a, b| {
a.display_order
.unwrap_or(999)
.cmp(&b.display_order.unwrap_or(999))
.then_with(|| a.cmd.name.cmp(b.cmd.name))
});
}

/// The bracketed notes after an entry's help: choices, environment, default.
fn annotations(out: &mut String, choices: &[&str], env: Option<&str>, default: &[&str]) {
if !choices.is_empty() {
Expand Down Expand Up @@ -1190,11 +1232,12 @@ pub fn long_help(spec: &Spec<'_>, path: &[&str], chain: &[&CommandMeta<'_>]) ->

// One column width per section, over its visible entries — the same two the reference
// computes, and separately, so a long flag does not push the arguments out.
let args: Vec<&ArgMeta<'_>> = meta
let mut args: Vec<&ArgMeta<'_>> = meta
.args
.iter()
.filter(|a| !a.hide && !a.hide_long_help)
.collect();
order_args(&mut args, meta.args);
let arg_col = args
.iter()
.map(|a| arg_usage(a).chars().count())
Expand Down Expand Up @@ -1430,7 +1473,8 @@ fn long_annotations(out: &mut String, choices: &[&str], env: Option<&str>, defau

/// The commands list, with each command's help beneath its usage.
fn long_commands_section(out: &mut String, path: &[&str], meta: &CommandMeta<'_>) {
let visible: Vec<&&CommandMeta<'_>> = meta.subcommands.iter().filter(|c| !c.hide).collect();
let mut visible: Vec<&&CommandMeta<'_>> = meta.subcommands.iter().filter(|c| !c.hide).collect();
order_commands(&mut visible);
if visible.is_empty() {
return;
}
Expand All @@ -1445,7 +1489,12 @@ fn long_commands_section(out: &mut String, path: &[&str], meta: &CommandMeta<'_>
(usage_line(&sub_path, sub), *sub)
})
.collect();
lines.sort_by(|a, b| a.0.cmp(&b.0));
lines.sort_by(|a, b| {
a.1.display_order
.unwrap_or(999)
.cmp(&b.1.display_order.unwrap_or(999))
.then_with(|| a.0.cmp(&b.0))
});

for (usage, sub) in &lines {
let _ = write!(out, " {usage}");
Expand Down Expand Up @@ -1479,7 +1528,7 @@ fn long_commands_section(out: &mut String, path: &[&str], meta: &CommandMeta<'_>

fn flat_commands_long(out: &mut String, path: &[&str], meta: &CommandMeta<'_>, width: usize) {
let mut visible: Vec<_> = meta.subcommands.iter().filter(|sub| !sub.hide).collect();
visible.sort_by_key(|sub| sub.cmd.name);
order_commands(&mut visible);
for sub in visible {
let mut sub_path = path.to_vec();
sub_path.push(sub.cmd.name);
Expand All @@ -1492,16 +1541,18 @@ fn flat_commands_long(out: &mut String, path: &[&str], meta: &CommandMeta<'_>, w
let _ = writeln!(out, "{}", about.trim_end());
}

let args: Vec<_> = sub
let mut args: Vec<_> = sub
.args
.iter()
.filter(|arg| !arg.hide && !arg.hide_long_help)
.collect();
let flags: Vec<&FlagMeta<'_>> = sub
order_args(&mut args, sub.args);
let mut flags: Vec<&FlagMeta<'_>> = sub
.flags
.iter()
.filter(|flag| !flag.flag.global && !flag.hide && !flag.hide_long_help)
.collect();
order_flags(&mut flags, sub.flags);
let arg_col = args
.iter()
.map(|arg| arg_usage(arg).chars().count())
Expand Down Expand Up @@ -1807,7 +1858,7 @@ fn own_and_global<'a>(
keep.push((f as *const _, show));
}
}
let inherited: Vec<(&FlagMeta<'_>, String)> = ancestors
let mut inherited: Vec<(&FlagMeta<'_>, String)> = ancestors
.iter()
.flat_map(|meta| meta.flags.iter())
.filter_map(|f| {
Expand All @@ -1816,6 +1867,18 @@ fn own_and_global<'a>(
.map(|(_, show)| (f, column_usage_masked(f, show)))
})
.collect();
let inherited_positions: Vec<*const FlagMeta<'_>> = inherited
.iter()
.map(|(flag, _)| *flag as *const _)
.collect();
inherited.sort_by_key(|(flag, _)| {
flag.display_order.unwrap_or_else(|| {
inherited_positions
.iter()
.position(|candidate| core::ptr::eq(*candidate, *flag as *const _))
.unwrap_or(usize::MAX)
})
});

// Last in the command's own section, which is where clap has them: they carry no
// `help_heading`, so a CLI that groups its flags gets them at the end of the ungrouped
Expand All @@ -1825,6 +1888,7 @@ fn own_and_global<'a>(
// negations, and a `--help` the page offers while something else binds it is exactly the
// lie this whole model exists to prevent.
let mut own = own;
order_flags(&mut own, here.flags);
Comment thread
jdx marked this conversation as resolved.
// Forms *and* negations: `long_flag` asks `find_negation` before it offers `--version`,
// so a declared negation beats a supplied flag even though it loses to a long.
let claimed: Vec<String> = taken
Expand Down
18 changes: 18 additions & 0 deletions argv/src/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,8 @@ pub struct CommandMeta<'a> {
pub hidden_aliases: &'a [&'a str],
/// Whether the command is hidden from help and completions.
pub hide: bool,
/// Explicit placement within the parent's command section.
pub display_order: Option<usize>,
/// What running this does to the world, for a caller deciding whether to
/// confirm first. clap cannot express this, which is why mise keeps a
/// 330-entry table to bolt it on afterwards.
Expand Down Expand Up @@ -778,6 +780,7 @@ impl CommandMeta<'_> {
long_about: None,
hidden_aliases: &[],
hide: false,
display_order: None,
effect: None,
mount: None,
restart_token: None,
Expand Down Expand Up @@ -805,6 +808,8 @@ impl CommandMeta<'_> {
#[derive(Debug, Clone, Copy)]
pub struct FlagMeta<'a> {
pub flag: &'a Flag<'a>,
/// Explicit placement within its help section.
pub display_order: Option<usize>,
/// Short forms accepted by the parser but omitted from help and completion.
pub hidden_shorts: &'a [u8],
/// Long forms accepted by the parser but omitted from help and completion.
Expand Down Expand Up @@ -913,6 +918,7 @@ impl FlagMeta<'_> {
complete: None,
complete_type: None,
flag: &Flag::BOOL,
display_order: None,
hidden_shorts: &[],
hidden_longs: &[],
help: None,
Expand Down Expand Up @@ -989,6 +995,8 @@ pub struct DefaultIf<'a> {
#[derive(Debug, Clone, Copy)]
pub struct ArgMeta<'a> {
pub arg: &'a Arg<'a>,
/// Explicit placement within its help section.
pub display_order: Option<usize>,
/// Ordered placeholders for a fixed-arity positional.
pub value_names: &'a [&'a str],
pub help: Option<&'a str>,
Expand Down Expand Up @@ -1044,6 +1052,7 @@ impl ArgMeta<'_> {
complete: None,
complete_type: None,
arg: &Arg::REQUIRED,
display_order: None,
value_names: &[],
help: None,
long_help: None,
Expand Down Expand Up @@ -1486,6 +1495,9 @@ fn write_command<'a>(
if meta.subcommand_required && !meta.cmd.subcommands.is_empty() {
out.push_str(" subcommand_required=#true");
}
if let Some(order) = meta.display_order {
write!(out, " display_order={order}")?;
}
if let Some(heading) = meta.subcommand_help_heading {
write!(out, " subcommand_help_heading={}", quoted(heading))?;
}
Expand Down Expand Up @@ -1689,6 +1701,9 @@ fn write_flag(out: &mut String, meta: &FlagMeta<'_>, depth: usize) -> core::fmt:
if let Some(heading) = meta.help_heading {
write!(out, " help_heading={}", quoted(heading))?;
}
if let Some(order) = meta.display_order {
write!(out, " display_order={order}")?;
}
if let Some(effect) = meta.effect {
write!(out, " effect={}", quoted(effect.as_str()))?;
}
Expand Down Expand Up @@ -1998,6 +2013,9 @@ fn write_arg(out: &mut String, meta: &ArgMeta<'_>, depth: usize) -> core::fmt::R
if let Some(heading) = meta.help_heading {
write!(out, " help_heading={}", quoted(heading))?;
}
if let Some(order) = meta.display_order {
write!(out, " display_order={order}")?;
}
if let Some(env) = meta.env {
write!(out, " env={}", quoted(env))?;
}
Expand Down
16 changes: 16 additions & 0 deletions clap_usage/tests/fidelity_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,22 @@ fn disabled_long_version_flag_is_reported() {
.any(|loss| loss.feature == FidelityFeature::DisableVersionFlag));
}

#[test]
fn display_order_is_lossless() {
let mut command = Command::new("ex")
.arg(Arg::new("second").long("second").display_order(20))
.arg(Arg::new("first").long("first").display_order(10))
.subcommand(Command::new("second").display_order(20))
.subcommand(Command::new("first").display_order(10));
let (spec, report) = spec_with_report(&mut command, "ex");

assert_eq!(spec.cmd.flags[0].display_order, Some(20));
assert_eq!(spec.cmd.flags[1].display_order, Some(10));
assert_eq!(spec.cmd.subcommands[0].display_order, Some(20));
assert_eq!(spec.cmd.subcommands[1].display_order, Some(10));
assert!(report.is_lossless(), "{report:#?}");
}

#[test]
fn reports_detectable_losses_with_locations() {
let mut command = Command::new("ex")
Expand Down
3 changes: 3 additions & 0 deletions conformance/src/tables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ pub fn build(
.into_boxed_slice(),
),
hide: cmd.hide,
display_order: cmd.display_order,
effect: cmd.effect.map(effect),
// A command carries at most one mount in the tables; a spec may list several, and the
// first is the one the tables can hold.
Expand Down Expand Up @@ -420,6 +421,7 @@ fn flag_meta(
required_unless: strs(&f.required_unless),
required_unless_all: strs(&f.required_unless_all),
help_heading: opt(&f.help_heading),
display_order: f.display_order,
effect: f.effect.map(effect),
complete_type: complete_type(completers, &f.name, arg.map(|a| a.name.as_str())),
complete: NO_COMPLETER,
Expand Down Expand Up @@ -448,6 +450,7 @@ fn arg_meta(
validate_error: a.validate_error.as_deref().map(leak),
required: a.required,
hide: a.hide,
display_order: a.display_order,
hide_default_value: a.hide_default_value,
hide_env: a.hide_env,
hide_env_values: a.hide_env_values,
Expand Down
Loading