-
Notifications
You must be signed in to change notification settings - Fork 2k
Plugin: support custom completions in command flags #16859
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
Conversation
1ebff6b to
804f9fd
Compare
a3383bf to
9ec6250
Compare
f559820 to
4be8fc9
Compare
|
It would be nice to have some integration tests to make sure that it's actually working from nu-cli. Sorry, I have no idea of how to do it. You can ignore this if you find it difficult to implement. |
Sure, I added a |
Smart move, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me on the completions side, just some minor comments. Do you want someone else from the core team to review the plugin side?
Of course it's better if anyone is available, perhaps @devyn and @cptpiepmatz ? |
|
While reviewing this I wondered why this is even needed. We have custom completions for externals: https://www.nushell.sh/book/custom_completions.html#custom-completion-and-extern I don't understand why we cannot use that in some smart way. Is it not dynamic or what is the issue here? Also even if we go with the direction of this PR (I like the impl so far, so I'm not against this PR itself), should we merge these two ideas? I'm not sure if this was talked already in this PR in the comments somewhere, sorry if I overlooked that. |
The main issue is that it's hard to get the internal state of a plugin, it may take a lot of effort to expose commands to query internal state. |
|
I see the issue. But how are the other completers implemented? Both feel to me as they try to solve very similar issues. Should we move the impl for them into this one, when this one lands? |
|
Not really, it's mainly for plugins, the plugin authors can add completion easily for there commands once it's landed |
The way custom completions work right now is, we parse a custom command whose parameters refer to some completers, and then we store the After this PR, builtins as well as plugin commands can provide dynamic completions, and then we can try having shared logic for this, the static completions added by Bahex ( |
ysthakur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excited for this! Thanks for sticking with this @WindSoilder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually @cptpiepmatz made me thinking that maybe it could/should be done at the Parameter level, instead of Command. If we need to call the command during dynamic completion, we can always resort to find_decl.
Anyways, I don't want to keep this PR on-hold for too long. We still get time to redesign it before the next release.
|
Thanks for reviewing! I'm going to land it. Regards to this:
Let's keep the diff smaller, I can try to implement it in another pr. |
…letion results (#17006) First step of [this](#16859 (comment)), mainly to reuse the type-based completions for flag values, as reported in #16977 . And some complement test cases. @WindSoilder @ysthakur ## Release notes summary - What our users need to know Fixed a bug where path/glob/directory typed flags don't get completions. ## Tasks after submitting
Closes: #15766
It's similar to #16383, but allows to generate completion dynamically, which can be useful for plugin commands.
To implement this, I added a new
get_dynamic_completionmethod toCommandtrait, so nushell gets the ability to ask plugins for a completion. Here is how it goes when I typeplugin-cmd --flag1 <tab>.ArgValueDynamicCompletionArgValueDynamicCompletionit callsget_dynamic_completion(ArgType::Flag(flag1))on the command, which can be plugin command, and it will issue aget_dynamic_completion(ArgType::Flag(flag1))call to the pluginSome(Vec<DynamicSemanticSuggestion>)if there are available items.It also supports dynamic completion for positional arguments, here is how it goes when I type
plugin-cmd x<tab>.ArgValueDynamicCompletionArgValueDynamicCompletionit callsget_dynamic_completion(ArgType::Positional(0))on the command, which can be plugin command, and it will issue aget_dynamic_completion(ArgType::Positional(0))call to the pluginSome(Vec<DynamicSemanticSuggestion>)if there are available items.Release notes summary - What our users need to know
Plugin developer: auto completion is available for plugin command arguments.
User can define auto completions for plugin commands, here is the way to define this:
For the detailed example, you can refer to
get_dynamic_completionincrates/nu_plugin_example/src/commands/arg_completion.rs.Tasks after submitting