Hi, On Sun, Oct 29, 2023 at 7:18 AM brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx> wrote: > > On 2023-10-29 at 06:17:09, Elijah Newren wrote: > > Hi, > > > > Overall, looks good. Just a couple questions... > > > > On Tue, Oct 24, 2023 at 12:58 PM brian m. carlson > > <sandals@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > > > From: "brian m. carlson" <bk2204@xxxxxxxxxx> > > > > > [...] > > > --- a/Documentation/git-merge-file.txt > > > +++ b/Documentation/git-merge-file.txt > > > @@ -12,6 +12,9 @@ SYNOPSIS > > > 'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]] > > > [--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>] > > > [--[no-]diff3] <current-file> <base-file> <other-file> > > > +'git merge-file' --object-id [-L <current-name> [-L <base-name> [-L <other-name>]]] > > > + [--ours|--theirs|--union] [-q|--quiet] [--marker-size=<n>] > > > + [--[no-]diff3] <current-oid> <base-oid> <other-oid> > > > > Why was the `[-p|--stdout]` option removed in the second synopsis? > > Elsewhere you explicitly call it out as a possibility to be used with > > --object-id. > > Originally because it implied `-p`, but I changed that to write into the > object store. I'll restore it. > > > Also, why the extra synopsis instead of just adding a `[--object-id]` > > option to the previous one? > > Because there's a relevant difference: the former has <current-file>, > <base-file>, and <other-file>, and the latter has the -oid versions. Ah, I looked over it a couple times and just kept missing that difference. Thanks for pointing it out. > > > Does "/dev/null" have any portability considerations? (I really don't > > know; just curious.) > > We already use it elsewhere in the codebase, so I assume it works. We > also have a test for that case and it worked in CI, so it's probably > fine. Seems reasonable to me; thanks.