Re: [PATCH 4/5] trailer: reuse ignore_non_trailer() to ignore conflict lines

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

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Christian Couder <chriscool@tuxfamily.org> writes:
>>
>>> Yeah, it won't be as efficient as using only one strbuf and only byte
>>> oriented functions, and it looks much less manly too :-) But over time in
>>> Git we have developed a number of less efficient but quite clean
>>> abstractions like strbuf, argv_array, sha1_array and so on, that we
>>> are quite happy with.
>>
>> Actually, all these examples you gave are fairly efficient and clean
>> abstractions.  I find it insulting to pretend that the "one line per
>> strbuf" is in the same league.  It isn't.
>>
>> And it is not about manliness.
>
> By the way, I do not mean to say that all of strbuf (which is a
> rather full API) is uniformly efficient and cleanly abstracted.
> ...

Having said all that, please notice that I said "might need to be
rethought before the code becomes ready for production."

After all, it would have perfectly been OK to experiment with this
new topic in a script; e.g. this topic being text processing, it
might have happened as a Perl script while we get the semantics and
desirable features nailed down before rewriting the real thing in C.

And if that were what happened here, I wouldn't have been talking
about data structures and unnecessary complexity having to have one
strbuf per line only to join them into one string (or printf() out
to a stream one-after-another) later.

In other words, I did not say "this code is too ugly to live and we
should clean it up as soon as possible or we should revert it from
'master'---it was a mistake to merge it".

I consider the "trailer" code in 'master' today as experimental (in
other words, something I might have written in a scripting language
as a mock-up if I were doing it) that needs to be worked on further
to still get the features and semantics right.  And the patch series
in this thread are hopefully taking us in the right direction to get
them right [*1*].

You could have just said "Yeah, I realize that the code is way
suboptimal, but lets get it feature complete first and then clean it
up", or something, instead of getting defensive with unnecessary
excuses.


[Footnote]

*1* ... even though this step exposes why the approach taken,
splitting the input lines first into individual lines without even
looking at them, is a wrong one---if you started from a single
buffer that is not yet split, it would have been much easier to find
where the trailer block is, and that is why you are re-joining the
buffer you have already split into lines (i.e. what I called
"impedance mismatch"), which shows that the initial splitting is
done prematurely.  Hopefully you would realize that by now ;-).