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

Mixing the && and || operators with "and"/"or" does not behave #6089

Closed
Phidica opened this issue Sep 4, 2019 · 5 comments
Closed

Mixing the && and || operators with "and"/"or" does not behave #6089

Phidica opened this issue Sep 4, 2019 · 5 comments
Assignees
Labels
bug
Milestone

Comments

@Phidica
Copy link
Contributor

@Phidica Phidica commented Sep 4, 2019

Tried using tagged release fish 3.0.2 as well as current master, built from source on Fedora 29, running in Gnome Terminal.

Problem

The command true && and true sets the exit code to 1 but is otherwise not indicated to be misbehaving.

Changing the command to true && and echo word reveals that the final statement is never executed (nothing is printed). Running true && and exits with code 2 instead and prints the help information for and. Running true && or exits with code 2 and prints the help info for or.

Similar results are observed with ||. false || and and false || or exit with code 2 and print the help info for and or or.

In all these cases, a command placed after the and or or is never executed.

Expected behaviour

I think that these two constructs shouldn't be allowed to be mixed in this way, as the results are unintuitive. The parser should expressly reject and or or when they follow && or ||, to prevent users accidentally creating structures that don't behave as they may anticipate, with commands that never execute.

@zanchey
Copy link
Member

@zanchey zanchey commented Sep 5, 2019

Yeah, that should be a syntax error.

@zanchey zanchey added the bug label Sep 5, 2019
@zanchey zanchey added this to the fish-future milestone Sep 5, 2019
@mqudsi
Copy link
Contributor

@mqudsi mqudsi commented Sep 6, 2019

Nice catch. It's more generic than that, things like true; and and foo are also accepted but broken.

@Phidica
Copy link
Contributor Author

@Phidica Phidica commented Sep 7, 2019

Indeed, chaining an arbitrary number of ands together used to be okay in fish 2.7. Now it appears the first is considered an operator, the second a command, and rest are arguments to the command. Naturally, that's broken.

@ridiculousfish ridiculousfish self-assigned this Sep 8, 2019
@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Sep 8, 2019

What's going on here is that and is recognized syntactically at parse time, but also has a double-life as a builtin. and is normally parsed as a "job decorator" but here it's becoming an ordinary command. For example true && and --help will print and's help.

Rejecting and as a command seems smart to me. Nice catch.

Speaking of which and --help is broken, bleh.

@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Sep 8, 2019

and --help fixed as fa6bac1

@ridiculousfish ridiculousfish removed this from the fish-future milestone Sep 8, 2019
@ridiculousfish ridiculousfish added this to the fish 3.2.0 milestone Sep 8, 2019
@zanchey zanchey removed this from the fish 3.2.0 milestone Sep 9, 2019
@zanchey zanchey added this to the fish 3.1.0 milestone Sep 9, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug
Projects
None yet
Development

No branches or pull requests

4 participants