error messages should provide more context (e.g., the function, file and line number)

[krader1961]

I've noticed that people are mystified by error messages like the following (from issue #2859):

test: Missing argument at index 2

I've seen such errors myself during the startup of a fish session and it was overly difficult to figure out what was wrong with my config.fish.

For error messages generated by fish, as opposed to external programs, it would be extremely helpful if they included some context. Such as the file and line number of the code being executed and/or the function being run at the time of the error.

[ridiculousfish]

The test case is weird because this is a syntax error in test's little DSL, not a syntax error in fish. So either test would have to print the backtrace, or fish would have to specially recognize and interpret test's return code. Either one feels like a layering violation.

[krader1961]

Either one feels like a layering violation.

No argument from me. But sometimes practicality trumps CS design principles. I'm willing to bet that the single most perplexing problem fish users experience is test emitting a diagnostic and there is no context to help identify which test invocation was at fault. And there are plenty of other examples. Such as character encoding/decoding errors.

I'll admit I haven't actually thought much about how to actually implement this. My hope would be that the debug() function would be able to ascertain the context that produced the message and automatically include it in the output. That way, assuming the builtin test code uses debug(0, ...) it wouldn't need to know about such things. Alternatively, we could add an additional context parameter that test could use when emitting an error.

P.S., This is probably a good time to repeat my observation in a different issue that how the debug() function is used is weird. I'd much rather see something akin to the Python logging subsystem where you pass a parameter that explicitly states whether its an error, warning, info, or debug level message. Alternatively, introduce error_msg(), warning_msg(), info_msg(), and debug_msg() functions (the actual names aren't important) with only the debug variant taking a verbosity level. Either of those approaches would greatly improve the clarity of the code.

[faho]

It does, since 785945c.

test is such a frequent source of bugs that I feel it's justified.

[floam]

Is it useful to have the backtrace show if we are interactive and input was stdin?

The backtrace output is pretty jarring IMO. It just kind of appears out of nowhere: It does not look like the short and sweet output you expect from the test builtin, but it's actually not very rich output either.

$ foo
test: Missing argument at index 2

~/.config/fish/functions/foo.fish (line 3): 
    test 1.3 -lt; 
    ^
in function 'foo'

(Type 'help test' for related documentation)
test: Missing argument at index 2

~/.config/fish/functions/foo.fish (line 4): 
test 1.4 -gt;
^
in function 'foo'

(Type 'help test' for related documentation)

An improvement which would make it less confusing would be to display the error in the backtrace as part of the flow like we do in parse errors. It might look like:

~/.config/fish/functions/foo.fish (line 3): test: Missing argument at index 2
    test 1.3 -lt;
    ^

Also, the carats always only point at the beginning of the current line, which is always a test command. It is not useful information. It is lame but also potentially confusing because we actually have errors that report about specific indices in the command line arguments, yet our the cursor helpfully points somewhere else. :(