Re: [PATCH v5 2/2] sideband: highlight keywords in remote sideband output

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 6, 2018, 17:51 UTC
Message-ID: <xmqq36vrpemu.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAFQ2z_Nrj5WhzYsxr6P5qLJ3pDVFzB4UDb9g4P-5U99wrAuB0w@mail.gmail.com>

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

Show 10 quoted lines

>> I think the code is OK without any assert() or BUG(), and that is
>> because the design is "we just paint the keyword at the beginning of
>> what the other side of the sideband wants us to show as a single
>> unit".  If the other side sends a payload with an embedded LF in a
>> single packet, that's their choice and we are free not to paint the
>> beginning of the second line after that LF.  So from that point of
>> view, perhaps we shouldn't even talk about "a single line only".
>
> I don't understand this remark. Isn't the call to strpbrk() meant to
> split the input on line endings?

Yes, but that happens only for band #2 and not band #3, to which this coloring also applies if I am reading your changes correctly.

And I still think not splitting band #3 packet into lines and painting only the beginning of the first line is perfectly OK.

Show 11 quoted lines

>> >  #define ANSI_SUFFIX "\033[K"
>> > -#define DUMB_SUFFIX "        "
>> > +#define DUMB_SUFFIX "             "
>> >
>>
>> Was this change intended and if so for what purpose?  I can drop
>> this hunk if this is a mere finger-slip without proofreading, but I
>> do not want to do so without making sure I am not missing anything
>> and not discarding a meaningful change.
>
> This was my poor use of the tabify function.

OK, so I can drop this hunk---eh, perhaps v6 won't have it so I won't have to worry about it ;-)

Thanks.