Re: [PATCH 05/10] sequencer: use the trailer iterator

From: Linus Arver <linusa@google.com>
Date: Jan 20, 2024, 20:04 UTC
Message-ID: <owlyle8jhiv3.fsf@fine.c.googlers.com>
In-Reply-To: <xmqq34uunoag.fsf@gitster.g>

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

Show 10 quoted lines

>> From: Linus Arver <linusa@google.com>
>> Also, teach the iterator about non-trailer lines, by adding a new field
>> called "raw" to hold both trailer and non-trailer lines. This is
>> necessary because a "trailer block" is a list of trailer lines of at
>> least 25% trailers (see 146245063e (trailer: allow non-trailers in
>> trailer block, 2016-10-21)), such that it may hold non-trailer lines.
>
> OK.  This would change behaviour, wouldn't it, in the sense that we
> used to yield a non-trailer line from the old iterator but the new
> one skips them?

I think it's the other way; the old iterator only iterated over trailer lines, skipping over non-trailer lines (see the "not a real trailer" deleted bit for trailer_iterator_advance()). The new one iterates over all lines found in the trailer block, whether they are trailer or non-trailer lines.

The function insert_records_from_trailers() from shortlog.c uses the new iterator, but has to be careful because the new iterator goes over non-trailer lines too. That's why it now does

    if (!iter.is_trailer)
            continue;

to do the skipping itself.

> Is that something we can demonstrate and protect in
> tests (presumably once we conclude these refactoring, we would be
> able to call into this machinery from unit-testing framework)?

Yup, that is exactly what I want to do once the dust around the trailer API settles down after this series (and also the larger series). :)