[PATCH] checkout: add --autostash option for branch switching

[Harald Nordgren <haraldnordgren@gmail.com>]

> This is looking good, there are just a few small issues. Hopefully the
> next iteration will be the last.

Thanks for the encouragement! 💪🏻

> s/would/will/

👍

> It is the changes in the files overlapping that causes the merge
> conflict, not the files overlapping
> 
>         When the `--merge` (`-m`) option is given and the local changes
>         overlap with the changes in the branch we're switching to,

👍

> I'd drop this line and say instead "a message is printed"

👍

> This needs updating to match the new conflict advice.

👍

> If you've not done so already it would be well worth checking the
> generated git-checkout.html and the man page

Good catch, I generated it now and yes it didn't look correct. I dropped
that last section now.

> Don't we show the modified files as well now?

Good catch, very good idea to actually generate the man html file and
check.

> As this function only sets up the flags for unpack_trees() I think we
> could call this "quiet" or "show_errors"

Good point!

> We've added a function parameter for this option but then we ignore it
> unless "merge" and "old_commit" are true which is confusing. The reason
> we used to check those was to set "quiet" automatically but we can't do
> that now, so why not just use the value the call requested?

Good point! I attempted to change this, hopefully it doesn't break anything!

> This is an "out" parameter, so it would make sense to keep it at the end
> of the parameter list.

👍

> To create a multi-line file it is clearer to use
> 
>         cat >expect.messages <<-\EOF &&
>         The following paths have local changes:
>         M       one
>         EOF

👍

> I've realized since I suggested this that we should be checking the
> reflog message as well since that's what's shown by "git stash list" so
> we need to run
> 
>    git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual
> 
> > +     sed /^index/d actual >actual.trimmed &&
> > +     cat >expect <<-EOF &&
> 
> and add
> 
> 
>    autostash while switching to ${SQ}side${SQ}

Make sense!

> Why the two calls to test_grep, rather than one? Anyway I've realized
> since I suggested this test that we also need to check the message only
> appears once to prevent a regression where merge_working_tree() calls
> unpack_trees() without setting "quiet" the first time it is called. We
> can do that by writing an expect file and calling test_cmp(), or by
> using "test_line_count = 1 err"

Excellent point. I went with test_cmp since it's multi-line output and
"test_line_count = 1" seemed to not work then.


Harald