FabioAntunes / fish-nvm

nvm wrapper for fish-shell
MIT License
511 stars 20 forks source link

type: Invalid combination of options #55

Closed torbjornvatn closed 3 years ago

torbjornvatn commented 3 years ago

In version 3.2.0 of fish type -fqP fails with the error type: Invalid combination of options

https://github.com/FabioAntunes/fish-nvm/blob/b532e737a620c54e2f47d28130819c0ab51e53e0/functions/__nvm_run.fish#L21

lowgos commented 3 years ago

This broke my environment too. Had this error on every node related command. Had to downgrade to fish 3.1.2.

faho commented 3 years ago

From the type documentation, going back to fish 2.7:

The -q, -p, -t and -P flags (and their long flag aliases) are mutually exclusive. Only one can be specified at a time.

-q asks to print no output, -p asks to print the path. That doesn't make sense, but the old type function had a bug that ignored it.

This should probably just be type -fq $argv[1] or probably command -sq $argv[1]- you are interested in checking a command if I understand correctly. The latter has the nice side effect of being faster in fish < 3.2.

nathan-gilbert commented 3 years ago

Addressed in #54

FabioAntunes commented 3 years ago

Apologies for the delay, for some reason I hadn't notifications, turned on! Thanks for the fix @nathan-gilbert

FabioAntunes commented 3 years ago

@faho I wrote this a long time ago, but I think the use case was:

-P or --force-path returns the path to the executable file NAME, presuming NAME is found in $PATH, or nothing otherwise. --force-path explicitly resolves only the path to executable files in $PATH, regardless of whether $NAME is shadowed by a function or builtin with the same name.

Because some names are shadowed, but I'm not interested in the resulting Path just making sure that I'm checking the command exists.

So my guess is that the fix should have been:

type -fP $argv[1] > /dev/null 2>&1?

Also is there a difference between type -P and type -fP reading the docs it seems to me that -P will ignore functions anyway?

faho commented 3 years ago

So my guess is that the fix should have been:

@FabioAntunes It should not. type -f already only checks commands, the -P only causes it to print the path. (yes, that "-f" is called "--no-functions" is unfortunate, honestly you want command -sq or in 3.2 just command -q)

It should be type -fq or command -sq.

Also is there a difference between type -P and type -fP reading the docs it seems to me that -P will ignore functions anyway?

type -p will print nothing if the first thing it finds is not a command. type -P will go on until it finds a command.

See e.g. type -p test (which will print nothing because test is a builtin) and type -P test (which will see the /bin/test binary and ignore the builtin).

fogelfish commented 3 years ago

Since this is closed does that mean an update is just around the corner? I'm asking because I'd like to decide if I should downgrade fish? Or wait X days for fish-nvm to be patched? Or do you know if there another temp workaround? Fish shell, nvm, is telling me that node and npm are “currently not installed” but they are

faho commented 3 years ago

@fogelfish https://github.com/FabioAntunes/fish-nvm/releases/tag/1.4.2