On 12/10, Duy Nguyen wrote:
> On Sun, Dec 9, 2018 at 9:05 PM Thomas Gummerer <t.gumme...@gmail.com> wrote:
> > @@ -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.

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.

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

> >                 OPT_END(),
> >         };
> >
> -- 
> Duy

Reply via email to