-
Notifications
You must be signed in to change notification settings - Fork 29
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(core): add shell_complete implementation for workflows and datasets #2512
Conversation
currently with this implementation there's a problem with dependency injection, namely:
|
other than that using |
39113b9
to
1d4022e
Compare
8749c7f
to
d444ec9
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.
This is a very cool feature! Thanks Viktor! I've used it and have the following comments:
- It prevents shell completion from working. For example, if I type
renku dataset add my-dataset <TAB>
to get a list of paths, nothing shows up; instead, it seems that Renku completions is called again. I believe disabling shell completions is not desirable. - Installation could be made easier: There is a
renku --install-completion
command that usesclick-completion
package to install the completion. Maybe you can check to see what it does and if it's useful here.
thanks for the feedback! see my answers below
good catch! this is indeed not good! i've just checked a bit into this. It's actually because of the following reason: the
would actually fix the problem. now of course this is semantically not what we want but in this specific case i think it solves our problem. because anyways there would be no sane way to autocomplete a proper url, like
oh i didn't know about this 🤦 i've just quickly checked and it's not really working for me, but i'll investigate and try to make the whole shell integration installation working with this command and add it to this patch |
d444ec9
to
b79b84e
Compare
b79b84e
to
5041e5a
Compare
@mohammad-sdsc so i've looked into the whole i'd do the same, but we could still keep the command that actually expands the shell's config file with the right command (see the how-to) if we wanna maintain that code-base 🙃 |
39bba61
to
c03490b
Compare
c03490b
to
f2644de
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.
Works great! Thank you!
Description
Using the feature of click's auto-complete implementation when
running various
renku dataset
sub-commands the actual dataset name can be auto-completedrunning various
renku workflow
sub-commands the actual workflow name can be auto-completedfix dependency injection
Mention in the documentation how to activate for bash, zsh or fish (basically the shells that are supported by click) auto-completion of renku commands