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

Please support user defined dynamic complete command #5293

Open
2 tasks done
liuhangbin opened this issue Jan 9, 2024 · 7 comments
Open
2 tasks done

Please support user defined dynamic complete command #5293

liuhangbin opened this issue Jan 9, 2024 · 7 comments
Labels
A-completion Area: completion generator C-enhancement Category: Raise on the bar on expectations E-medium Call for participation: Experience needed to fix: Medium / intermediate

Comments

@liuhangbin
Copy link

Please complete the following tasks

Clap Version

4.4.11

Describe your use case

Currently, the dynamic completion hides the parameter from the program's parameters and only shows with $ cmd complete -h, which is good. But it hard-codes the parameter with complete, which may conflict with or be similar to the program's existing parameter. This makes $ cmd c not work. It would be good if the user could define the complete parameter themself.

Describe the solution you'd like

Maybe add a new function fn set_comp_words in impl CompleteCommand. Then we can define the comp_words ourself.

Alternatives, if applicable

No response

Additional Context

No response

@liuhangbin liuhangbin added the C-enhancement Category: Raise on the bar on expectations label Jan 9, 2024
@epage
Copy link
Member

epage commented Jan 9, 2024

To clarify, you are wanting to replace the complete subcommand name, right?

That isn't too clear from the proposed solution as its unclear what comp_words is.

The clap maintainer also said that dynamic completion will replace static completion in future. So I chose to use dynamic completion, although it only supports bash and fish right now.

dynamic completions are also very early in their development so I also generally recommend people use static completions at this time but I encourage fixes and improvements on dynamic completions.

@liuhangbin
Copy link
Author

To clarify, you are wanting to replace the complete subcommand name, right?

Yes

That isn't too clear from the proposed solution as it's unclear what comp_words is.

In clap_complete/src/dynamic/shells/mod.rs it defines

/// Generally used via [`CompleteCommand`]
#[derive(clap::Args)]
#[command(arg_required_else_help = true)]
#[command(group = clap::ArgGroup::new("complete").multiple(true).conflicts_with("register"))]
#[allow(missing_docs)]
#[derive(Clone, Debug)]
pub struct CompleteArgs {
    /// Specify shell to complete for
    #[arg(long)]
    shell: Shell,

    /// Path to write completion-registration to
    #[arg(long, required = true)]
    register: Option<std::path::PathBuf>,

    #[arg(raw = true, hide_short_help = true, group = "complete")]
    comp_words: Vec<OsString>,
}

So I think adding a function in impl CompleteCommand to set the comp_words may fix this issue.

The clap maintainer also said that dynamic completion will replace static completion in future. So I chose to use dynamic completion, although it only supports bash and fish right now.

dynamic completions are also very early in their development so I also generally recommend people use static completions at this time but I encourage fixes and improvements on dynamic completions.

The dynamic completions could hide from the existing parameters, which is very useful. If we use the static completions and add a, e.g. generate parameter, we need to remove it if switch to dynamic completions.

@epage
Copy link
Member

epage commented Jan 10, 2024

So I think adding a function in impl CompleteCommand to set the comp_words may fix this issue.

Its not clear to me why someone would want to use this struct but override comp_words. comp_words is meant to come from the shell, not the calling program, to define what we should complete.

Are you trying to use CompleteCommand as a completion API-only and not for flattening into your CLI? If so, that isn't its role. The logic it implements is fairly light with most of the behavior coming from elsewhere.

The dynamic completions could hide from the existing parameters, which is very useful. If we use the static completions and add a, e.g. generate parameter, we need to remove it if switch to dynamic completions.

It sounds like the concern is with compatibility being broken with how you currently do completions? I can understand that. However, the current built-in interface isn't stable yet either. Its also likely that you could make a generate parameter used for static completions that will then work for dynamic completions. The shell support might be a little different but you can choose now for a subset of shells (fig is one that I know won't work with dynamic completions).

@liuhangbin
Copy link
Author

So I think adding a function in impl CompleteCommand to set the comp_words may fix this issue.

Its not clear to me why someone would want to use this struct but override comp_words. comp_words is meant to come from the shell, not the calling program, to define what we should complete.

It's just my guess. Not the way you should follow. And looks like I didn't read the code correctly. You can choose the best way to implement this feature.

Are you trying to use CompleteCommand as a completion API-only and not for flattening into your CLI? If so, that isn't its role. The logic it implements is fairly light with most of the behavior coming from elsewhere.

I use the way like dynamic example does.

The dynamic completions could hide from the existing parameters, which is very useful. If we use the static completions and add a, e.g. generate parameter, we need to remove it if switch to dynamic completions.

It sounds like the concern is with compatibility being broken with how you currently do completions? I can understand that. However, the current built-in interface isn't stable yet either. Its also likely that you could make a generate parameter used for static completions that will then work for dynamic completions. The shell support might be a little different but you can choose now for a subset of shells (fig is one that I know won't work with dynamic completions).

Yes, I tried the static way. What I am concerned about is that the generate completion is not related to the program functions. It's mainly used by the package maintainer.

So I like the idea that dynamic completion hides the complete parameter by default, which doesn't affect the normal program parameters.

@liuhangbin
Copy link
Author

So I like the idea that dynamic completion hides the complete parameter by default, which doesn't affect the normal program parameters.

OK, I just saw that I can use the .hide(true) for the static completion parameter.

@epage epage added E-medium Call for participation: Experience needed to fix: Medium / intermediate A-completion Area: completion generator labels Jan 11, 2024
@epage epage changed the title [clap_complete] Please support user defined dynamic complete command Please support user defined dynamic complete command Jan 11, 2024
@epage
Copy link
Member

epage commented Jan 11, 2024

Overall, anyone is able to define their own CompleteCommand. The core issue imo is that the registration script does not allow overriding it.

@liuhangbin
Copy link
Author

Overall, anyone is able to define their own CompleteCommand. The core issue imo is that the registration script does not allow overriding it.

Indeed.

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 E-medium Call for participation: Experience needed to fix: Medium / intermediate
Projects
None yet
Development

No branches or pull requests

2 participants