[m-rev.] for post-commit review: add color in det_report.m

[Zoltan Somogyi]

On 2024-04-29 20:32 +10:00 AEST, "Julien Fischer" <jfischer at opturion.com> wrote:
> The diff is fine, although looking at it, and the output of yesterday's
> ROTD, and have some thoughts:
> 
> - If something is colored, does it also need to be enclosed in `'
>   quotes?

If we want that something to be clearly delineated as the subject
that the rest of the message is about *even if colors are not enabled*,
then yes, at least for now. We could change that if we had a mechanism
in error_spec.m that says *wrap these pieces in this color if color is
enabled, and if not, wrap them in quotes*. However, that would not be
a trivial exercise, because the scope of the quotes and the scope of
the color changes are slightly different. We normally put quotes
around just names, while in most already-colored error messages,
the coloring extends to the punctuation following the name as well.
We could change that, but we would also need to decide what to do
with error messages where the color extends to a whole phrase,
and not just a name. (This is for cases where what is wrong is not the name,
but what the code is doing, or not doing, with the named entity.) In such cases,
the scope of the quotes and of the color change *have* to be different.
Having a test for "is color enabled" and then two distinct pieces
of code constructing the pieces for each case works, but it is also ugly.

Just for context:

I have tried to arrange it so that error messages about incorrect arities,
which have the form "... has the wrong arity (3, should be 2 or 4),
have the 3 colored as incorrect and 2/4 are colored as correct, but
doing so is not easy. The reason is that we add e.g. 3 just after the (
using a suffix piece, but the color change between the ( and the 3
screws up the correspondence. I am sure it can be fixed, but doing so
demands significant changes to the code of write_error_spec.m.

Of course, in some error messages, the quotes do not improve
readability, and can go. I am working on a change which does that,
which should be ready to post soon.

> - Should entities from the program source, notably program or type
>   variables, be colored when they are the subject of the error message?
> 
> Consider the following:
> 
> integral_constant_no_suffix.m:016: In clause for predicate `main'/2:
> integral_constant_no_suffix.m:016:   in argument 1 of call to predicate
> integral_constant_no_suffix.m:016:
> `integral_constant_no_suffix.life_universe_everything'/1:
> integral_constant_no_suffix.m:016:   type error: variable `N' has type
> integral_constant_no_suffix.m:016:     int;
> integral_constant_no_suffix.m:016:   expected type was
> integral_constant_no_suffix.m:016:     uint8.
> integral_constant_no_suffix.m:016:   A integer constant that consists
> only of
> integral_constant_no_suffix.m:016:   digits is always of type `int'.
> Unsigned
> integral_constant_no_suffix.m:016:   integer constants of the default
> size
> integral_constant_no_suffix.m:016:   should have the suffix `u';
> constants of
> integral_constant_no_suffix.m:016:   sized integer types should have an
> `i8',
> integral_constant_no_suffix.m:016:   `i16', `i32' or `i64' suffix if
> they are
> integral_constant_no_suffix.m:016:   signed, and an `u8', `u16', `u32'
> or `u64'
> integral_constant_no_suffix.m:016:   suffix if they are unsigned.
> 
> "int" and "uint8" are now colored, but nothing else is.  I think there
> is a case to be made that the variable "N", which is the subject of the
> message should stand out mode.

That is a good idea. Internally, we could call that color "color_subject",
as it denotes the subject of the error messsage. Does anyone have
a better name in mind? And what should the default color shade for be?
Some kind of blue, given that red, green and yellow are already in use
for other things? Or should we rejig the existing colors?

> Also, if all the syntax fragments in the above message were colored (and
> assuming that the color scheme is sensible), then they should be
> distinct enough from the surrounding text without the cluter of the
> quotes.

Again, we need to handle color being off; we don't want to make
error messages worse for e.g. color-blind people for whom the colors
they *can* see are not distinct enough to be all that helpful.

Thanks for the review. Please keep'em coming.

Zoltan.