Avoiding checkout -m autostash retries when no tracked local changes exist and visually separating autostash conflict advice from the subsequent branch-switch message.

Addresses #leftoverbits from here: https://lore.kernel.org/git/cfd09dbf-8d77-4464-8030-3a0ffb4aeae7@gmail.com/

Harald Nordgren (2):
  sequencer: teach autostash apply to report conflicts
  checkout -m: refine autostash fallback

 builtin/checkout.c | 18 ++++++++++++++----
 builtin/commit.c   |  2 +-
 builtin/merge.c    |  6 +++---
 sequencer.c        | 29 +++++++++++++++++++----------
 sequencer.h        |  3 ++-
 t/t7201-co.sh      | 17 ++++++++++++++++-
 6 files changed, 55 insertions(+), 20 deletions(-)

base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2364%2FHaraldNordgren%2Fhn%2Fgit-checkout-m-leftoverbits-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2364/HaraldNordgren/hn/git-checkout-m-leftoverbits-v1
Pull-Request: https://github.com/git/git/pull/2364

From: Harald Nordgren <haraldnordgren@gmail.com>

Add a conflicted parameter to apply_save_autostash_oid() and apply_save_autostash_ref() so callers can learn whether applying the stash resulted in conflicts. Thread the parameter through apply_autostash_ref() and update existing callers to pass NULL.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 builtin/checkout.c |  3 ++-
 builtin/commit.c   |  2 +-
 builtin/merge.c    |  6 +++---
 sequencer.c        | 29 +++++++++++++++++++----------
 sequencer.h        |  3 ++-
 5 files changed, 27 insertions(+), 16 deletions(-)

From: Harald Nordgren <haraldnordgren@gmail.com>

When unpack_trees() fails under "git checkout -m", only create an autostash and retry if there are tracked local changes. Without such changes, the fallback cannot help and merely repeats the same failure.

Use the conflict result from apply_autostash_ref() to print a blank line before the branch-switch message, visually separating it from the conflict advice.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 builtin/checkout.c | 17 +++++++++++++----
 t/t7201-co.sh      | 17 ++++++++++++++++-
 2 files changed, 29 insertions(+), 5 deletions(-)

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

Thanks.  Will queue.

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

This topic unfortunately has seen no interests from others on the list. Asking Phillip for help, as the leftoverbits comment cited in the cover letter is from him.

Thanks.

Hi Harald

On 25/07/2026 16:34, Harald Nordgren via GitGitGadget wrote:

It would be nicer to use the return value to indicate success/error/conflicts rather than adding yet another parameter. Apart from that this looks good.

Thanks

Phillip

Hi Harald

On 25/07/2026 16:34, Harald Nordgren via GitGitGadget wrote:

Unfortunately to do that we have to look for local changes before the first call to unpack_trees() so we're trading an occasional inconvenience of an unnecessary stash and unstash for the cost of looking for local changes on every invocation of "git checkout -m". I don't think that is a good trade off, especially as there is no guarantee that stashing the local changes will make unpack_trees() succeed. To do this effectively would require refactoring unpack_trees() to write its error messages to a buffer and return an error flag that indicates all the errors that were encountered. We could then check if the only thing that prevented upack_trees() from succeeding was local changes to files and stash them, or if there are other errors print the error message. I suspect such a change is far from straight forward.

This change is very welcome and could happily be squashed into the first patch as it motivates the changes in it.

I wonder whether we should just bite the bullet and check what gets printed to the screen with test_cmp, rather than grepping for all these separate parts of the message. Is there something in the message that makes that difficult?

Thanks

Phillip

On 26/08/2026 20:14, Junio C Hamano wrote:

That'll teach me to make suggestions for further work when I'm reviewing patches! Thanks for following up on those comments Harald. I've left some comments, in short I think the changes to the error message are a good idea, but I'm not sure sure about the other change in patch 2 though.

Thanks

Phillip

Is the int flexible enough to separate autostash conflict from other errors? Or should we introduce an enum here?

Harald

From: Harald Nordgren <haraldnordgren@gmail.com>

When "git checkout -m" stashes the user's local changes and then fails to re-apply them because of conflicts, the conflict advice is printed directly on top of the branch-switch message ("Switched to branch ..."), making the two messages hard to tell apart. Print a blank line in between when the stash apply conflicted, so that the advice and the branch-switch message are visually distinct.

To learn whether the stash apply resulted in conflicts, make the autostash apply functions return 1 when the stash could not be applied due to conflicts (and was stored back), while success and error remain 0 and -1 respectively. Checkout can then use the return value to decide whether to print the blank line.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
    checkout -m: refine autostash fallback
    
    Avoiding checkout -m autostash retries when no tracked local changes
    exist and visually separating autostash conflict advice from the
    subsequent branch-switch message. Addresses #leftoverbits from here:
    https://lore.kernel.org/git/cfd09dbf-8d77-4464-8030-3a0ffb4aeae7@gmail.com/
    
    Changes in v2:
    
     * Simplify logic and combine to one commit.
     * Test full output with test_cmp.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2364%2FHaraldNordgren%2Fhn%2Fgit-checkout-m-leftoverbits-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2364/HaraldNordgren/hn/git-checkout-m-leftoverbits-v2
Pull-Request: https://github.com/git/git/pull/2364

Range-diff vs v1:

 1:  b501b5fcd0 < -:  ---------- sequencer: teach autostash apply to report conflicts
 2:  37becf38c2 ! 1:  b44c53fcf2 checkout -m: refine autostash fallback
     @@ Metadata
      Author: Harald Nordgren <haraldnordgren@gmail.com>
      
       ## Commit message ##
     -    checkout -m: refine autostash fallback
     +    checkout: print blank line after autostash conflict advice
      
     -    When unpack_trees() fails under "git checkout -m", only create an
     -    autostash and retry if there are tracked local changes.  Without such
     -    changes, the fallback cannot help and merely repeats the same failure.
     +    When "git checkout -m" stashes the user's local changes and then fails
     +    to re-apply them because of conflicts, the conflict advice is printed
     +    directly on top of the branch-switch message ("Switched to branch ..."),
     +    making the two messages hard to tell apart.  Print a blank line in
     +    between when the stash apply conflicted, so that the advice and the
     +    branch-switch message are visually distinct.
      
     -    Use the conflict result from apply_autostash_ref() to print a blank line
     -    before the branch-switch message, visually separating it from the
     -    conflict advice.
     +    To learn whether the stash apply resulted in conflicts, make the
     +    autostash apply functions return 1 when the stash could not be applied
     +    due to conflicts (and was stored back), while success and error remain
     +    0 and -1 respectively.  Checkout can then use the return value to decide
     +    whether to print the blank line.
      
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
       ## builtin/checkout.c ##
     -@@ builtin/checkout.c: static void init_topts(struct unpack_trees_options *topts,
     - static int merge_working_tree(const struct checkout_opts *opts,
     - 			      struct branch_info *old_branch_info,
     - 			      struct branch_info *new_branch_info,
     --			      bool quiet,
     -+			      bool allow_autostash,
     - 			      int *writeout_error)
     - {
     - 	int ret;
     -+	bool can_autostash = false;
     - 	struct lock_file lock_file = LOCK_INIT;
     - 	struct tree *new_tree;
     - 
     -@@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *opts,
     - 			return 1;
     - 		}
     - 
     -+		if (allow_autostash)
     -+			can_autostash = has_unstaged_changes(the_repository, 1) ||
     -+				has_uncommitted_changes(the_repository, 1);
     -+
     - 		/* 2-way merge to the new branch */
     - 		init_topts(&topts, opts->show_progress,
     --			   opts->overwrite_ignore, quiet);
     -+			   opts->overwrite_ignore, can_autostash);
     - 		init_checkout_metadata(&topts.meta, new_branch_info->refname,
     - 				       new_branch_info->commit ?
     - 				       &new_branch_info->commit->object.oid :
     -@@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *opts,
     - 		clear_unpack_trees_porcelain(&topts);
     - 		if (ret == -1) {
     - 			rollback_lock_file(&lock_file);
     --			return MERGE_WORKING_TREE_UNPACK_FAILED;
     -+			return can_autostash ?
     -+				MERGE_WORKING_TREE_UNPACK_FAILED : 1;
     - 		}
     - 	}
     - 
      @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
       	int flag, writeout_error = 0;
       	int do_merge = 1;
       	int created_autostash = 0;
     -+	bool autostash_conflicted = false;
     ++	int autostash_res = 0;
       	struct strbuf old_commit_shortname = STRBUF_INIT;
       	struct strbuf autostash_msg = STRBUF_INIT;
       	const char *stash_label_base = NULL;
      @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
     - 					    "local",
     - 					    stash_label_base,
     - 					    autostash_msg.buf,
     --					    NULL);
     -+					    &autostash_conflicted);
     + 				git_config_push_parameter(cfg.buf);
     + 				strbuf_release(&cfg);
     + 			}
     +-			apply_autostash_ref(the_repository,
     +-					    "CHECKOUT_AUTOSTASH_HEAD",
     +-					    new_branch_info->name,
     +-					    "local",
     +-					    stash_label_base,
     +-					    autostash_msg.buf);
     ++			autostash_res = apply_autostash_ref(the_repository,
     ++				    "CHECKOUT_AUTOSTASH_HEAD",
     ++				    new_branch_info->name,
     ++				    "local",
     ++				    stash_label_base,
     ++				    autostash_msg.buf);
       		}
       		if (ret) {
       			branch_info_release(&old_branch_info);
     @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
       	if (!opts->quiet && !old_branch_info.path && old_branch_info.commit && new_branch_info->commit != old_branch_info.commit)
       		orphaned_commit_warning(old_branch_info.commit, new_branch_info->commit);
       
     -+	if (autostash_conflicted && !opts->quiet)
     ++	if (autostash_res == 1 && !opts->quiet)
      +		fputc('\n', stderr);
       	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
       
       	if (created_autostash) {
      
     + ## sequencer.c ##
     +@@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
     + 		if (label_base)
     + 			strvec_pushf(&child.args, "--label-base=%s", label_base);
     + 		strvec_push(&child.args, stash_oid);
     +-		ret = run_command(&child);
     ++		if (run_command(&child))
     ++			ret = 1;
     + 	}
     + 
     + 	if (attempt_apply && !ret)
     +
       ## t/t7201-co.sh ##
      @@ t/t7201-co.sh: test_expect_success 'checkout -m creates a recoverable stash on conflict' '
     - 	test_grep "git stash drop" actual &&
     - 	test_grep "git stash pop" actual &&
     - 	test_grep "The following paths have local changes" actual &&
     -+	sed -n "/apply the local changes later/,/Switched to branch/p" \
     -+		actual >separator.actual &&
     -+	cat >separator.expect <<-EOF &&
     + 	test_must_fail git checkout side 2>stderr &&
     + 	test_grep "Your local changes" stderr &&
     + 	git checkout -m side >actual 2>&1 &&
     +-	test_grep "resulted in conflicts" actual &&
     +-	test_grep "git stash drop" actual &&
     +-	test_grep "git stash pop" actual &&
     +-	test_grep "The following paths have local changes" actual &&
     ++	cat >expect <<-EOF &&
     ++	Your local changes are stashed, however applying them
     ++	resulted in conflicts.  You can either resolve the conflicts
     ++	and then discard the stash with "git stash drop", or, if you
     ++	do not want to resolve them now, run "git reset --hard" and
      +	apply the local changes later by running "git stash pop".
      +
      +	Switched to branch ${SQ}side${SQ}
     ++	The following paths have local changes:
     ++	M	one
      +	EOF
     -+	test_cmp separator.expect separator.actual &&
     ++	test_cmp expect actual &&
       	git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual &&
       	sed /^index/d actual >actual.trimmed &&
       	cat >expect <<-EOF &&
     -@@ t/t7201-co.sh: test_expect_success 'checkout -m creates a recoverable stash on conflict' '
     - 	git reset --hard
     - '
     - 
     --test_expect_success 'checkout -m which would overwrite untracked file' '
     -+test_expect_success 'checkout -m only retries untracked-file failure with local changes' '
     - 	git checkout -f --detach main &&
     - 	test_commit another-file &&
     - 	git checkout HEAD^ &&
     - 	>another-file.t &&
     -+	test_must_fail env GIT_TRACE2_EVENT="$(pwd)/trace" \
     -+		git checkout -m @{-1} 2>err &&
     -+	test_grep "untracked working tree files" err &&
     -+	grep "\"region_enter\".*\"category\":\"index\",\"label\":\"refresh\"" \
     -+		trace >refresh.events &&
     -+	test_line_count = 1 refresh.events &&
     -+
     - 	fill 1 2 3 4 5 >one &&
     - 	test_must_fail git checkout -m @{-1} 2>err &&
     - 	q_to_tab >expect <<-\EOF &&

 builtin/checkout.c | 15 +++++++++------
 sequencer.c        |  3 ++-
 t/t7201-co.sh      | 16 ++++++++++++----
 3 files changed, 23 insertions(+), 11 deletions(-)

On 31/08/2026 11:18, Harald Nordgren wrote:

I think a tri-state enum would be clearest - the callers are only interested it "stash applied without conflicts", "stash applied with conflicts" and "something went wrong - could not apply stash"

Thanks

Phillip

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

This does not look like the right way to have the function return 1 if the objective is to do so only when the spawned "git stash apply <oid>" process fails due to conflicts.

The reasons for a non-zero return value from run_command() range from failing to find the command to run, to the command crashing, to the command voluntarily exiting with a fixed exit status to signal how exactly it exited to the spawning process.

Skimming bultin/stash.c::do_apply_stash(), I find this bit:

	clean = merge_ort_nonrecursive(&o, head, merge, merge_base);

	/*
	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
	 * merge was clean, and nonzero if the merge was unclean or encountered
	 * an error.
	 */
	ret = clean >= 0 ? !clean : clean;

If 'clean' is 0 (i.e., unclean merge), we assign 1 (== !clean) to ret. If 'clean' is 1 or more (presumably a clean merge? Check what merge_ort_nonrecursive() does for details---I didn't), return 0. If merge-ort gave an error (i.e., negative), use it as-is. And this 'ret' is eventually returned to the caller apply_stash() , which returns that value as-is.

The value is given back to builtin/stash.c::cmd_stash() and the more detailed exit value unfortunately discarded ehre:

	if (fn)
		return !!fn(argc, argv, prefix, repo);

but if one wants to implement 

    > To learn whether the stash apply resulted in conflicts, make the
    > autostash apply functions return 1 when the stash could not be applied
    > due to conflicts (and was stored back),...

correctly, I think this patch needs a bit more work.  

 (1) Find the values returned by foo_stash() implementations of all
     'git stash foo' subcommands.  Do they follow the 0 == success,
     negative == failure, positive == something else convention?

 (2) Update the code above to something like

	if (fn) {
		int ret = fn(argc, argv, prefix, repo);

		if (ret < 0)
			return 1; /* as before */
		if (!ret)
			return 0; /* as before */
		return ret + 1;
	}

      or with whatever necessary tweak to allow the positive "I have
      something to tell the spawning process" case from the bog
      standard and boring 'return error("message");' cases.

 (3) document what each of 'git stash foo' signals with its exit
     status.

For expediency, it may be OK to assume any and all failures from "git stash apply <oid>" come from a conflicted stash application in your first version. If that is what your reviewer recommended, I would agree. But let's help users and future developers (who do not necessarily have to be you) by leaving a note that this code is not doing what it claims to do and needs more work in the code.

Thanks.

On 31/08/2026 18:19, Junio C Hamano wrote:

 > > For expediency, it may be OK to assume any and all failures from

I think if the objective of this patch is to tell the caller whether the conflicts message was printed or not then it is correct because the existing code is too caviler about printing that message. We should at least tighten that even if we don't change "git stash" (which I agree we should fix at some point).

	ret = run_command(&child);
	if (ret > 1)
		ret = -1;

would catch run_command() failing and stash dying or being killed by a signal. Then we should change the code below so that it only claims there were conflicts when "ret == 1" and prints a new error message explaining that "git stash apply" failed when "ret == -1"

Thanks

Phillip

From: Harald Nordgren <haraldnordgren@gmail.com>

"git checkout -m" stashes the user's local changes when it cannot perform the checkout, and then applies the stash. When applying the stash results in conflicts, the advice on how to deal with them is printed directly on top of the branch-switch message ("Switched to branch ..."), making the two hard to tell apart. Print a blank line in between so that the advice and the branch-switch message are visually distinct.

To make this possible, "git stash apply", "pop" and "branch" now exit with status 2 when applying the stash entry resulted in conflicts, in which case the stash entry is left in place; other failures exit with status 1, as before. The exit statuses are documented in the "git stash" documentation.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
    checkout -m: refine autostash fallback
    
    Avoiding checkout -m autostash retries when no tracked local changes
    exist and visually separating autostash conflict advice from the
    subsequent branch-switch message. Addresses #leftoverbits from here:
    https://lore.kernel.org/git/cfd09dbf-8d77-4464-8030-3a0ffb4aeae7@gmail.com/
    
    Changes in v3:
    
     * Use enum for git stash return values, to separate conflict from
       generic error.
    
    Changes in v2:
    
     * Simplify logic and combine to one commit.
     * Test full output with test_cmp.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2364%2FHaraldNordgren%2Fhn%2Fgit-checkout-m-leftoverbits-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2364/HaraldNordgren/hn/git-checkout-m-leftoverbits-v3
Pull-Request: https://github.com/git/git/pull/2364

Range-diff vs v2:

 1:  b44c53fcf2 ! 1:  8e1979dd6c checkout: print blank line after autostash conflict advice
     @@ Metadata
      Author: Harald Nordgren <haraldnordgren@gmail.com>
      
       ## Commit message ##
     -    checkout: print blank line after autostash conflict advice
     +    checkout: separate autostash conflict advice from branch-switch message
      
     -    When "git checkout -m" stashes the user's local changes and then fails
     -    to re-apply them because of conflicts, the conflict advice is printed
     -    directly on top of the branch-switch message ("Switched to branch ..."),
     -    making the two messages hard to tell apart.  Print a blank line in
     -    between when the stash apply conflicted, so that the advice and the
     -    branch-switch message are visually distinct.
     +    "git checkout -m" stashes the user's local changes when it cannot
     +    perform the checkout, and then applies the stash.  When applying the
     +    stash results in conflicts, the advice on how to deal with them is
     +    printed directly on top of the branch-switch message ("Switched to
     +    branch ..."), making the two hard to tell apart.  Print a blank line
     +    in between so that the advice and the branch-switch message are
     +    visually distinct.
      
     -    To learn whether the stash apply resulted in conflicts, make the
     -    autostash apply functions return 1 when the stash could not be applied
     -    due to conflicts (and was stored back), while success and error remain
     -    0 and -1 respectively.  Checkout can then use the return value to decide
     -    whether to print the blank line.
     +    To make this possible, "git stash apply", "pop" and "branch" now exit
     +    with status 2 when applying the stash entry resulted in conflicts, in
     +    which case the stash entry is left in place; other failures exit with
     +    status 1, as before.  The exit statuses are documented in the "git
     +    stash" documentation.
      
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
     + ## Documentation/git-stash.adoc ##
     +@@ Documentation/git-stash.adoc: include::includes/cmd-config-section-all.adoc[]
     + :git-stash: 1
     + include::config/stash.adoc[]
     + 
     ++EXIT STATUS
     ++-----------
     ++
     ++The `git stash` subcommands exit with status 0 on success and non-zero
     ++on failure.  The subcommands that apply a stash entry, i.e. `apply`,
     ++`pop` and `branch`, exit with status 2 when applying the stash entry
     ++resulted in conflicts, in which case the stash entry is left in place.
     ++Other failures exit with status 1 (usage errors exit with status 129).
     ++
     + 
     + SEE ALSO
     + --------
     +
       ## builtin/checkout.c ##
      @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
       	int flag, writeout_error = 0;
       	int do_merge = 1;
       	int created_autostash = 0;
     -+	int autostash_res = 0;
     ++	enum stash_apply_result autostash_res = STASH_APPLY_CLEAN;
       	struct strbuf old_commit_shortname = STRBUF_INIT;
       	struct strbuf autostash_msg = STRBUF_INIT;
       	const char *stash_label_base = NULL;
     @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
       	if (!opts->quiet && !old_branch_info.path && old_branch_info.commit && new_branch_info->commit != old_branch_info.commit)
       		orphaned_commit_warning(old_branch_info.commit, new_branch_info->commit);
       
     -+	if (autostash_res == 1 && !opts->quiet)
     ++	if (autostash_res == STASH_APPLY_CONFLICT && !opts->quiet)
      +		fputc('\n', stderr);
       	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
       
       	if (created_autostash) {
      
     + ## builtin/stash.c ##
     +@@
     + #include "object-name.h"
     + #include "parse-options.h"
     + #include "refs.h"
     ++#include "stash.h"
     + #include "lockfile.h"
     + #include "cache-tree.h"
     + #include "unpack-trees.h"
     +@@ builtin/stash.c: static void unstage_changes_unless_new(struct object_id *orig_tree)
     + 		die(_("could not write index"));
     + }
     + 
     +-static int do_apply_stash(const char *prefix, struct stash_info *info,
     +-			  int index, int quiet,
     ++static enum stash_apply_result do_apply_stash(const char *prefix,
     ++					      struct stash_info *info,
     ++					      int index, int quiet,
     + 			  const char *label_ours, const char *label_theirs,
     + 			  const char *label_base)
     + {
     +@@ builtin/stash.c: static int do_apply_stash(const char *prefix, struct stash_info *info,
     + 	clean = merge_ort_nonrecursive(&o, head, merge, merge_base);
     + 
     + 	/*
     +-	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
     +-	 * merge was clean, and nonzero if the merge was unclean or encountered
     +-	 * an error.
     ++	 * Translate the value of 'clean' so 'ret' is STASH_APPLY_CLEAN
     ++	 * when the merge was clean, STASH_APPLY_CONFLICT when it was
     ++	 * unclean, and a negative value if it encountered an error.
     + 	 */
     +-	ret = clean >= 0 ? !clean : clean;
     ++	ret = clean >= 0 ? (clean ? STASH_APPLY_CLEAN : STASH_APPLY_CONFLICT)
     ++			 : clean;
     + 
     + 	if (ret < 0)
     + 		rollback_lock_file(&lock);
     +@@ builtin/stash.c: static int do_apply_stash(const char *prefix, struct stash_info *info,
     + 
     + 	if (has_index) {
     + 		if (reset_tree(&index_tree, 0, 0))
     +-			ret = -1;
     ++			ret = STASH_APPLY_ERROR;
     + 	} else {
     + 		unstage_changes_unless_new(&c_tree);
     + 	}
     +@@ builtin/stash.c: int cmd_stash(int argc,
     + 	strbuf_addf(&stash_index_path, "%s.stash.%" PRIuMAX, index_file,
     + 		    (uintmax_t)pid);
     + 
     +-	if (fn)
     +-		return !!fn(argc, argv, prefix, repo);
     +-	else if (!argc)
     ++	if (fn) {
     ++		ret = fn(argc, argv, prefix, repo);
     ++
     ++		if (ret < 0)
     ++			return 1;
     ++		return ret;
     ++	} else if (!argc)
     + 		return !!push_stash_unassumed(0, NULL, prefix, repo);
     + 
     + 	/* Assume 'stash push' */
     +
       ## sequencer.c ##
     +@@
     + #include "commit.h"
     + #include "sequencer.h"
     + #include "run-command.h"
     ++#include "stash.h"
     + #include "hook.h"
     + #include "utf8.h"
     + #include "cache-tree.h"
     +@@ sequencer.c: void create_autostash_ref(struct repository *r, const char *refname,
     + 	create_autostash_internal(r, NULL, refname, message, silent);
     + }
     + 
     +-static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
     +-				    const char *label_ours, const char *label_theirs,
     +-				    const char *label_base,
     +-				    const char *stash_msg)
     ++static enum stash_apply_result apply_save_autostash_oid(const char *stash_oid,
     ++							int attempt_apply,
     ++							const char *label_ours,
     ++							const char *label_theirs,
     ++							const char *label_base,
     ++							const char *stash_msg)
     + {
     + 	struct child_process child = CHILD_PROCESS_INIT;
     +-	int ret = 0;
     ++	enum stash_apply_result ret = STASH_APPLY_CLEAN;
     + 
     + 	if (attempt_apply) {
     + 		child.git_cmd = 1;
      @@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
     - 		if (label_base)
       			strvec_pushf(&child.args, "--label-base=%s", label_base);
       		strvec_push(&child.args, stash_oid);
     --		ret = run_command(&child);
     -+		if (run_command(&child))
     -+			ret = 1;
     + 		ret = run_command(&child);
     ++		if (ret && ret != STASH_APPLY_CONFLICT)
     ++			ret = STASH_APPLY_ERROR;
       	}
       
     - 	if (attempt_apply && !ret)
     +-	if (attempt_apply && !ret)
     ++	if (attempt_apply && ret == STASH_APPLY_CLEAN)
     + 		fprintf(stderr, _("Applied autostash.\n"));
     + 	else {
     + 		struct child_process store = CHILD_PROCESS_INIT;
     +@@ sequencer.c: 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)
     ++		else if (attempt_apply && ret == STASH_APPLY_CONFLICT)
     + 			fprintf(stderr,
     + 				_("Your local changes are stashed, however applying them\n"
     + 				  "resulted in conflicts.  You can either resolve the conflicts\n"
     + 				  "and then discard the stash with \"git stash drop\", or, if you\n"
     + 				  "do not want to resolve them now, run \"git reset --hard\" and\n"
     + 				  "apply the local changes later by running \"git stash pop\".\n"));
     ++		else if (attempt_apply)
     ++			ret = error(_("could not apply autostash"));
     + 		else
     + 			fprintf(stderr,
     + 				_("Autostash exists; creating a new stash entry.\n"
     +@@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
     + 	return ret;
     + }
     + 
     +-static int apply_save_autostash(const char *path, int attempt_apply)
     ++static enum stash_apply_result apply_save_autostash(const char *path,
     ++						    int attempt_apply)
     + {
     + 	struct strbuf stash_oid = STRBUF_INIT;
     +-	int ret = 0;
     ++	enum stash_apply_result ret = STASH_APPLY_CLEAN;
     + 
     + 	if (!read_oneliner(&stash_oid, path,
     + 			   READ_ONELINER_SKIP_IF_EMPTY)) {
     + 		strbuf_release(&stash_oid);
     +-		return 0;
     ++		return STASH_APPLY_CLEAN;
     + 	}
     + 	strbuf_trim(&stash_oid);
     + 
     +@@ sequencer.c: static int apply_save_autostash(const char *path, int attempt_apply)
     + 	return ret;
     + }
     + 
     +-int save_autostash(const char *path)
     ++enum stash_apply_result save_autostash(const char *path)
     + {
     + 	return apply_save_autostash(path, 0);
     + }
     + 
     +-int apply_autostash(const char *path)
     ++enum stash_apply_result apply_autostash(const char *path)
     + {
     + 	return apply_save_autostash(path, 1);
     + }
     + 
     +-int apply_autostash_oid(const char *stash_oid)
     ++enum stash_apply_result apply_autostash_oid(const char *stash_oid)
     + {
     + 	return apply_save_autostash_oid(stash_oid, 1, NULL, NULL, NULL, NULL);
     + }
     + 
     +-static int apply_save_autostash_ref(struct repository *r, const char *refname,
     +-				    int attempt_apply,
     +-				    const char *label_ours, const char *label_theirs,
     +-				    const char *label_base,
     +-				    const char *stash_msg)
     ++static enum stash_apply_result apply_save_autostash_ref(struct repository *r,
     ++							const char *refname,
     ++							int attempt_apply,
     ++							const char *label_ours,
     ++							const char *label_theirs,
     ++							const char *label_base,
     ++							const char *stash_msg)
     + {
     + 	struct object_id stash_oid;
     + 	char stash_oid_hex[GIT_MAX_HEXSZ + 1];
     +-	int flag, ret;
     ++	int flag;
     ++	enum stash_apply_result ret;
     + 
     + 	if (!refs_ref_exists(get_main_ref_store(r), refname))
     +-		return 0;
     ++		return STASH_APPLY_CLEAN;
     + 
     + 	if (!refs_resolve_ref_unsafe(get_main_ref_store(r), refname,
     + 				     RESOLVE_REF_READING, &stash_oid, &flag))
     +-		return -1;
     ++		return STASH_APPLY_ERROR;
     + 	if (flag & REF_ISSYMREF)
     + 		return error(_("autostash reference is a symref"));
     + 
     +@@ sequencer.c: static int apply_save_autostash_ref(struct repository *r, const char *refname,
     + 	return ret;
     + }
     + 
     +-int save_autostash_ref(struct repository *r, const char *refname)
     ++enum stash_apply_result save_autostash_ref(struct repository *r,
     ++					   const char *refname)
     + {
     + 	return apply_save_autostash_ref(r, refname, 0,
     + 					NULL, NULL, NULL, NULL);
     + }
     + 
     +-int apply_autostash_ref(struct repository *r, const char *refname,
     +-			const char *label_ours, const char *label_theirs,
     +-			const char *label_base, const char *stash_msg)
     ++enum stash_apply_result apply_autostash_ref(struct repository *r,
     ++					    const char *refname,
     ++					    const char *label_ours,
     ++					    const char *label_theirs,
     ++					    const char *label_base,
     ++					    const char *stash_msg)
     + {
     + 	return apply_save_autostash_ref(r, refname, 1,
     + 					label_ours, label_theirs, label_base,
     +
     + ## sequencer.h ##
     +@@
     + 
     + #include "strbuf.h"
     + #include "strvec.h"
     ++#include "stash.h"
     + #include "wt-status.h"
     + 
     + struct commit;
     +@@ sequencer.h: void commit_post_rewrite(struct repository *r,
     + void create_autostash(struct repository *r, const char *path);
     + void create_autostash_ref(struct repository *r, const char *refname,
     + 			  const char *message, bool silent);
     +-int save_autostash(const char *path);
     +-int save_autostash_ref(struct repository *r, const char *refname);
     +-int apply_autostash(const char *path);
     +-int apply_autostash_oid(const char *stash_oid);
     +-int apply_autostash_ref(struct repository *r, const char *refname,
     +-			const char *label_ours, const char *label_theirs,
     +-			const char *label_base, const char *stash_msg);
     ++enum stash_apply_result save_autostash(const char *path);
     ++enum stash_apply_result save_autostash_ref(struct repository *r,
     ++					   const char *refname);
     ++enum stash_apply_result apply_autostash(const char *path);
     ++enum stash_apply_result apply_autostash_oid(const char *stash_oid);
     ++enum stash_apply_result apply_autostash_ref(struct repository *r,
     ++					    const char *refname,
     ++					    const char *label_ours,
     ++					    const char *label_theirs,
     ++					    const char *label_base,
     ++					    const char *stash_msg);
     + 
     + #define SUMMARY_INITIAL_COMMIT   (1 << 0)
     + #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
     +
     + ## stash.h (new) ##
     +@@
     ++#ifndef STASH_H
     ++#define STASH_H
     ++
     ++enum stash_apply_result {
     ++	/* The stash was applied cleanly, or there was nothing to apply. */
     ++	STASH_APPLY_CLEAN = 0,
     ++
     ++	/*
     ++	 * The stash could not be applied because it resulted in
     ++	 * conflicts.  The stash entry is left in place.  The "git stash
     ++	 * apply", "pop" and "branch" subcommands exit with this status
     ++	 * in this case.
     ++	 */
     ++	STASH_APPLY_CONFLICT = 2,
     ++
     ++	/* Something went wrong. */
     ++	STASH_APPLY_ERROR = -1,
     ++};
     ++
     ++#endif /* STASH_H */
     +
     + ## t/t3903-stash.sh ##
     +@@ t/t3903-stash.sh: test_expect_success 'apply with custom conflict labels' '
     + 	echo stashed >conflict-file &&
     + 	git stash push -m "stashed" &&
     + 	test_commit label-upstream conflict-file upstream-content &&
     +-	test_must_fail git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
     ++	test_expect_code 2 git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
     + 	test_grep "^<<<<<<< UP" conflict-file &&
     + 	test_grep "^||||||| Stash base" conflict-file &&
     + 	test_grep "^>>>>>>> STASH" conflict-file
     +@@ t/t3903-stash.sh: test_expect_success 'apply with empty conflict labels' '
     + 	echo stashed >conflict-file &&
     + 	git stash push -m "stashed" &&
     + 	test_commit empty-label-upstream conflict-file upstream-content &&
     +-	test_must_fail git stash apply --label-ours= --label-theirs= &&
     ++	test_expect_code 2 git stash apply --label-ours= --label-theirs= &&
     + 	test_grep "^<<<<<<<$" conflict-file &&
     + 	test_grep "^>>>>>>>$" conflict-file
     + '
     + 
     ++test_expect_success 'apply exits 2 on conflicts and keeps the stash entry' '
     ++	git reset --hard initial &&
     ++	test_commit exit-code-base conflict-file base-content &&
     ++	echo stashed >conflict-file &&
     ++	git stash push -m stashed &&
     ++	test_commit exit-code-upstream conflict-file upstream-content &&
     ++	test_expect_code 2 git stash apply &&
     ++	git stash list >list &&
     ++	test_grep stashed list
     ++'
     ++
     ++test_expect_success 'pop exits 2 on conflicts and keeps the stash entry' '
     ++	git reset --hard initial &&
     ++	test_commit pop-exit-code-base pop-file base-content &&
     ++	echo stashed >pop-file &&
     ++	git stash push -m pop-stashed &&
     ++	test_commit pop-exit-code-upstream pop-file upstream-content &&
     ++	test_expect_code 2 git stash pop &&
     ++	git stash list >list &&
     ++	test_grep pop-stashed list
     ++'
     ++
     + test_expect_success 'stash show --include-untracked includes untracked files' '
     + 	git reset --hard &&
     + 
      
       ## t/t7201-co.sh ##
      @@ t/t7201-co.sh: test_expect_success 'checkout -m creates a recoverable stash on conflict' '

 Documentation/git-stash.adoc |  9 +++++
 builtin/checkout.c           | 15 +++++----
 builtin/stash.c              | 27 +++++++++------
 sequencer.c                  | 65 ++++++++++++++++++++++--------------
 sequencer.h                  | 19 +++++++----
 stash.h                      | 20 +++++++++++
 t/t3903-stash.sh             | 26 +++++++++++++--
 t/t7201-co.sh                | 16 ++++++---
 8 files changed, 143 insertions(+), 54 deletions(-)
 create mode 100644 stash.h

Hi Harald

On 01/09/2026 10:49, Harald Nordgren via GitGitGadget wrote:

Other commands such as merge-tree and merge strategies use 1 to indicate conflicts and another non-zero exit code for errors. That matches the way grep and diff use the exit code to distinguish differences from errors. It is confusing if we start using a different convention here. I've left a few comments below, but the exit code is my main concern. It would be nice to separate out the stash changes into a separate commit as well.

Thanks for documenting this, I think we'd be better to avoid giving specific exit codes for errors and say "a non-zero exit code other than 1" unless we have a good way of enforcing that.

The indentation is strange here

Nested ternary operators are not particularly readable, if we stick with an exit code of 1 for conflicts the original code does not need to be modified.

This seems a bit pointless when we're still returning -1 implicitly everywhere else where we have "return error(...).

Looking at the callers of do_apply_stash(), apply_stash() returns the result of do_apply_stash(), pop_stash() and branch_stash() return the result of do_drop_stash() if do_apply_stash() returns 0. do_drop_stash() always returns 0 so we're safe, but that analysis should be in the commit message.

We only print this if we know there were conflicts - good.

We know we've saved the stash so we should tell the user that we have, rather than leaving when wondering what's happened to their stashed changes.

The rest of the changes in this file look good.

Aren't we testing that above?

We've just reset to a known starting point that has paths file and other-file, so why do we need to create a new commit in order to stash something?

This is good, we should be checking "stash branch" as well.

Nice, it is much easier to see what we're checking now

Thanks

Phillip

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

;-)

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

This is half my fault. I should have made it clear that my sample code was merely to illustrate the need and ability to allow the callers to tell general errors and specific failure modes. I agree with you that "1 if X, other non-zero exit if error" would fly much better.

Thanks.

Avoiding checkout -m autostash retries when no tracked local changes exist and visually separating autostash conflict advice from the subsequent branch-switch message. Addresses #leftoverbits from here: https://lore.kernel.org/git/cfd09dbf-8d77-4464-8030-3a0ffb4aeae7@gmail.com/

Changes in v4:

 * Conflicts now exit with status 1 like merge-tree, other failures exit 128
   so exit 1 unambiguously means conflicts. Stash changes split into their
   own commit.
 * The autostash apply helpers use the return value (enum
   stash_apply_result) instead of an out-parameter, and only claim conflicts
   when git stash apply actually reported them.

Changes in v3:

 * Use enum for git stash return values, to separate conflict from generic
   error.

Changes in v2:

 * Simplify logic and combine to one commit.
 * Test full output with test_cmp.

Harald Nordgren (2):
  stash: reserve exit status 1 for conflicts
  checkout: separate autostash conflict advice from branch-switch
    message

 Documentation/git-stash.adoc |  9 +++++
 builtin/checkout.c           | 15 ++++----
 builtin/stash.c              | 32 ++++++++++++-----
 sequencer.c                  | 66 ++++++++++++++++++++++--------------
 sequencer.h                  | 19 +++++++----
 stash.h                      | 21 ++++++++++++
 t/t3903-stash.sh             | 25 ++++++++++++--
 t/t7201-co.sh                | 16 ++++++---
 8 files changed, 149 insertions(+), 54 deletions(-)
 create mode 100644 stash.h

base-commit: 1630431f326e15fcde608827b5ff38422528eb59
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2364%2FHaraldNordgren%2Fhn%2Fgit-checkout-m-leftoverbits-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2364/HaraldNordgren/hn/git-checkout-m-leftoverbits-v4
Pull-Request: https://github.com/git/git/pull/2364

Range-diff vs v3:

 1:  8e1979dd6c ! 1:  ff43221802 checkout: separate autostash conflict advice from branch-switch message
     @@ Metadata
      Author: Harald Nordgren <haraldnordgren@gmail.com>
      
       ## Commit message ##
     -    checkout: separate autostash conflict advice from branch-switch message
     +    stash: reserve exit status 1 for conflicts
      
     -    "git checkout -m" stashes the user's local changes when it cannot
     -    perform the checkout, and then applies the stash.  When applying the
     -    stash results in conflicts, the advice on how to deal with them is
     -    printed directly on top of the branch-switch message ("Switched to
     -    branch ..."), making the two hard to tell apart.  Print a blank line
     -    in between so that the advice and the branch-switch message are
     -    visually distinct.
     +    "git stash apply", "pop" and "branch" exit with status 1 both when
     +    applying the stash entry resulted in conflicts and when they fail for
     +    other reasons, so callers cannot tell the two apart.
      
     -    To make this possible, "git stash apply", "pop" and "branch" now exit
     -    with status 2 when applying the stash entry resulted in conflicts, in
     -    which case the stash entry is left in place; other failures exit with
     -    status 1, as before.  The exit statuses are documented in the "git
     -    stash" documentation.
     +    Follow the convention of "git merge-tree" and the merge strategies,
     +    which exit with status 1 to indicate conflicts and with a different
     +    non-zero status for errors: those subcommands now exit with status 1
     +    only when applying the stash entry resulted in conflicts, in which
     +    case the stash entry is left in place, and exit with status 128, the
     +    status die() uses, when they fail for other reasons.  Document the
     +    exit statuses.
     +
     +    cmd_stash() used to collapse the return values of the subcommand
     +    implementations to a boolean.  It now maps negative values, which
     +    signal a failure, to 128 and passes everything else through as-is.
     +    The only implementations that return a positive value are "apply",
     +    "pop" and "branch", which return the value of do_apply_stash():
     +    "apply" returns it directly, and "pop" and "branch" drop the stash
     +    entry, via do_drop_stash(), which always returns 0, only when the
     +    application succeeded.  The positive value is always 1, as
     +    do_apply_stash() only returns a positive value when the three-way
     +    merge was unclean.
     +
     +    Make the convention explicit by introducing enum stash_apply_result
     +    with the values STASH_APPLY_CLEAN, STASH_APPLY_CONFLICT and
     +    STASH_APPLY_ERROR, and use it for the in-process autostash helpers,
     +    too.  They spawn "git stash apply" and can now tell conflicts apart
     +    from other failures, e.g. a crash or death by signal of the child,
     +    which map to exit statuses above 1.  Since we know the stash entry
     +    was saved, tell users so in the error message instead of leaving them
     +    wondering what happened to their stashed changes.
      
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
     @@ Documentation/git-stash.adoc: include::includes/cmd-config-section-all.adoc[]
      +EXIT STATUS
      +-----------
      +
     -+The `git stash` subcommands exit with status 0 on success and non-zero
     -+on failure.  The subcommands that apply a stash entry, i.e. `apply`,
     -+`pop` and `branch`, exit with status 2 when applying the stash entry
     -+resulted in conflicts, in which case the stash entry is left in place.
     -+Other failures exit with status 1 (usage errors exit with status 129).
     ++The `git stash` subcommands exit with status 0 on success.  The
     ++subcommands that apply a stash entry, i.e. `apply`, `pop` and `branch`,
     ++exit with status 1 when applying the stash entry resulted in conflicts,
     ++in which case the stash entry is left in place, and with a non-zero
     ++status other than 1 when they fail for other reasons.
      +
       
       SEE ALSO
       --------
      
     - ## builtin/checkout.c ##
     -@@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
     - 	int flag, writeout_error = 0;
     - 	int do_merge = 1;
     - 	int created_autostash = 0;
     -+	enum stash_apply_result autostash_res = STASH_APPLY_CLEAN;
     - 	struct strbuf old_commit_shortname = STRBUF_INIT;
     - 	struct strbuf autostash_msg = STRBUF_INIT;
     - 	const char *stash_label_base = NULL;
     -@@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
     - 				git_config_push_parameter(cfg.buf);
     - 				strbuf_release(&cfg);
     - 			}
     --			apply_autostash_ref(the_repository,
     --					    "CHECKOUT_AUTOSTASH_HEAD",
     --					    new_branch_info->name,
     --					    "local",
     --					    stash_label_base,
     --					    autostash_msg.buf);
     -+			autostash_res = apply_autostash_ref(the_repository,
     -+				    "CHECKOUT_AUTOSTASH_HEAD",
     -+				    new_branch_info->name,
     -+				    "local",
     -+				    stash_label_base,
     -+				    autostash_msg.buf);
     - 		}
     - 		if (ret) {
     - 			branch_info_release(&old_branch_info);
     -@@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
     - 	if (!opts->quiet && !old_branch_info.path && old_branch_info.commit && new_branch_info->commit != old_branch_info.commit)
     - 		orphaned_commit_warning(old_branch_info.commit, new_branch_info->commit);
     - 
     -+	if (autostash_res == STASH_APPLY_CONFLICT && !opts->quiet)
     -+		fputc('\n', stderr);
     - 	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
     - 
     - 	if (created_autostash) {
     -
       ## builtin/stash.c ##
      @@
       #include "object-name.h"
     @@ builtin/stash.c: static void unstage_changes_unless_new(struct object_id *orig_t
       
      -static int do_apply_stash(const char *prefix, struct stash_info *info,
      -			  int index, int quiet,
     +-			  const char *label_ours, const char *label_theirs,
     +-			  const char *label_base)
      +static enum stash_apply_result do_apply_stash(const char *prefix,
      +					      struct stash_info *info,
      +					      int index, int quiet,
     - 			  const char *label_ours, const char *label_theirs,
     - 			  const char *label_base)
     ++					      const char *label_ours,
     ++					      const char *label_theirs,
     ++					      const char *label_base)
       {
     + 	int clean, ret;
     + 	int has_index = index;
      @@ builtin/stash.c: static int do_apply_stash(const char *prefix, struct stash_info *info,
     - 	clean = merge_ort_nonrecursive(&o, head, merge, merge_base);
       
       	/*
     --	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
     + 	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
      -	 * merge was clean, and nonzero if the merge was unclean or encountered
      -	 * an error.
     -+	 * Translate the value of 'clean' so 'ret' is STASH_APPLY_CLEAN
     -+	 * when the merge was clean, STASH_APPLY_CONFLICT when it was
     -+	 * unclean, and a negative value if it encountered an error.
     ++	 * merge was clean, and 1 if the merge was unclean or a negative value
     ++	 * if it encountered an error.
       	 */
     --	ret = clean >= 0 ? !clean : clean;
     -+	ret = clean >= 0 ? (clean ? STASH_APPLY_CLEAN : STASH_APPLY_CONFLICT)
     -+			 : clean;
     + 	ret = clean >= 0 ? !clean : clean;
       
     - 	if (ret < 0)
     - 		rollback_lock_file(&lock);
     -@@ builtin/stash.c: static int do_apply_stash(const char *prefix, struct stash_info *info,
     - 
     - 	if (has_index) {
     - 		if (reset_tree(&index_tree, 0, 0))
     --			ret = -1;
     -+			ret = STASH_APPLY_ERROR;
     - 	} else {
     - 		unstage_changes_unless_new(&c_tree);
     - 	}
      @@ builtin/stash.c: int cmd_stash(int argc,
       	strbuf_addf(&stash_index_path, "%s.stash.%" PRIuMAX, index_file,
       		    (uintmax_t)pid);
     @@ builtin/stash.c: int cmd_stash(int argc,
      +	if (fn) {
      +		ret = fn(argc, argv, prefix, repo);
      +
     ++		/*
     ++		 * The subcommand implementations return 0 on success, a
     ++		 * negative value on failure, and STASH_APPLY_CONFLICT
     ++		 * when applying a stash entry resulted in conflicts.
     ++		 * Map failures to 128, the status die() uses, so that
     ++		 * exit status 1 unambiguously indicates conflicts.
     ++		 */
      +		if (ret < 0)
     -+			return 1;
     ++			return 128;
      +		return ret;
      +	} else if (!argc)
       		return !!push_stash_unassumed(0, NULL, prefix, repo);
     @@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int atte
       			strvec_pushf(&child.args, "--label-base=%s", label_base);
       		strvec_push(&child.args, stash_oid);
       		ret = run_command(&child);
     -+		if (ret && ret != STASH_APPLY_CONFLICT)
     ++		if (ret > 1)
      +			ret = STASH_APPLY_ERROR;
       	}
       
     @@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int atte
       				  "do not want to resolve them now, run \"git reset --hard\" and\n"
       				  "apply the local changes later by running \"git stash pop\".\n"));
      +		else if (attempt_apply)
     -+			ret = error(_("could not apply autostash"));
     ++			ret = error(_("could not apply autostash; "
     ++				      "your changes are safe in the stash"));
       		else
       			fprintf(stderr,
       				_("Autostash exists; creating a new stash entry.\n"
     @@ stash.h (new)
      +	 * The stash could not be applied because it resulted in
      +	 * conflicts.  The stash entry is left in place.  The "git stash
      +	 * apply", "pop" and "branch" subcommands exit with this status
     -+	 * in this case.
     ++	 * in this case, mirroring the convention of "git merge-tree" and
     ++	 * the merge strategies.
      +	 */
     -+	STASH_APPLY_CONFLICT = 2,
     ++	STASH_APPLY_CONFLICT = 1,
      +
      +	/* Something went wrong. */
      +	STASH_APPLY_ERROR = -1,
     @@ stash.h (new)
      +#endif /* STASH_H */
      
       ## t/t3903-stash.sh ##
     -@@ t/t3903-stash.sh: test_expect_success 'apply with custom conflict labels' '
     +@@ t/t3903-stash.sh: test_expect_success 'stash.index=false overridden by --index' '
     + 	test_cmp expect file
     + '
     + 
     +-test_expect_success 'apply with custom conflict labels' '
     ++test_expect_success 'apply exits 1 on conflicts' '
     + 	git reset --hard initial &&
     + 	test_commit label-base conflict-file base-content &&
       	echo stashed >conflict-file &&
       	git stash push -m "stashed" &&
       	test_commit label-upstream conflict-file upstream-content &&
      -	test_must_fail git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
     -+	test_expect_code 2 git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
     ++	test_expect_code 1 git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
       	test_grep "^<<<<<<< UP" conflict-file &&
       	test_grep "^||||||| Stash base" conflict-file &&
       	test_grep "^>>>>>>> STASH" conflict-file
     @@ t/t3903-stash.sh: test_expect_success 'apply with empty conflict labels' '
       	git stash push -m "stashed" &&
       	test_commit empty-label-upstream conflict-file upstream-content &&
      -	test_must_fail git stash apply --label-ours= --label-theirs= &&
     -+	test_expect_code 2 git stash apply --label-ours= --label-theirs= &&
     ++	test_expect_code 1 git stash apply --label-ours= --label-theirs= &&
       	test_grep "^<<<<<<<$" conflict-file &&
       	test_grep "^>>>>>>>$" conflict-file
       '
       
     -+test_expect_success 'apply exits 2 on conflicts and keeps the stash entry' '
     ++test_expect_success 'pop exits 1 on conflicts and keeps the stash entry' '
      +	git reset --hard initial &&
     -+	test_commit exit-code-base conflict-file base-content &&
     -+	echo stashed >conflict-file &&
     -+	git stash push -m stashed &&
     -+	test_commit exit-code-upstream conflict-file upstream-content &&
     -+	test_expect_code 2 git stash apply &&
     ++	echo stashed >file &&
     ++	git stash push -m pop-stashed &&
     ++	test_commit pop-upstream file upstream-content &&
     ++	test_expect_code 1 git stash pop &&
      +	git stash list >list &&
     -+	test_grep stashed list
     ++	test_grep pop-stashed list
      +'
      +
     -+test_expect_success 'pop exits 2 on conflicts and keeps the stash entry' '
     ++test_expect_success 'stash branch exits with a non-1 status on errors' '
      +	git reset --hard initial &&
     -+	test_commit pop-exit-code-base pop-file base-content &&
     -+	echo stashed >pop-file &&
     -+	git stash push -m pop-stashed &&
     -+	test_commit pop-exit-code-upstream pop-file upstream-content &&
     -+	test_expect_code 2 git stash pop &&
     ++	echo stashed >file &&
     ++	git stash push -m branch-stashed &&
     ++	test_expect_code 128 git stash branch conflicting-branch refs/heads/does-not-exist &&
      +	git stash list >list &&
     -+	test_grep pop-stashed list
     ++	test_grep branch-stashed list
      +'
      +
       test_expect_success 'stash show --include-untracked includes untracked files' '
       	git reset --hard &&
       
     -
     - ## t/t7201-co.sh ##
     -@@ t/t7201-co.sh: test_expect_success 'checkout -m creates a recoverable stash on conflict' '
     - 	test_must_fail git checkout side 2>stderr &&
     - 	test_grep "Your local changes" stderr &&
     - 	git checkout -m side >actual 2>&1 &&
     --	test_grep "resulted in conflicts" actual &&
     --	test_grep "git stash drop" actual &&
     --	test_grep "git stash pop" actual &&
     --	test_grep "The following paths have local changes" actual &&
     -+	cat >expect <<-EOF &&
     -+	Your local changes are stashed, however applying them
     -+	resulted in conflicts.  You can either resolve the conflicts
     -+	and then discard the stash with "git stash drop", or, if you
     -+	do not want to resolve them now, run "git reset --hard" and
     -+	apply the local changes later by running "git stash pop".
     -+
     -+	Switched to branch ${SQ}side${SQ}
     -+	The following paths have local changes:
     -+	M	one
     -+	EOF
     -+	test_cmp expect actual &&
     - 	git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual &&
     - 	sed /^index/d actual >actual.trimmed &&
     - 	cat >expect <<-EOF &&
 -:  ---------- > 2:  935fa0a9ae checkout: separate autostash conflict advice from branch-switch message

From: Harald Nordgren <haraldnordgren@gmail.com>

"git stash apply", "pop" and "branch" exit with status 1 both when applying the stash entry resulted in conflicts and when they fail for other reasons, so callers cannot tell the two apart.

Follow the convention of "git merge-tree" and the merge strategies, which exit with status 1 to indicate conflicts and with a different non-zero status for errors: those subcommands now exit with status 1 only when applying the stash entry resulted in conflicts, in which case the stash entry is left in place, and exit with status 128, the status die() uses, when they fail for other reasons. Document the exit statuses.

cmd_stash() used to collapse the return values of the subcommand implementations to a boolean. It now maps negative values, which signal a failure, to 128 and passes everything else through as-is. The only implementations that return a positive value are "apply", "pop" and "branch", which return the value of do_apply_stash(): "apply" returns it directly, and "pop" and "branch" drop the stash entry, via do_drop_stash(), which always returns 0, only when the application succeeded. The positive value is always 1, as do_apply_stash() only returns a positive value when the three-way merge was unclean.

Make the convention explicit by introducing enum stash_apply_result with the values STASH_APPLY_CLEAN, STASH_APPLY_CONFLICT and STASH_APPLY_ERROR, and use it for the in-process autostash helpers, too. They spawn "git stash apply" and can now tell conflicts apart from other failures, e.g. a crash or death by signal of the child, which map to exit statuses above 1. Since we know the stash entry was saved, tell users so in the error message instead of leaving them wondering what happened to their stashed changes.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 Documentation/git-stash.adoc |  9 +++++
 builtin/stash.c              | 32 ++++++++++++-----
 sequencer.c                  | 66 ++++++++++++++++++++++--------------
 sequencer.h                  | 19 +++++++----
 stash.h                      | 21 ++++++++++++
 t/t3903-stash.sh             | 25 ++++++++++++--
 6 files changed, 128 insertions(+), 44 deletions(-)
 create mode 100644 stash.h

From: Harald Nordgren <haraldnordgren@gmail.com>

"git checkout -m" stashes the user's local changes when it cannot perform the checkout, and then applies the stash. When applying the stash results in conflicts, the advice on how to deal with them is printed directly on top of the branch-switch message ("Switched to branch ..."), making the two hard to tell apart. Print a blank line in between so that the advice and the branch-switch message are visually distinct.

apply_autostash_ref() reports whether applying the stash resulted in conflicts via its enum stash_apply_result return value, so only print the blank line in the conflicted case.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 builtin/checkout.c | 15 +++++++++------
 t/t7201-co.sh      | 16 ++++++++++++----
 2 files changed, 21 insertions(+), 10 deletions(-)

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

The above is on the overly verbose side. The first two paragraphs give enough discussion and the remainder mostly repeats with small details sprinkled in, which can probably be shortened to 1/4 of the amount of text, but it is OK.

Great.

OK.

Style. Once one of "if", "else if" and "else" cascade gains {braches}, others should do so as well.

This kind of code that assigns any random "int" that is returned by run_command() to "enum ret" that has much narrower valid value range and then makes corrections annoys me a bit.

One way to do this cleanly might be to make a small helper function do_stash_apply(), and use it like so:

	if (attempt_apply)
		ret = do_stash_apply(stash_oid, label_ours, label_theirs,
				     label_base);

The implementation of do_stash_apply() would be like what you have in "if (attempt_apply) {...}" block, perhaps like:

	static enum stash_apply_result do_stash_apply(const char *stash_oid,
						      const char *label_ours,
						      const char *label_theirs,
						      const char *label_base)
	{
		struct child_process child = CHILD_PROCESS_INIT;

		child.git_cmd = 1;
		...
                strvec_push(&child.args, stash_oid);
                switch (run_command(&child)) {
		case 0: return STASH_APPLY_CLEAN;
		case 1: return STASH_APPLY_CONFLICT;
		default: return STASH_APPLY_ERROR;
		}
	}

Good, and the rest of this function is good.

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

This iteration looks good to me.

Junio C Hamano <gitster@pobox.com> writes:

Just for fun, I complained to an AI agent I had nearby with the above four lines of critique, which spit back the following as a replacement for the last two paragraphs.

    Update cmd_stash() to map negative return values to 128 while
    passing positive values through.  Formalize this with enum
    stash_apply_result and update the autostash helpers accordingly
    to distinguish conflicts from errors and inform users that their
    stash was preserved.

As I said it is OK already, you do not have to adopt this shortened version, but I personally think that this level of detail is enough.

On 02/09/2026 20:51, Junio C Hamano wrote:

I think the analysis in the middle of the third paragraph is useful to make it clear that the return paths have been audited correctly. I agree the rest could be condensed or cut.

Thanks

Phillip

Hi Harald

On 02/09/2026 19:29, Harald Nordgren via GitGitGadget wrote:

Thanks for changing the exit codes, I don't have anything to add to Junio's comments.

Thanks

Phillip

Avoiding checkout -m autostash retries when no tracked local changes exist and visually separating autostash conflict advice from the subsequent branch-switch message. Addresses #leftoverbits from here: https://lore.kernel.org/git/cfd09dbf-8d77-4464-8030-3a0ffb4aeae7@gmail.com/

Changes in v5:

 * Improve commit messages.
 * Create helper do_stash_apply.

Changes in v4:

 * Conflicts now exit with status 1 like merge-tree, other failures exit 128
   so exit 1 unambiguously means conflicts. Stash changes split into their
   own commit.
 * The autostash apply helpers use the return value (enum
   stash_apply_result) instead of an out-parameter, and only claim conflicts
   when git stash apply actually reported them.

Changes in v3:

 * Use enum for git stash return values, to separate conflict from generic
   error.

Changes in v2:

 * Simplify logic and combine to one commit.
 * Test full output with test_cmp.

Harald Nordgren (2):
  stash: reserve exit status 1 for conflicts
  checkout: separate autostash conflict advice from branch-switch
    message

 Documentation/git-stash.adoc |   9 +++
 builtin/checkout.c           |  15 +++--
 builtin/stash.c              |  33 +++++++---
 sequencer.c                  | 113 ++++++++++++++++++++++-------------
 sequencer.h                  |  19 +++---
 stash.h                      |  21 +++++++
 t/t3903-stash.sh             |  25 +++++++-
 t/t7201-co.sh                |  16 +++--
 8 files changed, 181 insertions(+), 70 deletions(-)
 create mode 100644 stash.h

base-commit: 3cb9185f65410273787f74333cc027d2ea5daada
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2364%2FHaraldNordgren%2Fhn%2Fgit-checkout-m-leftoverbits-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2364/HaraldNordgren/hn/git-checkout-m-leftoverbits-v5
Pull-Request: https://github.com/git/git/pull/2364

Range-diff vs v4:

 1:  ff43221802 ! 1:  fe22b1bfa6 stash: reserve exit status 1 for conflicts
     @@ Commit message
          status die() uses, when they fail for other reasons.  Document the
          exit statuses.
      
     -    cmd_stash() used to collapse the return values of the subcommand
     -    implementations to a boolean.  It now maps negative values, which
     -    signal a failure, to 128 and passes everything else through as-is.
     -    The only implementations that return a positive value are "apply",
     -    "pop" and "branch", which return the value of do_apply_stash():
     -    "apply" returns it directly, and "pop" and "branch" drop the stash
     -    entry, via do_drop_stash(), which always returns 0, only when the
     -    application succeeded.  The positive value is always 1, as
     -    do_apply_stash() only returns a positive value when the three-way
     -    merge was unclean.
     -
     -    Make the convention explicit by introducing enum stash_apply_result
     -    with the values STASH_APPLY_CLEAN, STASH_APPLY_CONFLICT and
     -    STASH_APPLY_ERROR, and use it for the in-process autostash helpers,
     -    too.  They spawn "git stash apply" and can now tell conflicts apart
     -    from other failures, e.g. a crash or death by signal of the child,
     -    which map to exit statuses above 1.  Since we know the stash entry
     -    was saved, tell users so in the error message instead of leaving them
     -    wondering what happened to their stashed changes.
     +    The only subcommand implementations that can return a positive value
     +    are "apply", "pop" and "branch", which return the value of
     +    do_apply_stash(): "apply" returns it directly, and "pop" and "branch"
     +    drop the stash entry, via do_drop_stash(), which always returns 0,
     +    only when the application succeeded.  do_apply_stash() only returns a
     +    positive value when the three-way merge was unclean.  cmd_stash() now
     +    maps negative values to 128 and passes positive values through as the
     +    exit status, so exit status 1 unambiguously indicates conflicts.
     +    enum stash_apply_result makes the convention explicit, and the
     +    autostash helpers use it to tell users that their stashed changes
     +    were saved when applying them fails.
      
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
     @@ builtin/stash.c: int cmd_stash(int argc,
      +		if (ret < 0)
      +			return 128;
      +		return ret;
     -+	} else if (!argc)
     ++	} else if (!argc) {
       		return !!push_stash_unassumed(0, NULL, prefix, repo);
     ++	}
       
       	/* Assume 'stash push' */
     + 	strvec_push(&args, "push");
      
       ## sequencer.c ##
      @@
     @@ sequencer.c: void create_autostash_ref(struct repository *r, const char *refname
      -				    const char *label_ours, const char *label_theirs,
      -				    const char *label_base,
      -				    const char *stash_msg)
     ++static enum stash_apply_result do_stash_apply(const char *stash_oid,
     ++					      const char *label_ours,
     ++					      const char *label_theirs,
     ++					      const char *label_base)
     + {
     + 	struct child_process child = CHILD_PROCESS_INIT;
     +-	int ret = 0;
     + 
     +-	if (attempt_apply) {
     +-		child.git_cmd = 1;
     +-		child.no_stdout = 1;
     +-		child.no_stderr = 1;
     +-		strvec_push(&child.args, "stash");
     +-		strvec_push(&child.args, "apply");
     +-		if (label_ours)
     +-			strvec_pushf(&child.args, "--label-ours=%s", label_ours);
     +-		if (label_theirs)
     +-			strvec_pushf(&child.args, "--label-theirs=%s", label_theirs);
     +-		if (label_base)
     +-			strvec_pushf(&child.args, "--label-base=%s", label_base);
     +-		strvec_push(&child.args, stash_oid);
     +-		ret = run_command(&child);
     +-	}
     +-
     +-	if (attempt_apply && !ret)
     ++	child.git_cmd = 1;
     ++	child.no_stdout = 1;
     ++	child.no_stderr = 1;
     ++	strvec_push(&child.args, "stash");
     ++	strvec_push(&child.args, "apply");
     ++	if (label_ours)
     ++		strvec_pushf(&child.args, "--label-ours=%s", label_ours);
     ++	if (label_theirs)
     ++		strvec_pushf(&child.args, "--label-theirs=%s", label_theirs);
     ++	if (label_base)
     ++		strvec_pushf(&child.args, "--label-base=%s", label_base);
     ++	strvec_push(&child.args, stash_oid);
     ++
     ++	switch (run_command(&child)) {
     ++	case 0:
     ++		return STASH_APPLY_CLEAN;
     ++	case STASH_APPLY_CONFLICT:
     ++		return STASH_APPLY_CONFLICT;
     ++	default:
     ++		return STASH_APPLY_ERROR;
     ++	}
     ++}
     ++
      +static enum stash_apply_result apply_save_autostash_oid(const char *stash_oid,
      +							int attempt_apply,
      +							const char *label_ours,
      +							const char *label_theirs,
      +							const char *label_base,
      +							const char *stash_msg)
     - {
     - 	struct child_process child = CHILD_PROCESS_INIT;
     --	int ret = 0;
     ++{
      +	enum stash_apply_result ret = STASH_APPLY_CLEAN;
     - 
     - 	if (attempt_apply) {
     - 		child.git_cmd = 1;
     -@@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
     - 			strvec_pushf(&child.args, "--label-base=%s", label_base);
     - 		strvec_push(&child.args, stash_oid);
     - 		ret = run_command(&child);
     -+		if (ret > 1)
     -+			ret = STASH_APPLY_ERROR;
     - 	}
     - 
     --	if (attempt_apply && !ret)
     ++
     ++	if (attempt_apply)
     ++		ret = do_stash_apply(stash_oid, label_ours, label_theirs,
     ++				     label_base);
     ++
      +	if (attempt_apply && ret == STASH_APPLY_CLEAN)
       		fprintf(stderr, _("Applied autostash.\n"));
       	else {
 2:  935fa0a9ae = 2:  d18ff3ea9a checkout: separate autostash conflict advice from branch-switch message

From: Harald Nordgren <haraldnordgren@gmail.com>

"git stash apply", "pop" and "branch" exit with status 1 both when applying the stash entry resulted in conflicts and when they fail for other reasons, so callers cannot tell the two apart.

Follow the convention of "git merge-tree" and the merge strategies, which exit with status 1 to indicate conflicts and with a different non-zero status for errors: those subcommands now exit with status 1 only when applying the stash entry resulted in conflicts, in which case the stash entry is left in place, and exit with status 128, the status die() uses, when they fail for other reasons. Document the exit statuses.

The only subcommand implementations that can return a positive value are "apply", "pop" and "branch", which return the value of do_apply_stash(): "apply" returns it directly, and "pop" and "branch" drop the stash entry, via do_drop_stash(), which always returns 0, only when the application succeeded. do_apply_stash() only returns a positive value when the three-way merge was unclean. cmd_stash() now maps negative values to 128 and passes positive values through as the exit status, so exit status 1 unambiguously indicates conflicts. enum stash_apply_result makes the convention explicit, and the autostash helpers use it to tell users that their stashed changes were saved when applying them fails.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 Documentation/git-stash.adoc |   9 +++
 builtin/stash.c              |  33 +++++++---
 sequencer.c                  | 113 ++++++++++++++++++++++-------------
 sequencer.h                  |  19 +++---
 stash.h                      |  21 +++++++
 t/t3903-stash.sh             |  25 +++++++-
 6 files changed, 160 insertions(+), 60 deletions(-)
 create mode 100644 stash.h

From: Harald Nordgren <haraldnordgren@gmail.com>

"git checkout -m" stashes the user's local changes when it cannot perform the checkout, and then applies the stash. When applying the stash results in conflicts, the advice on how to deal with them is printed directly on top of the branch-switch message ("Switched to branch ..."), making the two hard to tell apart. Print a blank line in between so that the advice and the branch-switch message are visually distinct.

apply_autostash_ref() reports whether applying the stash resulted in conflicts via its enum stash_apply_result return value, so only print the blank line in the conflicted case.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 builtin/checkout.c | 15 +++++++++------
 t/t7201-co.sh      | 16 ++++++++++++----
 2 files changed, 21 insertions(+), 10 deletions(-)

I would love to have a linter rule for this, since I keep forgetting and it's a waste of time for others to remind me.

Maybe we should copy this over from Linux? https://github.com/torvalds/linux/blob/v7.2/scripts/checkpatch.pl#L6270

Harald

Harald Nordgren <haraldnordgren@gmail.com> writes:

Does our .clang-format file already cover this?

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

Both patches look good to me.  Thanks.  Will queue.

I don't think so. When GitHub CI runs, it doesn't complain about mismatched braces.

Harald

Harald Nordgren <haraldnordgren@gmail.com> writes:

A local "make style" while you develop runs

	git clang-format --style file --diff --extensions c,h

which tells the command to check only the parts of the system that you touched.

I do not think it is used in CI.  

It seems to run as part of CI and doesn't catch it then: '.github/workflows/check-style.yml'

Harald

[Cc'd Karthik for a view on the CI style job]

On 04/09/2026 09:16, Harald Nordgren wrote:

My recollection is that we made that job never fail because clang-format does not always do a good job of following our style guide and the output is not guaranteed to be stable across different versions. That makes its value debatable as I don't think many people (anyone?) bother checking the output to see what suggestions it made. It would be a lot simpler for contributors if we just devolved style decisions to clang-format so no one had to think about or comment on the style. Whether that tradeoff is worth it depends on how far the output of clang-format deviates from our preferred style.

Thanks

Phillip

Harald Nordgren <haraldnordgren@gmail.com> writes:

I never make any pull request to this project, so I didn't even notice its existence.

My question was more about what is in .clang-format file we already have and if it would have caught this. I was actually hoping that "RemoveBracesLLVM: true" we have at the end, which is explained like so

    # Remove optional braces of control statements (if, else, for, and while)
    # according to the LLVM coding style. This avoids braces on simple
    # single-statement bodies of statements but keeps braces if one side of
    # if/else if/.../else cascade has multi-statement body.

would help us, but it only can do a small subset of what we want, i.e., excess braces around both or either if/else body are removed (see [*] below), and it cannot add braces around all the bodies of if/else if.../else cascade only when one of them require them, it seems.

[*] effect of RemoveBracesLLVM.

git clang-format --style file --diff --extensions c,h

On Fri, Sep 4, 2026 at 5:09 PM Phillip Wood <phillip.wood123@gmail.com> wrote:

Thanks for the history! Seems that when linter output is not consistent between versions, then version pinning is necessary.

My professional opinion is that a consistent style, auto-enforceable + auto-fixable, is extremely valuable, because it removes an entire class of code review discussions. I almost never care which style is chosen, as long as it's the same everywhere.

I tried today to apply the full clang-format output and it was A LOT. Doesn't mean it shouldn't be done, but it would need to be done at a carefully chosen time because there will be lots of conflicts on all in-flight topics.

In the best of worlds, the preferred style is exactly what the linter dictates, then these two are equivalent.

Harald