[m-rev.] for review: color schemes

[Zoltan Somogyi]

On 2024-06-04 17:19 +10:00 AEST, "Peter Wang" <novalazy at gmail.com> wrote:
>> Are you saying that because you think that is a useful capability,
>> or just out of general concerns about the length of the string
>> required to specify all four colors?
> 
> It's just how I expected something like this to behave.
> 
> Ignoring unassigned roles would have the benefit that if a new role is
> added, when a user updates the compiler, the compiler will continue
> working instead of refusing to do anything until the user assigns a
> color to the new role. That would be pretty annoying.

OK, that argument convinced me. I will make the change to allow
color roles to be unassigned. To allow that change to be reviewed,
I will commit this diff without that change, and post that change later today
or tomorrow.

>> One purpose is add additional info about an error, info that tells the user
>> about the usual cause of the error. For this use, the word "hint" is a perfect
>> description.
>> 
>> The other purpose occurs when we have an error that reports not that
>> some part of the user's code is definitely wrong, but that two parts of
>> the user's code are inconsistent with one another. In that case, one
>> of those pieces of code is wrong, but we don't which one, so we
>> can't color one correct and one incorrect, so we color both with possible_cause.
>> For this use, the word "possible" would work, as a shorthand for "possibly incorrect".
>> "inconsistent" would also work.
>> 
>> Should these roles be split? I am fine with either answer.
> 
> Sure, "hint" and "inconsistent" sound fine.

I will make this split, and again do so a later diff. This will take longer,
due to the need to reexamine all the places that currently use possible_cause
as a color name.

> But, ideally, --color-scheme in a Mercury.options file ought to have
> lower priority than the environment variable. I believe that Unix
> programs generally take their setting from these sources, in increasing
> order of priority:
> 
>   1. system wide config files
>   2. local config files
>   3. environment variables
>   4. command line arguments

The problem is that 1, 2 and 4 record their decisions in the option table,
but 3 does not. With the option table, later options override earlier ones,
but this is not true for 3.

The obvious solution would be to turn env var values into options,
and stick them into the argument list given to getopt.m in position listed above.
That works in most respects, but it does have the drawback that any errors
in the *arguments* of those options cannot correctly attribute the location
of the error: if the diagnostic says something about a bug in an option value,
that is misleading if the option was constructed by the compiler for an envvar,
and vice versa. This is not a problem for options without arguments, or for
options with arguments whose values are not subject to any checks,
but --color-scheme has an argument that *is* subject to checks.

I suppose I can solve the problem by making --color-scheme be a special
option that sets two other options, one that records its argument value
and a bool option that says "that argument came from an option". The code
that sets the argument value from the envvar would then *clear* the bool
option.

> mmc --make also follows that, at least the manual says:
> 
>     All variables in ‘Mercury.options’ are treated as if they are
>     assigned using ‘:=’. Variables may also be set in the environment,
>     overriding settings in options files.

I will have a look to see how it does it, but I have a hunch that it ignores
the problem I just outlined.

> Anyway, I expect users will set MERCURY_COLOR_SCHEME in the shell
> startup file once, and --color-scheme will roughly never be used.
> So it doesn't actually matter in practice.

Agreed.

>> > Is there any reason to keep the --set-color-* options?
>> 
>> We have to keep them, because that is the only way that write_error_spec.m
>> has to access the chosen color scheme. However, we don't have to let users know
>> about them. We don't even need them to be specifiable on the command line;
>> the system should work without that. If we made them unspecifiable in that way,
>> then obviously they wouldn't be an input to the color selection process;
>> they would just record its output.
> 
> Ok, let's make --set-color-* unspecifiable.

Done.

> BTW, I had another look at https://no-color.org
> According to their informal standard,
> we should disable color if NO_COLOR != "",
> but then color *may* still be enabled by command line options.

This can be done by turning a nonempty value of NO_COLOR
into the --no-enable-color-diagnostics option in position 3 of your list above.
I will do handle it in a third later diff, together with MERCURY_COLOR_SCHEME.

Thanks for the review.

Zoltan.