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

Support for multicall binaries #1120

Closed
lucab opened this issue Nov 30, 2017 · 3 comments · Fixed by #2817
Closed

Support for multicall binaries #1120

lucab opened this issue Nov 30, 2017 · 3 comments · Fixed by #2817
Labels
E-medium Call for participation: Experience needed to fix: Medium / intermediate
Milestone

Comments

@lucab
Copy link
Contributor

lucab commented Nov 30, 2017

I did a bit of digging in the ticket tracker, but I didn't see this topic covered before. In short, I would like to explore how to support multicall binaries in clap.

Multicall binaries share a single codebase, but different codepaths can be activated based on basename(argv[0]). A good example to target would be the hostname util: https://sources.debian.net/src/hostname/3.18/hostname.c/#L6-L12.

What I would like to be able to do with clap is:

  • detect which flavor of a multicall binary has been called, with optional missing & catchall cases
  • define arguments which are specific to some flavor of a multicall binary
  • define arguments which are specific to all flavors of a multicall binary

All of this in order in order to use normal clap features transparently in a multicall binary. This can be already done with a bit of boilerplate (here a very dirty example), but if possible I'd like to see this directly supported in clap as I've found myself repeating this logic in several places lately.

I'll be happy to try contribute code/test/review for this, but it will need design inputs from @kbknapp first.

PS: thanks for maintaining clap, it is a great library!

@kbknapp
Copy link
Member

kbknapp commented Dec 2, 2017

Thanks for the suggestion!

This could be done in two ways, either an external crate that uses clap internally and just matches argv[0] and calls the appropriate clap::App or a new AppSettings::Multicall variant.

If you'd like to test the AppSettings way here's how you could do it:

The basic premise would be argv[0] would be matched in src/app/mod.rs:get_matches_from_safe_borrow first. If it matches self.p.meta.name (i.e. what's given to App::new), everything functions exactly like it already does in clap (i.e. all the "applets" are just subcommands).

However, if it doesn't match, you could check that it matches a subcommand name and set self.p.meta.bin_name and self.p.meta.name to a blank string and simply continue parsing like normal because clap will simply treat it as a subcommand. The reason you set self.p.meta.bin_name is to that all usage strings and error messages say applet [options] and not q applet [options].

Here's how you can add those specifics:

  • Add a new variant to src/app/settings.rs just following the examples set by all the other variants
  • Add any tests to the tests/app_settings.rs file
    • For testing purposes you can run cargo test --test app_settings so you don't need to run the full test suite while developing
    • All tests can be run by calling App::get_matches_from(vec![]) and just ensuring the first item in the vec is your "applet name" to see if it all works correct
  • I would also add an example to examples/ since this is kind of a bigger and strange use case
  • I would also include a help output test that can put in tests/help.rs following the examples in there to make sure when you call q applet --foo the help looks correct, as well as when you call applet --foo

At the end I envision this working:

let matches = App::new("q")
    .setting(AppSettings::Multicall)
    .subcommand(SubCommand::with_name("foo")
        .arg(Arg::from_usage("-f, --foo-flag 'some foo flag'")))
    .subcommand(SubCommand::with_name("bar")
        .arg(Arg::from_usage("-b, --bar-flag 'some bar flag'")))
    .arg(Arg::from_usage("-q, --q-flag 'some q flag'"))
    .get_matches();

And being able to work with any of these

$ q --q-flag
$ foo --foo-flag
$ bar --bar-flag
$ q foo --foo-flag
$ q bar --bar-flag

@kbknapp kbknapp added C: settings E-medium Call for participation: Experience needed to fix: Medium / intermediate labels Dec 2, 2017
@kbknapp
Copy link
Member

kbknapp commented Dec 2, 2017

Note this also somewhat similar to #975

@lucab
Copy link
Contributor Author

lucab commented Dec 5, 2017

Thanks for the feedback, it was more detailed than expected! I started code-wrangling based on your hints and I have a few more questions.

a new AppSettings::Multicall variant.

This seems to be the busybox-style applets approach, where busybox foo and foo are both fine and equivalent. I've called it MulticallApplets for the moment and I think it makes sense to implement as you suggested. However my multicall usecases are more similar to the hostname one above, where hostname and dnsdomainname are fine but hostname dnsdomainname is not. For that one, do you envision a similar AppSetting? How should it work, perhaps always restricting matches to subcommands only? Or should this be an external create instead?

and set self.p.meta.bin_name and self.p.meta.name to a blank string and simply continue parsing like normal because clap will simply treat it as a subcommand.

I'm having some difficulties following how this would work. First, I assume you mean self.p.meta.bin_name = Some(arg0.clone()); self.p.meta.name = "".to_owned(); in order to clear the top-level name. But then, how would the parser proceed to the subcommand? A quick test doesn't seem to proceed that way, and I would expect needing to forward-call subcmd.p.get_matches_with() myself (instead of current self). In that case I may have to reconcile some mutability mismatches, but before venturing there I wanted to double-check my understandings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-medium Call for participation: Experience needed to fix: Medium / intermediate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants