Subject: Re: [PATCH 2/4] builtin/interpret-trailers: suppress blank line

From: "Michael S. Tsirkin" <[email-masked]>
Date: Sun, 10 Apr 2016 17:56:00 +0300

On Thu, Apr 07, 2016 at 10:34:51AM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin" <[email-masked]> writes:
> 
> > No - but then I will need to re-run mailinfo to parse the result,
> > will I not?
> 
> By the way, I suspect (if Christian did his implementation right
> when he did interpret-trailers) all these points may become moot.
> 
> I haven't re-reviewed what is in interpret-trailers, but the vision
> has been that its internal workings should be callable directly into
> instead of running it via run_commands() interface passing the data
> via on-disk file.  In the codepath you touch in 3/4 and 4/4, you
> already have not just mi.log_message but msg that has the whole
> payload to create a commit object out of already, so shouldn't it be
> just the matter of passing <msg.buf, msg.len> to some API function
> that was prepared to implement interpret-trailers?

That's certainly possible, though it will need a rework
of the internal API: we currently have:

void process_trailers(const char *file, int in_place, int trim_empty,
                      int suppress_blank_line, struct string_list *trailers)
{
        struct trailer_item *in_tok_first = NULL;
        struct trailer_item *in_tok_last = NULL;
        struct trailer_item *arg_tok_first;
        struct strbuf **lines;
        int trailer_end;
        FILE *outfile = stdout;

        /* Default config must be setup first */
        git_config(git_trailer_default_config, NULL);
        git_config(git_trailer_config, NULL);

        lines = read_input_file(file);

So process_trailers can be changed to get struct strbuf ** instead.

But it seems that the output would have to go into a temporary file
anyway, unless trailer.c is completely rewritten, since it
currently does all output by writing it into a file.
Is that an issue?

-- 
MST