Re: [PATCH 4/6] unpack-trees: avoid nuking untracked dir in way of locally deleted file

[Elijah Newren <newren@gmail.com>]

On Sun, Sep 19, 2021 at 6:52 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Sat, Sep 18 2021, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >  t/t2500-untracked-overwriting.sh | 2 +-
> >  unpack-trees.c                   | 4 ++++
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/t/t2500-untracked-overwriting.sh b/t/t2500-untracked-overwriting.sh
> > index 017946a494f..d4d9dc928aa 100755
> > --- a/t/t2500-untracked-overwriting.sh
> > +++ b/t/t2500-untracked-overwriting.sh
> > @@ -218,7 +218,7 @@ test_expect_success 'git am --abort and untracked dir vs. unmerged file' '
> >       )
> >  '
> >
> > -test_expect_failure 'git am --skip and untracked dir vs deleted file' '
> > +test_expect_success 'git am --skip and untracked dir vs deleted file' '
> >       test_setup_sequencing am_skip_and_untracked &&
> >       (
> >               cd sequencing_am_skip_and_untracked &&
> > diff --git a/unpack-trees.c b/unpack-trees.c
> > index 3b3d1c0ff40..858595a13f1 100644
> > --- a/unpack-trees.c
> > +++ b/unpack-trees.c
> > @@ -2395,7 +2395,11 @@ static int deleted_entry(const struct cache_entry *ce,
> >               if (verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_REMOVED, o))
> >                       return -1;
> >               return 0;
> > +     } else {
> > +             if (verify_absent_if_directory(ce, ERROR_WOULD_LOSE_UNTRACKED_REMOVED, o))
> > +                     return -1;
> >       }
>
> Maybe just "else if" ?

Yeah, that makes sense.

> [...]

That's kind of misleading.  ;-)  You trimmed out a single line here,
and in particular one that only contained a trailing curly brace.
Thus, your "trimming" here actually made things longer.

>
> > +
>
> Stray whitespace change

No, the whitespace addition was after making the if-block above it
more complex with the extra else block.  That if-block is now
approximately 2/3 of the length of the function, and is the part that
is relevant to the comment above it.  Since the code that follows the
if-block is separate from the comment above and the if-block became
more complex, it felt natural to add a bit of spacing.  So, it wasn't
stray, but intentional and related to the changes above.