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

feat(completion): add option to create default command alone #1392

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

scop
Copy link
Contributor

@scop scop commented May 6, 2021

The current implementation of the default completion (and help) commands makes them available only when there are some other commands defined. This rules them out for apps that have no other subcommands, either by design anew, or for historical behavior, even though they would benefit from completions both in the first place, and the convenience of having the default generated.

This adds a new completion option for having the default init happen in those cases. Not that clean in the first place and the variable name is a mouthful, but the functionality would be great to have.

Untested, mainly for discussion purposes, but in absence of something better, I'd be fine with something like this.

@marckhouzam
Copy link
Collaborator

Thanks for the PR @scop. I now realize that there is value in a completion command even for tools that don't have subcommands. It would allow flags to be completed, as well as allowing for custom completions of arguments and flag values.

However I am not sure of the right approach for the best user experience. Allowing the creation of the completion command changes the help text because sub-commands now exist. There may be other impacts to the tool when creating a first sub-commands, I'm not sure.

And completion itself isn't as smooth as usual because when doing completion of arguments, the completion and help commands will also be suggested, which are not "arguments", so may cause confusion.

But I agree that it probably has more value to have the completion command than those small disadvantages. And with your proposal, it is up to the developper to decide to enable the completion command, so they can evaluate the impacts.

I just wanted to continue your discussion and see if others had suggestions about this.

@github-actions
Copy link

This PR is being marked as stale due to a long period of inactivity

@marckhouzam
Copy link
Collaborator

Still valid

@scop
Copy link
Contributor Author

scop commented Jul 10, 2021

Sorry about the delay in following up. I agree with the concerns flagged by @marckhouzam, and made an alternative implementation in #1450 that doesn't have those problems. It's not that nice that the completion command is completely hidden, but better than the alternatives I believe.

@scop
Copy link
Contributor Author

scop commented Jul 10, 2021

A middle ground could be worth considering as well: let the default completion generate the help section in the usage output, but avoid them being offered as completions. Guess that would be somewhat more code though.

@fabstu
Copy link

fabstu commented Jul 16, 2021

We could also introduce a command like ForceCreationDespiteLoneCommand along the lines of DisableDefaultCmd. For me it was counter-intuitive that the completion command wasn't available and that not being configurable using a completion option.

@marckhouzam
Copy link
Collaborator

I believe this has been replaced by #1450

@scop
Copy link
Contributor Author

scop commented Sep 25, 2021

As mentioned in a couple of comments here and in #1450, #1450 is an alternative implementation. It will replace this one if the approach in it is chosen, and vice versa, depends on the decision which one is replaced by which.

@scop scop force-pushed the feat/default-completion-cmd-alone branch from 548b27b to be2d93c Compare October 23, 2021 07:24
@scop
Copy link
Contributor Author

scop commented Apr 8, 2024

Just to drop an explicit note here, #1559 is s superseding implementation of both this and #1450.

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

Successfully merging this pull request may close these issues.

3 participants