Re: [PATCH v4 16/28] trailer: finish formatting unification

[Linus Arver <linusa@google.com>]

Christian Couder <christian.couder@gmail.com> writes:

> On Tue, Feb 6, 2024 at 6:12 AM Linus Arver via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>
>>  /*
>> - * Format the trailers from the commit msg "msg" into the strbuf "out".
>> - * Note two caveats about "opts":
>> - *
>> - *   - this is primarily a helper for pretty.c, and not
>> - *     all of the flags are supported.
>> - *
>> - *   - this differs from process_trailers slightly in that we always format
>> - *     only the trailer block itself, even if the "only_trailers" option is not
>> - *     set.
>
> This makes me wonder if there was actually a good reason why
> format_trailers() and format_trailer_info() were 2 different
> functions. Is there info about this in the commit message of the
> commit which introduced this comment?

Good question. I see a388b10fc1 (pretty: move trailer formatting to
trailer.c, 2017-08-15) and there it says:

    pretty: move trailer formatting to trailer.c
    
    The next commit will add many features to the %(trailer)
    placeholder in pretty.c. We'll need to access some internal
    functions of trailer.c for that, so our options are either:
    
      1. expose those functions publicly
    
    or
    
      2. make an entry point into trailer.c to do the formatting
    
    Doing (2) ends up exposing less surface area, though do note
    that caveats in the docstring of the new function.

so it looks like this function started out from pretty.c and did not
have access to all of the trailer implementation internals, and was
never intended to replace (unify) the formatting machinery in trailer.c
both before and after that commit. This seems like good information to
include in the commit message, so I will do so in the next reroll.

Aside: it is interesting that the current patch series is taking the
direction of (1) instead of (2) as was done in the past (we had a choice
to "libify" at that time but did not do so, in order to "expos[e] less
surface area [in <trailer.h>]").