Location: Highlight multi-line locations

[Julow]

This PR changes the code highlighting in error messages to highlight several lines using the red underline made of ^. Previously, this was disabled as soon as the location spanned more than one line and was replaced with an echo of the code with the part outside of range replaced with dots (.).

I find the dots delimiting the code to look at to be very confusing. Code around is hidden and it's hard to recognise the original code. The dots stick to my mind and distract me from even remembering what was the error.

The new messages are more sparse but the regularity of the highlighting helps skip the code to the error message and back.

Before:

File "./test.ml", lines 4-5, characters 2-15:
4 | ..print_lines (List.map string_of_int [ 1; 2; 3; 4; 5 ])
5 |   print_endline......

After:

File "./test.ml", lines 4-5, characters 2-15:
4 |   print_lines (List.map string_of_int [ 1; 2; 3; 4; 5 ])
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5 |   print_endline "foo"
      ^^^^^^^^^^^^^

[gasche]

I'm interested in hearing that there is dissatisfaction with the previous multiline printing strategy, but I'm not convinced that the new one is a strong improvement. In particular, it double the number of lines printed, which I think could be annoying in practice in terminals (or CI logs, etc.).

Looking at the testsuite changes (plenty of examples of modified multi-line quotations), I see that almost always the characters before the quotation start on the first line are whitespace. We could special-case this situation and not print dots in this case, only the whitespace. At the end of the quotation, all testsuite examples with remaining dots have a ;; after the quotation. We could special-case this but I think it is more ad-hoc and there is less value -- our use of toplevel inputs in the testsuite makes ;; over-represented compared to normal OCaml code.

To summarize:

• I'm not fond of the new proposal.
• Okay, let's think about whether we can do something about those dots you find confusing.
• We should start by not using dots for whitespace, and see where that gets us in practice.

[Armael]

Related: #9118 . AFAIR the initial motivation was different, but there we also ended up discussing the dot situation, thought about not using dots for whitespace, then you (!) rightfully pointed out that it would end up looking weird in many cases. Then we finally agreed on some fairly involved strategy, which I think I timed out trying to implement...

[Armael]

I'm also open to improving the current situation for multi-line error but are not super convinced by the new proposal.
What do other languages do for highlighting multi-line errors? (I investigated that in the past but have now forgotten; also the situation may have improved in the meantime) @Julow , would you maybe have time to collect some samples for comparison? (off the top of my head, possibilities might be Rust, Haskell, C++/gcc/clang, elm (?))

[dbuenzli]

I find @Julow's proposal much better. By showing you the context it makes it easier to rematch the location in your source.

Why not use what he proposes when OCAML_COLOR is never and bold (and/or colors) to highlight when colors can be used (and thus prevent the line doubling) ?

[gasche]

When the terminal is clever, it is also possible to underline without using any extra vertical space, this is what the toplevel uses in particular. That form would also be fine.

[Julow]

I've played a bit with https://godbolt.org/ and I find that (non-parsing) error messages are generally short and straight to the point (Go, Java, Python, Scala, etc...) and code highlighting is not polished, if any (eg. no color, no highlighting, no hints).
But the languages you mentionned are interesting.

Rust highlight the code in a compact way but is not afraid to add empty lines, which are also nice for readability. It even has a second multi-line bit of code as an example of correct code. It has a compiler error index, CC ocaml/RFCs#33.

error[E0005]: refutable pattern in local binding: `None` not covered
--> <source>:9:9
   |
9  |       let Some(
   |  _________^
10 | |             y
11 | |             ) = x;
   | |_____________^ pattern `None` not covered
   |
   = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
   = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
note: `Option<Option<i32>>` defined here
   = note: the matched value is of type `Option<Option<i32>>`
help: you might want to use `if let` to ignore the variant that isn't matched
   |
9  ~     let y = if let Some(
10 |             y
11 ~             ) = x { y } else { todo!() };
   |

C++ has a message similar to mine :) Locations are bold, messages are blue and indented, it's very easy to jump between messages with the eyes while ignoring highlighted code and hints.

<source>: In function 'int main()':
<source>:8:11: error: too many arguments to function 'int square(int)'
    8 |     square(
      |     ~~~~~~^
    9 |         1,
      |         ~~ 
   10 |         2
      |         ~  
   11 |     )
      |     ~      
<source>:2:5: note: declared here
    2 | int square(int num) {
      |     ^~~~~~
<source>:11:6: error: expected ';' before '}' token
   11 |     )
      |      ^
      |      ;
   12 | }
      | ~     

Elm has similar message to OCaml, with a few more empty lines. For some syntax errors, it shows an example of code. It also seems that error messages have identifier internally because the editor has a short name for it (not a multiline error):

I am trying to parse a declaration, but I am getting stuck here:

3| _ =

   ^
When a line has no spaces at the beginning, I expect it to be a declaration like
one of these:

    greet : String -> String
    greet name =
      "Hello " ++ name ++ "!"
    
    type User = Anonymous | LoggedIn String

Try to make your declaration look like one of those? Or if this is not supposed
to be a declaration, try adding some spaces before it?

Haskell don't seem to have multi-line highlighting but has very detailed messages about the types (quite hard to read for me).

[Julow]

I think removing the dots for whitespaces at the beginning of lines is enough to fix #9118. This PR should also fix it.

It can be a good idea to remove the highlighting completely when in a "quiet" mode (eg. COLOR=never, or some configuration that CI might give) but CI generally run with --warn-error (due to Dune) and the compiler like stopping at the first error. I wouldn't expect a spam of error messages.

Clever terminal highlighting would bring concise messages but they would disappear on copy/paste. Rust has a good compromise, it adds a line at the beginning of the span and one at the end (in exchange it also adds two empty lines). The bar on the left of the code has the property of being easy to skip.

[Julow]

Not ready for review.

I rebased this PR because I think the changes in the printing algorithm can help implement #11918 and #11917.