[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