Re: [PATCH 5/8] checkout: introduce --{,no-}overlay option

From: Duy Nguyen <pclouds@gmail.com>
Date: Dec 10, 2018, 16:42 UTC
Message-ID: <CACsJy8BWZyFWZ70y4beCEYvjbc2+X-2K+gOiY+=toK0y3xYzKw@mail.gmail.com>
In-Reply-To: <20181209200449.16342-6-t.gummerer@gmail.com>

On Sun, Dec 9, 2018 at 9:05 PM Thomas Gummerer <t.gummerer@gmail.com> wrote:

Show 25 quoted lines

> @@ -302,15 +310,29 @@ static int checkout_paths(const struct checkout_opts *opts,
>                 ce->ce_flags &= ~CE_MATCHED;
>                 if (!opts->ignore_skipworktree && ce_skip_worktree(ce))
>                         continue;
> -               if (opts->source_tree && !(ce->ce_flags & CE_UPDATE))
> -                       /*
> -                        * "git checkout tree-ish -- path", but this entry
> -                        * is in the original index; it will not be checked
> -                        * out to the working tree and it does not matter
> -                        * if pathspec matched this entry.  We will not do
> -                        * anything to this entry at all.
> -                        */
> -                       continue;
> +               if (opts->source_tree && !(ce->ce_flags & CE_UPDATE)) {
> +                       if (!opts->overlay_mode &&
> +                           ce_path_match(&the_index, ce, &opts->pathspec, ps_matched)) {
> +                               /*
> +                                * "git checkout --no-overlay <tree-ish> -- path",
> +                                * and the path is not in tree-ish, but is in
> +                                * the current index, which means that it should
> +                                * be removed.
> +                                */
> +                               ce->ce_flags |= CE_MATCHED | CE_REMOVE | CE_WT_REMOVE;
> +                               continue;
> +                       } else {

In non-overlay mode but when pathspec does not match, we come here too.

> +                               /*
> +                                * "git checkout tree-ish -- path", but this
> +                                * entry is in the original index; it will not

I think the missing key point in this comment block is "..is in the original index _and it's not in tree-ish_". In non-overlay mode, if pathspec does not match then it's safe to ignore too. But this logic starts too get to complex and hurt my brain.

Show 11 quoted lines

> +                                * be checked out to the working tree and it
> +                                * does not matter if pathspec matched this
> +                                * entry.  We will not do anything to this entry
> +                                * at all.
> +                                */
> +                               continue;
> +                       }
> +               }
>                 /*
>                  * Either this entry came from the tree-ish we are
>                  * checking the paths out of, or we are checking out

Show 5 quoted lines

> @@ -1266,6 +1299,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>                             "checkout", "control recursive updating of submodules",
>                             PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater },
>                 OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")),
> +               OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode")),

maybe add " (default)" to the help string.

>                 OPT_END(),
>         };
>

-- 
Duy