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

>  === 3. Merge conflict
>  
> -When a merge conflict happens during switching branches with
> -the `-m` option, you would see something like this:
> +When the `--merge` (`-m`) option is in effect and the locally
> +modified files overlap with files that need to be updated by the
> +branch switch, the changes are stashed and reapplied after the
> +switch.  If the stash application results in conflicts, they are not
> +resolved and the stash is saved to the stash list:

"results in conflicts, they are not resolved" felt a bit funny thing
to say.  The definition of resulting in conflicits is to be left
with unresolvable conflicts ;-), so ...

    ... and reapplied after the switch.  If this process results in
    conflicts, a stash entry is saved and made available in "git
    stash list".  This is so that you can revisit it in case if you
    fail to resolve conflicts correctly.

or something.  But reading what you have further down, I think the
last sentence "This is so that ..." is probably unnecessary.

> ...
> +Your local changes are stashed, however, applying it to carry
> +forward your local changes resulted in conflicts:
>  
> + - You can try resolving them now.  If you resolved them
> +   successfully, discard the stash entry with "git stash drop".
>  
> + - Alternatively you can "git reset --hard" if you do not want
> +   to deal with them right now, and later "git stash pop" to
> +   recover your local changes.
>  ------------
> +
> +You can try resolving the conflicts now.  Edit the conflicting files
> +and mark them resolved with `git add` as usual, then run `git stash
> +drop` to discard the stash entry.  Alternatively, you can clear the
> +working tree with `git reset --hard` and recover your local changes
> +later with `git stash pop`.

Very well written.  I am very happy to see this text.

> diff --git a/Documentation/git-switch.adoc b/Documentation/git-switch.adoc
> index 87707e9265..3147023dc3 100644
> --- a/Documentation/git-switch.adoc
> +++ b/Documentation/git-switch.adoc
> @@ -126,15 +126,16 @@ variable.
>  	If you have local modifications to one or more files that are
>  	different between the current branch and the branch to which
>  	you are switching, the command refuses to switch branches in
> +	order to preserve your modifications in context.  With this
> +	option, the conflicting local changes are automatically
> +	stashed before the switch and reapplied afterwards.  If the
> +	local changes do not overlap with the differences between
> +	branches, the switch proceeds without stashing.  If
> +	reapplying the stash results in conflicts, the entry is
> +	saved to the stash list.  Resolve the conflicts and run
> +	`git stash drop` when done, or clear the working tree
> +	(e.g. with `git reset --hard`) before running `git stash pop`
> +	later to re-apply your changes.

We lost "however" but it is OK.

We would want to hint that "The command refuses to" refers to the
default behaviour when option being described ("--merge") is not
given by some other means, which was what the "however" we lost from
the updated text was trying to do.

> @@ -217,15 +218,15 @@ $ git switch mytopic
>  error: You have local changes to 'frotz'; not switching branches.
>  ------------
>  
> -You can give the `-m` flag to the command, which would try a three-way
> -merge:
> +You can give the `-m` flag to the command, which would carry your local
> +changes to the new branch:

Yes, saying "carry your local changes to" is much clearer than "try
a three-way merge".  I very much like this new text.

>  ------------
>  $ git switch -m mytopic
> -Auto-merging frotz
> +Switched to branch 'mytopic'
>  ------------
>  
> -After this three-way merge, the local modifications are _not_
> +After the switch, the local modifications are reapplied and are _not_
>  registered in your index file, so `git diff` would show you what
>  changes you made since the tip of the new branch.


So now to the more exciting part.  But there isn't much to comment
on, as it stayed the same since a previous iteration where Phillip's
excellent "we do not have to waste cycles to dry-run at all" idea.

> diff --git a/t/t7201-co.sh b/t/t7201-co.sh
> index 9bcf7c0b40..857a5a6bd1 100755
> --- a/t/t7201-co.sh
> +++ b/t/t7201-co.sh
> @@ -210,6 +210,194 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
>  	test_cmp expect two
>  '
>  
> +test_expect_success 'checkout --merge --conflict=zdiff3 <branch>' '
> +	git checkout -f main &&
> +	git reset --hard &&
> +	git clean -f &&
> +
> +	fill a b X d e >two &&
> +	git checkout --merge --conflict=zdiff3 simple &&
> +
> +	cat <<-EOF >expect &&
> +	a
> +	<<<<<<< simple
> +	c
> +	||||||| main
> +	b
> +	c
> +	d
> +	=======
> +	b
> +	X
> +	d
> +	>>>>>>> local
> +	e
> +	EOF
> +	test_cmp expect two
> +'

OK.  Do we want to verify with "git stash show -p" or something what
we stashed away?  I see we later have an explicit test to see that
the stash entry can actually be used in "stash pop" after clearing
the stage with "checkout -f", so perhaps it is OK to leave it to
that test, instead of checking in every step.

> +test_expect_success 'checkout -m skips stash when no conflict' '
> +	git checkout -f main &&
> +	git clean -f &&
> +
> +	fill 0 x y z >same &&
> +	git checkout -m side >actual 2>&1 &&
> +	test_grep ! "Created autostash" actual &&
> +	fill 0 x y z >expect &&
> +	test_cmp expect same
> +'

This one may not just check "test_grep !" but also check the output
of "git stash list", perhaps take one before and after "checkout -m"
and compare them?

> +test_expect_success 'checkout -m stashes and applies on conflicting changes' '
> +	git checkout -f main &&
> +	git clean -f &&
> +
> +	fill 1 2 3 4 5 6 7 >one &&
> +	git checkout -m side >actual 2>&1 &&
> +	test_grep ! "Created autostash" actual &&
> +	test_grep "Applied autostash" actual &&
> +	fill 1 2 3 4 5 6 7 >expect &&
> +	test_cmp expect one
> +'

Nice.

> +test_expect_success 'checkout -m stashes on truly conflicting changes' '
> +	git checkout -f main &&
> +	git clean -f &&
> +
> +	fill 1 2 3 4 5 >one &&
> +	test_must_fail git checkout side 2>stderr &&
> +	test_grep "Your local changes" stderr &&
> +	git checkout -m side >actual 2>&1 &&
> +	test_grep ! "Created autostash" actual &&
> +	test_grep "resulted in conflicts" actual &&
> +	test_grep "git stash drop" actual &&
> +	git stash drop &&
> +	git reset --hard
> +'

OK.  This is an emulation of a user who gives up resolving and punts.

> +test_expect_success 'checkout -m produces usable stash on conflict' '
> +	git checkout -f main &&
> +	git clean -f &&
> +
> +	fill 1 2 3 4 5 >one &&
> +	git checkout -m side >actual 2>&1 &&
> +	test_grep "recover your local changes" actual &&
> +	git checkout -f main &&
> +	git stash pop &&
> +	fill 1 2 3 4 5 >expect &&
> +	test_cmp expect one
> +'

Nice.

> +test_expect_success 'checkout -m applies stash cleanly with non-overlapping changes in same file' '
> +	git checkout -f main &&
> +	git reset --hard &&
> +	git clean -f &&
> +
> +	git checkout -b nonoverlap_base &&
> +	fill a b c d >file &&
> +	git add file &&
> +	git commit -m "add file" &&
> +
> +	git checkout -b nonoverlap_child &&
> +	fill a b c INSERTED d >file &&
> +	git commit -a -m "insert line near end of file" &&
> +
> +	fill DIRTY a b c INSERTED d >file &&
> +
> +	git checkout -m nonoverlap_base 2>stderr &&
> +	test_grep "Applied autostash" stderr &&
> +	test_grep ! "resulted in conflicts" stderr &&
> +
> +	fill DIRTY a b c d >expect &&
> +	test_cmp expect file &&

Should we or should we not see an extra stack entry saved at this point?
Don't we want to test it?

> +
> +	git checkout -f main &&
> +	git branch -D nonoverlap_base &&
> +	git branch -D nonoverlap_child
> +'

Overall very nicely done.

> Should we or should we not see an extra stack entry saved at this point?
> Don't we want to test it?

All of these should be fixed as well. Thanks!


Harald

On Thu, Apr 9, 2026 at 12:18 PM Harald Nordgren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> ... If reapplying causes conflicts, the stash is
> kept and the user is told they can resolve and run "git stash drop",
> or run "git reset --hard" and later "git stash pop" to recover their
> changes.

I might suggest that this should recommend "git stash pop --index"
(either always, or if the stashed index differs from the stash's parent).

Chris

> I might suggest that this should recommend "git stash pop --index"
> (either always, or if the stashed index differs from the stash's parent).

Interesting! This is a new option that I've never seen before.



Harald

On Fri, Apr 10, 2026 at 09:01:13PM +0000, Harald Nordgren via GitGitGadget wrote:

>  	if (do_merge) {
>  		ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
> +		if (ret && opts->merge) {
> +			create_autostash_ref_silent(the_repository,
> +						   "CHECKOUT_AUTOSTASH");

This tries to create a root-level ref called CHECKOUT_AUTOSTASH, which
violates the syntax rules given in gitglossary's "ref" entry:

  Ref names must either start with refs/ or be located in the root of
  the hierarchy. For the latter, their name must follow these rules:

    •   The name consists of only upper-case characters or underscores.

    •   The name ends with "_HEAD" or is equal to "HEAD".

Our enforcement of these rules has some holes, but I have a local series
to fix that (which is how I noticed the problem). The entry continues to
list some exceptions:

  There are some irregular refs in the root of the hierarchy that do not
  match these rules. The following list is exhaustive and shall not be
  extended in the future:

    •   AUTO_MERGE

    •   BISECT_EXPECTED_REV

    •   NOTES_MERGE_PARTIAL

    •   NOTES_MERGE_REF

    •   MERGE_AUTOSTASH

We can add CHECKOUT_AUTOSTASH to the list of exceptions, but I wonder if
there is another name we could use that would conform to the usual
rules.

-Peff

> This tries to create a root-level ref called CHECKOUT_AUTOSTASH, which
> violates the syntax rules given in gitglossary's "ref" entry:
> 
>   Ref names must either start with refs/ or be located in the root of
>   the hierarchy. For the latter, their name must follow these rules:
> 
>     •   The name consists of only upper-case characters or underscores.
> 
>     •   The name ends with "_HEAD" or is equal to "HEAD".


So maybe easiest is just to rename it to CHECKOUT_AUTOSTASH_HEAD?


Harald

On Sat, Apr 11, 2026 at 02:38:23PM -0400, Jeff King wrote:

> Our enforcement of these rules has some holes, but I have a local series
> to fix that (which is how I noticed the problem). The entry continues to
> list some exceptions:
> 
>   There are some irregular refs in the root of the hierarchy that do not
>   match these rules. The following list is exhaustive and shall not be
>   extended in the future:
> 
>     •   AUTO_MERGE
> 
>     •   BISECT_EXPECTED_REV
> 
>     •   NOTES_MERGE_PARTIAL
> 
>     •   NOTES_MERGE_REF
> 
>     •   MERGE_AUTOSTASH
> 
> We can add CHECKOUT_AUTOSTASH to the list of exceptions, but I wonder if
> there is another name we could use that would conform to the usual
> rules.

Actually, I misread the documentation I quoted as "list is not
exhaustive". ;) So we would be violating its promise to add
CHECKOUT_AUTOSTASH to the list.

-Peff

On Sat, Apr 11, 2026 at 08:51:09PM +0200, Harald Nordgren wrote:

> > This tries to create a root-level ref called CHECKOUT_AUTOSTASH, which
> > violates the syntax rules given in gitglossary's "ref" entry:
> > 
> >   Ref names must either start with refs/ or be located in the root of
> >   the hierarchy. For the latter, their name must follow these rules:
> > 
> >     •   The name consists of only upper-case characters or underscores.
> > 
> >     •   The name ends with "_HEAD" or is equal to "HEAD".
> 
> 
> So maybe easiest is just to rename it to CHECKOUT_AUTOSTASH_HEAD?

Yeah, that is syntactically valid, if a mouthful. I can't offhand think
of a shorter variant.

-Peff

Hi Harald

For the subject line I think

    checkout -m: autostash when switching branches

would be more in keeping with our usual style.

On 14/04/2026 13:59, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
> > When switching branches with "git checkout -m", local modifications
> can block the switch. Really? Isn't the point of "checkout -m" to merge the local modifications into the branch that's being checked out?

> Teach the -m flow to create a temporary stash
> before switching and reapply it after.  On success, only "Applied
> autostash." is shown. and a diff of the local changes?

> If reapplying causes conflicts, the stash is
> kept and the user is told they can resolve and run "git stash drop",
> or run "git reset --hard" and later "git stash pop" to recover their
> changes.
> > Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>

I've skipped the docs as I'm short on time

> @@ -783,8 +783,10 @@ static int merge_working_tree(const struct checkout_opts *opts,
>   	struct tree *new_tree;
>   >   	repo_hold_locked_index(the_repository, &lock_file, LOCK_DIE_ON_ERROR);
> -	if (repo_read_index_preload(the_repository, NULL, 0) < 0)
> +	if (repo_read_index_preload(the_repository, NULL, 0) < 0) {
> +		rollback_lock_file(&lock_file);
>   		return error(_("index file corrupt"));
> +	}
>   >   	resolve_undo_clear_index(the_repository->index);
>   	if (opts->new_orphan_branch && opts->orphan_from_empty_tree) {
> @@ -797,14 +799,18 @@ static int merge_working_tree(const struct checkout_opts *opts,
>   	} else {
>   		new_tree = repo_get_commit_tree(the_repository,
>   						new_branch_info->commit);
> -		if (!new_tree)
> +		if (!new_tree) {
> +			rollback_lock_file(&lock_file);
>   			return error(_("unable to read tree (%s)"),
>   				     oid_to_hex(&new_branch_info->commit->object.oid));
> +		}
>   	}
>   	if (opts->discard_changes) {
>   		ret = reset_tree(new_tree, opts, 1, writeout_error, new_branch_info);
> -		if (ret)
> +		if (ret) {
> +			rollback_lock_file(&lock_file);
>   			return ret;
> +		}
>   	} else {
>   		struct tree_desc trees[2];
>   		struct tree *tree;
> @@ -814,6 +820,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
>   		refresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL);
>   >   		if (unmerged_index(the_repository->index)) {
> +			rollback_lock_file(&lock_file);
>   			error(_("you need to resolve your current index first"));
>   			return 1;

The changes up to here look like fixes for an existing bug and so would be better in a separate patch.

Sometimes we return "1" and sometimes "-1" what does that signal to the caller?

>   		}
> @@ -846,82 +853,8 @@ static int merge_working_tree(const struct checkout_opts *opts,
>   		ret = unpack_trees(2, trees, &topts);
>   		clear_unpack_trees_porcelain(&topts);
>   		if (ret == -1) {
> [lots of deletions]
> -			if (ret)
> -				return ret;
> +			rollback_lock_file(&lock_file);
> +			return 1;
>   		}
>   	}
>   > @@ -1166,6 +1099,10 @@ static int switch_branches(const struct checkout_opts *opts,
>   	struct object_id rev;
>   	int flag, writeout_error = 0;
>   	int do_merge = 1;
> +	int created_autostash = 0;

This can be a bool

> +	struct strbuf old_commit_shortname = STRBUF_INIT;
> +	struct strbuf autostash_msg = STRBUF_INIT;
> +	const char *stash_label_base = NULL;
>   >   	trace2_cmd_mode("branch");
>   > @@ -1203,10 +1140,37 @@ static int switch_branches(const struct checkout_opts *opts,
>   			do_merge = 0;
>   	}
>   > +	if (old_branch_info.name)
> +		stash_label_base = old_branch_info.name;
> +	else if (old_branch_info.commit) {
> +		strbuf_add_unique_abbrev(&old_commit_shortname,
> +					 &old_branch_info.commit->object.oid,
> +					 DEFAULT_ABBREV);
> +		stash_label_base = old_commit_shortname.buf;
> +	}
> +
>   	if (do_merge) {
>   		ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
> +		if (ret && opts->merge) {

As we saw above merge_working_tree() can return non-zero for a variety of reasons. We only want to try stashing if the call to unpack_trees() failed. Even then if you look at the list of errors in unpack-trees.h you'll see that only a few of them relate to problems that can be solved by stashing. The old code just tried merging whenever unpack_trees() failed so it probably not so bad to do the same here but we should not be stashing if merge_working_tree() returns before calling unpack_trees().

> +			strbuf_addf(&autostash_msg,
> +				    "autostash while switching to '%s'",
> +				    new_branch_info->name);
> +			create_autostash_ref_with_msg_silent(the_repository,
> +						   "CHECKOUT_AUTOSTASH_HEAD",

It's a shame we have to create a ref here. MERGE_AUTOSTASH exists so that "git merge --continue" can apply the stash once the user has resolved any merge conflicts. We don't have that problem here because there is no user interaction and we could just hold onto the stash oid in a variable.

> +						   autostash_msg.buf);
> +			created_autostash = 1;
> +			ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
> +		}
>   		if (ret) {

I'm confused by this - if we stash then don't we expect the call to unpack_trees() in merge_working_tree() to succeed and therefore return 0? If opts->merge is false then we should not be trying to apply the stash when merge_working_tree() fails.

> +			apply_autostash_ref_with_labels(the_repository,
> +						       "CHECKOUT_AUTOSTASH_HEAD",
> +						       new_branch_info->name,
> +						       "local",
> +						       stash_label_base,
> +						       autostash_msg.len ? autostash_msg.buf : NULL);

Can we create an autostash without setting a message in autostash_msg?

>   			branch_info_release(&old_branch_info);
> +			strbuf_release(&old_commit_shortname);
> +			strbuf_release(&autostash_msg);
>   			return ret;
>   		}
>   	}
> @@ -1216,8 +1180,31 @@ static int switch_branches(const struct checkout_opts *opts,
>   >   	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
>   > +	if (opts->conflict_style >= 0) {
> +		struct strbuf cfg = STRBUF_INIT;
> +		strbuf_addf(&cfg, "merge.conflictStyle=%s",
> +			    conflict_style_name(opts->conflict_style));
> +		git_config_push_parameter(cfg.buf);

This is rather unfortunate - it might be nicer for create_autostash_internal() to set the conflict style.

> +		strbuf_release(&cfg);
> +	}
> +	apply_autostash_ref_with_labels(the_repository, "CHECKOUT_AUTOSTASH_HEAD",
> +				       new_branch_info->name, "local",
> +				       stash_label_base,
> +				       autostash_msg.len ? autostash_msg.buf : NULL);
> +
> +	discard_index(the_repository->index);
> +	if (repo_read_index(the_repository) < 0)
> +		die(_("index file corrupt"));
> +
> +	if (created_autostash && !opts->discard_changes && !opts->quiet &&

Wouldn't it be a bug if we've created and autostash when opts->discard_changes is set? Why do we need to check it?

> +	    new_branch_info->commit)
> +		show_local_changes(&new_branch_info->commit->object,
> +				   &opts->diff_options);

So this is a change to the output when using "checkout -m"? If so it might be better as a separate change.


I'll have to leave it there for now, I'll try and look at the rest of the changes later in the week.

Thanks

Phillip

> +
>   	ret = post_checkout_hook(old_branch_info.commit, new_branch_info->commit, 1);
>   	branch_info_release(&old_branch_info);
> +	strbuf_release(&old_commit_shortname);
> +	strbuf_release(&autostash_msg);
>   >   	return ret || writeout_error;
>   }
> diff --git a/sequencer.c b/sequencer.c
> index c2516000bd..b78a8ff092 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4770,15 +4770,23 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
>   		strvec_push(&store.args, stash_oid);
>   		if (run_command(&store))
>   			ret = error(_("cannot store %s"), stash_oid);
> +		else if (attempt_apply)
> +			fprintf(stderr,
> +				_("Your local changes are stashed, however, applying it to carry\n"
> +				  "forward your local changes resulted in conflicts:\n"
> +				  "\n"
> +				  " - You can try resolving them now.  If you resolved them\n"
> +				  "   successfully, discard the stash entry with \"git stash drop\".\n"
> +				  "\n"
> +				  " - Alternatively you can \"git reset --hard\" if you do not want\n"
> +				  "   to deal with them right now, and later \"git stash pop\" to\n"
> +				  "   recover your local changes.\n"));
>   		else
>   			fprintf(stderr,
> -				_("%s\n"
> +				_("Autostash exists; creating a new stash entry.\n"
>   				  "Your changes are safe in the stash.\n"
>   				  "You can run \"git stash pop\" or"
> -				  " \"git stash drop\" at any time.\n"),
> -				attempt_apply ?
> -				_("Applying autostash resulted in conflicts.") :
> -				_("Autostash exists; creating a new stash entry."));
> +				  " \"git stash drop\" at any time.\n"));
>   	}
>   >   	return ret;
> diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
> index ad3ba6a984..e4e2cb19ce 100755
> --- a/t/t3420-rebase-autostash.sh
> +++ b/t/t3420-rebase-autostash.sh
> @@ -61,18 +61,30 @@ create_expected_failure_apply () {
>   	First, rewinding head to replay your work on top of it...
>   	Applying: second commit
>   	Applying: third commit
> -	Applying autostash resulted in conflicts.
> -	Your changes are safe in the stash.
> -	You can run "git stash pop" or "git stash drop" at any time.
> +	Your local changes are stashed, however, applying it to carry
> +	forward your local changes resulted in conflicts:
> +
> +	 - You can try resolving them now.  If you resolved them
> +	   successfully, discard the stash entry with "git stash drop".
> +
> +	 - Alternatively you can "git reset --hard" if you do not want
> +	   to deal with them right now, and later "git stash pop" to
> +	   recover your local changes.
>   	EOF
>   }
>   >   create_expected_failure_merge () {
>   	cat >expected <<-EOF
>   	$(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual)
> -	Applying autostash resulted in conflicts.
> -	Your changes are safe in the stash.
> -	You can run "git stash pop" or "git stash drop" at any time.
> +	Your local changes are stashed, however, applying it to carry
> +	forward your local changes resulted in conflicts:
> +
> +	 - You can try resolving them now.  If you resolved them
> +	   successfully, discard the stash entry with "git stash drop".
> +
> +	 - Alternatively you can "git reset --hard" if you do not want
> +	   to deal with them right now, and later "git stash pop" to
> +	   recover your local changes.
>   	Successfully rebased and updated refs/heads/rebased-feature-branch.
>   	EOF
>   }
> diff --git a/t/t7201-co.sh b/t/t7201-co.sh
> index 9bcf7c0b40..c474c6759f 100755
> --- a/t/t7201-co.sh
> +++ b/t/t7201-co.sh
> @@ -210,6 +210,214 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
>   	test_cmp expect two
>   '
>   > +test_expect_success 'checkout --merge --conflict=zdiff3 <branch>' '
> +	git checkout -f main &&
> +	git reset --hard &&
> +	git clean -f &&
> +
> +	fill a b X d e >two &&
> +	git checkout --merge --conflict=zdiff3 simple &&
> +
> +	cat <<-EOF >expect &&
> +	a
> +	<<<<<<< simple
> +	c
> +	||||||| main
> +	b
> +	c
> +	d
> +	=======
> +	b
> +	X
> +	d
> +	>>>>>>> local
> +	e
> +	EOF
> +	test_cmp expect two
> +'
> +
> +test_expect_success 'checkout -m respects merge.conflictStyle config' '
> +	git checkout -f main &&
> +	git reset --hard &&
> +	git clean -f &&
> +
> +	test_config merge.conflictStyle diff3 &&
> +	fill b d >two &&
> +	git checkout -m simple &&
> +
> +	cat <<-EOF >expect &&
> +	<<<<<<< simple
> +	a
> +	c
> +	e
> +	||||||| main
> +	a
> +	b
> +	c
> +	d
> +	e
> +	=======
> +	b
> +	d
> +	>>>>>>> local
> +	EOF
> +	test_cmp expect two
> +'
> +
> +test_expect_success 'checkout -m skips stash when no conflict' '
> +	git checkout -f main &&
> +	git clean -f &&
> +
> +	fill 0 x y z >same &&
> +	git stash list >stash-before &&
> +	git checkout -m side >actual 2>&1 &&
> +	test_grep ! "Created autostash" actual &&
> +	git stash list >stash-after &&
> +	test_cmp stash-before stash-after &&
> +	fill 0 x y z >expect &&
> +	test_cmp expect same
> +'
> +
> +test_expect_success 'checkout -m skips stash with non-conflicting dirty index' '
> +	git checkout -f main &&
> +	git clean -f &&
> +
> +	fill 0 x y z >same &&
> +	git add same &&
> +	git checkout -m side >actual 2>&1 &&
> +	test_grep ! "Created autostash" actual &&
> +	fill 0 x y z >expect &&
> +	test_cmp expect same
> +'
> +
> +test_expect_success 'checkout -m stashes and applies on conflicting changes' '
> +	git checkout -f main &&
> +	git clean -f &&
> +
> +	fill 1 2 3 4 5 6 7 >one &&
> +	git checkout -m side >actual 2>&1 &&
> +	test_grep ! "Created autostash" actual &&
> +	test_grep "Applied autostash" actual &&
> +	fill 1 2 3 4 5 6 7 >expect &&
> +	test_cmp expect one
> +'
> +
> +test_expect_success 'checkout -m with mixed staged and unstaged changes' '
> +	git checkout -f main &&
> +	git clean -f &&
> +
> +	fill 0 x y z >same &&
> +	git add same &&
> +	fill 1 2 3 4 5 6 7 >one &&
> +	git checkout -m side >actual 2>&1 &&
> +	test_grep ! "Created autostash" actual &&
> +	test_grep "Applied autostash" actual &&
> +	fill 0 x y z >expect &&
> +	test_cmp expect same &&
> +	fill 1 2 3 4 5 6 7 >expect &&
> +	test_cmp expect one
> +'
> +
> +test_expect_success 'checkout -m stashes on truly conflicting changes' '
> +	git checkout -f main &&
> +	git clean -f &&
> +
> +	fill 1 2 3 4 5 >one &&
> +	test_must_fail git checkout side 2>stderr &&
> +	test_grep "Your local changes" stderr &&
> +	git checkout -m side >actual 2>&1 &&
> +	test_grep ! "Created autostash" actual &&
> +	test_grep "resulted in conflicts" actual &&
> +	test_grep "git stash drop" actual &&
> +	git stash drop &&
> +	git reset --hard
> +'
> +
> +test_expect_success 'checkout -m produces usable stash on conflict' '
> +	git checkout -f main &&
> +	git clean -f &&
> +
> +	fill 1 2 3 4 5 >one &&
> +	git checkout -m side >actual 2>&1 &&
> +	test_grep "recover your local changes" actual &&
> +	git checkout -f main &&
> +	git stash pop &&
> +	fill 1 2 3 4 5 >expect &&
> +	test_cmp expect one
> +'
> +
> +test_expect_success 'checkout -m autostash message includes target branch' '
> +	git checkout -f main &&
> +	git clean -f &&
> +
> +	fill 1 2 3 4 5 >one &&
> +	git checkout -m side >actual 2>&1 &&
> +	git stash list >stash-list &&
> +	test_grep "autostash while switching to .side." stash-list &&
> +	git stash drop &&
> +	git checkout -f main &&
> +	git reset --hard
> +'
> +
> +test_expect_success 'checkout -m stashes on staged conflicting changes' '
> +	git checkout -f main &&
> +	git clean -f &&
> +
> +	fill 1 2 3 4 5 >one &&
> +	git add one &&
> +	git checkout -m side >actual 2>&1 &&
> +	test_grep ! "Created autostash" actual &&
> +	test_grep "resulted in conflicts" actual &&
> +	test_grep "git stash drop" actual &&
> +	git stash drop &&
> +	git reset --hard
> +'
> +
> +test_expect_success 'checkout -m applies stash cleanly with non-overlapping changes in same file' '
> +	git checkout -f main &&
> +	git reset --hard &&
> +	git clean -f &&
> +
> +	git checkout -b nonoverlap_base &&
> +	fill a b c d >file &&
> +	git add file &&
> +	git commit -m "add file" &&
> +
> +	git checkout -b nonoverlap_child &&
> +	fill a b c INSERTED d >file &&
> +	git commit -a -m "insert line near end of file" &&
> +
> +	fill DIRTY a b c INSERTED d >file &&
> +
> +	git stash list >stash-before &&
> +	git checkout -m nonoverlap_base 2>stderr &&
> +	test_grep "Applied autostash" stderr &&
> +	test_grep ! "resulted in conflicts" stderr &&
> +
> +	git stash list >stash-after &&
> +	test_cmp stash-before stash-after &&
> +
> +	fill DIRTY a b c d >expect &&
> +	test_cmp expect file &&
> +
> +	git checkout -f main &&
> +	git branch -D nonoverlap_base &&
> +	git branch -D nonoverlap_child
> +'
> +
> +test_expect_success 'checkout -m -b skips stash with dirty tree' '
> +	git checkout -f main &&
> +	git clean -f &&
> +
> +	fill 0 x y z >same &&
> +	git checkout -m -b newbranch >actual 2>&1 &&
> +	test_grep ! "Created autostash" actual &&
> +	fill 0 x y z >expect &&
> +	test_cmp expect same &&
> +	git checkout main &&
> +	git branch -D newbranch
> +'
> +
>   test_expect_success 'switch to another branch while carrying a deletion' '
>   	git checkout -f main &&
>   	git reset --hard &&
> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 9838094b66..cbef8a534e 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -914,7 +914,7 @@ test_expect_success 'merge with conflicted --autostash changes' '
>   	git diff >expect &&
>   	test_when_finished "test_might_fail git stash drop" &&
>   	git merge --autostash c3 2>err &&
> -	test_grep "Applying autostash resulted in conflicts." err &&
> +	test_grep "your local changes resulted in conflicts" err &&
>   	git show HEAD:file >merge-result &&
>   	test_cmp result.1-9 merge-result &&
>   	git stash show -p >actual &&
> diff --git a/xdiff-interface.c b/xdiff-interface.c
> index f043330f2a..5ee2b96d0a 100644
> --- a/xdiff-interface.c
> +++ b/xdiff-interface.c
> @@ -325,6 +325,18 @@ int parse_conflict_style_name(const char *value)
>   		return -1;
>   }
>   > +const char *conflict_style_name(int style)
> +{
> +	switch (style) {
> +	case XDL_MERGE_DIFF3:
> +		return "diff3";
> +	case XDL_MERGE_ZEALOUS_DIFF3:
> +		return "zdiff3";
> +	default:
> +		return "merge";
> +	}
> +}
> +
>   int git_xmerge_style = -1;
>   >   int git_xmerge_config(const char *var, const char *value,
> diff --git a/xdiff-interface.h b/xdiff-interface.h
> index fbc4ceec40..ce54e1c0e0 100644
> --- a/xdiff-interface.h
> +++ b/xdiff-interface.h
> @@ -55,6 +55,7 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line, int cflags);
>   void xdiff_clear_find_func(xdemitconf_t *xecfg);
>   struct config_context;
>   int parse_conflict_style_name(const char *value);
> +const char *conflict_style_name(int style);
>   int git_xmerge_config(const char *var, const char *value,
>   		      const struct config_context *ctx, void *cb);
>   extern int git_xmerge_style;

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

> Hi Harald
>
> For the subject line I think
>
>      checkout -m: autostash when switching branches
>
> would be more in keeping with our usual style.

Thanks.  I agree.  Alternatively, 

    checkout: autostash when switching branches with -m

would also work.

>> When switching branches with "git checkout -m", local modifications
>> can block the switch. 
>
> Really? Isn't the point of "checkout -m" to merge the local 
> modifications into the branch that's being checked out?

Yeah, either "git checkout -m" -> "git checkout" (without "-m")
or "can block" -> "can cause conflict during".  Also, I think a bit
more description of what happens in the current system without this
patch series would clarify the motivation.  Perhaps something like...

    When switching branches with "git checkout -m", the attempted
    merge of local modifications may cause conflicts with the
    changes made on the other branch, which the user may not want to
    (or may not be able to) resolve right now.  Because there is no
    easy way to recover from this situation, we discouraged users from
    using "checkout -m" unless they are certain their changes are
    trivial and within their ability to resolve conflicts.

... would contrast well with "the user can resolve or reset and
postpone 'stash pop' to some later time" we will give at the end of
the message.

>> Teach the -m flow to create a temporary stash
>> before switching and reapply it after.  On success, only "Applied
>> autostash." is shown. 
>
> and a diff of the local changes?

I noticed that we show the same output as a successful "git checkout
other-branch" without "-m" shows, i.e., short list of modified
paths.  I am not sure what it is officially called but calling it
"diff" is probably misleading.

>> If reapplying causes conflicts, the stash is
>> kept and the user is told they can resolve and run "git stash drop",
>> or run "git reset --hard" and later "git stash pop" to recover their
>> changes.

Good.

>> @@ -814,6 +820,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
>>   		refresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL);
>>   
>>   		if (unmerged_index(the_repository->index)) {
>> +			rollback_lock_file(&lock_file);
>>   			error(_("you need to resolve your current index first"));
>>   			return 1;
>
> The changes up to here look like fixes for an existing bug and so would 
> be better in a separate patch.

Good point.

> Sometimes we return "1" and sometimes "-1" what does that signal to the 
> caller?

This too.

>> @@ -846,82 +853,8 @@ static int merge_working_tree(const struct checkout_opts *opts,
>>   		ret = unpack_trees(2, trees, &topts);
>>   		clear_unpack_trees_porcelain(&topts);
>>   		if (ret == -1) {
>> [lots of deletions]
>> -			if (ret)
>> -				return ret;
>> +			rollback_lock_file(&lock_file);
>> +			return 1;
>>   		}
>>   	}
>>   
>> @@ -1166,6 +1099,10 @@ static int switch_branches(const struct checkout_opts *opts,
>>   	struct object_id rev;
>>   	int flag, writeout_error = 0;
>>   	int do_merge = 1;
>> +	int created_autostash = 0;
>
> This can be a bool

So are many things (like do_merge, writeout_error).  I wouldn't
worry too much about the difference between int and bool unless it
is a function parameter.

>> +			strbuf_addf(&autostash_msg,
>> +				    "autostash while switching to '%s'",
>> +				    new_branch_info->name);
>> +			create_autostash_ref_with_msg_silent(the_repository,
>> +						   "CHECKOUT_AUTOSTASH_HEAD",
>
> It's a shame we have to create a ref here. MERGE_AUTOSTASH exists so 
> that "git merge --continue" can apply the stash once the user has 
> resolved any merge conflicts. We don't have that problem here because 
> there is no user interaction and we could just hold onto the stash oid 
> in a variable.

Hmph, so it is not a shame and we can do without it?

> The changes up to here look like fixes for an existing bug and so would 
> be better in a separate patch.

👍

> Sometimes we return "1" and sometimes "-1" what does that signal to the 
> caller?

I just tried to follow a pattern, I'm not knowlegable of how this return code will be used. Futher down in the file we check 'ret == -1' and turn it into 1, so maybe 1 is correct?

> > +                                                    autostash_msg.len ? autostash_msg.buf : NULL);
> 
> Can we create an autostash without setting a message in autostash_msg?

No, seems not. I'll simplify it!

> > +     if (created_autostash && !opts->discard_changes && !opts->quiet &&
> 
> Wouldn't it be a bug if we've created and autostash when
> opts->discard_changes is set? Why do we need to check it?

I'll simplify it!

> > +	    new_branch_info->commit)
> > +		show_local_changes(&new_branch_info->commit->object,
> > +				   &opts->diff_options);
>
> So this is a change to the output when using "checkout -m"? If so it 
> might be better as a separate change.

Do you mean to drop if from my patchset, or just make it a separate
commit within this series?


Harald

> > +                     strbuf_addf(&autostash_msg,
> > +                                 "autostash while switching to '%s'",
> > +                                 new_branch_info->name);
> > +                     create_autostash_ref_with_msg_silent(the_repository,
> > +                                                "CHECKOUT_AUTOSTASH_HEAD",
> 
> It's a shame we have to create a ref here. MERGE_AUTOSTASH exists so
> that "git merge --continue" can apply the stash once the user has
> resolved any merge conflicts. We don't have that problem here because
> there is no user interaction and we could just hold onto the stash oid
> in a variable.

I don't know how to actually do that. Maybe better to do later?

> > +                                                autostash_msg.buf);
> > +                     created_autostash = 1;
> > +                     ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
> > +             }
> >               if (ret) {
> 
> I'm confused by this - if we stash then don't we expect the call to
> unpack_trees() in merge_working_tree() to succeed and therefore return
> 0? If opts->merge is false then we should not be trying to apply the
> stash when merge_working_tree() fails.

Same here, I'm not sure how to get this to work. Maybe better to do later?


Harald

> > +	if (old_branch_info.name)
> > +		stash_label_base = old_branch_info.name;
> > +	else if (old_branch_info.commit) {
> > +		strbuf_add_unique_abbrev(&old_commit_shortname,
> > +					 &old_branch_info.commit->object.oid,
> > +					 DEFAULT_ABBREV);
> > +		stash_label_base = old_commit_shortname.buf;
> > +	}
> > +
> >   	if (do_merge) {
> >   		ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
> > +		if (ret && opts->merge) {
> 
> As we saw above merge_working_tree() can return non-zero for a variety 
> of reasons. We only want to try stashing if the call to unpack_trees() 
> failed. Even then if you look at the list of errors in unpack-trees.h 
> you'll see that only a few of them relate to problems that can be solved 
> by stashing. The old code just tried merging whenever unpack_trees() 
> failed so it probably not so bad to do the same here but we should not 
> be stashing if merge_working_tree() returns before calling unpack_trees().

What you are saying makes a lot of sense.

I gave this a shot now, trying to return an error code that only attempts
the stashing when it has a chance of improving the outcome. Not at all sure
if it's correct though!

> > +						   autostash_msg.buf);
> > +			created_autostash = 1;
> > +			ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
> > +		}
> >   		if (ret) {
> 
> I'm confused by this - if we stash then don't we expect the call to 
> unpack_trees() in merge_working_tree() to succeed and therefore return 
> 0? If opts->merge is false then we should not be trying to apply the 
> stash when merge_working_tree() fails.

I'm attempting to fix this by making call to apply_autostash_ref
conditional on whether or not the autostash was actually created. Makes
sense?


Harald

> > > +                     strbuf_addf(&autostash_msg,
> > > +                                 "autostash while switching to '%s'",
> > > +                                 new_branch_info->name);
> > > +                     create_autostash_ref_with_msg_silent(the_repository,
> > > +                                                "CHECKOUT_AUTOSTASH_HEAD",
> > 
> > It's a shame we have to create a ref here. MERGE_AUTOSTASH exists so
> > that "git merge --continue" can apply the stash once the user has
> > resolved any merge conflicts. We don't have that problem here because
> > there is no user interaction and we could just hold onto the stash oid
> > in a variable.
> 
> I don't know how to actually do that. Maybe better to do later?

A gave this a try, but it becomes a very big change. Or maybe I'm missing
some key knowledge here.

> > > +                                                autostash_msg.buf);
> > > +                     created_autostash = 1;
> > > +                     ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
> > > +             }
> > >               if (ret) {
> > 
> > I'm confused by this - if we stash then don't we expect the call to
> > unpack_trees() in merge_working_tree() to succeed and therefore return
> > 0? If opts->merge is false then we should not be trying to apply the
> > stash when merge_working_tree() fails.
> 
> Same here, I'm not sure how to get this to work. Maybe better to do later?

I think I succeeded with this one.


Harald

On 15/04/2026 09:19, Harald Nordgren wrote:
>>>> +                     strbuf_addf(&autostash_msg,
>>>> +                                 "autostash while switching to '%s'",
>>>> +                                 new_branch_info->name);
>>>> +                     create_autostash_ref_with_msg_silent(the_repository,
>>>> +                                                "CHECKOUT_AUTOSTASH_HEAD",
>>>
>>> It's a shame we have to create a ref here. MERGE_AUTOSTASH exists so
>>> that "git merge --continue" can apply the stash once the user has
>>> resolved any merge conflicts. We don't have that problem here because
>>> there is no user interaction and we could just hold onto the stash oid
>>> in a variable.
>>
>> I don't know how to actually do that. Maybe better to do later?
> > A gave this a try, but it becomes a very big change. Or maybe I'm missing
> some key knowledge here.

Maybe leave that for now then

>>>> +                                                autostash_msg.buf);
>>>> +                     created_autostash = 1;
>>>> +                     ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
>>>> +             }
>>>>                if (ret) {
>>>
>>> I'm confused by this - if we stash then don't we expect the call to
>>> unpack_trees() in merge_working_tree() to succeed and therefore return
>>> 0? In that case we apply the stash lower down so that's fine.

>>> If opts->merge is false then we should not be trying to apply the
>>> stash when merge_working_tree() fails.
>>
>> Same here, I'm not sure how to get this to work. Maybe better to do later?
> > I think I succeeded with this one.

This one definitely needs fixing but it should be simple to do as I think it is just a logic error. We should not be trying to re-apply the stash unless we created it and we can check "created_autostash" to do that.

Thanks

Phillip

> > > Harald

On 14/04/2026 21:06, Harald Nordgren wrote:
>> The changes up to here look like fixes for an existing bug and so would
>> be better in a separate patch.
> > 👍
> >> Sometimes we return "1" and sometimes "-1" what does that signal to the
>> caller?
> > I just tried to follow a pattern, I'm not knowlegable of how this return
> code will be used. Futher down in the file we check 'ret == -1' and
> turn it into 1, so maybe 1 is correct?

But you can read the code to see how it is used. Tracing the return path of merge_working_tree(), the return value get propagated back up to the top of the call stack i.e. cmd_checkout() or cmd_switch() and used as the return value there. I had wondered if we were using the value on the way back up the stack and doing something different based on the whether it was "1" or "-1" but we don't so it only affects the exit code of "git checkout". That means returning "1" is sensible I think.

> Do you mean to drop if from my patchset, or just make it a separate
> commit within this series?

A separate commit in this series. As "git checkout" without "-m" can also carry local changes across we probably should do the same there as well.

Thanks

Phillip

On 15/04/2026 09:16, Harald Nordgren wrote:
>>> +	if (old_branch_info.name)
>>> +		stash_label_base = old_branch_info.name;
>>> +	else if (old_branch_info.commit) {
>>> +		strbuf_add_unique_abbrev(&old_commit_shortname,
>>> +					 &old_branch_info.commit->object.oid,
>>> +					 DEFAULT_ABBREV);
>>> +		stash_label_base = old_commit_shortname.buf;
>>> +	}
>>> +
>>>    	if (do_merge) {
>>>    		ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
>>> +		if (ret && opts->merge) {
>>
>> As we saw above merge_working_tree() can return non-zero for a variety
>> of reasons. We only want to try stashing if the call to unpack_trees()
>> failed. Even then if you look at the list of errors in unpack-trees.h
>> you'll see that only a few of them relate to problems that can be solved
>> by stashing. The old code just tried merging whenever unpack_trees()
>> failed so it probably not so bad to do the same here but we should not
>> be stashing if merge_working_tree() returns before calling unpack_trees().
> > What you are saying makes a lot of sense.
> > I gave this a shot now, trying to return an error code that only attempts
> the stashing when it has a chance of improving the outcome. Not at all sure
> if it's correct though!

That sounds like the right approach

>>> +						   autostash_msg.buf);
>>> +			created_autostash = 1;
>>> +			ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
>>> +		}
>>>    		if (ret) {
>>
>> I'm confused by this - if we stash then don't we expect the call to
>> unpack_trees() in merge_working_tree() to succeed and therefore return
>> 0? If opts->merge is false then we should not be trying to apply the
>> stash when merge_working_tree() fails.
> > I'm attempting to fix this by making call to apply_autostash_ref
> conditional on whether or not the autostash was actually created. Makes
> sense?

Yes, exactly

Thanks

Phillip

"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 &&

> 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

Fixed, thanks!


Harald

Hi Harald

On 09/04/2026 20:17, Harald Nordgren via GitGitGadget wrote:
> 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.

> 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.

> 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?

> +	(
> +		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)

> +		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

> +	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)

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

>> +test_expect_success 'apply with empty conflict labels' '
>
> Why do we want to support empty labels rather than making them an error?

Why not?

There are applications that do not require, and prefer to have more
stable output that will not be affected by UI updates to improve
human-user experience.  Even though rerere database is not populated
with the facility this patch implements, we can see in

    $ grep -C2 -e '^\([<=>]\)\1\{6,\}$' .git/rr-cache/*/preimage*

how having labels make the output more noisy, and more importantly,
will make it misleading given how rerere is designed to work,
treating the same merge conflicts in both directions equivalents.
It is not a huge stretch of imagination that our users will find
similar needs, I would imagine.

> 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.

Wow, that command is so powerful! Thanks for sharing that!

Will shift that definition to an earlier commit in my set.

> Why do we need to create a new repository just to stash some changes?

Isn't it good to do it in isolation, for when the test and/or its cleanup
fails. I tried to change it now, but it's not trivial, I quickly broke a
lot of subsequent tests.

> We have a helper test_commit() for creating commits (it is documented in 
> t/test-lib-functions.sh)

Thanks, will update!


Harald

Hi Harald

On 14/04/2026 13:59, Harald Nordgren via GitGitGadget wrote:
> 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.

> 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

> +		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)

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

>> +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.
>
>> 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.

Both are exellent points.  

I also agree with your comments on create_autostash_ref() in [2/4],
extending apply_autostash_ref() with optional three or four extra
parameters and updating existing callers in [3/4].

I have v10 already merged to 'next', but I think it is better to
revert the merge and give these finishing touches, as we are not
in a rush to add more topics to 'next' before 2.54 final anyway.

Thanks.

> 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.

Also a good point, and I will update it.

Harald

> > +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.

Yes, I want this too, but I had some problems to get it to work. Found a
way now I think, but the cleanup is not 100% trivial (this is the only
reason to run anything inside a new repo).

Harald

On 14/04/2026 21:08, Harald Nordgren wrote:
>>> +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.
> > Yes, I want this too, but I had some problems to get it to work. Found a
> way now I think, but the cleanup is not 100% trivial (this is the only
> reason to run anything inside a new repo).

Normally the first test would setup some commits with test_commit() that creates a tag so you can just use "git reset --hard <tag>" to start your test from a known state. Unfortunately setup_stash() does not use test_commit() so there are no tags. It would be useful to fix that by adding a line that creates a tag so that future test authors do not face the same problem.

Thanks

Phillip

> Normally the first test would setup some commits with test_commit() that
> creates a tag so you can just use "git reset --hard <tag>" to start your
> test from a known state. Unfortunately setup_stash() does not use
> test_commit() so there are no tags. It would be useful to fix that by
> adding a line that creates a tag so that future test authors do not face
> the same problem.

Sounds reasonable, but it's surprisingly easy to break the subsequent
tests.

My solution now will be to move these tests to last in the test file.


Harald