[m-rev.] for possible post-commit review: handle color changes next to prefixes/suffixes

Julien Fischer jfischer at opturion.com
Sat May 4 01:02:38 AEST 2024



On Sat, 4 May 2024, Zoltan Somogyi wrote:

>
> On 2024-05-04 00:40 +10:00 AEST, "Julien Fischer" <jfischer at opturion.com> wrote:
>>
>> On Fri, 3 May 2024, Zoltan Somogyi wrote:
>>
>>> Handle color changes next to prefixes/suffixes.
>>>
>>
>> That looks fine.
>
> Thanks. The attached diff to a comment should have been part
> of that change.

> diff --git a/compiler/write_error_spec.m b/compiler/write_error_spec.m
> index 45f356510..54f368e47 100644
> --- a/compiler/write_error_spec.m
> +++ b/compiler/write_error_spec.m
> @@ -1011,6 +1011,21 @@ convert_words_to_paragraphs_acc(ColorDb, [Word | Words],
>          record_text_word(TextWord, !Done, !InWork)
>      ;
>          Word = word_color(ColorChange),
> +        % Note how with color_start, we want to put any space *before*
> +        % the color change, while with color_end, we want to put any space
> +        % *after* the color change. The objective is to enable color
> +        % only for the shortest span of characters to accomplish the
> +        % intended coloring effect. This may not matter much when the
> +        % output we generate gets viewed by a user, but it matters when
> +        % we as developers work with .err_exp files in the test directories
> +        % that test diagnostic outputs.
> +        %
> +        % It simply easier to visually check whether the output of a diff

It *is*

> +        % between an .err file containing new or updated diagnostic output
> +        % and the existing .err_exp file contains just the expected changes
> +        % if neither file contains color change escape sequences that are
> +        % effectively no-ops. (The addition or deletion of such a no-op
> +        % sequence would show up in the diff as a red herring.)
>          (
>              ColorChange = color_start(_),
>              Done0 = !.Done,

Julien.


More information about the reviews mailing list