This would print the default "Argument is invalid" error string, which
is *true* but not super obvious, because `test` doesn't always perform
numeric conversion, and that's the bit that failed here.
Prior to this change, builtins would take their arguments as `wchar_t **`.
This implies that the order of the arguments may be changed (which is
true, `wgetopter` does so) but also that the strings themselves may be
changed, which no builtin should do.
Switch them all to take `const wchar_t **` instead: now the arguments may
be rearranged but their contents may no longer be modified.
builtin_test stashes some variables in statics, to support
the `test -t` expression. However this will cause conflicts with
concurrent execution, where we may want to run two `test` expressions at
once. Do the grunt work of threading the data into all places it needs
to go.
Since builtins don't actually have the streams connected, but instead
read input via the io_streams_t objects, this would just always say
what *fish's* fds were.
Instead, pass along some of the stream data to check those
specifically - nobody cares that `test`s fd 0 *technically* is stdin.
What they want to know is that, if they used another program in that
place, it would connect to the TTY.
This is pretty hacky - I abused static variables for this, but
since it's two bools and an int it's probably okay.
See #1228.
Fixes#4766.
I really kinda hate how insistent clang-format is to have line
breaks *IFF THE LINE IS TOO LONG*.
Like... lemme just add a break if it looks better, will you?
But it is the style at this time, so we shall tie an onion to our
belt.
We are 1-indexed, and so it's weird to have `test` count its arguments
from 0.
For `test 1 =` this changes the error from
test: Missing argument at index 2
1 =
^
to
test: Missing argument at index 3
1 =
^
test loves error messages like
test: Missing argument at index 2
without explaining where that "index 2" is.
So now, we print the arguments below that, with a caret pointing to
the place where the error occured.
For example:
> test 1 = 2 and echo true or echo false
test: Expected a combining operator like '-a' at index 3
1 = 2 and echo true or echo false
^
(Type 'help test' for related documentation)
Fixes#6030.
I kinda hate how fussy clang-format is. It reflows text
constantly (line limit), forces things onto one line *except* when
they're too long, and wants to turn this:
```c++
return true;;
```
into this:
```c++
return true;
;
```
instead of, you know, eliminating the second semicolon?
Anyway, it is what it is and we use it, I'll just look into getting some
more slack.
This runs build_tools/style.fish, which runs clang-format on C++, fish_indent on fish and (new) black on python.
If anything is wrong with the formatting, we should fix the tools, but automated formatting is worth it.
Return STATUS_INVALID_ARGS when failing due to evaluation errors,
so we can tell the difference between an error and falseness.
Add a test for the ERANGE error
We were not parsing an in-range number when we claimed we were,
and were thus failing to error with invalid numbers and returned
a wrong test result. Fixed#5414
Also, provide the detail we can for the other error cases.
This reverts commit 1cb8b2a87b.
argv[0] has the full path in it for a user when he executes it
out of $PATH. This is really annoying in the title which uses $_.
... rather than hard code it to "fish". This affects
what is found in $_ and improves the errors:
For example, if fish was ran with ./fish, instead of
something like:
fish: Expected 3 surprises, only got 2 surprises
we'll see:
./fish: Expected 3 surprises, only got 2 surprises
like most other shell utilities. It's just a tiny bit
of detail that can avoid confusion.
This changes all of the builtins to behave like `string` to return
STATUS_INVALID_ARGS (121) if the args passed to the command don't make
sense. Also change several of the builtins to use the existing symbols
(e.g., STATUS_CMD_OK and STATUS_CMD_ERROR) rather than hardcoded "0"
and "1" for consistency and to make it easier to find such values in
the future.
Fixes#3985
This primarily replaces "STATUS_BUILTIN_OK" with "STATUS_CMD_OK" and
"STATUS_BUILTIN_ERROR" with "STATUS_CMD_ERROR". That is because we want
to make it clear these status codes are applicable to fish functions as
well as builtins. Future changes will make it easier to use these
symbols and values in functions.
Defining aliases for existing symbols serves only to obscure the code.
So remove the following symbols and replace them with the primary
symbols:
enum { BUILTIN_TEST_SUCCESS = STATUS_BUILTIN_OK, BUILTIN_TEST_FAIL =
STATUS_BUILTIN_ERROR };
See issue #3985.
I recently upgraded the software on my macOS server and was dismayed to
see that cppcheck reported a huge number of format string errors due to
mismatches between the format string and its arguments from calls to
`assert()`. It turns out they are due to the macOS header using `%lu`
for the line number which is obviously wrong since it is using the C
preprocessor `__LINE__` symbol which evaluates to a signed int.
I also noticed that the macOS implementation writes to stdout, rather
than stderr. It also uses `printf()` which can be a problem on some
platforms if the stream is already in wide mode which is the normal case
for fish.
So implement our own `assert()` implementation. This also eliminates
double-negative warnings that we get from some of our calls to
`assert()` on some platforms by oclint.
Also reimplement the `DIE()` macro in terms of our internal
implementation.
Rewrite `assert(0 && msg)` statements to `DIE(msg)` for clarity and to
eliminate oclint warnings about constant expressions.
Fixes#3276, albeit not in the fashion I originally envisioned.
