Re: [PATCH v1 04/11] restore: disable overlay mode by default

[Elijah Newren <newren@gmail.com>]

On Fri, Mar 8, 2019 at 2:17 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>
> Overlay mode is considered confusing when the command is about
> restoring files on worktree. Disable it by default. The user can still
> turn it on, or use 'git checkout' which still has overlay mode on by
> default.
>
> While at there make the check in checkout_branch() stricter. Neither
> --overlay or --no-overlay should be accepted in branch switching mode.

s/While at there/While at it,/ ?

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/checkout.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index c52ce13d2a..9e59bf792f 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -1196,9 +1196,9 @@ static int checkout_branch(struct checkout_opts *opts,
>                 die(_("'%s' cannot be used with switching branches"),
>                     "--patch");
>
> -       if (!opts->overlay_mode)
> +       if (opts->overlay_mode != -1)
>                 die(_("'%s' cannot be used with switching branches"),
> -                   "--no-overlay");
> +                   "--[no]-overlay");
>
>         if (opts->writeout_stage)
>                 die(_("'%s' cannot be used with switching branches"),
> @@ -1313,7 +1313,6 @@ static struct option *add_checkout_path_options(struct checkout_opts *opts,
>                 OPT_BOOL('p', "patch", &opts->patch_mode, N_("select hunks interactively")),
>                 OPT_BOOL(0, "ignore-skip-worktree-bits", &opts->ignore_skipworktree,
>                          N_("do not limit pathspecs to sparse entries only")),
> -               OPT_BOOL(0, "overlay", &opts->overlay_mode, N_("use overlay mode (default)")),
>                 OPT_END()
>         };
>         struct option *newopts = parse_options_concat(prevopts, options);
> @@ -1333,7 +1332,6 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
>         opts->overwrite_ignore = 1;
>         opts->prefix = prefix;
>         opts->show_progress = -1;
> -       opts->overlay_mode = -1;
>
>         git_config(git_checkout_config, opts);
>
> @@ -1505,6 +1503,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>                 OPT_BOOL('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")),
>                 OPT_BOOL(0, "guess", &opts.dwim_new_local_branch,
>                          N_("second guess 'git checkout <no-such-branch>' (default)")),
> +               OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode (default)")),
>                 OPT_END()
>         };
>         int ret;
> @@ -1517,6 +1516,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>         opts.accept_pathspec = 1;
>         opts.implicit_detach = 1;
>         opts.empty_pathspec_ok = 1;
> +       opts.overlay_mode = -1;
>
>         options = parse_options_dup(checkout_options);
>         options = add_common_options(&opts, options);
> @@ -1551,6 +1551,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.overlay_mode = -1;
>
>         options = parse_options_dup(switch_options);
>         options = add_common_options(&opts, options);
> @@ -1569,6 +1570,7 @@ int cmd_restore(int argc, const char **argv, const char *prefix)
>         struct option restore_options[] = {
>                 OPT_STRING('s', "source", &opts.from_treeish, "<tree-ish>",
>                            N_("where the checkout from")),
> +               OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode")),
>                 OPT_END()
>         };
>         int ret;
> @@ -1577,6 +1579,7 @@ int cmd_restore(int argc, const char **argv, const char *prefix)
>         opts.accept_ref = 0;
>         opts.accept_pathspec = 1;
>         opts.empty_pathspec_ok = 0;
> +       opts.overlay_mode = 0;
>
>         options = parse_options_dup(restore_options);
>         options = add_common_options(&opts, options);
> --
> 2.21.0.rc1.337.gdf7f8d0522

The rest all looks good.