Re: [PATCH 2/2] Highlight keywords in remote sideband output.

[Junio C Hamano <gitster@pobox.com>]

Han-Wen Nienhuys <hanwen@google.com> writes:

> On Wed, Aug 1, 2018 at 5:41 PM Junio C Hamano <gitster@pobox.com> wrote:
>
>> Hmm, do we actually say things like "Error: blah"?  I am not sure if
>> I like this strncasecmp all that much.
>
> this is for the remote end, so what we (git-core) says isn't all that
> relevant.

It is very relevant, I would think.  Because the coloring is
controlled at the client end with this implementation, third-party
remote implementations have strong incentive to follow what our
remote end says and not to deviate.  Preventing them from being
different just to be different does help the users, no?

>> > So, despite the explanation in the commit message, this function isn't
>> > _generally_ highlighting keywords in the sideband. Instead, it is
>> > highlighting a keyword only if it finds it at the start of string
>> > (ignoring whitespace). Perhaps the commit message could be more clear
>> > about that.
>>
>> Sounds good.
>>
>> I didn't comment on other parts of your review posed as questions
>> (that require some digging and thinking), but I think they are all
>> worthwhile thing to think about.
>
> Sorry for being dense, but do you want me to send an updated patch or
> not based on your and Eric's comments or not?

It would help to see the comments responded with either "such a
change is not needed for such and such reasons", "it may make sense
but let's leave it to a follow-up patch later," etc., or with a
"here is an updated patch, taking all the comments to the previous
version into account---note that I rejected that particular comment
because of such and such reasons".