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

Failed wildcards still execute #2394

Closed
fizzfaldt opened this issue Sep 16, 2015 · 53 comments
Closed

Failed wildcards still execute #2394

fizzfaldt opened this issue Sep 16, 2015 · 53 comments
Labels
bug Something that's not working as intended
Milestone

Comments

@fizzfaldt
Copy link

From the documentation:
http://fishshell.com/docs/current/#expand-wildcard

If none of the wildcarded arguments sent to a command result in any matches, the command will not be executed. If this happens when using the shell interactively, a warning will also be printed.
It still gets executed after being deleted.
This used to work for me, it might be related to my fish version.

fizzfaldt@buk ~/fishtest> fish --version
fish, version 2.2.0
fizzfaldt@buk ~/fishtest> dpkg -l | grep fish
ii  fish                                                        2.2.0-1~trusty                                      amd64        friendly interactive shell

Create a directory containing "foo" and "bar"

fizzfaldt@buk ~/fishtest> command /bin/ls -1 -l
total 0
-rw-rw-r-- 1 fizzfaldt fizzfaldt 0 Sep 16 00:38 bar
-rw-rw-r-- 1 fizzfaldt fizzfaldt 0 Sep 16 00:38 foo

If none of the wildcarded arguments sent to a command result in any matches, the command will not be executed.

This fails.. it expands the wildcards to zero arguments, but it still EXECUTES the command:

fizzfaldt@buk ~/fishtest> command /bin/ls -1 -l c*
No matches for wildcard “c*”.
fish: command /bin/ls -1 -l c*
                            ^
total 0
-rw-rw-r-- 1 fizzfaldt fizzfaldt 0 Sep 16 00:38 bar
-rw-rw-r-- 1 fizzfaldt fizzfaldt 0 Sep 16 00:38 foo
fizzfaldt@buk ~/fishtest>

Note that if no matches are found for a specific wildcard, it will expand into zero arguments, i.e. to nothing.

This one works: If I have multiple wildcards, but at least one of them matches something, I get a warning, the non-matching ones become nothing, the matching one is expanded, and the command is executed:

fizzfaldt@buk ~/fishtest> command /bin/ls -1 -l b* c* d*
No matches for wildcard “c*”.
fish: command /bin/ls -1 -l b* c* d*
                               ^
No matches for wildcard “d*”.
fish: command /bin/ls -1 -l b* c* d*
                                  ^
-rw-rw-r-- 1 fizzfaldt fizzfaldt 0 Sep 16 00:38 bar
fizzfaldt@buk ~/fishtest> command /bin/ls -1 -l c* b* d*
No matches for wildcard “c*”.
fish: command /bin/ls -1 -l c* b* d*
                            ^
No matches for wildcard “d*”.
fish: command /bin/ls -1 -l c* b* d*
                                  ^
-rw-rw-r-- 1 fizzfaldt fizzfaldt 0 Sep 16 00:38 bar
@zanchey
Copy link
Member

zanchey commented Sep 16, 2015

See #1920, #1482.

@fizzfaldt
Copy link
Author

At the very least, assuming the change in behavior is intentional, the documentation needs to be updated.
For example, just deleting the line:

If none of the wildcarded arguments sent to a command result in any matches, the command will not be executed.

I do feel we have the worst of all worlds right now.
We don't pass globs to subcommands, so we often have to escape them, and we have no control over how it works (no options to switch behavior).

I liked getting errors when the globs failed, and would have preferred they always fail even if there were multiples. Is there no intention to have options then?

@ridiculousfish
Copy link
Member

Today's "ls *.txt prints everything" behavior is horrible.

@faho
Copy link
Member

faho commented Sep 16, 2015

Yes, it is. But what's the correct behavior? The previous "ls *.txt fails, and you can't do anything to detect it or to suppress the error message" wasn't perfect either.

For the ls case, you clearly don't want to have globs expand to nothing, but for for i in * or set foo * you probably want just that (and I believe we all agree that passing the bare "*" character is the worst possible solution).

@bgeron
Copy link

bgeron commented Sep 22, 2015

I think the best option is to fail by default (so converts from sh don't hurt themselves), and add a emptyglob command mentioned in #1920 .

Today's "ls *.txt prints everything" behavior is horrible.

+1.

@ridiculousfish
Copy link
Member

Another possibility is to make * and ** fail by default, but introduce new globs *? and **? that expand to empty instead of failing. I think I like that a bit better because it attaches to the glob instead of the command - what do others think?

@faho
Copy link
Member

faho commented Sep 27, 2015

introduce new globs ? and *? that expand to empty instead of failing

First of all, we already have a "?" for globs. I presume you mean to ditch that and replace it with this?

Otherwise, the idea of attaching it to globs sounds nice. I'm not sure if a more extensible syntax would make sense - zsh has glob qualifiers, which are very powerful but utterly unreadable. Unfortunately, I can't come up with a nice syntax.

@ridiculousfish
Copy link
Member

? matches any one character, while * matches zero or more characters. So *? has no real utility: any character matched by the ? could also be matched by the *. I propose grabbing this currently useless syntax to make a new glob *? that does the emptyglob thing.

Ok, that's not really true: * matches the empty string, while *? only matches one-or-more characters. But one-or-more would still be available as ?* after this change.

@ridiculousfish
Copy link
Member

I guess another, more conservative option is to make emptyglob a command instead of a command modifier. So instead of emptyglob ls *.txt it would be ls (emptyglob *.txt).

@faho
Copy link
Member

faho commented Sep 27, 2015

Ok, that's not really true: * matches the empty string, while ? only matches one-or-more characters. But one-or-more would still be available as ? after this change.

And a one character glob that should come up empty instead of fail? "??" would either match any two characters or fail, or match any one character or come up empty - it's ambiguous.

I guess another, more conservative option is to make emptyglob a command instead of a command modifier. So instead of emptyglob ls *.txt it would be ls (emptyglob *.txt).

The issue I have with that is that I'd have to use it almost always.

The best I could come up with is something like *[empty] - the brackets are already special for other substitutions and are currently not used for globs, so it makes sense to use them for something. It's also extensible - ?[empty,file] or *[0..5], but I'm not sure if it's fishy enough.

Well, that or fail and make certain commands (set and for come to mind) special.

@anordal
Copy link
Contributor

anordal commented Sep 27, 2015

regex > emptyglob

@anordal
Copy link
Contributor

anordal commented Sep 27, 2015

I like how ? in *? and **? borrows syntax from regex. There it means, match once or nothing.

@ridiculousfish
Copy link
Member

@faho can you elaborate on how you'd have to use it almost always?

@faho
Copy link
Member

faho commented Sep 27, 2015

I wrote a rather long and confused comment here before I recognized that I probably forgot that making globs fail also means not setting $status to 0. Currently globs behave a little confusing here - set and for fail with status 124, everything else (?) gets an empty argument and may therefore return 0 - meaning you can't detect if a glob with, say, ls failed.

Frankly, all I'd need in that regard is that failing globs set status to non-zero (so that a simple or can work if you don't care why it failed) and a way to detect that the glob failed, not the command itself (a $globstatus variable?).

So: Disregard, me thinkings ungood.

@zanchey zanchey added this to the next-2.x milestone Oct 16, 2015
@anordal
Copy link
Contributor

anordal commented Feb 8, 2016

Suggestion: #2719

@krader1961
Copy link
Contributor

I too have been mildly annoyed that typing something like ls *foobar* (when no file names contain foobar) causes the the command to be run with no arguments. I'm inclined to agree with @faho when he says

I believe we all agree that passing the bare "*" character is the worst possible solution

with respect to the for, set, or count commands.

What is less clear is what should happen with respect to other commands (especially external commands). Other shells I've used, such as ksh and zsh, either refuse to run the command or pass it the unexpanded glob. In my opinion either of those is preferable to the current fish behavior.

The proposed change by @anordal seems to allow someone who cares about distinguishing between a matching glob and a non matching glob to use the set command with a subsequent if command. This, however, breaks backward compatibility and therefore requires a lot of discussion.

P.S., Pull request #2719 is a good example of a change where the "philosophical" discussions should occur in an issue like this rather than in the pull request. If a pull request is causing feedback about the appropriateness (as opposed to the correctness) of the change then the discussion should be moved to an open issue and the pull request rejected.

@anordal
Copy link
Contributor

anordal commented Feb 9, 2016

I understand that this is a philosophical discussion, but it's also a bugreport. Strictly speaking, the current behaviour is a bug, since it goes against the documentation. As such, we should just mercilessly revert back to full failglob behaviour ASAP, close this issue, and continue the discussion in feature requests like #1482 and #1920.

backward compatibility

…with a bug. I know, that's why I didn't propose the above — we probably agree that nullglob, more often than not, is the only sane alternative for scripting. But if we want any of the failglob-by-default behaviour back, without making a separate language for scripting, I don't see a way around this one.

passing the bare "*" character is the worst possible solution

Period, I'd say. Nobody needs it! And for the purpose of solving this bug, it's a separate discussion.

@krader1961
Copy link
Contributor

@anordal: Take a deep breath and calm down. Then take another look at issue #1482 (which is closed by the way so is not the place to continue this discussion). This was a deliberate change in behavior. The change did not get reflected in the documentation. The mistake (or "bug" if you prefer) was in not updating the documentation to match the new semantics.

Breaking backward compatibility always requires discussion and careful consideration of the impact. The fact the previous change apparently occurred without such a discussion is a problem in its own right. But the current behavior has been in place for over a year. Changing it again should not be done casually.

nullglob, more often than not, is the only sane alternative for scripting

  • passing the bare "" character is the worst possible solution... Period, I'd say. Nobody needs it!

Wow. I guess the fact I've been programming for 39 years and disagree means nothing. Clearly you know more than anyone else who has thought about this issue (including the writers of other shells).

@faho
Copy link
Member

faho commented Feb 9, 2016

@krader1961: Could you elaborate on why you think passing "*" is a good idea?

To be clear, the situation we're talking about is something like

echo foo*

This could do the following:

  • Fail, with or without error message and with or without setting $status (this is what bash calls "failglob" - which is zsh's default behavior)
  • Succeed but print nothing (this would be "nullglob" - which is fish's current non-interactive behavior)
  • Succeed but print a literal "foo*" (the bash/ksh/etc default behavior, I'll call it "passglob")

To me, the issue with nullglob is that it may result in different behavior than you wanted, including ls foo* printing the current directory. This behavior is potentially dangerous (though I'm struggling to find an example), but one could argue that it's up to the script to check.

The issue with failglob is that it can't be done 100% - you need at least the ability to check for a failing glob. Whether that can be done inline or needs to be done via doing a set first or maybe even an additional function is an implementation detail, and one we'd have to decide on before merging a solution. I'd be okay with either.

The issue with passglob is that it's mixing paths with things that aren't paths. "foo_" is one or more paths if it matches and something that is not a path if it doesn't. The problem isn't when you'd want to do ls foo_ and it doesn't match, because ls will then tell you that it's not a valid directory - though in that case it also wouldn't hurt for the shell to tell you instead. The problem is when someone wants to pass a literal "foo*" and then it ends up matching because something about PWD changes. Our version of switch/case would be bound to cause this for some people. This leads to very weird bugs.

In other words: All three variants (I can't think of any else) can fail, but failglob fails in the most obvious way and has the least potential to cause other issues (simply because it doesn't execute anything).

@anordal
Copy link
Contributor

anordal commented Feb 9, 2016

@krader1961 Sorry for flaming.

nullglob, more often than not, is the only sane alternative for scripting

faho says it much better: The issue with failglob is that it can't be done 100% - you need at least the ability to check for a failing glob.

About passglob:

Nobody needs it!

I didn't mean this as a matter of opinion. "You can always quote", is what I meant.

@krader1961
Copy link
Contributor

@faho:

Could you elaborate on why you think passing "*" is a good idea?

I am not claiming that is the optimal solution or even a good idea. I am claiming that most other shells I've used do so (including Bourne, Korn, Zsh, Bash). Too, that isn't a good example. A better example is something like "*.wtf". We should deviate from that well established behavior only with extremely good reasons.

Also, consider that fish can't possibly know how a given command will behave if the glob expands to nothing. The command might operate on every file in the PWD if it doesn't get any file names. Whereas it will likely throw an error if handed a glob that didn't match any files. Which means passing the unexpanded glob to the command could have fewer unintended side-effects.

Having said all that I would be extremely careful about writing a script that might misbehave if a glob didn't match anything. I would typically test whether the glob actually matches at least one file before doing anything with the glob expansion. The question for us is what should fish do to maximize the principle of least surprise (or "principle of least astonishment").

@anordal:

I didn't mean this as a matter of opinion. "You can always quote", is what I meant.

I must be misunderstanding you because that doesn't make any sense to me. If I quote the glob then obviously it won't be expanded and will be passed through literally. That isn't the issue. The issue is what should happen when a glob fails to match any files.

@krader1961
Copy link
Contributor

@faho wrote:

The issue with failglob is that it can't be done 100% - you need at least the ability to check for a failing glob. Whether that can be done inline or needs to be done via doing a set first or maybe even an additional function is an implementation detail, ...

Exactly. Anyone who actually cares whether a glob matches anything should be doing so explicitly via set and test commands (or some equivalent construct). The question is what should fish do when someone isn't careful and does the equivalent of random_command *wtf* where the glob doesn't match any files? The principle of least surprise says to pass the literal glob to the command and let it deal with it because that is what has happened for three decades in most shells.

@bgeron
Copy link

bgeron commented Feb 10, 2016

There's some value in sticking with history, but I think both passglob
and nullglob is not what users want by default. And I think people do
care by default. I don't think fish needs extremely good reasons to
deviate from history, just good reasons, but that's been discussed
before in the script case.

Personally, when I have a failing glob that's passed to a command, I
often put echo before it so I can see what was the expanded command
line. It seems to make sense that fish helps me with this when nothing
matches. I don't think it's weird to have to escape *, because loads of
special characters have meaning and need to be escaped.

To be honest failglob still surprises me regularly, but it's a very
mild surprise.

Cheers, Bram

On Wed, 10 Feb 2016, at 03:23 AM, Kurtis Rader wrote:

@faho[1] wrote:

The issue with failglob is that it can't be done 100% - you need at
least the ability to check for a failing glob. Whether that can be
done inline or needs to be done via doing a set first or maybe even
an additional function is an implementation detail, ...

Exactly. Anyone who actually cares whether a glob matches anything
should be doing so explicitly via set and test commands (or some
equivalent construct). The question is what should fish do when
someone isn't careful and does the equivalent of random_command
wtf where the glob doesn't match any files? The principle of least
surprise says to pass the literal glob to the command and let it
deal with it because that is what has happened for three decades in
most shells.

— Reply to this email directly or view it on GitHub[2].

Links:

  1. https://github.com/faho
  2. Failed wildcards still execute #2394 (comment)

@anordal
Copy link
Contributor

anordal commented Feb 10, 2016

If I quote the glob then obviously it won't be expanded and will be passed through literally. That isn't the issue. The issue is what should happen when a glob fails to match any files.

@krader1961: I mean that since one can always pass the glob literally by quoting it, we can infer that nobody needs other ways to pass a glob literally (such as hoping that the shell will do it). That's how I conclude that passglob is not a strictly necessary feature. That's all I meant with that.

While I'm on it, I would also argue that whenever passglob is helpful, it comes at the price of brittleness.

For example, find is a command you may want to pass a literal glob to. Of course, this will misbehave in interesting ways unless you quote the glob:

find -name *.foo

And some commands don't take globs, yet people seem to think so. This one barfs an ugly error if the glob doesn't expand to exactly 1 argument — better rewrite this in terms of count *.foo:

test -f *.foo

These are some examples I can think of that wouldn't work with either nullglob or failglob, that I commonly see in shellscripts. But they have serious other problems and are better rewritten in any case.

Regarding the find command, I've witnessed more than one bash user not knowing how to quote the glob. Then, I observed one newly converted fish user that knew this perfectly well… Just alluding to failglob's pedagogical value here — I guess the mistake isn't obvious if it tends to work.

That lousy find command might end up in a backup script near you

@krader1961
Copy link
Contributor

@anordal: What sentient person who has used a UNIX shell for more than a few minutes expects a bare asterisk in a shell command to be a reliable way to pass a literal asterisk to a command? Same for your find example. Sure, an absolute newbie will make that mistake. But it shouldn't take long for them to realize they need to quote or escape those chars. Similarly, if I'm relying on a backup script written by someone that inexperienced (or clueless) then I deserve what I get. Your argument leaves me scratching my head in puzzlement.

@zanchey
Copy link
Member

zanchey commented Feb 11, 2016

For reference, there has been some discussion on this in the past. I agree with @ridiculousfish in #683 (comment). See also #796 and #799.

bash's default behavior with respect to wildcards is dangerous and misleading:

> echo foo*
foo*
> touch foobar ; echo foo*
foobar

So if a wildcard does not expand to at least one match, it is not expanded at all. That is why your command works in bash. But now do this:

touch -- --include=foo.xml

that makes a file that matches your wildcard, and now your command will fail.

@krader1961
Copy link
Contributor

@zanchey: That example makes my point rather than those arguing for "nullglob" semantics. If someone has done the equivalent of that touch command then the glob will be expanded by fish. You can construct any number of such scenarios. It is impossible to make shells like the original Bourne shell and more modern shells like Bash immune to such problems. Which is precisely why they should not be used when their environment makes them at risk to such problems. This is why options where added to the find and xargs command to null-terminate strings to avoid splitting on whitespace.

Fish is much better in this regard than shells that conform to the Posix standard which is heavily based on the original Bourne shell. But it is still at risk.

That

some commands don't take globs, yet people seem to think so.

as mentioned by @anordal is not relevant. Or at least only partially relevant given that people will make mistakes in any programming language. The question for us is how to minimize the effects of such mistakes while maximizing the utility of the fish language.

@bgeron
Copy link

bgeron commented Feb 11, 2016

Okay, so if I understand correctly, you @krader1961 think that passglob isn't a good design decision either, but the Unix ecosystem has gotten used to the quirk and so it's not entirely horrible so why change it. Do I understand that correctly?

My argument to move from passglob to failglob is Unix is nice, but they nevertheless made a lot of mistakes (which are only obvious in hindsight). We very very rarely get the chance to fix them; let's fix them now that we can. I'm not sure we get the chance again in our lifetime, and I think fish users are generally okay with backwards-incompatible improvements.

Do I further understand correctly in thinking that apart from @krader1961 there is concensus for failglob and that we want fish to revert to pre-2.2 behaviour and the documentation to not change (that is, describe failglob behaviour)?

@ridiculousfish
Copy link
Member

Command substitutions and globs both participate in completions.

@faho faho added bug Something that's not working as intended and removed bug Something that's not working as intended labels Feb 15, 2016
@faho
Copy link
Member

faho commented Feb 15, 2016

I've now closed #1920. Since there's been plenty of discussion about more ways to make clear that a glob is not supposed to fail a command, let's track that here.

@faho faho modified the milestones: next-major, next-2.x Feb 15, 2016
@anordal
Copy link
Contributor

anordal commented Feb 15, 2016

@ridiculousfish How? It doesn't look like fish evaluates command substitutions as part of filename completion.

Suppose I do this first:

mkdir comp{lete,rehend}
touch comp{lete,rehend}/this
  • Glob: If I write ls comp*/ and press TAB, fish finds the "this" part and completes the command line into ls comp*/this.
  • Command substitution: If I write ls comp(printf 'lete\nrehend\n')/ and press TAB, nothing happens!
  • Variable: set both (printf 'lete\nrehend\n'), then ls comp$both/ and pressing TAB works — arrays ftw again.

The behaviour of not executing command substitutions until you press enter is very sensible in general (as fish can't know if an arbitrary command has side effects or takes 100 years to finish), but sounds too much like a deal breaker for a glob command IMHO.

@ridiculousfish
Copy link
Member

Sorry, by "command substitutions participate in completions" I mean that you can complete inside command substitutions, that's all. It doesn't evaluate them.

@anordal
Copy link
Contributor

anordal commented Feb 18, 2016

So #2719 got merged. Thanks everyone!

we can add any other nullglob command or syntax later

Agreed. I suppose command modifiers are a clean way forward. An example mentioned in #2188 (comment):

emptyglobs read_some_yaml *.yaml *.yml
noglobs wget http://foo.com?bar

That said, I do think we have the most user friendly solution right now — it encourages quoting globs that should be quoted, and it encourages checking the number of expanded arguments. I'm struggling to think of any popular command you would want to use nullglob for, that successfully does nothing when that glob fails to expand — the check usually has to be there anyway.

@ridiculousfish
Copy link
Member

nullglob is nice for rm, like rm obj/* bin/*. If the glob fails to expand, then there was nothing to remove, which is usually what you want.

@anordal
Copy link
Contributor

anordal commented Feb 19, 2016

… but that's not success as far as rm is concerned:

rm: missing operand
Try 'rm --help' for more information.

I mean that most commands will barf errors like that, making the check necessary for scripting purposes.

@bgeron
Copy link

bgeron commented Feb 19, 2016

I think ridiculousfish meant 'rm -f', which is common in Makefiles. Make
has nullglob behaviour if I'm not mistaken.

Cheers, Bram

On Fri, 19 Feb 2016, at 05:42 PM, anordal wrote:

… but that's not success as far as rm is concerned:

rm: missing operand
Try 'rm --help' for more information.

I mean that scripts have to check the number of arguments in order to
not barf errors like that.

— Reply to this email directly or view it on GitHub[1].

Links:

  1. Failed wildcards still execute #2394 (comment)

@anordal
Copy link
Contributor

anordal commented Feb 19, 2016

Good point about rm -f! I'll count that as example number 1.

If you mean targets in Makefiles, make picks the first target in the file if you don't specify any, so no nullglob behaviour wrt targets. But the wildcard function in Make has nullglob behaviour!

@bgeron
Copy link

bgeron commented Feb 19, 2016

No, I meant rules.

On Fri, 19 Feb 2016, at 06:10 PM, anordal wrote:

Good point about rm -f! I'll count that as example number 1.

If you mean targets in Makefiles, make picks the first target in the
file if you don't specify any, so no.

— Reply to this email directly or view it on GitHub[1].

Links:

  1. Failed wildcards still execute #2394 (comment)

@ridiculousfish
Copy link
Member

Yes, I should have typed rm -f, thanks

@faho
Copy link
Member

faho commented Feb 19, 2016

Specifically, for rm -f, it's often okay if one of the globs matches, and if none matches it might still count as a success.

@jplews
Copy link

jplews commented Mar 3, 2016

This behaviour got me with cp where a source dir ended up empty and cp complained about not having a destination, in a situation with multiple sources that seems potentially dangerous, count seems to be the thing so all is well now.

IMO makes sense to have glob --null and additional syntax for interactive efficiency, and re-purposing ? and adding ??, although * looks more like the symbol nullglob should have, with ? for failblob.

Command substitution seems to cover everything, and would make more sense to a beginner. Having deliberately not used [ ] for my entire life I like the approach Fish is taking, and people should be pushed to using bash sub-processes like I often see with perl (even though bash has string stuff).

I also think that the documentation is incorrect still, and perhaps it would be worth mentioning count there too.

@faho
Copy link
Member

faho commented Mar 3, 2016

I also think that the documentation is incorrect still, and perhaps it would be worth mentioning count there too.

Count is mentioned in the documentation, unless you mean the ones hosted at fishshell.com, which are the ones from the last release.

@faho
Copy link
Member

faho commented May 19, 2016

There's not been new discussion in a while and I've not seen a huge need for syntactic sugar in practice (set works just fine), so I'm closing this for now. Maybe it'll come up again later.

@faho faho closed this as completed May 19, 2016
@faho faho modified the milestones: 2.3.0, next-major May 19, 2016
@faho faho added the bug Something that's not working as intended label May 19, 2016
dideler added a commit to dideler/dotfiles that referenced this issue Apr 17, 2017
Otherwise fish (2.3.0 and up) warns about non-match.
fish-shell/fish-shell#2394 (comment)
dideler added a commit to dideler/dotfiles that referenced this issue Sep 3, 2017
Otherwise fish (2.3.0 and up) warns about non-match.
See fish-shell/fish-shell#2394
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something that's not working as intended
Projects
None yet
Development

No branches or pull requests

8 participants