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

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

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

>> 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.

>> >  #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.