Re: [PATCH 1/1] merge-file: add an option to process object IDs

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Oct 29, 2023, 14:18 UTC
Message-ID: <ZT5p0WUw7EeaY8vW@tapette.crustytoothpaste.net>
In-Reply-To: <CABPp-BG9Y6aZ+TWdkL4QE9e12fu3n61V16G6DLtawEDe=g9F4w@mail.gmail.com>

On 2023-10-29 at 06:17:09, Elijah Newren wrote:

Show 23 quoted lines

> Hi,
> 
> Overall, looks good.  Just a couple questions...
> 
> On Tue, Oct 24, 2023 at 12:58 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> >
> > From: "brian m. carlson" <bk2204@github.com>
> >
> [...]
> > --- 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.

> 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.

-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA