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

On 12/10, Duy Nguyen wrote:

On Sun, Dec 9, 2018 at 9:05 PM Thomas Gummerer [off-list ref] wrote:

quoted

@@ -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.

quoted

+                               /*
+                                * "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.

Yes, that would make it a bit easier to read. I took a while to try
and refactor this to make it easier to read, but couldn't come up with
anything much better unfortunately.  I'll have another stab at
simplifying the logic a bit for v2.

quoted

+                                * 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

quoted

@@ -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.

Makes sense, will add.

quoted

                OPT_END(),
        };

-- 
Duy