From: Karthik Nayak <karthik.188@gmail.com>
Date: Tue, 10 Mar 2026 14:31:43 GMT
Subject: Re: [PATCH v2] advice: add stashBeforeCheckout advice for dirty branch switches
Message-ID: <CAOLa=ZRfaSR2CisUrW0gLf_45KQj1wQZ70F4PZ5XcwWZ--+HhQ@mail.gmail.com>
In-Reply-To: <pull.2233.v2.git.git.1773140364525.gitgitgadget@gmail.com>

"Arsh Srivastava via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Arsh Srivastava <arshsrivastava00@gmail.com>
>
> 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 <file>..." to update what will be committed)
  (use "git restore <file>..." 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 <paths>\" 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 -- <file>"));
> +}

This doesn't seem to be formatted with tabs.