[PATCH] stash: allow custom conflict labels for pop

[PATCH] stash: allow custom conflict labels for pop

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

Since 13817db274 (stash: add --label-ours, --label-theirs, --label-base
for apply, 2026-04-28), "git stash apply" accepts custom labels for
conflict markers, but "git stash pop" does not, although it applies the
entry the same way and only differs by dropping it afterward. A caller
that wants its own labels has to use apply and drop the entry itself.

Teach "git stash pop" the same three options.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
    stash: allow custom conflict labels for pop
    
    git stash pop now accepts the conflict label options that git stash
    apply gained in 2.55.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2430%2FHaraldNordgren%2Fstash-pop-labels-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2430/HaraldNordgren/stash-pop-labels-v1
Pull-Request: https://github.com/git/git/pull/2430

 Documentation/git-stash.adoc |  4 ++--
 builtin/stash.c              | 11 +++++++++--
 t/t3903-stash.sh             | 23 +++++++++++++++++++++++
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index fc6a9a008c..187b1a50d3 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -11,7 +11,7 @@ SYNOPSIS
 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 pop [--index] [-q | --quiet] [--label-ours=<label>] [--label-theirs=<label>] [--label-base=<label>] [<stash>]
 git stash apply [--index] [-q | --quiet] [--label-ours=<label>] [--label-theirs=<label>] [--label-base=<label>] [<stash>]
 git stash branch <branchname> [<stash>]
 git stash [push] [-p | --patch] [-S | --staged] [-k | --[no-]keep-index] [-q | --quiet]
@@ -198,7 +198,7 @@ apply the changes as they were originally).
 `--label-ours=<label>`::
 `--label-theirs=<label>`::
 `--label-base=<label>`::
-	These options are only valid for the `apply` command.
+	These options are only valid for `pop` and `apply` commands.
 +
 Use the given labels in conflict markers instead of the default
 "Updated upstream", "Stashed changes", and "Stash base".
diff --git a/builtin/stash.c b/builtin/stash.c
index 7a9843413b..3a3c46d6cf 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -43,7 +43,7 @@
 #define BUILTIN_STASH_DROP_USAGE \
 	N_("git stash drop [-q | --quiet] [<stash>]")
 #define BUILTIN_STASH_POP_USAGE \
-	N_("git stash pop [--index] [-q | --quiet] [<stash>]")
+	N_("git stash pop [--index] [-q | --quiet] [--label-ours=<label>] [--label-theirs=<label>] [--label-base=<label>] [<stash>]")
 #define BUILTIN_STASH_APPLY_USAGE \
 	N_("git stash apply [--index] [-q | --quiet] [--label-ours=<label>] [--label-theirs=<label>] [--label-base=<label>] [<stash>]")
 #define BUILTIN_STASH_BRANCH_USAGE \
@@ -885,11 +885,18 @@ static int pop_stash(int argc, const char **argv, const char *prefix,
 	int ret = -1;
 	int index = use_index;
 	int quiet = 0;
+	const char *label_ours = NULL, *label_theirs = NULL, *label_base = NULL;
 	struct stash_info info = STASH_INFO_INIT;
 	struct option options[] = {
 		OPT__QUIET(&quiet, N_("be quiet, only report errors")),
 		OPT_BOOL(0, "index", &index,
 			 N_("attempt to recreate the index")),
+		OPT_STRING(0, "label-ours", &label_ours, N_("label"),
+			   N_("label for the upstream side in conflict markers")),
+		OPT_STRING(0, "label-theirs", &label_theirs, N_("label"),
+			   N_("label for the stashed side in conflict markers")),
+		OPT_STRING(0, "label-base", &label_base, N_("label"),
+			   N_("label for the base in diff3 conflict markers")),
 		OPT_END()
 	};
 
@@ -900,7 +907,7 @@ static int pop_stash(int argc, const char **argv, const char *prefix,
 		goto cleanup;
 
 	if ((ret = do_apply_stash(prefix, &info, index, quiet,
-				  NULL, NULL, NULL)))
+				  label_ours, label_theirs, label_base)))
 		printf_ln(_("The stash entry is kept in case "
 			    "you need it again."));
 	else
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 721158606f..58a41f4c65 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1841,6 +1841,29 @@ test_expect_success 'pop exits 1 on conflicts and keeps the stash entry' '
 	test_grep pop-stashed list
 '
 
+test_expect_success 'pop with custom conflict labels' '
+	git reset --hard initial &&
+	test_commit pop-label-base conflict-file base-content &&
+	echo stashed >conflict-file &&
+	git stash push -m "stashed" &&
+	test_commit pop-label-upstream conflict-file upstream-content &&
+	test_expect_code 1 git -c merge.conflictStyle=diff3 stash pop --label-ours=UP --label-theirs=STASH &&
+	test_grep "^<<<<<<< UP" conflict-file &&
+	test_grep "^||||||| Stash base" conflict-file &&
+	test_grep "^>>>>>>> STASH" conflict-file
+'
+
+test_expect_success 'pop with empty conflict labels' '
+	git reset --hard initial &&
+	test_commit pop-empty-label-base conflict-file base-content &&
+	echo stashed >conflict-file &&
+	git stash push -m "stashed" &&
+	test_commit pop-empty-label-upstream conflict-file upstream-content &&
+	test_expect_code 1 git stash pop --label-ours= --label-theirs= &&
+	test_grep "^<<<<<<<$" conflict-file &&
+	test_grep "^>>>>>>>$" conflict-file
+'
+
 test_expect_success 'stash branch exits with a non-1 status on errors' '
 	git reset --hard initial &&
 	echo stashed >file &&

base-commit: a018953688f1b10bddf91bff8747068f5f4746a4
-- 
gitgitgadget

Re: [PATCH] stash: allow custom conflict labels for pop

[Junio C Hamano]

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

> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> Since 13817db274 (stash: add --label-ours, --label-theirs, --label-base
> for apply, 2026-04-28), "git stash apply" accepts custom labels for
> conflict markers, but "git stash pop" does not, although it applies the
> entry the same way and only differs by dropping it afterward. A caller
> that wants its own labels has to use apply and drop the entry itself.
>
> Teach "git stash pop" the same three options.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>     stash: allow custom conflict labels for pop
>     
>     git stash pop now accepts the conflict label options that git stash
>     apply gained in 2.55.

This is not a new problem, but is it just me who finds this
"feature" more about "because we can do it", not "because we need to
have it"?  Stepping back a bit, why did we add these three options
to "stash apply" in the first place?

If there is no good use case, perhaps what we should be doing is to
remove from "git stash apply" these three options, not adding the
same to another command.

I know that the underlying machinery to allow different labels were
invented for "checkout" that automatically stashes and then pops
while switching branches, and the "checkout" command wanted to use
labels that are different from what "git stash pop/apply" uses.  So
I would not question that there is a very good use case for the
underlying machinery to allow us to use different labels.

But was it really helpful and necessary, beyond "Having the feature
exposed to lower level component command like 'stash apply' makes it
slightly easier to debug", to add these three options to the "git
stash apply" command in the first place?  Who in their right mind
would type

    $ git stash pop --label-base=B --label-ours=O --label-theirs=T

every time they unstash a saved change?

Maybe I am not seeing an obvious use case, but I would blame the
lack of justification in the proposed log message for that.  And "We
can add the same three options" is not it.  "A caller that wants its
own labels has to..." is not it either.  Why does that caller want
such a strange thing?  What we have in the proposed log message is
exactly "because we can" and not "because we need them in order to
do X".

Re: [PATCH] stash: allow custom conflict labels for pop

[Phillip Wood]

On 30/09/2026 22:33, Junio C Hamano wrote:
> "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> This is not a new problem, but is it just me who finds this
> "feature" more about "because we can do it", not "because we need to
> have it"?  Stepping back a bit, why did we add these three options
> to "stash apply" in the first place?

So we could have meaningful conflict labels for "git checkout -m".
> If there is no good use case, perhaps what we should be doing is to
> remove from "git stash apply" these three options, not adding the
> same to another command.

I think having better labels for commands that are autostashing is a 
good use case for adding labels to "apply" but I'm not convinced there 
is a good use case for "pop". As you say below is anyone really going to 
type out the labels when they pop a stash? Scripts should probably be 
using "create" and "apply" rather than "push" and "pop" so are already 
covered.

Thanks

Phillip

> I know that the underlying machinery to allow different labels were
> invented for "checkout" that automatically stashes and then pops
> while switching branches, and the "checkout" command wanted to use
> labels that are different from what "git stash pop/apply" uses.  So
> I would not question that there is a very good use case for the
> underlying machinery to allow us to use different labels.
> 
> But was it really helpful and necessary, beyond "Having the feature
> exposed to lower level component command like 'stash apply' makes it
> slightly easier to debug", to add these three options to the "git
> stash apply" command in the first place?  Who in their right mind
> would type
> 
>      $ git stash pop --label-base=B --label-ours=O --label-theirs=T
> 
> every time they unstash a saved change?
> 
> Maybe I am not seeing an obvious use case, but I would blame the
> lack of justification in the proposed log message for that.  And "We
> can add the same three options" is not it.  "A caller that wants its
> own labels has to..." is not it either.  Why does that caller want
> such a strange thing?  What we have in the proposed log message is
> exactly "because we can" and not "because we need them in order to
> do X".

Re: [PATCH] stash: allow custom conflict labels for pop

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 30/09/2026 22:33, Junio C Hamano wrote:
>> "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> 
>> This is not a new problem, but is it just me who finds this
>> "feature" more about "because we can do it", not "because we need to
>> have it"?  Stepping back a bit, why did we add these three options
>> to "stash apply" in the first place?
>
> So we could have meaningful conflict labels for "git checkout -m".

Yes, I know (as I already written in the part you quoted below your
"Thanks").  What I didn't realize was that we spawned "git stash
apply" as a subprocess from sequencer, not as an internal subroutine
call, in do_stash_apply().  Of course, with that calling sequence,
we do need to expose these options to "git stash apply".

>> If there is no good use case, perhaps what we should be doing is to
>> remove from "git stash apply" these three options, not adding the
>> same to another command.
>
> I think having better labels for commands that are autostashing is a 
> good use case for adding labels to "apply" but I'm not convinced there 
> is a good use case for "pop". As you say below is anyone really going to 
> type out the labels when they pop a stash? Scripts should probably be 
> using "create" and "apply" rather than "push" and "pop" so are already 
> covered.

Yup.

Re: [PATCH] stash: allow custom conflict labels for pop

[Harald Nordgren]

> This is not a new problem, but is it just me who finds this
> "feature" more about "because we can do it", not "because we need to
> have it"?  Stepping back a bit, why did we add these three options
> to "stash apply" in the first place?
>
> If there is no good use case, perhaps what we should be doing is to
> remove from "git stash apply" these three options, not adding the
> same to another command.

Let's scrap this.

Would be interesting to remove it from 'apply' too. Can we "hide" it
by keeping it but just not documenting it?

Maybe that's a very bad idea.


Harald

Re: [PATCH] stash: allow custom conflict labels for pop

[Junio C Hamano]

Harald Nordgren <haraldnordgren@gmail.com> writes:

> Would be interesting to remove it from 'apply' too. Can we "hide" it
> by keeping it but just not documenting it?

If we rewrote the in-tree caller that spawns 'git stash apply' as a
separate subprocess via the run_command() interface, and instead
implemented the feature as a function call, the need to "hide" it
would disappear.  With the possibility for such a real improvement
in the future in mind, labelling them as "for internal use only, do
not use, as it may disappear in the future" might have some merit as
a short-term measure.

Thanks.