Re: [PATCH v2] advice: add stashBeforeCheckout advice for dirty branch switches

>
> "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.