From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 09 Apr 2026 17:25:55 GMT
Subject: Re: [PATCH v7 1/4] stash: add --ours-label, --theirs-label, --base-label for apply
Message-ID: <xmqq7bqgujto.fsf@gitster.g>
In-Reply-To: <284075600aeb1fc2711be72ca201de3efd81e365.1775741265.git.gitgitgadget@gmail.com>

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> Allow callers of "git stash apply" to pass custom labels for conflict
> markers instead of the default "Updated upstream" and "Stashed changes".
> Document the new options and add a test.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>  Documentation/git-stash.adoc | 11 ++++++++++-
>  builtin/stash.c              |  2 +-
>  t/t3903-stash.sh             | 18 ++++++++++++++++++
>  3 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
> index b05c990ecd..6829ba1140 100644
> --- a/Documentation/git-stash.adoc
> +++ b/Documentation/git-stash.adoc
> @@ -12,7 +12,7 @@ git stash list [<log-options>]
>  git stash show [-u | --include-untracked | --only-untracked] [<diff-options>] [<stash>]
>  git stash drop [-q | --quiet] [<stash>]
>  git stash pop [--index] [-q | --quiet] [<stash>]
> -git stash apply [--index] [-q | --quiet] [<stash>]
> +git stash apply [--index] [-q | --quiet] [--ours-label=<label>] [--theirs-label=<label>] [--base-label=<label>] [<stash>]
>  git stash branch <branchname> [<stash>]
>  git stash [push] [-p | --patch] [-S | --staged] [-k | --[no-]keep-index] [-q | --quiet]
>  	     [-u | --include-untracked] [-a | --all] [(-m | --message) <message>]
> @@ -195,6 +195,15 @@ the index's ones. However, this can fail, when you have conflicts
>  (which are stored in the index, where you therefore can no longer
>  apply the changes as they were originally).
>  
> +`--ours-label=<label>`::
> +`--theirs-label=<label>`::
> +`--base-label=<label>`::
> +	These options are only valid for the `apply` command.
> ++
> +Use the given labels in conflict markers instead of the default
> +"Updated upstream", "Stashed changes", and "Stash base".
> +`--base-label` only has an effect with merge.conflictStyle=diff3.
> +
>  `-k`::
>  `--keep-index`::
>  `--no-keep-index`::
> diff --git a/builtin/stash.c b/builtin/stash.c
> index 0d27b2fb1f..54bcb6ac73 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -44,7 +44,7 @@
>  #define BUILTIN_STASH_POP_USAGE \
>  	N_("git stash pop [--index] [-q | --quiet] [<stash>]")
>  #define BUILTIN_STASH_APPLY_USAGE \
> -	N_("git stash apply [--index] [-q | --quiet] [<stash>]")
> +	N_("git stash apply [--index] [-q | --quiet] [--ours-label=<label>] [--theirs-label=<label>] [--base-label=<label>] [<stash>]")
>  #define BUILTIN_STASH_BRANCH_USAGE \
>  	N_("git stash branch <branchname> [<stash>]")
>  #define BUILTIN_STASH_STORE_USAGE \
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index 70879941c2..dd47c1322a 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -1666,6 +1666,24 @@ test_expect_success 'restore untracked files even when we hit conflicts' '
>  	)
>  '
>  
> +test_expect_success 'apply with custom conflict labels' '
> +	git init conflict_labels &&
> +	(
> +		cd conflict_labels &&
> +		echo base >file &&
> +		git add file &&
> +		git commit -m base &&
> +		echo stashed >file &&
> +		git stash push -m "stashed" &&
> +		echo upstream >file &&
> +		git add file &&
> +		git commit -m upstream &&
> +		test_must_fail git stash apply --ours-label=UP --theirs-label=STASH &&
> +		grep "^<<<<<<< UP" file &&
> +		grep "^>>>>>>> STASH" file
> +	)
> +'

Two and a half things I noticed.

 * use "test_grep" to validate the result, like you did in other
   patches to the tests.  t3903 is rather old and has uses of raw
   "grep" but majority of the tests should already be using
   test_grep.

 * Not validating the base line is a bit unexpected.  Even without
   giving --base-label to the "stash apply" command, we could make
   sure that the output says "|||||||" (and nothing else) for the
   base label.

 * When these labels are set to an empty string, I think we should
   refrain from adding a trailing " " after these marker characters.
   Should we add a test case for that, e.g.

	test_must_fail git stash apply --ours-l= --theirs-l= &&
	test_grep "^<<<<<<<$" file &&
	test_grep "^>>>>>>>$" file

>  test_expect_success 'stash create reports a locked index' '
>  	test_when_finished "rm -rf repo" &&
>  	git init repo &&