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

Fix for #22: stop error from test command #23

Closed
wants to merge 1 commit into from
Closed

Fix for #22: stop error from test command #23

wants to merge 1 commit into from

Conversation

tomsaleeba
Copy link
Contributor

Fixes #22.
I'm not sure if this is the behaviour you want but at least it gets rid of that error from test

…ther param is provided, change the variable name so it doesn't shadow a built in (command)
@brigand
Copy link
Owner

brigand commented May 10, 2018

Doesn't this throw from the argv access if there are no arguments?

@tomsaleeba
Copy link
Contributor Author

Yeah, it throws from here:

set -l command $argv[1]

A simple test case is this script:

#!/usr/bin/fish
echo 'before'
set -l c $argv[1]
if test $c = 'use'
  echo 'yep, use'
end
echo 'after'

Assume we save it as blah.fish and it explodes when you try to access that non-existant index of $argv, like you say:

$ fish blah.fish asdf
before
after
$ fish blah.fish
before
test: Missing argument at index 2
after

My goal is to stop that error message. Setting a default "command" has the added bonus of making the script more user friendly; you can run it with no args happily.

@brigand
Copy link
Owner

brigand commented May 11, 2018

You need to use (count $argv) and not read the item if it's 0.

@tomsaleeba
Copy link
Contributor Author

How about this approach: https://github.com/tomsaleeba/fast-nvm-fish/commit/26bef18115ef6aae190f6f491821081e58a7c99f#diff-5aa7c1aafb1d525d781f1c83ca10e0c2

It achieves the same goal as this PR but goes about it a bit differently. It does what you mentioned by branching based on the result of the count of the whole argv list, not the element at index 1.

This pull request was closed.
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.

Running with no args prints error
2 participants