Re: [PATCH v2] stash: fix incorrect branch name in stash message

> > +
>
> Addition of trailing whitespace?
>
> You can avoid such mistakes in the future by enabling our sample
> pre-commit hook, which essentially does
>
>         git diff-index --check --cached $against --
>

> where $against is HEAD (or an empty tree object while preparing for
> an initial commit).
>
> >       branch_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> >                                            "HEAD", 0, NULL, &flags);
> > -     if (flags & REF_ISSYMREF)
> > -             skip_prefix(branch_ref, "refs/heads/", &branch_name);
> > +
> > +     if (flags & REF_ISSYMREF) {
> > +             if (skip_prefix(branch_ref, "refs/heads/", &branch_name))
> > +                     branch_name = branch_name_buf = xstrdup(branch_name);
> > +     } else
> > +             branch_name = "(no branch)";
>
> Do we need the else clause?  The original did not have it and showed
> the "(no branch)" message without an issue, and I do not see anything
> is changed by what happens inside the other side of this if statement.
> Am I missing something?

>
> > @@ -1495,6 +1501,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
> >       strbuf_release(&msg);
> >       strbuf_release(&untracked_files);
> >       free_commit_list(parents);
> > +     free(branch_name_buf);
> >       return ret;
> >  }
>
> Makes sense.
>
> This is a common pattern we use with a variable whose name contains
> "to_free" (e.g., "branch_name_to_free"), but "branch_name_buf" is
> pleanty readable and easy to understand what is going on.
>
> > +test_expect_success 'stash reflog message uses superproject branch, not submodule branch' '
>
> The title looks a bit on the overly-long side.  Would
>
>     stash message records the superproject branch
>
> be sufficient?  The fact that the stash is implemented as reflog
> is invidible and irrelevant at this level, so "reflog message" is
> wasting bytes without adding any useful information.
>
> What we want to make sure is that the message records the current
> branch name, whether the project has any submodules or not, and from
> that point of view,
>
>     stash message records the correct branch name
>
> ought to be good, but not quite, because this test is trying to
> trigger a bug that was present only when there are submodules, so
> not mentioning superproject/submodule at all would not work well.
>
> Would
>
>     submodules does not affect the branch recorded in stash message
>
> work?  That is the best one I can come up with offhand.
>

> > +     git init sub_project &&
> > +     (
> > +             cd sub_project &&
> > +             echo "Initial content in sub_project" >sub_file.txt &&
> > +             git add sub_file.txt &&
> > +             git commit -q -m "Initial commit in sub_project"
> > +     ) &&
>
> It is easier to debug the test script if you avoid using --quiet too
> much.  Regular "sh ./t3903-stash.sh" will squelch these output
> anyway, and they can be seen when the test script is run with "-v".
>

> > +     git init main_project &&
> > +     (
> > +             cd main_project &&
> > +             echo "Initial content in main_project" >main_file.txt &&
> > +             git add main_file.txt &&
> > +             git commit -q -m "Initial commit in main_project" &&
> > +
> > +             git -c protocol.file.allow=always submodule add --quiet ../sub_project sub &&
> > +             git commit -q -m "Added submodule sub_project" &&
> > +
> > +             git checkout -q -b feature_main &&
>
>
> > +             cd sub &&
> > +             git checkout -q -b feature_sub &&
> > +             cd .. &&
>
> These three lines can be written more compactly as:
>
>                 git -C sub checkout -b feature_sub &&
>

> > +             git checkout -q -b work_branch &&
> > +             echo "Important work to be stashed" >work_item.txt &&
> > +             git add work_item.txt &&
> > +             git stash push -q -m "custom stash for work_branch" &&
> > +
> > +             git stash list >../actual_stash_list.txt &&
> > +             grep "On work_branch: custom stash for work_branch" ../actual_stash_list.txt
> > +     )
> > +'
> > +
> >  test_done
>
> Thanks.