On 29/03/2019 10:39, Nguyễn Thái Ngọc Duy wrote:
> Unless you know what you're doing, switching to another branch to do
> something then switching back could be confusing. Worse, you may even
> forget that you're in the middle of something. By the time you realize,
> you may have done a ton of work and it gets harder to go back.
>
> A new option --ignore-in-progress was considered but dropped because it
> was not exactly clear what should happen. Sometimes you can switch away
> and get back safely and resume the operation. Sometimes not. And the
> git-checkout behavior is automatically clear merge/revert/cherry-pick,
> which makes it a bit even more confusing [1].
>
> We may revisit and add this option in the future. But for now play it
> safe and not allow it (you can't even skip this check with --force).
I think this is a good compromise, lets see how it goes (I think I
broadly agree with Elijah's suggestion to allow the switch if we can
safely switch back again if we want to add --ignore-in-progress in the
future).
> The
> user is suggested to cancel the operation by themselves (and hopefully
> they do consider the consequences, not blindly type the command), or to
> create a separate worktree instead of switching. The third option is
> the good old "git checkout", but it's not mentioned.
>
> [1] CACsJy8Axa5WsLSjiscjnxVK6jQHkfs-gH959=YtUvQkWriAk5w@xxxxxxxxxxxxxx
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> builtin/checkout.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index f7967cdb7c..5f100c1552 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -24,6 +24,7 @@
> #include "tree.h"
> #include "tree-walk.h"
> #include "unpack-trees.h"
> +#include "wt-status.h"
> #include "xdiff-interface.h"
>
> static const char * const checkout_usage[] = {
> @@ -56,6 +57,7 @@ struct checkout_opts {
> int accept_pathspec;
> int switch_branch_doing_nothing_is_ok;
> int only_merge_on_switching_branches;
> + int can_switch_when_in_progress;
>
> const char *new_branch;
> const char *new_branch_force;
> @@ -1202,6 +1204,39 @@ static void die_expecting_a_branch(const struct branch_info *branch_info)
> die(_("a branch is expected, got '%s'"), branch_info->name);
> }
>
> +static void die_if_some_operation_in_progress(void)
> +{
> + struct wt_status_state state;
> +
> + memset(&state, 0, sizeof(state));
> + wt_status_get_state(the_repository, &state, 0);
> +
> + if (state.merge_in_progress)
> + die(_("cannot switch branch while merging\n"
> + "Consider \"git merge --quit\" "
> + "or \"git worktree add\"."));
I'm not sure merge --quit exists, 'git grep \"quit origin/pu' shows
matches for builtin/{am.c,rebase.c,revert.c}. The --quit option for the
sequencer command does not touch the index or working tree (that's the
difference between --quit and --abort) so the switch can still fail due
changes in the index and worktree that would be overwritten by the switch.
Best Wishes
Phillip
> + if (state.am_in_progress)
> + die(_("cannot switch branch in the middle of an am session\n"
> + "Consider \"git am --quit\" "
> + "or \"git worktree add\"."));
> + if (state.rebase_interactive_in_progress || state.rebase_in_progress)
> + die(_("cannot switch branch while rebasing\n"
> + "Consider \"git rebase --quit\" "
> + "or \"git worktree add\"."));
> + if (state.cherry_pick_in_progress)
> + die(_("cannot switch branch while cherry-picking\n"
> + "Consider \"git cherry-pick --quit\" "
> + "or \"git worktree add\"."));
> + if (state.revert_in_progress)
> + die(_("cannot switch branch while reverting\n"
> + "Consider \"git revert --quit\" "
> + "or \"git worktree add\"."));
> + if (state.bisect_in_progress)
> + die(_("cannot switch branch while bisecting\n"
> + "Consider \"git bisect reset HEAD\" "
> + "or \"git worktree add\"."));
> +}
> +
> static int checkout_branch(struct checkout_opts *opts,
> struct branch_info *new_branch_info)
> {
> @@ -1257,6 +1292,9 @@ static int checkout_branch(struct checkout_opts *opts,
> !new_branch_info->path)
> die_expecting_a_branch(new_branch_info);
>
> + if (!opts->can_switch_when_in_progress)
> + die_if_some_operation_in_progress();
> +
> if (new_branch_info->path && !opts->force_detach && !opts->new_branch &&
> !opts->ignore_other_worktrees) {
> int flag;
> @@ -1514,6 +1552,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
> opts.only_merge_on_switching_branches = 0;
> opts.accept_pathspec = 1;
> opts.implicit_detach = 1;
> + opts.can_switch_when_in_progress = 1;
>
> options = parse_options_dup(checkout_options);
> options = add_common_options(&opts, options);
> @@ -1549,6 +1588,7 @@ int cmd_switch(int argc, const char **argv, const char *prefix)
> opts.switch_branch_doing_nothing_is_ok = 0;
> opts.only_merge_on_switching_branches = 1;
> opts.implicit_detach = 0;
> + opts.can_switch_when_in_progress = 0;
>
> options = parse_options_dup(switch_options);
> options = add_common_options(&opts, options);
>
