Don't skip caret for some errors

This checked specifically for "| and" and "a=b" and then just gave the
error without a caret at all.

E.g. for a /tmp/broken.fish that contains

```fish
echo foo

echo foo | and cat
```

This would print:

```
/tmp/broken.fish (line 3): The 'and' command can not be used in a pipeline
warning: Error while reading file /tmp/broken.fish
```

without any indication other than the line number as to the location
of the error.

Now we do

```
/tmp/broken.fish (line 3): The 'and' command can not be used in a pipeline
echo foo | and cat
           ^~^
warning: Error while reading file /tmp/broken.fish
```

Another nice one:

```
fish --no-config -c 'echo notprinted; echo foo; a=b'
```

failed to give the error message!

(Note: Is it really a "warning" if we failed to read the one file we
wer told to?)

We should check if we should either centralize these error messages
completely, or always pass them and remove this "code" system, because
it's only used in some cases.
This commit is contained in:
Fabian Boehm 2022-08-12 17:04:30 +02:00
parent 232ca25ff9
commit 8d7416048d
4 changed files with 26 additions and 3 deletions

View file

@ -44,14 +44,17 @@ parse_error_code_t parse_error_from_tokenizer_error(tokenizer_error_t err) {
wcstring parse_error_t::describe_with_prefix(const wcstring &src, const wcstring &prefix,
bool is_interactive, bool skip_caret) const {
wcstring result = prefix;
// Some errors don't have their message passed in, so we construct them here.
// This affects e.g. `eval "a=(foo)"`
switch (code) {
default:
if (skip_caret && this->text.empty()) return L"";
result.append(this->text);
break;
case parse_error_andor_in_pipeline:
append_format(result, INVALID_PIPELINE_CMD_ERR_MSG,
src.substr(this->source_start, this->source_length).c_str());
return result;
break;
case parse_error_bare_variable_assignment: {
wcstring assignment_src = src.substr(this->source_start, this->source_length);
maybe_t<size_t> equals_pos = variable_assignment_equals_pos(assignment_src);
@ -60,10 +63,9 @@ wcstring parse_error_t::describe_with_prefix(const wcstring &src, const wcstring
wcstring value = assignment_src.substr(*equals_pos + 1);
append_format(result, ERROR_BAD_COMMAND_ASSIGN_ERR_MSG, variable.c_str(),
value.c_str());
return result;
break;
}
}
result.append(this->text);
size_t start = source_start;
size_t len = source_length;

View file

@ -4,9 +4,13 @@ set -xl LANG C # uniform quotes
eval 'true | and'
# CHECKERR: {{.*}}: The 'and' command can not be used in a pipeline
# CHECKERR: true | and
# CHECKERR: ^~^
eval 'true | or'
# CHECKERR: {{.*}}: The 'or' command can not be used in a pipeline
# CHECKERR: true | or
# CHECKERR: ^^
# Verify and/or behavior with if and while
if false; or true

View file

@ -94,3 +94,14 @@ $fish --no-config -c 'echo $$ oh no syntax error' -c 'echo this works'
$fish --no-config .
# CHECKERR: error: Unable to read input file: Is a directory
# CHECKERR: warning: Error while reading file .
$fish --no-config -c 'echo notprinted; echo foo; a=b'
# CHECKERR: fish: Unsupported use of '='. In fish, please use 'set a b'.
# CHECKERR: echo notprinted; echo foo; a=b
# CHECKERR: ^~^
$fish --no-config -c 'echo notprinted | and true'
# CHECKERR: fish: The 'and' command can not be used in a pipeline
# CHECKERR: echo notprinted | and true
# CHECKERR: ^~^

View file

@ -87,10 +87,16 @@ complete -C'a=b envxalias '
# Eval invalid grammar to allow fish to parse this file
eval 'a=(echo b)'
# CHECKERR: {{.*}}: Unsupported use of '='. In fish, please use 'set a (echo b)'.
# CHECKERR: a=(echo b)
# CHECKERR: ^~~~~~~~~^
eval ': | a=b'
# CHECKERR: {{.*}}: Unsupported use of '='. In fish, please use 'set a b'.
# CHECKERR: : | a=b
# CHECKERR: ^~^
eval 'not a=b'
# CHECKERR: {{.*}}: Unsupported use of '='. In fish, please use 'set a b'.
# CHECKERR: not a=b
# CHECKERR: ^~^
complete -c foo -xa '$a'
a=b complete -C'foo '