-
Notifications
You must be signed in to change notification settings - Fork 353
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
cli: completion: Add support for Nushell completions #3073
Conversation
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.
LGTM, could you please adhere to our commit style too, like cli:
and lib:
?
And it seems someone forgot to update the snapshot of the cli-reference. |
c90888b
to
febb8dc
Compare
@PhilipMetzger done to both ! |
Note that in #2353 this was also attempted, but due to a bug in |
fee6052
to
4851997
Compare
Yes, and it's been released with 4.5.1, so this PR is now ready. Thanks for pointing out the existing issues and PRs :) |
Can you add a Closes #2323 to the commit message or the PR description? Then it should be good to land. |
Yes done |
4851997
to
71df079
Compare
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.
lgtm 👍🏻
71df079
to
201446c
Compare
Rebased on top of #3062 so I can finally merge them sequentially without them interfering with each other in the changelog |
201446c
to
2e21067
Compare
Incidentally, I also fixed outdated docs in the command help for other shells (still using
--<shell>
instead of the positional form)Checklist
If applicable:
CHANGELOG.md
Dependencies
Needs fix(clap_complete_nushell): correctly handle commands whose short description is more than one line clap-rs/clap#5359 to work
Needs completion: Update docs for new style with positional argument #3074
Closes cli: add nushell completion support #2353
Closes FR: nushell completion support #2323