Fix compatibility with fish 3 #91
Conversation
The former is deprecated in fish 3.0, and can even optionally be turned off.
Fish 3's new math builtin defaults to float output (with trailing 0s
removed), and modulo on things with a non-integer component retains
that component, i.e.
math 3 % 2 # is 1
math 3.5 % 2 # is 1.5 - the ".5" is just not touched
This requires fish 2.4.0, I'm not sure if anything else in pure does.
Alternatively, one could `string replace -r '\..*' ''` on every math
call here.
| set -l minutes (math "$milliseconds / 60000 % 60") | ||
| set -l hours (math "$milliseconds / 3600000 % 24") | ||
| set -l days (math "$milliseconds / 86400000") | ||
| set -l seconds (math -s0 "$milliseconds / 1000 % 60") |
There was a problem hiding this comment.
Do you mind using long option format in order to make code less cryptic and easier to contribute to?
| set -l seconds (math -s0 "$milliseconds / 1000 % 60") | |
| set -l seconds (math --scale=0 "$milliseconds / 1000 % 60") |
There was a problem hiding this comment.
We shouldn't, because fish 2.4.0's math actually does not support that. So that would bump the required version to 2.7.0 (the first (and only) version of math to use argparse).
| set -l hours (math "$milliseconds / 3600000 % 24") | ||
| set -l days (math "$milliseconds / 86400000") | ||
| set -l seconds (math -s0 "$milliseconds / 1000 % 60") | ||
| set -l minutes (math -s0 "$milliseconds / 60000 % 60") |
There was a problem hiding this comment.
| set -l minutes (math -s0 "$milliseconds / 60000 % 60") | |
| set -l minutes (math --scale=0 "$milliseconds / 60000 % 60") |
| set -l days (math "$milliseconds / 86400000") | ||
| set -l seconds (math -s0 "$milliseconds / 1000 % 60") | ||
| set -l minutes (math -s0 "$milliseconds / 60000 % 60") | ||
| set -l hours (math -s0 "$milliseconds / 3600000 % 24") |
There was a problem hiding this comment.
| set -l hours (math -s0 "$milliseconds / 3600000 % 24") | |
| set -l hours (math --scale=0 "$milliseconds / 3600000 % 24") |
| set -l seconds (math -s0 "$milliseconds / 1000 % 60") | ||
| set -l minutes (math -s0 "$milliseconds / 60000 % 60") | ||
| set -l hours (math -s0 "$milliseconds / 3600000 % 24") | ||
| set -l days (math -s0 "$milliseconds / 86400000") |
There was a problem hiding this comment.
| set -l days (math -s0 "$milliseconds / 86400000") | |
| set -l days (math --scale=0 "$milliseconds / 86400000") |
|
Our CI runs on Ubuntu trusty/ Our package manager tests suite runs on a container based on ❯ docker run --rm colstrom/fish --version
fish, version 2.7.1
❯ docker run --rm edouardlopez/pure-fish fish --version
fish, version 2.7.1 My opinion is to officially support only the versions against our tests run, i.e. to this date, only So I there is no reason to block this issue on this matter. We can discuss this further in a dedicated issue #93. |
|
Thanks didn't notice that. For now, I will add a requirement for fish However, why do you mention the |
Hi!
The upcoming fish 3 has a rather large number of changes, some of them not entirely backwards compatible.
Among them:
^to redirect stderr is deprecated and can be turned off via a new "feature flags" system. The intention is to default it to off in the next major release after 3.0, and to remove it entirely in the one after that.mathis now a builtin (which makes pure about 5x faster), and defaults to float output (with trailing 0s removed - so e.g.5.0000will come out as5)Pure hits both of this, this PR rectifies that.
I'm not sure if you still support fish < 2.4 (meaning 2.3, as that's what introduced
string), the second commit requires 2.4. If you prefer, I can add a solution usingstringthat's a teensy bit uglier.