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

port CLI from argh to bpaf #185

Merged
merged 13 commits into from
Jan 30, 2025
Merged

port CLI from argh to bpaf #185

merged 13 commits into from
Jan 30, 2025

Conversation

untitaker
Copy link
Owner

@untitaker untitaker commented Jan 30, 2025

context: #184 #176

fixes #184

this is a work-in-progress port from argh to bpaf. I am not convinced
bpaf is a good solution here.

First, I don't see an obvious way to have optional subcommands and
global options. For example:

 hyperlink -j 4 dump-paragraphs -f foo.md

...should parse the same -j as:

 hyperlink -j 4 ./public/

in argh this worked fine (even though dump-paragraphs does not use -j
right now), but in bpaf I didn't find a way to write subcommands without
making them mutually exclusive with global options.

additionally, a main reason I would like to move away from argh is its
poor handling of help texts compared to clap. The helptext for
dump-paragraphs currently looks like this:

$ ./target/release/hyperlink dump-paragraphs --help
Usage: hyperlink dump-paragraphs --file <file>

Dump out internal data for markdown or html file. This is mostly useful to figure out why a source file is not properly matched up with its target html file. NOTE: This is a tool for debugging and development. Usage:    vimdiff <(hyperlink dump-paragraphs src/foo.md) <(hyperlink dump-paragraphs public/foo.html) Each line on the left represents a Markdown paragraph. Each line on the right represents a HTML paragraph. If there are minor formatting differences in two lines that are supposed to match, you found the issue that needs fixing in `src/paragraph.rs`. There may also be entire lines missing from either side, in which case the logic for detecting paragraphs needs adjustment, either in `src/markdown.rs` or `src/html.rs`. Note that the output for HTML omits paragraphs that do not have links, while for Markdown all paragraphs are dumped.

Options:
  --file            markdown or html file
  --help            display usage information

This, from argh, is a mess, but bpaf is worse:

$ ./target/debug/hyperlink dump-paragraphs --help
Dump out internal data for markdown or html file.

Usage: hyperlink dump-paragraphs --file=ARG

Available options:
        --file=ARG  markdown or html file
    -h, --help      Prints help information

Where did all that content go? It seems bpaf just drops all data after
the first paragraph. I looked at the docs and found that bpaf has some
special handling for two successive blank lines, but I don't have those
in my code (only single blank line).

Overall I found the API surface overwhelming and confusing. I can mix
and match the derive and combinators API, and that led me very far
astray when I couldn't figure out easily how to make optional
subcommands work. I still don't really know how to make it work exactly
the same as in argh/clap.

this is a work-in-progress port from argh to bpaf. I am not convinced
bpaf is a good solution here.

First, I don't see an obvious way to have optional subcommands and
global options. For example:

     hyperlink -j 4 dump-paragraphs -f foo.md

...should parse the same -j as:

     hyperlink -j 4 ./public/

in argh this worked fine (even though dump-paragraphs does not use -j
right now), but in bpaf I didn't find a way to write subcommands without
making them mutually exclusive with global options.

additionally, a main reason I would like to move away from argh is its
poor handling of help texts compared to clap. The helptext for
dump-paragraphs currently looks like this:

    $ ./target/release/hyperlink dump-paragraphs --help
    Usage: hyperlink dump-paragraphs --file <file>

    Dump out internal data for markdown or html file. This is mostly useful to figure out why a source file is not properly matched up with its target html file. NOTE: This is a tool for debugging and development. Usage:    vimdiff <(hyperlink dump-paragraphs src/foo.md) <(hyperlink dump-paragraphs public/foo.html) Each line on the left represents a Markdown paragraph. Each line on the right represents a HTML paragraph. If there are minor formatting differences in two lines that are supposed to match, you found the issue that needs fixing in `src/paragraph.rs`. There may also be entire lines missing from either side, in which case the logic for detecting paragraphs needs adjustment, either in `src/markdown.rs` or `src/html.rs`. Note that the output for HTML omits paragraphs that do not have links, while for Markdown all paragraphs are dumped.

    Options:
      --file            markdown or html file
      --help            display usage information

This, from argh, is a mess, but bpaf is worse:

    $ ./target/debug/hyperlink dump-paragraphs --help
    Dump out internal data for markdown or html file.

    Usage: hyperlink dump-paragraphs --file=ARG

    Available options:
            --file=ARG  markdown or html file
        -h, --help      Prints help information

Where did all that content go? It seems bpaf just drops all data after
the first paragraph. I looked at the docs and found that bpaf has some
special handling for two successive blank lines, but I don't have those
in my code (only single blank line).

Overall I found the API surface overwhelming and confusing. I can mix
and match the derive and combinators API, and that led me very far
astray when I couldn't figure out easily how to make optional
subcommands work. I still don't really know how to make it work exactly
the same as in argh/clap.
@pacak
Copy link

pacak commented Jan 30, 2025

First, I don't see an obvious way to have optional subcommands and
global options.

Normally you'd define a parser for a subcommand as if it was a required then add it to the top level with a combination of external and optional

#[derive(Bpaf, Clone, Debug)]
#[bpaf(command)]
struct Sub {
    /// does something
    opt: bool,
}

#[derive(Bpaf, Clone, Debug)]
struct Options {
    some: bool,
    global: usize,
    options: PathBuf,
    #[bpaf(external, optional)]
    sub: Option<Sub>
}

Where did all that content go? It seems bpaf just drops all data after
the first paragraph. I looked at the docs and found that bpaf has some
special handling for two successive blank lines, but I don't have those
in my code (only single blank line).

Two successive blank lines separate "header", "description" and "footer" parts of the top level option parser. For all other help bits a single blank line separates "brief help" from "detailed help" - you can pass --help --help or -hh to see the rest of it

@pacak
Copy link

pacak commented Jan 30, 2025

Overall I found the API surface overwhelming and confusing.

Sorry about that, writing documentation is hard :)

It's all about some simple parsers created with flag, positional, argument that you can combine into struct or enum using construct! macro or derive mechanism. Parser trait allows you to chain transformations on simple parsers - similar to iterators. Until eventually you declare the parser you like to be your top level parser with #[bpaf(options)]. Any top level parser can become a subcommand - back to a regular parser...

@pacak
Copy link

pacak commented Jan 30, 2025

Anyway, if you have questions - I'm happy to help.

@untitaker
Copy link
Owner Author

untitaker commented Jan 30, 2025

Thank you for directly engaging with this feedback!

Another thing i found is that bpaf derive does not have good error messages. I get things like:

    #[bpaf(external, optional)]
    subcommand: Option<Subcommand>

error[E0599]: no method named `optional` found for struct `OptionParser` in the current scope
  --> src/main.rs:29:10
   |
29 | #[derive(Bpaf, PartialEq, Debug)]
   |          ^^^^ method not found in `OptionParser<Subcommand>`
   |

Here the issue was that the Subcommand enum actually did not have the right annotation (bpaf(options) vs bpaf(command)), but the error was not attached to the right struct nor did it directly point at the right line.


I now updated the CLI to use your suggested structure for optional subcommands. However, this does not actually work as I also have an optional base-path argument. The help looks like this:

$ ./target/debug/hyperlink dump-external-links --help
Usage: hyperlink [-j=ARG] [--check-anchors] [--sources=ARG] [--github-actions] [-V] [BASE-PATH] [
COMMAND ...]

A command-line tool to find broken links in your static site.
    -j, --jobs=ARG        how many threads to use, default is to try and saturate CPU
        --check-anchors   whether to check for valid anchor references
        --sources=ARG     path to directory of markdown files to use for reporting errors
        --github-actions  enable specialized output for GitHub actions
    -V, --version         print version information and exit
    BASE-PATH             the static file path to check
    subcommand

Available options:
    -h, --help            Prints help information

There's a few issues:

  • options are shown above "Available options"
  • subcommands are not enumerated
  • running hyperlink dump-external-links does not actually work, as it is interpreted as basepath

Two successive blank lines separate "header", "description" and "footer" parts of the top level option parser. For all other help bits a single blank line separates "brief help" from "detailed help" - you can pass --help --help or -hh to see the rest of it

this is very unexpected. is there a way to turn this off? I want to show one paragraph for each subcommand on hyperlink --help, but hyperlink <subcommand> --help should show the entire text as there is plenty of space, or otherwise allow me to write more than one paragraph there.

I couldn't find any documentation for this behavior, and even if bpaf did document it, I think my users won't expect it. I personally don't know of any CLI that handles --help --help differently from --help

Sorry about that, writing documentation is hard :)

I do think the architecture makes sense, but for getting started I would prefer a focus on realistic, large examples instead of three separate tutorials: one how to parse an option, one how to parse a subcommand, and then one generic tutorial that tells you hwo to combine arbitrary parsers. While this shows the composability really well it also requires me to put the pieces together myself. One tutorial that goes step-by-step on how to build something really complex like a clone of the git CLI (or the cargo CLI or even the npm CLI) would help here.

@untitaker
Copy link
Owner Author

I went with a suggestion from @Dav1dde to use a wrapper struct around my command enum, and it works fine (and the helptext seems correct)

@untitaker
Copy link
Owner Author

this helptext is ok but still formatted in an unfortunate way:

A command-line tool to find broken links in your static site.

Usage: [-V] [-j=ARG] (COMMAND ... | [--check-anchors] [--sources=ARG] [--github-actions] [BASE-PATH
])

(the line-break is there)

@pacak
Copy link

pacak commented Jan 30, 2025

but the error was not attached to the right struct nor did it directly point at the right line.

Hmm... I guess I can improve this error with [diagnostic::on_unimplemented]

this is very unexpected. is there a way to turn this off?

Not at the moment. Might be easier in upcoming 0.10. I'd skip those blank lines and force linebreaks in places you want with a single space

/// this is one line
///  this should go on a separate line

I couldn't find any documentation for this behavior, and even if bpaf did document it, I think my users won't expect it.

Should be included in any .help() method documentation https://docs.rs/bpaf/0.9.16/bpaf/params/struct.NamedArg.html#method.help but I get it - not the the first place you'd look.

One tutorial that goes step-by-step on how to build something really complex like a clone of the git CLI (or the cargo CLI or even the npm CLI) would help here.

Will see what I can do. There's a bunch of stuff in /examples - did you had a chance to look there?

this helptext is ok but still formatted in an unfortunate way:
...
(the line-break is there)

It tries to fit everything within a fixed width and there's no fancy logic about line breaking. There's a ticket about it though, but I'm not sure how to do it yet :) You can hide some items from the generated usage line (hide_usage will do that, items will still be around in help) or add some short aliases - say -C for --check-anchors - it prefers short names by default.

@untitaker
Copy link
Owner Author

the link to the help() method contained sufficient information to make this work. I am a bit uncomfortable contorting the sourcecode to get multi-paragraph help by default, but it works. ty!

@untitaker untitaker marked this pull request as ready for review January 30, 2025 18:06
@pacak
Copy link

pacak commented Jan 30, 2025

   // TODO: use bpaf-native version option
    /// print version information and exit

This should be just #[bpaf(options, version)] I think - it will take version from cargo

@untitaker
Copy link
Owner Author

I found that the output is not what I want (hyperlink <version> vs Version: <version>), I'm ok with that code though.

@untitaker untitaker changed the title port from argh to bpaf port CLI from argh to bpaf Jan 30, 2025
@untitaker untitaker merged commit 6fb163d into main Jan 30, 2025
6 checks passed
@untitaker untitaker deleted the bpaf branch January 30, 2025 18:15
@pacak
Copy link

pacak commented Jan 30, 2025

I found that the output is not what I want (hyperlink vs Version: ), I'm ok with that code though.

Interesting problem, never thought about it this way. Made pacak/bpaf#409 - should be doable in 0.10 once it's done.

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.

Help output is missing newlines
2 participants