Volume XXII, number 279Tuesday, October 6, 2026Latest message 33 minutes ago

The Git List

News and archive of git@vger.kernel.org, since April 2005

patchstash: allow custom conflict labels for pop

6 messages between Sep 30, 2026 and Oct 2, 2026, from Harald Nordgren via GitGitGadget, Junio C Hamano, Phillip Wood, Harald Nordgren.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

Harald Nordgren via GitGitGadgetSep 30, 2026, 20:58 UTC on lore
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(-)
Show changes to 3 files +34 −4

Documentation/git-stash.adoc, builtin/stash.c, t/t3903-stash.sh

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
Junio C HamanoSep 30, 2026, 21:33 UTC in reply to Harald Nordgren via GitGitGadget on lore

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

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 16 quoted lines
> 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".

Phillip WoodOct 1, 2026, 09:47 UTC in reply to Junio C Hamano on lore

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

On 30/09/2026 22:33, Junio C Hamano wrote:
Show 6 quoted lines
> "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
Show 24 quoted lines
> 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".
Junio C HamanoOct 1, 2026, 17:58 UTC in reply to Phillip Wood on lore

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

Phillip Wood <phillip.wood123@gmail.com> writes:
Show 9 quoted lines
> 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".

Show 10 quoted lines
>> 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.
Harald NordgrenOct 2, 2026, 07:21 UTC in reply to Junio C Hamano on lore

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

Show 8 quoted lines
> 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
Junio C HamanoOct 2, 2026, 14:31 UTC in reply to Harald Nordgren on lore

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

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.

Back to recent threads