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 automatic subcommands for completion #810

Open
colemickens opened this issue Jan 7, 2017 · 19 comments
Open

Support automatic subcommands for completion #810

colemickens opened this issue Jan 7, 2017 · 19 comments
Labels
A-completion Area: completion generator C-enhancement Category: Raise on the bar on expectations S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing

Comments

@colemickens
Copy link

It seems to be something of a pattern for apps to support completion via the following:

$ source <(app completion zsh)
# or
$ source <(app completion bash)

It'd be nice if there were a helper on App so that we could call .add_completion_subcommands() and have this wired up automatically.

@kbknapp
Copy link
Member

kbknapp commented Jan 8, 2017

This would be an easy addition, although it'd probably get implemented as an AppSettings variant.

The one thing that gives me pause, is that I've seen this done in a few different ways by different applications, such as $ prog completion[s] [--shell] <shell>.

Is the prog completion <shell> (singular form of "completion") a standard? Because if it were only up to me, I'd implement it as prog completions <shell> (plural form of "completion" with a single positional argument accepting the shell name), but if there's a written standard I haven't seen I'd rather follow the standard.

@colemickens
Copy link
Author

I don't know if one is more popular/"standard" than another. I thought I had other examples up my sleeve, but the only one I can think of now is kubectl which takes the form kubectl completion zsh. I have only a tiny preference for the subcommand style, rather than flag, but it seems to just be cosmetic.

@kbknapp
Copy link
Member

kbknapp commented Jan 8, 2017

In all honesty it'd be possible to give options for both, which is how I think I'll tackle this.

  • AppSettings::GenerateCompletionsSubcommand
    • Will generate prog completions <shell>
  • AppSettings::GenerateCompletionsArg
    • Will generate prog --shell-completions <shell>

@kbknapp kbknapp added this to the 2.21.0 milestone Jan 30, 2017
@kbknapp kbknapp removed this from the 2.21.0 milestone Mar 27, 2017
@kbknapp kbknapp mentioned this issue Feb 15, 2018
87 tasks
@kbknapp kbknapp added this to the v3.0 milestone Jul 22, 2018
@rharriso
Copy link
Contributor

rharriso commented Jul 4, 2019

Is this possible anymore?

The clap_generate package depends on clap. Won't this be a circular package dependency?
Also. I'm having trouble using clap_generate

working in this branch:
https://github.com/rharriso/clap/tree/v3-master

@pksunkara
Copy link
Member

pksunkara commented Feb 14, 2020

@rharriso Try the clap_generate now, it should work. But yeah, this would become a circular dependency. I am not sure we want this.

Maybe we could do some kind of macro.

@pksunkara
Copy link
Member

@CreepySkeleton Would love your input here.

@CreepySkeleton
Copy link
Contributor

CreepySkeleton commented Mar 3, 2020

That's possible. We can make clap_generate just another module in clap (i.e it would be no longer a separate crate), possibly behind a feature flag.

Pros:

  • Very convenient for end users.

  • We will no longer need the #[doc(hidden)] pub things.

    Let me catch your attention here: currently, a lot of stuff in clap is doc(hidden) precisely and only because clap_generate needs it. It's much more that you might have thought; many things (structs and enums) are public only because they are parts of such a "public" field, or a part of some function's signature (also pub hidden one). The point is: this pseudo-publicity expands onto other things transitively, pulling them into the club even though they have no need to be publich.

    In my estimation, about 30% of code is pub hidden now, and those erase the line between "internal API used by clap_generate" and really "public API". Once/if we embrace the generate into the clap itself, we'll be able to change them to pub(crate), as it should be. A big improvement if you ask me.

Cons:
* clap's own codebase would grow a bit. Well, we're the ones who's gonna have to maintain it one way or another 🤷‍♂ .

@TeXitoi
Copy link
Contributor

TeXitoi commented Mar 3, 2020

@CreepySkeleton You mean clap_generate instead of clap_derive ?

@pksunkara
Copy link
Member

pksunkara commented Mar 3, 2020 via email

@CreepySkeleton
Copy link
Contributor

@TeXitoi right 😨

@CreepySkeleton
Copy link
Contributor

I don't see any reason, honestly. Not any I would call a good one.

@TeXitoi
Copy link
Contributor

TeXitoi commented Mar 3, 2020

The advantage is that you will not compile it if you don't use it. The other solution is features, and I personally prefer to keep features to the bare minimum as they complexify the code considerably.

But, having this implementation hole is quite dirty. I think the clean way would be:

  • having the introspection API used by clap_generate public and documented
  • having clap_generate in the clap crate

The second way seems easier now. But that's just my humble opinion, I didn't even seen the code.

@pksunkara
Copy link
Member

I made as much of introspection API public. The remaning stuff are doc(hidden) because they are macro helpers for the introspection API. If the whole system isn't macros, they would have been private.

@CreepySkeleton
Copy link
Contributor

CreepySkeleton commented Mar 3, 2020

The other solution is features, and I personally prefer to keep features to the bare minimum as they complexify the code considerably.

That's gonna be one module level cfg and one another cfg somewhere in AppSettings. Sounds simple enough.

I've seen the code, I've been trying to implement an introspection API of sorts (you just read my mind 😃, ha!), and I feel like I'm failing. calp_generate and clap are so closely tied to each other that we may as well make them all public. I'll show you the result of this attempt soon, but I vote for pulling it into clap.

I made as much of introspection API public.

You did? Mind elaborate?

@CreepySkeleton
Copy link
Contributor

The remaning stuff are doc(hidden) because they are macro helpers for the introspection API. If the whole system isn't macros, they would have been private.

I hate to be rude, but that's actually not true at all. Just grep for #[doc(hidden)]: 140 matches across 17 files. Ridiculous.

@CreepySkeleton
Copy link
Contributor

CreepySkeleton commented Mar 3, 2020

@TeXitoi They seem to be implementing explicit support for features in docs.rs, see dtolnay/syn#734 . We might as well do that too.

Looks neat unless you have lots of cfg'ed items (we don't): https://docs.rs/tokio/0.2.13/tokio/index.html#modules

@pksunkara pksunkara added A-completion Area: completion generator and removed C: settings labels Apr 10, 2020
@CreepySkeleton
Copy link
Contributor

CreepySkeleton commented Jul 1, 2020

@pksunkara @kbknapp So can we finally decide whether we want this or not? More specifically, are we open to inclusion clap_generate as a clap module? If we aren't, than we can close this as impossible.

@pksunkara pksunkara self-assigned this Jul 1, 2020
@pksunkara
Copy link
Member

I will take care of this. I have a few ideas on how to fix this.

@epage
Copy link
Member

epage commented Jul 21, 2021

btw another way of solving this is instead of having a setting for this, to provide types that implement Args and Subcommand (through derive or not). A user could then choose to flatten those into their arguments.

@pksunkara pksunkara removed the W: 3.x label Aug 13, 2021
@epage epage modified the milestones: 3.0, 3.1 Oct 16, 2021
@epage epage added C-enhancement Category: Raise on the bar on expectations S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing and removed T: new setting labels Dec 8, 2021
@epage epage removed this from the 3.1 milestone Dec 9, 2021
@pksunkara pksunkara removed their assignment Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-completion Area: completion generator C-enhancement Category: Raise on the bar on expectations S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing
Projects
None yet
Development

No branches or pull requests

7 participants