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

Cannot use variable when typing path to command #154

Closed
kballard opened this Issue Jun 19, 2012 · 52 comments

Comments

Projects
None yet
@kballard
Copy link
Contributor

kballard commented Jun 19, 2012

Fish does not accept $HOME/bin/mvim as a valid typed command. This is perfectly legitimate in bash and other shells, and I wonder why Fish disallows it.

> $HOME/bin/mvim
fish: Variables may not be used as commands. Instead, define a function. See the help section
for the function command by typing 'help function'.

Even more curious, if I quote the variable, I get a slightly corrupted error message back.

> "$HOME"/bin/mvim
fish: Unknown command 'HOME/bin/mvim'

That garbage character is U+F003

@jonclayden

This comment has been minimized.

Copy link

jonclayden commented Jun 26, 2012

I've bumped into this several times in the process of converting bash aliases to fish functions. It's quite annoying. For scripts you can obviously work around it with /bin/sh $HOME/bin/myscript or similar, but it's a real problem when you're referring to a native executable. If this is by design I'd like to know how to get around it.

@maxfl

This comment has been minimized.

Copy link
Contributor

maxfl commented Jun 26, 2012

As far as I understand this is done as protection in order to make sure variables will not be used as if,else,case,switch,end,etc since they are parsed in the same way as other commands. And this is partly good point.

From my point of view, the bad thing is that you can not execute neither 'command $cmd' nor 'builtin $cmd'. Moreover
there is now way to execute 'call $cmd' which could execute builtin or command as fish does for usual executions.

Current workaround is to call 'eval $HOME/bin/mvim', which is not very comfortable if you have arguments with escaped characters, eval will loose them.

Another workaround is to use a function (attached). It's not a direct way, but it works and correctly handles escaped variables. But the redirection/suspending is specified indirectly, as first argument if needed:
call $vim
call -r ' ^/dev/null' $make # to redirect
call -r '>/dev/null &' make # to suspend
call echo "asdf sdf"
call $vim 'long filename with spaces'
And it works in a nested way:
call eval eval call call call echo 123

 function call --description 'Run a builtin or program' --no-scope-shadowing
          set -l redir
          switch "$argv[1]";
              case -r --redirection
              set redir $argv[2]
              set -e argv[1 2]
          end
          set -l cmd $argv[1]
          set -e argv[1]

          # 
          # Set job control for interactive mode
          #
          set -l __call_recover_mode
          if status --is-interactive-job-control
              set __call_recover_mode interactive
          else
              if status --is-full-job-control
                  set __call_recover_mode full
              else
                  set __call_recover_mode none
              end
          end
          if status --is-interactive
              status --job-control full
          end

          #
          # Define main function and call
          #
          echo "function -S __run_internal_function_name; $cmd \$argv $redir; end"  | .
          set -e cmd
          set -e redir
          __run_internal_function_name $argv

          #
          # Clear and return
          #
          set -l stat $status
          functions -e __run_internal_function_name
          status --job-control $__call_recover_mode
          return $stat
      end
@kballard

This comment has been minimized.

Copy link
Contributor

kballard commented Jun 26, 2012

What is --no-scope-shadowing? That's not documented.

-Kevin

On Jun 26, 2012, at 6:02 AM, maxfl reply@reply.github.com wrote:

function call --description 'Run a builtin or program' --no-scope-shadowing

@jonclayden

This comment has been minimized.

Copy link

jonclayden commented Jun 26, 2012

Thanks for the code, and for highlighting eval. I guess it is the intended approach, and in fact the error points you at it if the variable appears within the path, rather than at the start:

$ set test (whoami)
$ /Users/$test/bin/myscript
fish: Commands may not contain variables.
Use the eval builtin instead, like 'eval /Users/$test/bin/myscript'.
See the help section for the eval command by typing 'help eval'.

This is actually discussed in this fish manifesto article:

Since the use of variables as commands in regular shells is allowed, it is impossible to reliably check the syntax of a script. [...] fish solves this by disallowing variables as commands. Anything you can do with variables as commands can be done in a much cleaner way using either the eval command or by using functions.

I just hadn't realised that this restriction would be so tight, since in my mind using a variable to form part of the path to an executable is different to using the variable as a command in its own right.

@maxfl

This comment has been minimized.

Copy link
Contributor

maxfl commented Jun 27, 2012

--no-scope-shadowing is not documented, though it should be.

This option allows to operate on local variables of outer scope. For example it can be needed in following function:
function inc --description 'Increase the value of variable' --no-scope-shadowing
set $argv[1](expr $$argv[1] + 1)
end

So you can call 'inc index' to increase the value of $index by 1, even if $index is local variable.

@Ericson2314

This comment has been minimized.

Copy link

Ericson2314 commented Jul 31, 2015

This restriction makes me a bit uncomfortable -- reminds me of a "two lisp" like common lisp, where functions and data are bound in different namespaces.

Since the use of variables as commands in regular shells is allowed, it is impossible to reliably check the syntax of a script. [...] fish solves this by disallowing variables as commands. Anything you can do with variables as commands can be done in a much cleaner way using either the eval command or by using functions.

Huh? Higher order functions != implict eval. Parsing should happen before evaluation absent eval:

> (echo echo) asdf
asdf
> set foo echo
> $foo asdf
echo echo
> set bar = if
> $bar true; echo asdf; else; echo qwer; end
# syntax error

It is fine if else and friends look like builtin commands, but they certainly are something more. else without if gives a syntax error, not an unbound error, for example. That "secondary parsing" that ensures such "special builtins" appear in a proper syntax also happens before variable expansion.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Apr 7, 2016

I cross-linked a few more issues that are duplicates of this one. And I know I missed many more. This really needs to become a priority to fix. I've managed to live without this capability since switching to fish a few months ago but was extremely annoyed when I noticed I couldn't safely invoke external commands via variable substitution. Worse, there is no good reason not to allow it.

The documentation regarding why this restriction exists is almost incoherent. The restriction is because fish uses a parsing model that doesn't re-tokenize the statement after variable substitution. And that is a good thing. That POSIX shells, based on the original Bourne shell, do that re-tokenization is the source of many of the challenges to safely using them.

The idea of a call command that evals just the first argument hints at the proper solution. That is, modify the command builtin to perform variable substitution on its first argument. Then people can do things like command $EDITOR $argv and command $HOME/bin/mvim.

Symmetry argues for modifying the builtin builtin to do the same variable substitution. However, that is far less likely to be useful and could even be dangerous so we shouldn't do that unless someone can make a good argument for it.

@krader1961 krader1961 self-assigned this Apr 7, 2016

@Ericson2314

This comment has been minimized.

Copy link

Ericson2314 commented Apr 7, 2016

The restriction is because fish uses a parsing model that doesn't re-tokenize the statement after variable substitution.

This is the non-problem I was trying to get at. Do not reparse but just treat the first element of the expanded array as the exe and the rest as (prepended) args.

@Ericson2314

This comment has been minimized.

Copy link

Ericson2314 commented Apr 7, 2016

The change to command is still good, what I proposed above can be made distinct by making it work for functions too (but still not builtins). This is because all functions and external commands both take an arbitrary list; there is no special parsing that could be done.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Apr 8, 2016

See the discussion in issue #564; especially the point made by ridiculousfish on 2013-02-07.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Apr 8, 2016

@Ericson2314: Despite what I said a few comments earlier we definitely do not want to allow the indirection for builtins. Not even those like command, which could theoretically be implemented as a builtin but are currently special-cased by the parser.

I'm ambivalent about allowing indirection through a variable to functions. I've read all the issues where this problem has been discussed and have not seen a truly compelling argument for supporting it. You could argue functions should be supported in as much as you might want to choose from several alternative wrapper functions at run time, just as you want to be able to choose among external editors by setting $EDITOR. Yet you can get the same effect by defining another function whose name is invariant and having it redirect to the desired implementation. Like we do today with functions such as fish_title. A stronger argument for indirecting thru a var to a function is to allow callbacks without having to use eval in the function receiving the callback. That's a capability I use quite a bit in Python and C but I'm not sure how I feel about supporting it in fish.

@Ericson2314

This comment has been minimized.

Copy link

Ericson2314 commented Apr 8, 2016

Yeah to be clear I am 100% against implicit eval / reparsing too. Gross hack, poor security, etc, as you say.

@ghost

This comment has been minimized.

Copy link
Contributor

ghost commented Apr 8, 2016

From #564 ridiculousfish:

-==-
Restricting variables to be only functions, external commands, and "certain" builtins is somewhat better. (jakwings: agree, just like bash, $variables cannot map to keywords and special symbols) However it muddies the water about functions - now it seems like there's two ways to make user-defined commands (variables and functions) (jakwings: just substitution and plain text, problems?), and the user has to understand the rules governing both, and choose between them. The fact that fish does not have aliases, special prompt syntax, etc. is one of its great victories of simplification. (jakwings: I hope function output is not buffered)
-==-

I want to know whether bash is really doing this bad. Or it is just the fish parser does not work that way? (e.g. command is special-cased)

@Ericson2314

This comment has been minimized.

Copy link

Ericson2314 commented Apr 8, 2016

@krader1961 yeah to only be able to do callbacks (higher order functions) with eval means puts users in a security-vs-expressiveness bind. The fact that $asdf extra-args... has such predictable semantics regardless of the value of $asdf is a huge benefit.

@Ericson2314

This comment has been minimized.

Copy link

Ericson2314 commented Apr 8, 2016

@jakwings its the fact that bash will support more than functions and external commands in there that is so suspect. It's a weakened, but still dangerous eval, (unlike what we propose for fish) with bash.

lgarron added a commit to lgarron/dotfiles that referenced this issue Apr 12, 2016

[chrome.fish] Add an alias for net unit tests.
I would really like to unify all the test functions into a single implementation, but there is no straightforward way to evaluate a dynamic binary path with correctly quoted arguments.

Not only would

    eval "./out/Release/$TEST_BINARY" $argv

be unsafe, but it actually completely fails in a common case for me, which is test arguments containing asterisks that are not to be interpreted by the shell.

See fish-shell/fish-shell#154 for the relevant `fish` issues.
@lgarron

This comment has been minimized.

Copy link

lgarron commented Apr 12, 2016

To expand on my commit message above: I work on Google Chrome, which has many kinds of tests. I compile and run them as follows:

ninja -C out/Release browser_tests
./out/Release/browser_tests

Or to run a specific test:

ninja -C out/Release content_unittests
./out/Release/chrome-release-build-content-unit-tests --gtest_filter="WebURLLoaderImplTest.PopulateURLResponseSecurityDetails"

Or to run a group of tests.

ninja -C out/Release net_unittests
./out/Release/net_unittests --gtest_filter="CipherSuiteNamesTest.*"

So, I'd like to write a function as follows:

function build-and-run-tests
  set TEST_BINARY "$argv[1]"
  set -e argv[1]

  ninja -C out/Release "$TEST_BINARY"
  "./out/Release/$TEST_BINARY" $argv
end

However, there is no straightforward way to do the "./out/Release/$TEST_BINARY" $argv call. Using eval is unsafe because of the quoting issues described by others in this issue. In fact, trying to use eval (in the naive way) already fails for the simple and common case of trying to run a group of tests:

build-and-run-tests net_unittests --gtest_filter="CipherSuiteNamesTest.*"

Because there is an asterisk in the argument, it is interpreted as a file glob. Since there is no file that resembles the commandline argument, the argument is effectively dropped. (Unless you happen to have unlikely files called --gtest_filter=CipherSuiteNamesTest.abc or --gtest_filter=CipherSuiteNamesTest.xyz, in which case each file name is passed on as an argument.)

I use fish because it has awesome (and safe) defaults that usually don't require hacky workarounds, so I don't want to use something like maxfl's implementation of call or a separate bash script to do the dirty work. For now, I'm using a bunch of redundant implementations of build-and-run-tests with a binary name hardcoded in each one. But I'd prefer not to keep adding them manually. It would be very useful to have a safe exec function that only performs variable substitution for a binary specified in the first argument.

I don't know if this a compelling use case, but I thought I should at least write it up here.

@faho

This comment has been minimized.

Copy link
Member

faho commented Apr 12, 2016

It would be very useful to have a safe exec function that only performs variable substitution for a binary specified in the first argument.

@lgarron:

In the next release/current git, this is quite easy to do. I've written something like that in my comment on 2261

function call
    set -l cmd $argv[1]
    set -e argv[1]
    eval $cmd (string escape -- $argv)
end

Or rather this leaves the variable substitution up to the caller (no "--no-scope-shadowing"), but the end result is the same. Note that this does not escape the first argument to allow e.g. $EDITOR with a value of "emacs -nw" (one element), so you'd need to escape that manually if you so wish. string escape is meant to be fed into eval.

I'm still not sure if we want to ship something like this or leave it up to the user.

@Ericson2314

This comment has been minimized.

Copy link

Ericson2314 commented Apr 12, 2016

@faho any use of eval still has the poor security problem. Also the nature of this solution -- parsing escaping, reparsing, etc. feels much more overcomplex and bashy than what we are proposing.

@faho

This comment has been minimized.

Copy link
Member

faho commented Apr 12, 2016

@Ericson2314: As I said, string escape is meant to fix that. It's meant to be handed to eval, so any remaining issue is now a bug of string escape. So far I haven't found any.

For example

>string escape -n -- 'echo' '(printf "%s\n" $PAGER)'
echo
\(printf\ \"\%s\\n\"\ \$PAGER\)
>eval (string escape -- '/tmp/(blah)/test.fish') # A script that prints "test"
test

Of course that call implementation above wouldn't work with the latter example since it explicitly does not escape the first variable. It's a trade-off, though - do you want the first variable to be split ("$EDITOR" is used by programs other than fish, and therefore needs to be set to one element) or can it contain weird characters?

@Ericson2314

This comment has been minimized.

Copy link

Ericson2314 commented Apr 12, 2016

Consider with what we are proposing this is no tradeoff on argv0. "$cmd" will concatenate the array, and "$cmd $args will invoke a function or external command like $cmd[1] $cmd[2] $cmd[3]... $args[1] $args[2] $args[3]....

@Ericson2314

This comment has been minimized.

Copy link

Ericson2314 commented Apr 12, 2016

I don't want to say that escape+eval is always broken. But I find that it needless flirts with danger---invoking eval when no evaluation is intended.

@lgarron

This comment has been minimized.

Copy link

lgarron commented Apr 12, 2016

@Ericson2314: As I said, string escape is meant to fix that. It's meant to be handed to eval, so any remaining issue is now a bug of string escape. So far I haven't found any.

I'm glad that there will be a reasonable workaround, but I share the concern of @Ericson2314 that this "needlessly flirts with danger" for the common use case (executable path stored in a variable, other values to be passed as usual).

If we don't want to run on the bleeding edge of the latest git checkout, when can we look forward to the next release with strings?

@faho

This comment has been minimized.

Copy link
Member

faho commented Apr 12, 2016

I don't want to say that escape+eval is always broken. But I find that it needless flirts with danger---invoking eval when no evaluation is intended.

Don't misunderstand me - I'm not saying this shouldn't be done "properly", but now there's a reasonable workaround to tide you over.

If we don't want to run on the bleeding edge of the latest git checkout, when can we look forward to the next release with strings?

Any day now.

@Ericson2314

This comment has been minimized.

Copy link

Ericson2314 commented Apr 12, 2016

@faho Yes, certainly it's good to have a stop gap, and your suggestion works great as that.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Jan 4, 2017

The discussion on proof of concept PR #3596 has reached a tentative consensus. Expansion of the first arg for the exec decorator should be allowed to explicitly replace the fish process with an external command that might involve indirection via one or more variables. We should not allow indirection via the builtin or command decorators. That's because we would need to introduce an equivalent for functions to be consistent and neither is needed since we're going to implement a more generic mechanism that handles all three cases as described below.

A new decorator command should be introduced to allow running a function, builtin, or external command indirectly via variable expansion using the usual precedence rules. TBD is whether the new decorator should be named call, run, or something else. I favor call since it is consistent with historical convention by many languages that allow invoking a function indirectly.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Jan 5, 2017

While working on the change to implement this feature I noticed something that might require more discussion. The proof of concept implementation in PR #3596 only supports expansion of variables with a single value. This means this will fail:

set cmd echo hello
exec $cmd again

I think that is the desired behavior. Indirecting through a variable should only specify the name of the command to be run. It should not be possible to inject arguments to the command thru that indirection. If you want to do that then use eval. If anyone strongly disagrees please speak up now. Because if you don't that limitation will likely be part of the implementation.

I am quite aware that fish variables are arrays that can have zero or more elements; unlike other shells such as bash and zsh. Thus, you could argue that as a consequence of fish's behavior vis-a-vis variables the example I gave above should result in hello again being written to stdout rather than an error. I think that is too dangerous.

@floam

This comment has been minimized.

Copy link
Member

floam commented Jan 5, 2017

@krader1961 That was entirely intentional on my part. I had (maybe done a bad job) of demonstrating my desired behavior with the "spaces" demonstration. I think it'd be dangerous.

@floam

This comment has been minimized.

Copy link
Member

floam commented Jan 5, 2017

Side note: To be perfectly honest, I can't think of a great rationalization as to why command $foo should be illegal while exec $foo or call $foo works. But I recognized that ship sailed with consensus and I was too busy to argue.

@faho

This comment has been minimized.

Copy link
Member

faho commented Jan 5, 2017

To be perfectly honest, I can't think of a great rationalization as to why command $foo should be illegal while exec $foo or call $foo works

Me neither, so how about reopening that part of the discussion?

@krader1961, @ridiculousfish: Care to explain to my dumb brain that hasn't properly slept this year?

The proof of concept implementation in PR #3596 only supports expansion of variables with a single value.

If anyone strongly disagrees please speak up now.

I do. Both because it's inconsistent (the mental model that variable expansion happens before "the command" sees anything is broken) and because it removes some of the utility. And I feel like this necessitates more discussion.

For example, I have my $EDITOR set to emacs -nw, so interacting in the terminal doesn't suddenly open up a graphical program. Note that I currently set as one value because otherwise it'd be exported as emacs\x1e-nw which is unreadable to other programs. I'm not asking for it to be split on spaces again, but I feel like it should be easy to call something like that as well.

As another point of discussion: How does this interact with command substitutions? I feel like if call $EDITOR works, then call (string split " " -- $EDITOR) should as well - it again fits the model of "expand, then execute", and it would allow me to keep my $EDITOR set in a way that makes it useful outside of fish (e.g. git uses $EDITOR).

@kopischke

This comment has been minimized.

Copy link

kopischke commented Jan 5, 2017

[…] it's inconsistent (the mental model that variable expansion happens before "the command" sees anything is broken) and because it removes some of the utility.

Aye to both points.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Jan 6, 2017

Okay, I'm not crazy. Despite how I phrased my previous comment I too felt that behavior was wrong. But it was how the original proof of concept implementation behaved and nobody complained so I wanted someone other than me to say that was wrong. I deliberately used provocative phrasing in hopes of forcing someone to say "hold it, that seems wrong." Having said that I hope we all agree that set EDITOR "emacs -n -w" followed by exec $EDITOR /some/file should fail because we should not split the $EDITOR value on whitespace. The user has to separate the command and args when defining the variable; e.g., set EDITOR 'emacs' '-n' '-w'.

... why command $foo should be illegal while exec $foo or call $foo works.

Mostly because it a) isn't needed AFAICT and b) introduces an inconsistency since you can't indirectly run a function that way. That is, there is no function $foo equivalent to command $foo. Having said that I'm not completely opposed to supporting it since it should be mostly a matter of cutting and pasting some code and updating the "command" man page. But I would be happier if someone could provide a good example of why command $foo (and builtin $foo) are useful before we implement that feature. I'm assuming, of course, we allow indirection via exec and add a call builtin which appears to make command $foo unnecessary.

@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Jan 6, 2017

Also, I have no objection to allowing call (string split " " -- $EDITOR) (i.e., command substitutions), @faho, but do feel strongly that fish should not expand and whitespace split call $EDITOR. That would be inconsistent with how fish behaves everywhere else and is a good example of the legacy Bourne shell behavior that fish deliberately chooses not to support because it causes so many problems.

@floam

This comment has been minimized.

Copy link
Member

floam commented Jan 6, 2017

I mean, by that token - having a builtin or command statement at all is inconsistent and they shouldn't exist since we can't call a function directly. To me, it just seems strange that we wouldn't expand here. If we added a functions --run or whatever other names have been proposed - it'd ought to do the same thing. What makes exec special over command with regards to expansion? I might be missing something obvious?

@faho

This comment has been minimized.

Copy link
Member

faho commented Jan 6, 2017

But I would be happier if someone could provide a good example of why command $foo (and builtin $foo) are useful before we implement that feature.

Same reason command foo is useful - sometimes you don't want a function, or you want to call a command that has been shadowed by a builtin.

Also, again, for consistency's sake, so this stuff is easily understandable. If all of these "decorations" are allowed, then you just need to remember "I can launch something with a variable as long as it's started indirectly, by another 'command'". Whether that "command" is command, call, env or whatever. If only some are, then you need to remember which exactly.

introduces an inconsistency since you can't indirectly run a function that way. That is, there is no function $foo equivalent to command $foo

The obvious solution here would be to add something to run "a function". We don't really need it because if a function for something exists it'll be used by default and if it doesn't a functions --run or funcall or call --function would fail anyway, but for consistency it might be nice.

@faho

This comment has been minimized.

Copy link
Member

faho commented Jan 6, 2017

I have no objection to allowing call (string split " " -- $EDITOR) (i.e., command substitutions), @faho, but do feel strongly that fish should not expand and whitespace split call $EDITOR.

Obviously, I agree - which is why I said "I'm not asking for it to be split on spaces again".

The situation with $EDITOR is a bit annoying since for ease of use in fish it should be a list of "command option1 option2", but to be useful outside of fish it can only be a single string.

The only obvious and complete solution is to fix unix to allow list-variables. And that'd be quite a bit of work.

@faho faho referenced this issue Jan 6, 2017

Closed

eval $argv unparses the arguments #3708

2 of 2 tasks complete
@krader1961

This comment has been minimized.

Copy link
Contributor

krader1961 commented Jan 7, 2017

The case of an env var (e.g., $EDITOR) as a string containing a command plus options separated by whitespace (or colons, etc.) is another pain point discussed in issue #436. Hopefully someday soon we'll implement a mechanism to make importing and exporting such vars easier such that when used inside fish they are proper arrays.

@Ericson2314

This comment has been minimized.

Copy link

Ericson2314 commented Jan 10, 2017

Just want to say I'm excited that this is being worked out! The arguments on why eval is bad, array expansion is good, fish functions are overkill for this, etc, are exactly what I'd hope to bring up. Still feel like command/call aren't needed, but that's a small point I wouldn't want this to be hung up on!

@pgan002

This comment has been minimized.

Copy link

pgan002 commented May 28, 2017

If variables can be executed with a decorator command, then that duplicates much of what functions do. Should we remove functions as a concept in Fish?

As for a decorator name, how about do $var, do function $var, do command $var, do builtin $var to replace builtin and command? Or even do exec $var instead of exec? This is slightly more verbose but groups these commands that have quite similar meaning into one command with parameters.

@faho

This comment has been minimized.

Copy link
Member

faho commented May 28, 2017

If variables can be executed with a decorator command, then that duplicates much of what functions do. Should we remove functions as a concept in Fish?

I can't see how it does. I mean there's function PAGER; less; end vs call $PAGER, but that's such a minor use-case that it barely matters.

I notice that we haven't specified what happens if a variable contains syntax relating to multiple commands. That doesn't even work in bash (foo="echo a | less"; $foo prints "a | less") and I don't think it should in fish either. With that out of the way, variables cannot contain branches, so they are not a replacement for functions in any way but the most mundane (the above).

As for a decorator name, how about do $var, do function $var, do command $var, do builtin $var to replace builtin and command?

I find the idea of using subcommands here interesting. I'm not sure that it would be okay to allow no subcommand (to run anything). What happens if the first argument then looks like a subcommand? E.g. do $var - with var set to foo a b c, it would execute a foo function/builtin/command, but with var set to function a b c, it would try to execute a function a. That seems like a recipe for confusion - if you get a variable from somewhere else, you have to remember to use do any $var or check against the subcommands yourself. If we instead disallow do $var (where the first word of $var does not look like a subcommand), then it would be clear. I.e. make it fail in 90%, not 10%, of cases so the failure is visible.

I still think call is clearer than do, which is as generic as it gets, though.

@krader1961 krader1961 modified the milestones: fish-3.0, fish-future Jul 5, 2017

ammgws added a commit to ammgws/dotfiles that referenced this issue Jul 20, 2017

Fix call to scrot
Fish doesn't appear to support variables as commands:
fish-shell/fish-shell#154
Might change in the future, but for now we will have to live with
not being able to define the scrot path at the top of the file...

@krader1961 krader1961 removed their assignment Aug 25, 2017

@cben

This comment has been minimized.

Copy link
Contributor

cben commented Aug 13, 2018

Implicit cd also doesn't work with vars:

~ $ $GOPATH/src/github.com/exercism/cli/
fish: Variables may not be used as commands. In fish, please define a function or use 'eval $GOPATH/src/github.com/exercism/cli/'.
~ $ set PKG github.com/exercism/cli/
~ $ ~/go/src/$PKG
fish: Variables may not be used as commands. In fish, please define a function or use 'eval /home/bpaskinc/go/src/$PKG'.
~ $ command ~/go/src/$PKG
fish: Variables may not be used as commands. In fish, please define a function or use 'eval /home/bpaskinc/go/src/$PKG'.
command ~/go/src/$PKG
        ^
~ $ eval ~/go/src/$PKG
~/go/src/git/exe/cli (master|✔) $ 

Now of course cd ~/go/src/$PKG is the simplest solution here, zero reason to resort to eval / command or proposed call / do.
In a script, I'd probably write explicit cd dir/ instead of implicit ./dir/ for clarity anyway.
It's just a one more (minor) annoyance where this restriction violates law of orthogonality.

@ridiculousfish

This comment has been minimized.

Copy link
Member

ridiculousfish commented Sep 1, 2018

Revisiting this, I no longer think do or call is necessary. It's fine to allow variables in commands. If a command expands to zero strings, it's an error; if it expands to multiple strings, the first one is the command and the remainder become arguments. No further expansion takes place for these arguments.

@mqudsi

This comment has been minimized.

Copy link
Contributor

mqudsi commented Oct 18, 2018

@ridiculousfish what about this (overly contrived) example:

(which foo) --bar

I hit a real-world case today where I would have been forced to use eval before, but thanks to 865a464 I was able to store the result of the command substitution in a variable then execute that instead, like so:

set foo (which foo)
$foo --bar

But given that it's equivalent to the (currently illegal) first example, is there a good reason to not enable that as well?

@ridiculousfish

This comment has been minimized.

Copy link
Member

ridiculousfish commented Oct 18, 2018

Don't do that, that's what env is for: env foo --bar...

@mqudsi

This comment has been minimized.

Copy link
Contributor

mqudsi commented Oct 18, 2018

Of course. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment