Re: [PATCH v2] checkout: print blank line after autostash conflict advice
From: Phillip Wood <hidden>
Date: 2026-09-01 09:31:54
On 31/08/2026 18:19, Junio C Hamano wrote:
"Harald Nordgren via GitGitGadget" [off-list ref] writes:quoted
diff --git a/sequencer.c b/sequencer.c index 65afd100d9..5ed9ae86c4 100644 --- a/sequencer.c +++ b/sequencer.c@@ -4815,7 +4815,8 @@ 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; }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. [...]
> > 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.
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