Skip to content
This repository has been archived by the owner on Jan 1, 2022. It is now read-only.

Support automatic subcommands for completion #68

Open
epage opened this issue Dec 6, 2021 · 19 comments
Open

Support automatic subcommands for completion #68

epage opened this issue Dec 6, 2021 · 19 comments

Comments

@epage
Copy link
Owner

epage commented Dec 6, 2021

Issue by colemickens
Saturday Jan 07, 2017 at 23:48 GMT
Originally opened as clap-rs/clap#810


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.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by kbknapp
Sunday Jan 08, 2017 at 03:14 GMT


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.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by colemickens
Sunday Jan 08, 2017 at 04:26 GMT


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.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by kbknapp
Sunday Jan 08, 2017 at 05:11 GMT


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>

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by rharriso
Thursday Jul 04, 2019 at 22:06 GMT


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

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by pksunkara
Friday Feb 14, 2020 at 14:42 GMT


@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.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by pksunkara
Tuesday Mar 03, 2020 at 12:45 GMT


@CreepySkeleton Would love your input here.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by CreepySkeleton
Tuesday Mar 03, 2020 at 15:43 GMT


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 🤷‍♂ .

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by TeXitoi
Tuesday Mar 03, 2020 at 15:47 GMT


@CreepySkeleton You mean clap_generate instead of clap_derive ?

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by pksunkara
Tuesday Mar 03, 2020 at 15:51 GMT


There must have been a reason Kevin wanted to make it a separate crate.

On Tue, Mar 3, 2020, 16:47 Guillaume P. [email protected] wrote:

@CreepySkeleton https://github.com/CreepySkeleton You mean clap_generate
instead of clap_derive ?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
clap-rs/clap#810,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABKU3ZGNHAF4PG3EOK2DJ3RFURBVANCNFSM4C3WHVVA
.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by CreepySkeleton
Tuesday Mar 03, 2020 at 15:53 GMT


@TeXitoi right 😨

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by CreepySkeleton
Tuesday Mar 03, 2020 at 15:54 GMT


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

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by TeXitoi
Tuesday Mar 03, 2020 at 16:47 GMT


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.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by pksunkara
Tuesday Mar 03, 2020 at 16:55 GMT


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.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by CreepySkeleton
Tuesday Mar 03, 2020 at 17:00 GMT


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?

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by CreepySkeleton
Tuesday Mar 03, 2020 at 17:05 GMT


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.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by CreepySkeleton
Tuesday Mar 03, 2020 at 17:24 GMT


@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

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by CreepySkeleton
Wednesday Jul 01, 2020 at 02:27 GMT


@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.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by pksunkara
Wednesday Jul 01, 2020 at 08:15 GMT


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

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by epage
Wednesday Jul 21, 2021 at 19:58 GMT


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.

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

No branches or pull requests

1 participant