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

math no longer supports logical expressions #4777

Closed
mqudsi opened this Issue Mar 4, 2018 · 2 comments

Comments

Projects
None yet
3 participants
@mqudsi
Copy link
Contributor

mqudsi commented Mar 4, 2018

I'm not sure if @krader1961's math implementation with muparser featured it, but the released version of fish that uses bc for math supports logical tests like math 1 == 1, which the new tinyexpr-based builtin doesn't.

Under fish 2.7.0, the following output is generated:

~> math 1 == 1; echo $status;
1
0
~> math 1 == 0; echo $status;
0
1

(I stumbled across an old script of mine which used math instead of test to evaluate the return value of an external command.)

Not sure what we want to do about this or how hard it would be to add in support, but in all cases we should make a decision either way whether this functionality is explicitly dropped or not. Naturally things like math 1 >= 0 never worked thanks to the redirection operator (though it would trivial to fix that if we end up choosing to keep logical tests in math functional).

@faho

This comment has been minimized.

Copy link
Member

faho commented Mar 4, 2018

I'm not sure if @krader1961's math implementation with muparser featured it

It did. However, it did not return a failing status for a result of 0, so it wasn't really useful as a test. "a == b" just printed "1" when it was true and "0" otherwise.

The math function returned a status of 1 for a result of 0:

case 0
    # For historical reasons a zero result translates to a failure status.
    echo 0
    return 1

No matter the function. "==" just happens to have a result of 0 if the operands aren't equal.

how hard it would be to add in support

For "==", actually surprisingly hard. This does char-comparison for operators to avoid having to strip whitespace or stop at the next token, so operators that are more than one character long pose a bit of a problem (I've thought about adding ** as an alternative for exponentiation).

we should make a decision either way whether this functionality is explicitly dropped or not. Naturally things like math 1 >= 0 never worked thanks to the redirection operator (though it would trivial to fix that if we end up choosing to keep logical tests in math functional).

In that case, I vote for dropping it. This is duplicated functionality, and always relied on a weird quirk (returning 1 on zero).

Use an explicit test.

@faho faho added the RFC label Mar 5, 2018

@ridiculousfish

This comment has been minimized.

Copy link
Member

ridiculousfish commented Mar 8, 2018

Yes, this is what test is for. math doesn't need to do this.

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