Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(mangen): Support flatten_help #5769

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

aparcar
Copy link

@aparcar aparcar commented Oct 7, 2024

The flatten_help argument combines all subcommands on a single page. Until now this wasn't supported for mangen. With this command the sections SYNOPSIS as well as (SUB)COMMANDS are changed to imitate the style of git stash --help.

I reached out via discussions and it looks like this doesn't work just yet #5761

This is not necessarily the most beautiful implementation due to my limited knowledge of both clap and Rust itself.

I implemented this for a project I'm working on and the results look quite okay:

image

Not using man.render() but call each command manually feels a bit awkward but does work just fine, see example here https://github.com/rosenpass/rosenpass/pull/434/files#diff-352422e105afd3138a54ea14e33af782243398f1d768d271655729dd4bcb99d2R18

clap_mangen/src/lib.rs Outdated Show resolved Hide resolved
clap_mangen/src/lib.rs Outdated Show resolved Hide resolved
@aparcar aparcar force-pushed the flat-man branch 3 times, most recently from 3f70e85 to c2c7c26 Compare October 16, 2024 15:15
clap_mangen/src/render.rs Outdated Show resolved Hide resolved
@aparcar aparcar marked this pull request as ready for review October 16, 2024 16:09
for subcommand in cmd.get_subcommands() {
ord_v.push((
subcommand.get_display_order(),
format!("{} {}", name, subcommand.get_name().to_owned()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we manually building this, rather than using subcommand.get_bin_name().unwrap_or_else(|| cmd.get_name());

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With that change, do we even need a name parameter?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleaned up

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not addressed. We are still passing in the name and still using get_name

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change no longer exists, it's marked outdated

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whether the lines this originally applied to are there or not, the problem is still there.

clap_mangen/src/render.rs Outdated Show resolved Hide resolved
clap_mangen/src/render.rs Outdated Show resolved Hide resolved
clap_mangen/src/render.rs Outdated Show resolved Hide resolved
clap_mangen/src/render.rs Outdated Show resolved Hide resolved
@aparcar aparcar force-pushed the flat-man branch 2 times, most recently from 5094ea2 to 77e56f6 Compare October 17, 2024 13:10
@aparcar aparcar marked this pull request as draft October 17, 2024 13:42
@aparcar
Copy link
Author

aparcar commented Oct 17, 2024

I added a wip commit to handle is_subcommand_required. Still trying to figure out is_args_conflicts_with_subcommands_set Incorporated is_subcommand_required and is_args_conflicts_with_subcommands_set

@aparcar aparcar force-pushed the flat-man branch 2 times, most recently from 7acd84c to 52a7a4c Compare October 18, 2024 09:31
@aparcar aparcar marked this pull request as ready for review October 18, 2024 09:44
@aparcar
Copy link
Author

aparcar commented Oct 21, 2024

I added the differences between flattened --help and flattened Man to the commit description, below are screenshots of what I used for comparison.

--help

image

Man

image

@aparcar
Copy link
Author

aparcar commented Oct 21, 2024

@epage hey Ed I covered your comments as good as I could and think this is ready

Comment on lines 46 to 68
}
roff.text(line);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something is off about this commit

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's marked as outdated, I don't see this anymore.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still seeing it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Differences between flattened --help and the flattened man:

  • --help prints the description at the very beginning while Man uses a
    section called DESCRIPTION below the USAGE.
  • --help prints placeholder [OPTIONS] while Man prints all options
  • --help prints positional options (aka arguments) in its own section
    called arguments while Man prints them at the end of OPTIONS.
  • --help prints subcommands as their own section after the options
    section while Man uses an extra section called SUBCOMMANDS
  • --help prints after_long_help at the very end while Man uses the
    section EXTRA which comes before the sections VERSION and AUTHORS

Most of these differences sound unrelated to flattening

@@ -105,3 +105,17 @@ fn value_name_without_arg() {
cmd,
);
}

#[test]
fn flatten_help_false() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably cover more cases that help covers, see

clap/tests/builder/help.rs

Lines 3280 to 3971 in 61f5ee5

#[test]
fn flatten_basic() {
let cmd = Command::new("parent")
.flatten_help(true)
.about("parent command")
.arg(Arg::new("parent").long("parent"))
.subcommand(
Command::new("test")
.about("test command")
.arg(Arg::new("child").long("child")),
);
let expected = str![[r#"
parent command
Usage: parent [OPTIONS]
parent test [OPTIONS]
parent help [COMMAND]...
Options:
--parent <parent>
-h, --help Print help
parent test:
test command
--child <child>
-h, --help Print help
parent help:
Print this message or the help of the given subcommand(s)
[COMMAND]... Print help for the subcommand(s)
"#]];
utils::assert_output(cmd, "parent -h", expected, false);
}
#[test]
fn flatten_short_help() {
let cmd = Command::new("parent")
.flatten_help(true)
.about("parent command")
.arg(
Arg::new("parent")
.long("parent")
.help("foo")
.long_help("bar"),
)
.subcommand(
Command::new("test")
.about("test command")
.long_about("long some")
.arg(Arg::new("child").long("child").help("foo").long_help("bar")),
);
let expected = str![[r#"
parent command
Usage: parent [OPTIONS]
parent test [OPTIONS]
parent help [COMMAND]...
Options:
--parent <parent> foo
-h, --help Print help (see more with '--help')
parent test:
test command
--child <child> foo
-h, --help Print help (see more with '--help')
parent help:
Print this message or the help of the given subcommand(s)
[COMMAND]... Print help for the subcommand(s)
"#]];
utils::assert_output(cmd, "parent -h", expected, false);
}
#[test]
fn flatten_long_help() {
let cmd = Command::new("parent")
.flatten_help(true)
.about("parent command")
.arg(
Arg::new("parent")
.long("parent")
.help("foo")
.long_help("bar"),
)
.subcommand(
Command::new("test")
.about("test command")
.long_about("long some")
.arg(Arg::new("child").long("child").help("foo").long_help("bar")),
);
let expected = str![[r#"
parent command
Usage: parent [OPTIONS]
parent test [OPTIONS]
parent help [COMMAND]...
Options:
--parent <parent>
bar
-h, --help
Print help (see a summary with '-h')
parent test:
test command
--child <child>
bar
-h, --help
Print help (see a summary with '-h')
parent help:
Print this message or the help of the given subcommand(s)
[COMMAND]...
Print help for the subcommand(s)
"#]];
utils::assert_output(cmd, "parent --help", expected, false);
}
#[test]
fn flatten_help_cmd() {
let cmd = Command::new("parent")
.flatten_help(true)
.about("parent command")
.arg(
Arg::new("parent")
.long("parent")
.help("foo")
.long_help("bar"),
)
.subcommand(
Command::new("test")
.about("test command")
.long_about("long some")
.arg(Arg::new("child").long("child").help("foo").long_help("bar")),
);
let expected = str![[r#"
parent command
Usage: parent [OPTIONS]
parent test [OPTIONS]
parent help [COMMAND]...
Options:
--parent <parent>
bar
-h, --help
Print help (see a summary with '-h')
parent test:
test command
--child <child>
bar
-h, --help
Print help (see a summary with '-h')
parent help:
Print this message or the help of the given subcommand(s)
[COMMAND]...
Print help for the subcommand(s)
"#]];
utils::assert_output(cmd, "parent help", expected, false);
}
#[test]
fn flatten_with_global() {
let cmd = Command::new("parent")
.flatten_help(true)
.about("parent command")
.arg(Arg::new("parent").long("parent").global(true))
.subcommand(
Command::new("test")
.about("test command")
.arg(Arg::new("child").long("child")),
);
let expected = str![[r#"
parent command
Usage: parent [OPTIONS]
parent test [OPTIONS]
parent help [COMMAND]...
Options:
--parent <parent>
-h, --help Print help
parent test:
test command
--child <child>
-h, --help Print help
parent help:
Print this message or the help of the given subcommand(s)
[COMMAND]... Print help for the subcommand(s)
"#]];
utils::assert_output(cmd, "parent -h", expected, false);
}
#[test]
fn flatten_arg_required() {
let cmd = Command::new("parent")
.flatten_help(true)
.about("parent command")
.arg(Arg::new("parent").long("parent").required(true))
.subcommand(
Command::new("test")
.about("test command")
.arg(Arg::new("child").long("child").required(true)),
);
let expected = str![[r#"
parent command
Usage: parent --parent <parent>
parent --parent <parent> test --child <child>
parent --parent <parent> help [COMMAND]...
Options:
--parent <parent>
-h, --help Print help
parent --parent <parent> test:
test command
--child <child>
-h, --help Print help
parent --parent <parent> help:
Print this message or the help of the given subcommand(s)
[COMMAND]... Print help for the subcommand(s)
"#]];
utils::assert_output(cmd, "parent -h", expected, false);
}
#[test]
fn flatten_with_external_subcommand() {
let cmd = Command::new("parent")
.flatten_help(true)
.about("parent command")
.allow_external_subcommands(true)
.arg(Arg::new("parent").long("parent"))
.subcommand(
Command::new("test")
.about("test command")
.arg(Arg::new("child").long("child")),
);
let expected = str![[r#"
parent command
Usage: parent [OPTIONS]
parent test [OPTIONS]
parent help [COMMAND]...
Options:
--parent <parent>
-h, --help Print help
parent test:
test command
--child <child>
-h, --help Print help
parent help:
Print this message or the help of the given subcommand(s)
[COMMAND]... Print help for the subcommand(s)
"#]];
utils::assert_output(cmd, "parent -h", expected, false);
}
#[test]
fn flatten_without_subcommands() {
let cmd = Command::new("parent")
.flatten_help(true)
.about("parent command")
.arg(Arg::new("parent").long("parent"));
let expected = str![[r#"
parent command
Usage: parent [OPTIONS]
Options:
--parent <parent>
-h, --help Print help
"#]];
utils::assert_output(cmd, "parent -h", expected, false);
}
#[test]
fn flatten_with_subcommand_required() {
let cmd = Command::new("parent")
.flatten_help(true)
.about("parent command")
.subcommand_required(true)
.arg(Arg::new("parent").long("parent"))
.subcommand(
Command::new("test")
.about("test command")
.arg(Arg::new("child").long("child")),
);
let expected = str![[r#"
parent command
Usage: parent test [OPTIONS]
parent help [COMMAND]...
Options:
--parent <parent>
-h, --help Print help
parent test:
test command
--child <child>
-h, --help Print help
parent help:
Print this message or the help of the given subcommand(s)
[COMMAND]... Print help for the subcommand(s)
"#]];
utils::assert_output(cmd, "parent -h", expected, false);
}
#[test]
fn flatten_with_args_conflicts_with_subcommands() {
let cmd = Command::new("parent")
.flatten_help(true)
.about("parent command")
.subcommand_required(true)
.args_conflicts_with_subcommands(true)
.arg(Arg::new("parent").long("parent"))
.subcommand(
Command::new("test")
.about("test command")
.arg(Arg::new("child").long("child")),
);
let expected = str![[r#"
parent command
Usage: parent [OPTIONS]
parent test [OPTIONS]
parent help [COMMAND]...
Options:
--parent <parent>
-h, --help Print help
parent test:
test command
--child <child>
-h, --help Print help
parent help:
Print this message or the help of the given subcommand(s)
[COMMAND]... Print help for the subcommand(s)
"#]];
utils::assert_output(cmd, "parent -h", expected, false);
}
#[test]
fn flatten_single_hidden_command() {
let cmd = Command::new("parent")
.flatten_help(true)
.about("parent command")
.arg(Arg::new("parent").long("parent"))
.subcommand(
Command::new("child1")
.hide(true)
.about("child1 command")
.arg(Arg::new("child").long("child1")),
);
let expected = str![[r#"
parent command
Usage: parent [OPTIONS]
Options:
--parent <parent>
-h, --help Print help
"#]];
utils::assert_output(cmd, "parent -h", expected, false);
}
#[test]
fn flatten_hidden_command() {
let cmd = Command::new("parent")
.flatten_help(true)
.about("parent command")
.arg(Arg::new("parent").long("parent"))
.subcommand(
Command::new("child1")
.about("child1 command")
.arg(Arg::new("child").long("child1")),
)
.subcommand(
Command::new("child2")
.about("child2 command")
.arg(Arg::new("child").long("child2")),
)
.subcommand(
Command::new("child3")
.hide(true)
.about("child3 command")
.arg(Arg::new("child").long("child3")),
);
let expected = str![[r#"
parent command
Usage: parent [OPTIONS]
parent child1 [OPTIONS]
parent child2 [OPTIONS]
parent help [COMMAND]...
Options:
--parent <parent>
-h, --help Print help
parent child1:
child1 command
--child1 <child>
-h, --help Print help
parent child2:
child2 command
--child2 <child>
-h, --help Print help
parent help:
Print this message or the help of the given subcommand(s)
[COMMAND]... Print help for the subcommand(s)
"#]];
utils::assert_output(cmd, "parent -h", expected, false);
}
#[test]
fn flatten_recursive() {
let cmd = Command::new("parent")
.flatten_help(true)
.about("parent command")
.arg(Arg::new("parent").long("parent"))
.subcommand(
Command::new("child1")
.flatten_help(true)
.about("child1 command")
.arg(Arg::new("child").long("child1"))
.subcommand(
Command::new("grandchild1")
.flatten_help(true)
.about("grandchild1 command")
.arg(Arg::new("grandchild").long("grandchild1"))
.subcommand(
Command::new("greatgrandchild1")
.about("greatgrandchild1 command")
.arg(Arg::new("greatgrandchild").long("greatgrandchild1")),
)
.subcommand(
Command::new("greatgrandchild2")
.about("greatgrandchild2 command")
.arg(Arg::new("greatgrandchild").long("greatgrandchild2")),
)
.subcommand(
Command::new("greatgrandchild3")
.about("greatgrandchild3 command")
.arg(Arg::new("greatgrandchild").long("greatgrandchild3")),
),
)
.subcommand(
Command::new("grandchild2")
.about("grandchild2 command")
.arg(Arg::new("grandchild").long("grandchild2")),
)
.subcommand(
Command::new("grandchild3")
.about("grandchild3 command")
.arg(Arg::new("grandchild").long("grandchild3")),
),
)
.subcommand(
Command::new("child2")
.about("child2 command")
.arg(Arg::new("child").long("child2")),
)
.subcommand(
Command::new("child3")
.hide(true)
.about("child3 command")
.arg(Arg::new("child").long("child3"))
.subcommand(
Command::new("grandchild1")
.flatten_help(true)
.about("grandchild1 command")
.arg(Arg::new("grandchild").long("grandchild1"))
.subcommand(
Command::new("greatgrandchild1")
.about("greatgrandchild1 command")
.arg(Arg::new("greatgrandchild").long("greatgrandchild1")),
)
.subcommand(
Command::new("greatgrandchild2")
.about("greatgrandchild2 command")
.arg(Arg::new("greatgrandchild").long("greatgrandchild2")),
)
.subcommand(
Command::new("greatgrandchild3")
.about("greatgrandchild3 command")
.arg(Arg::new("greatgrandchild").long("greatgrandchild3")),
),
)
.subcommand(
Command::new("grandchild2")
.about("grandchild2 command")
.arg(Arg::new("grandchild").long("grandchild2")),
)
.subcommand(
Command::new("grandchild3")
.about("grandchild3 command")
.arg(Arg::new("grandchild").long("grandchild3")),
),
);
let expected = str![[r#"
parent command
Usage: parent [OPTIONS]
parent child1 [OPTIONS]
parent child1 grandchild1 [OPTIONS]
parent child1 grandchild1 greatgrandchild1 [OPTIONS]
parent child1 grandchild1 greatgrandchild2 [OPTIONS]
parent child1 grandchild1 greatgrandchild3 [OPTIONS]
parent child1 grandchild1 help [COMMAND]
parent child1 grandchild2 [OPTIONS]
parent child1 grandchild3 [OPTIONS]
parent child1 help [COMMAND]
parent child2 [OPTIONS]
parent help [COMMAND]...
Options:
--parent <parent>
-h, --help Print help
parent child1:
child1 command
--child1 <child>
-h, --help Print help
parent child1 grandchild1:
grandchild1 command
--grandchild1 <grandchild>
-h, --help Print help
parent child1 grandchild1 greatgrandchild1:
greatgrandchild1 command
--greatgrandchild1 <greatgrandchild>
-h, --help Print help
parent child1 grandchild1 greatgrandchild2:
greatgrandchild2 command
--greatgrandchild2 <greatgrandchild>
-h, --help Print help
parent child1 grandchild1 greatgrandchild3:
greatgrandchild3 command
--greatgrandchild3 <greatgrandchild>
-h, --help Print help
parent child1 grandchild1 help:
Print this message or the help of the given subcommand(s)
parent child1 grandchild2:
grandchild2 command
--grandchild2 <grandchild>
-h, --help Print help
parent child1 grandchild3:
grandchild3 command
--grandchild3 <grandchild>
-h, --help Print help
parent child1 help:
Print this message or the help of the given subcommand(s)
parent child2:
child2 command
--child2 <child>
-h, --help Print help
parent help:
Print this message or the help of the given subcommand(s)
[COMMAND]... Print help for the subcommand(s)
"#]];
utils::assert_output(cmd, "parent -h", expected, false);
}
#[test]
fn flatten_not_recursive() {
let cmd = Command::new("parent")
.flatten_help(true)
.about("parent command")
.arg(Arg::new("parent").long("parent"))
.subcommand(
Command::new("child1")
.about("child1 command")
.arg(Arg::new("child").long("child1"))
.subcommand(
Command::new("grandchild1")
.about("grandchild1 command")
.arg(Arg::new("grandchild").long("grandchild1")),
)
.subcommand(
Command::new("grandchild2")
.about("grandchild2 command")
.arg(Arg::new("grandchild").long("grandchild2")),
)
.subcommand(
Command::new("grandchild3")
.about("grandchild3 command")
.arg(Arg::new("grandchild").long("grandchild3")),
),
)
.subcommand(
Command::new("child2")
.about("child2 command")
.arg(Arg::new("child").long("child2")),
)
.subcommand(
Command::new("child3")
.about("child3 command")
.arg(Arg::new("child").long("child3")),
);
let expected = str![[r#"
parent command
Usage: parent [OPTIONS]
parent child1 [OPTIONS] [COMMAND]
parent child2 [OPTIONS]
parent child3 [OPTIONS]
parent help [COMMAND]...
Options:
--parent <parent>
-h, --help Print help
parent child1:
child1 command
--child1 <child>
-h, --help Print help
parent child2:
child2 command
--child2 <child>
-h, --help Print help
parent child3:
child3 command
--child3 <child>
-h, --help Print help
parent help:
Print this message or the help of the given subcommand(s)
[COMMAND]... Print help for the subcommand(s)
"#]];
utils::assert_output(cmd, "parent -h", expected, false);
}

e.g.

  • we don't need to cover short vs long
  • we should cover recursion

It'd be a big help to reuse the same commands as those tests so we can easily compare the help output vs man output

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added all relevant tests from help.rs and at least the recursive stuff isn't working right now. Is the recursive stuff strictly required for this PR to move forward?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The recursive behavior is part of the definition of flatten_help. We could possibly split it out but we'd need to be tracking it somehow. Its likely best to go ahead and do that as it might affect the design in important ways

clap_mangen/src/render.rs Outdated Show resolved Hide resolved
This test shows that the output stays the same independent of the value
of `flatten_help`. Further commits may implement `flatten_help` for
`clap_mangen`.

Signed-off-by: Paul Spooren <[email protected]>
The function prints the usage of a (sub)command.

Signed-off-by: Paul Spooren <[email protected]>
The `flatten_help` argument combines all subcommands on a single page.
Until now this wasn't supported for `mangen`. With this command the
sections `SYNOPSIS` as well as `SUBCOMMANDS` are changed to imitate the
style of `git stash --help`.

Differences between flattened `--help` and the flattened man:
* `--help` prints the description at the very beginning while Man uses a
  section called DESCRIPTION below the USAGE.
* `--help` prints placeholder `[OPTIONS]` while Man prints all options
* `--help` prints positional options (aka arguments) in its own section
  called arguments while Man prints them at the end of OPTIONS.
* `--help` prints subcommands as their own section after the options
  section while Man uses an extra section called SUBCOMMANDS
* `--help` prints `after_long_help` at the very end while Man uses the
  section EXTRA which comes before the sections VERSION and AUTHORS

Signed-off-by: Paul Spooren <[email protected]>
Add all tests from tests/builder/help.rs to mangen, too.

Signed-off-by: Paul Spooren <[email protected]>
}

#[test]
fn flatten_recursive() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add these tests like the other ones, in a commit before the feature work

Comment on lines +116 to +136
#[test]
fn flatten_help_true() {
let name = "my-app";
let cmd = common::basic_command(name).flatten_help(true);
common::assert_matches(snapbox::file!["../snapshots/flatten_help.roff"], cmd);
}

#[test]
fn flatten_help_true_subcommand_required_true() {
let name = "my-app";
let cmd = common::basic_command(name)
.flatten_help(true)
.subcommand_required(true);
common::assert_matches(
snapbox::file!["../snapshots/flatten_help_subcommand_required.roff"],
cmd,
);
}

#[test]
fn flatten_help_true_subcommand_args_conflicts_with_subcommands() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these tests still needed with the addition of the --help tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants