Hi,

On Sun, Oct 29, 2023 at 7:18 AM brian m. carlson
[off-list ref] wrote:

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

quoted

Hi,

Overall, looks good.  Just a couple questions...

On Tue, Oct 24, 2023 at 12:58 PM brian m. carlson
[off-list ref] wrote:

quoted

From: "brian m. carlson" <redacted>

[...]

quoted

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

quoted

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.

quoted

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.