Re: [PATCH v8 1/4] stash: add --ours-label, --theirs-label, --base-label for apply
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Apr 10, 2026, 15:39 UTC
- Message-ID
- <0d1c7bf2-6404-4779-a0d6-6db592510a04@gmail.com>
- In-Reply-To
- <8fcf3778205d4742a56ed2e4c3b97defa21a1538.1775762235.git.gitgitgadget@gmail.com>
Hi Harald
On 09/04/2026 20:17, Harald Nordgren via GitGitGadget wrote:
Show 5 quoted lines
> 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.
Sounds sensible and the documentation looks good.
Show 13 quoted lines
> 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 \This patch seems to be missing the implementation of these new options. Before submitting a patch series I find it is very helpful to run
git rebase --keep-base -x make -x 'cd t && prove -j6 <tests that I think might fail>'
to catch any mistakes.
Show 10 quoted lines
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh > index 70879941c2..d4e4e4d7b6 100755 > --- a/t/t3903-stash.sh > +++ b/t/t3903-stash.sh > @@ -1666,6 +1666,43 @@ test_expect_success 'restore untracked files even when we hit conflicts' ' > ) > ' > > +test_expect_success 'apply with custom conflict labels' ' > + git init conflict_labels &&
Why do we need to create a new repository just to stash some changes?
Show 5 quoted lines
> + ( > + cd conflict_labels && > + echo base >file && > + git add file && > + git commit -m base &&
We have a helper test_commit() for creating commits (it is documented in t/test-lib-functions.sh)
Show 9 quoted lines
> + echo stashed >file && > + git stash push -m "stashed" && > + echo upstream >file && > + git add file && > + git commit -m upstream && > + test_must_fail git -c merge.conflictStyle=diff3 stash apply --ours-label=UP --theirs-label=STASH && > + test_grep "^<<<<<<< UP" file && > + test_grep "^||||||| Stash base" file && > + test_grep "^>>>>>>> STASH" file
Hurray for the use of test_grep here!
> + ) > +' > + > +test_expect_success 'apply with empty conflict labels' '
Why do we want to support empty labels rather than making them an error?
Thanks
Phillip
Show 37 quoted lines
> + git init empty_labels &&
> + (
> + cd empty_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= --theirs-label= &&
> + test_grep "^<<<<<<<$" file &&
> + test_grep "^>>>>>>>$" file
> + )
> +'
> +
> test_expect_success 'stash create reports a locked index' '
> test_when_finished "rm -rf repo" &&
> git init repo &&
> diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
> index 29dad98c49..659ad4ec97 100644
> --- a/xdiff/xmerge.c
> +++ b/xdiff/xmerge.c
> @@ -199,9 +199,9 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,
> int size, int i, int style,
> xdmerge_t *m, char *dest, int marker_size)
> {
> - int marker1_size = (name1 ? strlen(name1) + 1 : 0);
> - int marker2_size = (name2 ? strlen(name2) + 1 : 0);
> - int marker3_size = (name3 ? strlen(name3) + 1 : 0);
> + int marker1_size = (name1 && *name1 ? strlen(name1) + 1 : 0);
> + int marker2_size = (name2 && *name2 ? strlen(name2) + 1 : 0);
> + int marker3_size = (name3 && *name3 ? strlen(name3) + 1 : 0);
> int needs_cr = is_cr_needed(xe1, xe2, m);
>
> if (marker_size <= 0)