Re: [PATCH v3 5/5] format-rev: introduce builtin for on-demand pretty formatting

From: Phillip Wood <phillip.wood123@gmail.com>
Date: May 2, 2026, 10:00 UTC
Message-ID: <0f57e309-62a0-438e-a1d8-7c367379ef01@gmail.com>
In-Reply-To: <20260501182718.27853-2-kristofferhaugsbakk@fastmail.com>

Hi Kristoffer

On 01/05/2026 19:27, kristofferhaugsbakk@fastmail.com wrote:

Show 20 quoted lines

> On Fri, May 1, 2026, at 12:16, Phillip Wood wrote:
>>> [snip]
>>> • You can’t feed commits piecemeal to these commands, one input
>>>     for one output; they block until standard in is closed
>>
>> So you can feed them piecemeal but you don't get any output until you
>> close stdin. That can be helpful as it means the calling process can
>> write to "git log --stdin" and then read the output without worrying
>> about getting deadlocked.
> 
> Okay. I don’t have much experience with concurrent programming.
> 
>> The Implementation below works fine if there
>> are separate processes or threads writing to and reading from "git
>> format-rev", but if we want a single process to be able to read from and
>> write to "git format-rev --stdin-mode=text" there will need to be a way
>> to delimit message boundaries so that git knows where the input message
>> ends and the caller knows where the response ends.
> 
> Okay, so I guess a null-terminator mode for output.

I think that's a good idea

>> We'll also need to be
>> careful about flushing the output at the end of a processed message.
> 
> I don’t get why this takes special care. I’ll think about it.

Because the output from printf() is buffered, unless you explicitly flush it you can get into a state where git thinks it has printed the output and is waiting for the caller to write more input, but the caller is still waiting to read git's output and so they are deadlocked. Calling maybe_flush_or_die() is the usual way to handle this I think - see 344a107b55 (merge-tree --stdin: flush stdout to avoid deadlock, 2025-02-18)

Show 13 quoted lines

>> For "--stdin-mode=revs" the caller cannot know how many lines the output
>> will span because formats like %(trailers) will produce a variable
>> number of lines depending on which trailers are present. It is also
>> possible for a rev name to span more than one line. The following
>> example finds the most recent commit that mentions 'cherry-pick' in the
>> subject line
>>
>> :/^[^
>> ]cherry-pick
>>
>> so we need a way to delimit the input and output records there as well.
> 
> Okay, so a null-terminator mode for input as well?

Yes I think "-z" should mean NUL terminated input and output.

Show 19 quoted lines

>> I think the functionality implemented here is useful (transforming the
>> output of 'git blame' or 'git-last-modified' are convicing examples) and
>> it is probably better to do it as a command rather than adding a
>> "--format" option to name-rev.
>>
>>> • You can’t feed a list of possibly duplicate commits, like the output
>>>     of git-last-modified(1); they effectively deduplicate the output
>>
>> That is definitely a problem
> 
> Great, thanks.
> 
>>> Beyond these two points there’s also the input massage problem: you
>>
>> s/massagge/message/?
> 
> No. I meant massaging the input so that it can be processed by whatever
> tool you have. :) In this case splitting the object name column and file
> column because tools like git-log(1) can only deal with revision input.

Oh I see

Thanks

Phillip

> Thanks for reviewing the usability design.
>