Re: [PATCH 0/4] rev-list: introduce NUL-delimited output mode

From: Jeff King <peff@peff.net>
Date: Mar 13, 2025, 05:33 UTC
Message-ID: <20250313053329.GC94015@coredump.intra.peff.net>
In-Reply-To: <sm5xvh7bipyrlpw45bptlctbuhey7v3qktpskal5dzexomt34f@wgfziyhcf3t6>

On Wed, Mar 12, 2025 at 05:09:41PM -0500, Justin Tobler wrote:

Show 9 quoted lines

> >      If it is possible to see some effect from "-z" now (I didn't dig
> >      very far), then it may be better to continue to let the diff
> >      options parser handle it, and simply pick the result out of
> >      revs.diffopt.line_termination. As your patch 3 is written, I think
> >      the diff code probably doesn't see it anymore at all.
> 
> As currently implemented, the early parsing of -z doesn't effect the
> diff options parsing in `setup_revisions()`. The early parsing doesn't
> remove the option and thus it continues to be set in the diff options.

Ah, OK. From the diff context I didn't realize it was not in the main option parsing loop. That makes sense.

> Furthermore, revision and pathspec argument parsing is all handled in
> `setup_revisions()` so if we want to NUL-delimit arguments parsed on
> stdin with -z, we would still need to parse the option early anyway. I
> think it should be fine to leave the early -z option parsing as-is.

Makes sense. And I guess we might not want to have setup_revisions() do that handling of "-z" for input, as that would make:

  git log --stdin --raw -z

behave differently (since it does not currently change stdin handling, only the diff output). Though that does mean that these two commands will behave differently:

  git log --stdin -z
  git rev-list --stdin -z

which seems...not great. My earlier suggestion to tie "-z" to stdin handling was for consistency with other tools like grep. But if we already have cases where "-z" is only for output, maybe it is better to stay consistent with other parts of git. I.e., I was worried about us painting ourselves into a corner with your patches, but we may have already done so years ago. ;)

-Peff