From: Harald Nordgren Date: Wed, 15 Apr 2026 08:16:59 GMT Subject: Re: [PATCH] checkout: add --autostash option for branch switching Message-ID: <20260415081659.86783-1-haraldnordgren@gmail.com> In-Reply-To: > > + 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