On Wed, Mar 12, 2025 at 05:09:41PM -0500, Justin Tobler wrote: > > 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
