Some changes fix actual problems, e.g. missing spaces in square bracket tests,
and backticks unintentionally causing code execution when intended as formatting.
Others, such as conservative quoting probably work fine in the old version in
most situations, but it's nice to have some additional safety.
Using `{ ..; }` instead of `(..)` is just a small performance enhancement.
Many of these issues were identified by shellcheck, which might be useful in CI
as well.
We assume that you use something like hyperfine to run warmups, like
our driver script does.
This allows the script to be run e.g. in valgrind without being too
much of a pain in the gluteus.
To allow paths with spaces, give a shell-quoted path to hyperfine. We
could make this a bit shorter by quoting as early as possible, like
FISH_PATH=$(quote "$1")
and then use $FISH_PATH unquoted. I discarded that idea because it
probably looks surprising.
Closes#8559
Results after 14908322a9, compared to 3.1.2:
math.fish
fish
rusage self:
user time: 916 ms
sys time: 39 ms
total time: 955 ms
max rss: 35028 kb
signals: 0
build/fish
rusage self:
user time: 769 ms
sys time: 60 ms
total time: 829 ms
max rss: 34868 kb
signals: 0
Benchmark #1: fish benchmarks/benchmarks/math.fish > /dev/null
Time (mean ± σ): 955.2 ms ± 32.5 ms [User: 897.2 ms, System: 57.9 ms]
Range (min … max): 896.3 ms … 1002.5 ms 10 runs
Benchmark #2: build/fish benchmarks/benchmarks/math.fish > /dev/null
Time (mean ± σ): 840.3 ms ± 21.5 ms [User: 784.4 ms, System: 54.8 ms]
Range (min … max): 802.4 ms … 869.0 ms 10 runs
Summary
'build/fish benchmarks/benchmarks/math.fish > /dev/null' ran
1.14 ± 0.05 times faster than 'fish benchmarks/benchmarks/math.fish > /dev/null'
This did some weird unescaping to try to extract the first word.
So we're now more likely to be *correct*, and the alias benchmark is
about 20% *faster*.
Call it a win-win.