Re: [PATCH v12 1/4] stash: add --label-ours, --label-theirs, --label-base for apply
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Apr 14, 2026, 14:05 UTC
- Message-ID
- <d5a47638-545b-44b3-9da5-803c06b3f98a@gmail.com>
- In-Reply-To
- <9ab5431b4773c29097ae9bdd497822477c7ba56a.1776171585.git.gitgitgadget@gmail.com>
Hi Harald
On 14/04/2026 13:59, Harald Nordgren via GitGitGadget wrote:
Show 21 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. > > Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com> > [...] > diff --git a/builtin/stash.c b/builtin/stash.c > index 0d27b2fb1f..00314e2b13 100644 > --- a/builtin/stash.c > +++ b/builtin/stash.c > @@ -44,7 +44,7 @@ > [...] > -static int do_apply_stash(const char *prefix, struct stash_info *info, > - int index, int quiet) > +static int do_apply_stash_with_labels(const char *prefix, > + struct stash_info *info, > + int index, int quiet, > + const char *label_ours, const char *label_theirs, > + const char *label_base)
There are only four callers of do_apply_stash so it might be better just to change the function signature and update the existing callers rather than adding another function.
Show 11 quoted lines
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh > index 70879941c2..00bcb1f802 100755 > --- a/t/t3903-stash.sh > +++ b/t/t3903-stash.sh > @@ -1666,6 +1666,35 @@ test_expect_success 'restore untracked files even when we hit conflicts' ' > ) > ' > > +test_expect_success 'apply with custom conflict labels' ' > + git init conflict_labels && > + (
I'm still unclear why we're creating a new repository here. Our test suite is slow enough already without each test spending time creating its own repository. There doesn't seem to be anything here that requires isolating the test in this way.
Apart from that everything else looks good to me
Thanks
Phillip
Show 46 quoted lines
> + cd conflict_labels &&
> + test_commit base file &&
> + echo stashed >file &&
> + git stash push -m "stashed" &&
> + test_commit upstream file &&
> + test_must_fail git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
> + test_grep "^<<<<<<< UP" file &&
> + test_grep "^||||||| Stash base" file &&
> + test_grep "^>>>>>>> STASH" file
> + )
> +'
> +
> +test_expect_success 'apply with empty conflict labels' '
> + git init empty_labels &&
> + (
> + cd empty_labels &&
> + test_commit base file &&
> + echo stashed >file &&
> + git stash push -m "stashed" &&
> + test_commit upstream file &&
> + test_must_fail git stash apply --label-ours= --label-theirs= &&
> + 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)