From: Arsh Srivastava Date: Tue, 10 Mar 2026 14:40:57 GMT Subject: Re: [PATCH v2] advice: add stashBeforeCheckout advice for dirty branch switches Message-ID: In-Reply-To: Subject: Re: [GSOC] advice: add stashBeforeCheckout advice for dirty branch switches Patrick Steinhardt writes: > It is used in "add.c", but not magically so. The function that you have > introduced is the only site that uses the new advice, but the function > is never called as far as I can see. So ultimately, the proposed change > does not have any effect on the user-observable behaviour. Thank you for the correction and for the bottom-posting reminder. You are right. The function advise_on_checkout_dirty_files() is defined but never called anywhere, so the patch has no user-observable effect. I also looked into the existing behaviour more carefully and found that unpack-trees.c already handles this case and prints a message telling the user to commit or stash their changes before switching branches. So the patch as written is both incomplete and duplicates existing behaviour. I will rework it in v3 to instead enhance the existing message in unpack-trees.c to also mention 'git checkout -m' for users who want to carry their local changes over to the new branch. Signed-off-by: Arsh Srivastava On Tue, 10 Mar 2026 at 20:07, Arsh Srivastava wrote: > > Subject: Re: [GSOC] advice: add stashBeforeCheckout advice for dirty > branch switches > > Karthik Nayak writes: > > > Doesn't 'ADVICE_COMMIT_BEFORE_MERGE' already do this? > > So won't this simply be duplicating the same message? > > Thank you for the detailed review. You are correct, the existing message > in unpack-trees.c already handles this case and my patch duplicates it. > I also acknowledge the other issues raised: > > - The newly introduced function was never called anywhere in the codebase > - No tests were added > - The bullet points in the commit message used '>' instead of '-' or '*' > - The advice message was not formatted with tabs > > Rather than duplicating the existing behaviour, I think the better > approach would be to enhance the existing message in unpack-trees.c to > also mention 'git checkout -m' as an option for users who want to carry > their local changes over to the new branch, since the current message > only says "commit or stash" without mentioning that option. > > I will rework the patch in that direction and send a v4. > > Signed-off-by: Arsh Srivastava > > On Tue, 10 Mar 2026 at 20:01, Karthik Nayak wrote: > > > > "Arsh Srivastava via GitGitGadget" writes: > > > > > From: Arsh Srivastava > > > > > > Add a new advice type ADVICE_STASH_BEFORE_CHECKOUT to guide users > > > when they attempt to switch branches with local modifications that > > > would be overwritten by the operation. > > > > > > This includes: > > >> New ADVICE_STASH_BEFORE_CHECKOUT enum value in advice.h > > >> Corresponding "stashBeforeCheckout" entry in advice_setting[] > > >> New advise_on_checkout_dirty_files() function that lists the > > > affected files and suggests using git stash push/pop > > >> Documentation entry in Documentation/config/advice.txt > > > > > > > Nit: Did you mean to add bullet point here? '>' is generally used to > > quote text. Perhaps use '-' or '*'. > > > > [snip] > > > > > > > > Documentation/config/advice.adoc | 5 +++++ > > > advice.c | 27 +++++++++++++++++++++++++++ > > > advice.h | 2 ++ > > > 3 files changed, 34 insertions(+) > > > > > > > Hmm. Shouldn't there be changes which actually call the newly introduced > > function? Also shouldn't there be tests added? > > > > > diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc > > > index 257db58918..8752e05636 100644 > > > --- a/Documentation/config/advice.adoc > > > +++ b/Documentation/config/advice.adoc > > > @@ -126,6 +126,11 @@ all advice messages. > > > Shown when a sparse index is expanded to a full index, which is likely > > > due to an unexpected set of files existing outside of the > > > sparse-checkout. > > > + stashBeforeCheckout:: > > > + Shown when the user attempts to switch branches but has > > > + local modifications that would be overwritten by the > > > + operation, to suggest using linkgit:git-stash[1] to > > > + save changes before switching. > > > > Doesn't 'ADVICE_COMMIT_BEFORE_MERGE' already do this? > > > > In one of my repos: > > > > ❯ git status > > On branch master > > Your branch is up to date with 'origin/master'. > > > > nothing to commit, working tree clean > > > > ❯ echo "aldjf" >> LICENSE > > > > ❯ git status > > On branch master > > Your branch is up to date with 'origin/master'. > > > > Changes not staged for commit: > > (use "git add ..." to update what will be committed) > > (use "git restore ..." to discard changes in working directory) > > modified: LICENSE > > > > no changes added to commit (use "git add" and/or "git commit -a") > > > > ❯ git checkout 0-1-stable > > error: Your local changes to the following files would be overwritten > > by checkout: > > LICENSE > > Please commit your changes or stash them before you switch branches. > > Aborting > > > > So won't this simply be duplicating the same message? > > > > > statusAheadBehind:: > > > Shown when linkgit:git-status[1] computes the ahead/behind > > > counts for a local ref compared to its remote tracking ref, > > > diff --git a/advice.c b/advice.c > > > index 0018501b7b..e1264f525c 100644 > > > --- a/advice.c > > > +++ b/advice.c > > > @@ -81,6 +81,7 @@ static struct { > > > [ADVICE_SET_UPSTREAM_FAILURE] = { "setUpstreamFailure" }, > > > [ADVICE_SKIPPED_CHERRY_PICKS] = { "skippedCherryPicks" }, > > > [ADVICE_SPARSE_INDEX_EXPANDED] = { "sparseIndexExpanded" }, > > > + [ADVICE_STASH_BEFORE_CHECKOUT] = { "stashBeforeCheckout" }, > > > [ADVICE_STATUS_AHEAD_BEHIND_WARNING] = { "statusAheadBehindWarning" }, > > > [ADVICE_STATUS_HINTS] = { "statusHints" }, > > > [ADVICE_STATUS_U_OPTION] = { "statusUoption" }, > > > @@ -312,3 +313,29 @@ void advise_on_moving_dirty_path(struct string_list *pathspec_list) > > > "* Use \"git add --sparse \" to update the index\n" > > > "* Use \"git sparse-checkout reapply\" to apply the sparsity rules")); > > > } > > > + > > > +void advise_on_checkout_dirty_files(struct string_list *file_list) > > > +{ > > > + struct string_list_item *item; > > > + > > > + if (!file_list->nr) > > > + return; > > > + > > > + fprintf(stderr, _("The following files have local modifications that would\n" > > > + "be overwritten by switching branches:\n")); > > > + for_each_string_list_item(item, file_list) > > > + fprintf(stderr, "\t%s\n", item->string); > > > + > > > + advise_if_enabled(ADVICE_STASH_BEFORE_CHECKOUT, > > > + _("You can save your local changes before switching by running:\n" > > > + "\n" > > > + "\tgit stash push\n" > > > + "\n" > > > + "Then restore them after switching with:\n" > > > + "\n" > > > + "\tgit stash pop\n" > > > + "\n" > > > + "Or to discard your local changes, use:\n" > > > + "\n" > > > + "\tgit checkout -- ")); > > > +} > > > > This doesn't seem to be formatted with tabs.