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

Completion for conda, the package manager #4837

Merged
merged 8 commits into from Mar 26, 2018
Merged

Conversation

@luc-j-bourhis
Copy link
Contributor

@luc-j-bourhis luc-j-bourhis commented Mar 19, 2018

I propose completion for the package manager conda. Since fish ships with support for completing brew, pacaur, pip, etc, I think it is appropriate to also include the rather popular conda.

# Complete using -n to select the given conda subcommand
# and passing the rest of the arguments to `complete`
# The goal here is to reduce clutter in the definitions below
# This function will be deleted at the end
Copy link
Member

@faho faho Mar 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's probably not fantastic since the name is so generic.

If the user has a function with that name, you'll first redefine and then delete it.

Loading

Copy link
Member

@faho faho Mar 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call it __conda or something.

Loading

Copy link
Contributor Author

@luc-j-bourhis luc-j-bourhis Mar 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed by 4b45243

Loading


# Option name
for cmd in create install list remove search update
if test $cmd = create
Copy link
Member

@faho faho Mar 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move create out of the for-loop since there's no shared part.

Loading

Copy link
Contributor Author

@luc-j-bourhis luc-j-bourhis Mar 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed by 64d5b94

Loading

function __fish_conda_needs_command -a cmd
# We need to filter out -x and --xx options so as to complete
# the arguments of said options
test (string match -r '^\w+' -- (commandline -opc))[-1] = $cmd
Copy link
Member

@faho faho Mar 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be really careful with test - it has some surprisingly sharp edges. This'll fail when the command substitution returns nothing, and it effectively does the same thing __fish_use_subcommand does.

We do have __fish_seen_subcommand_from, which just tests if a given word has been given. Usually I'll do not __fish_seen_subcommand_from $allcmds (where $allcmds contains all the subcommands that the command accepts).

That's still not perfect, because it'll see something like git -C log as using the "log" subcommand instead of the argument to the "-C" option. To do it properly, you need to actually know which options accept arguments - see how the git or systemctl completions do it - kinda complicated, right?

Loading

Copy link
Contributor Author

@luc-j-bourhis luc-j-bourhis Mar 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed by 10ff253

Loading

@faho faho added this to the fish-3.0 milestone Mar 25, 2018
faho
faho approved these changes Mar 26, 2018
@faho faho merged commit c116843 into fish-shell:master Mar 26, 2018
1 check passed
Loading
@faho
Copy link
Member

@faho faho commented Mar 26, 2018

Nice, merged!

Loading

@luc-j-bourhis luc-j-bourhis deleted the conda branch Mar 26, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants