Crystal: Some suggestions for the new compile error formatting & output

[r00ster91]

I have some suggestions for the new compile error formatting & output introduced by #7748.

The Missing Colon

In file.cr:25:1
Error: Unterminated string literal

I think there's a colon missing here after In file.cr:25:1, those two sentences look pretty uncatenated right now. Like they are just thrown there. I think it would look better with a colon.

Inconsistency

$ crystal cr.cr
In cr.cr:2:1
Error: Unterminated string literal
$ crystal eval '"'
syntax error in eval:1
Error: Unterminated string literal

Both the eval code and cr.cr contain only " which of course raises an error. But why is there syntax error at the beginning of the eval error but not at the beginning of the cr.cr error? Doesn't really make sense, since the errors are both the same. I think it should be like the eval error but with the "s" of "syntax error" capitalized and a colon (see chapter 1, The Missing Colon), i.e. Syntax error in eval:1:

Error Indicator

Showing last frame. Use --error-trace for full trace.

In cr.cr:9:19

9 | objects << Object.ne
                      ^-
Error: undefined method 'ne' for Object.class

Did you mean 'new'?

(This is just my opinion) Here ^- doesn't look very pretty, it looks acceptable when there's at least 2 or 3 dashes after the ^ but in this case it looks a bit weird. I propose to just make this whole thing ^-only, i.e.:

Showing last frame. Use --error-trace for full trace.

In file.cr:9:23

9 | objects << Object.wjeihwdwewe
                      ^^^^^^^^^^^
Error: undefined method 'wjeihwdwewe' for Object.class

This would work for every case and probably also gets the attention of the developer faster, I think it looks more apparent. However maybe it's a bit too pointy? Probably arguable, but I feel like it's ok.

Potential Bug

In file.cr:10:1

 10 | hello
      ^----
Error: expanding macro


In file.cr:10:1

 10 | hello
      ^
Error: expanding macro


There was a problem expanding macro 'hello'

Called macro defined in file.cr:7:1

 7 | macro hello

Which expanded to:

 > 1 | wjihrwdikfojieo
Error: undefined local variable or method 'wjihrwdikfojieo' for top-level

I don't think

In file.cr:10:1

 10 | hello
      ^----
Error: expanding macro

should be shown twice here. That doesn't really add new information. Seems like a bug.

The --error-trace Message

In most cases the first error line shown is Showing last frame. Use --error-trace for full trace.. This is not really related to the error in the code and doesn't really help fixing the error itself (I mean, enabling the error trace can help but for common simple errors it's not very helpful).
I feel like it should be made a bit more unapparent by using the colorize color dark_gray for the text.

[oprypin]

Disagree with the 1st point.
file.cr:25:1 is a syntax that text editors understand, they can open the file at this exact position, if this is passed as a command line argument.
So better not mess with it, and keep it easily selectable.

[RX14]

Error Indicator

I'd actually advocate for ^^ being used when n <= 2 and ^--- for n > 2. This is literally a 1-character difference though so very likely not worth the effort.

Potential Bug

@asterite should probably chip in but this is probably a bug. If it is, create a new issue for it.

[asterite]

Yeah, the compiler sometimes includes a stack trace twice. It's been doing that for a long time already. You can create a bug for it if you want.

[r00ster91]

Oh I actually did that in the past already: #7346. I thought it's related to #7748 but seems compiler-related as well.

[r00ster91]

Are you maybe interested in this @martimatix? Since you recently touched this stuff in your PR you might know how to fix some of this.

[martimatix]

Can't contribute to this at the moment but here are some thoughts for anyone that may be interested.

I didn't know about crystal eval until I was notified about this issue. Would need to look at how eval works to fix the inconsistency.

Modifying error indicator would be straight forwards.

Since the process of printing stack traces is recursive, it would be possible to pass in the previous error message to the next frame and not print it if it is a duplicate. However, I feel this would be a stopgap solution and does not address the underlying issue of why there is a duplicate in the first place.

Altering the styling of the --error-trace message is trivial. It's worth noting that in the RFC, there was a suggestion of toggling it via an environment variable.