From: Linus Arver <linus@ucla.edu>
To: Christian Couder <christian.couder@gmail.com>,
Linus Arver via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Christian Couder <chriscool@tuxfamily.org>,
Junio C Hamano <gitster@pobox.com>,
Emily Shaffer <nasamuffin@google.com>,
Josh Steadmon <steadmon@google.com>,
"Randall S. Becker" <rsbecker@nexbridge.com>,
Kristoffer Haugsbakk <code@khaugsbakk.name>
Subject: Re: [PATCH v3 03/10] trailer: teach iterator about non-trailer lines
Date: Mon, 29 Apr 2024 21:55:53 -0700 [thread overview]
Message-ID: <CAMo6p=GRcmy6NDGRskuNrauMYTkAgk_p_0TJJTPC2=axVrWuvQ@mail.gmail.com> (raw)
In-Reply-To: <CAMo6p=EggYjK60fWmNC2XRFf0zRf4PCcUT-vmFM6RH2J0i3hrw@mail.gmail.com>
Linus Arver <linus@ucla.edu> writes:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> (Sorry I just realized that I had sent this email to Linus only.)
>>
>> On Fri, Apr 26, 2024 at 2:26 AM Linus Arver via GitGitGadget
>> <gitgitgadget@gmail.com> wrote:
>>>
>>> From: Linus Arver <linusa@google.com>
>>>
>>> Previously the iterator did not iterate over non-trailer lines. This was
>>> somewhat unfortunate, because trailer blocks could have non-trailer
>>> lines in them since 146245063e (trailer: allow non-trailers in trailer
>>> block, 2016-10-21), which was before the iterator was created in
>>> f0939a0eb1 (trailer: add interface for iterating over commit trailers,
>>> 2020-09-27).
>>>
>>> So if trailer API users wanted to iterate over all lines in a trailer
>>> block (including non-trailer lines), they could not use the iterator and
>>> were forced to use the lower-level trailer_info struct directly (which
>>> provides a raw string array that includes all lines in the trailer
>>> block).
>>>
>>> Change the iterator's behavior so that we also iterate over non-trailer
>>> lines, instead of skipping over them. The new "raw" member of the
>>> iterator allows API users to access previously inaccessible non-trailer
>>> lines. Reword the variable "trailer" to just "line" because this
>>> variable can now hold both trailer lines _and_ non-trailer lines.
>>>
>>> The new "raw" member is important because anyone currently not using the
>>> iterator is using trailer_info's raw string array directly to access
>>> lines to check what the combined key + value looks like. If we didn't
>>> provide a "raw" member here, iterator users would have to re-construct
>>> the unparsed line by concatenating the key and value back together again
>>> --- which places an undue burden for iterator users.
>>>
>>> The next commit demonstrates the use of the iterator in sequencer.c as an
>>> example of where "raw" will be useful, so that it can start using the
>>> iterator.
>>>
>>> For the existing use of the iterator in builtin/shortlog.c, we don't
>>> have to change the code there because that code does
>>>
>>> trailer_iterator_init(&iter, body);
>>> while (trailer_iterator_advance(&iter)) {
>>> const char *value = iter.val.buf;
>>>
>>> if (!string_list_has_string(&log->trailers, iter.key.buf))
>>> continue;
>>>
>>> ...
>>>
>>> and the
>>>
>>> if (!string_list_has_string(&log->trailers, iter.key.buf))
>>>
>>> condition already skips over non-trailer lines (iter.key.buf is empty
>>> for non-trailer lines, making the comparison still work even with this
>>> commit).
>>>
>>> Rename "num_expected_trailers" to "num_expected_objects" in
>>> t/unit-tests/t-trailer.c because the items we iterate over now include
>>> non-trailer lines.
>>
>> I think it would be simpler if the previous patch used just
>> "num_expected" or "expected". It's not like the other fields in the
>> struct ("msg" and "name") are very explicit, so why this one only?
>
> I didn't give it much thought TBH. "num_expected" SGTM. Will update.
Another thing: I will rename "trailer_assertions" in Path 10 to probably
"trailer_contents" because it sounds simpler. I am replying here instead
of on Patch 10 because my mail setup still has some rough edges for the
transition away from @google.com (I no longer work there).
And on that note, I'll have to update the SOB lines to match my new
email address.
next prev parent reply other threads:[~2024-04-30 4:55 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-16 6:27 [PATCH 0/6] Make trailer_info struct private (plus sequencer cleanup) Linus Arver via GitGitGadget
2024-03-16 6:27 ` [PATCH 1/6] trailer: teach iterator about non-trailer lines Linus Arver via GitGitGadget
2024-03-16 6:27 ` [PATCH 2/6] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-03-16 6:27 ` [PATCH 3/6] interpret-trailers: access trailer_info with new helpers Linus Arver via GitGitGadget
2024-03-16 6:27 ` [PATCH 4/6] trailer: make parse_trailers() return trailer_info pointer Linus Arver via GitGitGadget
2024-03-16 6:27 ` [PATCH 5/6] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-03-16 6:27 ` [PATCH 6/6] trailer: retire trailer_info_get() from API Linus Arver via GitGitGadget
2024-03-16 17:06 ` [PATCH 0/6] Make trailer_info struct private (plus sequencer cleanup) Junio C Hamano
2024-03-26 22:00 ` Junio C Hamano
2024-04-19 5:36 ` Linus Arver
2024-04-19 5:22 ` [PATCH v2 0/8] " Linus Arver via GitGitGadget
2024-04-19 5:22 ` [PATCH v2 1/8] Makefile: sort UNIT_TEST_PROGRAMS Linus Arver via GitGitGadget
2024-04-19 5:22 ` [PATCH v2 2/8] trailer: add unit tests for trailer iterator Linus Arver via GitGitGadget
2024-04-19 5:33 ` Linus Arver
2024-04-19 18:46 ` Linus Arver
2024-04-19 21:52 ` Junio C Hamano
2024-04-20 0:14 ` Linus Arver
2024-04-19 5:22 ` [PATCH v2 3/8] trailer: teach iterator about non-trailer lines Linus Arver via GitGitGadget
2024-04-19 5:22 ` [PATCH v2 4/8] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-04-23 21:19 ` Junio C Hamano
2024-04-19 5:22 ` [PATCH v2 5/8] interpret-trailers: access trailer_info with new helpers Linus Arver via GitGitGadget
2024-04-19 5:22 ` [PATCH v2 6/8] trailer: make parse_trailers() return trailer_info pointer Linus Arver via GitGitGadget
2024-04-23 23:17 ` Junio C Hamano
2024-04-19 5:22 ` [PATCH v2 7/8] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-04-23 23:27 ` Junio C Hamano
2024-04-25 3:17 ` Linus Arver
2024-04-19 5:22 ` [PATCH v2 8/8] trailer: retire trailer_info_get() from API Linus Arver via GitGitGadget
2024-04-23 23:27 ` Junio C Hamano
2024-04-24 0:27 ` [PATCH v2 0/8] Make trailer_info struct private (plus sequencer cleanup) Junio C Hamano
2024-04-26 0:26 ` [PATCH v3 00/10] " Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 01/10] Makefile: sort UNIT_TEST_PROGRAMS Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 02/10] trailer: add unit tests for trailer iterator Linus Arver via GitGitGadget
2024-04-26 14:51 ` Christian Couder
2024-04-26 16:20 ` Junio C Hamano
2024-04-26 16:25 ` Linus Arver
2024-04-26 0:26 ` [PATCH v3 03/10] trailer: teach iterator about non-trailer lines Linus Arver via GitGitGadget
2024-04-27 12:50 ` Christian Couder
2024-04-30 4:42 ` Linus Arver
2024-04-30 4:55 ` Linus Arver [this message]
2024-04-26 0:26 ` [PATCH v3 04/10] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 05/10] interpret-trailers: access trailer_info with new helpers Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 06/10] trailer: make parse_trailers() return trailer_info pointer Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 07/10] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 08/10] trailer: retire trailer_info_get() from API Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 09/10] trailer: document parse_trailers() usage Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 10/10] trailer unit tests: inspect iterator contents Linus Arver via GitGitGadget
2024-04-27 12:51 ` [PATCH v3 00/10] Make trailer_info struct private (plus sequencer cleanup) Christian Couder
2024-05-02 4:54 ` [PATCH v4 " Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 01/10] Makefile: sort UNIT_TEST_PROGRAMS Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 02/10] trailer: add unit tests for trailer iterator Linus Arver via GitGitGadget
2024-05-02 16:54 ` Junio C Hamano
2024-05-02 4:54 ` [PATCH v4 03/10] trailer: teach iterator about non-trailer lines Linus Arver via GitGitGadget
2024-05-04 15:33 ` Phillip Wood
2024-05-05 1:37 ` Linus Arver
2024-05-05 14:09 ` Phillip Wood
2024-05-09 7:11 ` Linus Arver
2024-05-13 15:11 ` Phillip Wood
2024-05-13 15:13 ` Phillip Wood
2024-05-02 4:54 ` [PATCH v4 04/10] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 05/10] interpret-trailers: access trailer_info with new helpers Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 06/10] trailer: make parse_trailers() return trailer_info pointer Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 07/10] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 08/10] trailer: retire trailer_info_get() from API Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 09/10] trailer: document parse_trailers() usage Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 10/10] trailer unit tests: inspect iterator contents Linus Arver via GitGitGadget
2024-05-02 17:15 ` [PATCH v4 00/10] Make trailer_info struct private (plus sequencer cleanup) Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMo6p=GRcmy6NDGRskuNrauMYTkAgk_p_0TJJTPC2=axVrWuvQ@mail.gmail.com' \
--to=linus@ucla.edu \
--cc=chriscool@tuxfamily.org \
--cc=christian.couder@gmail.com \
--cc=code@khaugsbakk.name \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=nasamuffin@google.com \
--cc=rsbecker@nexbridge.com \
--cc=steadmon@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).