Re: [PATCH v7 1/3] ls_files.c: bugfix for --deleted and --modified

>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > This situation may occur in the original code: lstat() failed
> > but we use `&st` to feed ie_modified() later.
> >
> > Therefore, we can directly execute show_ce without the judgment of
> > ie_modified() when lstat() has failed.
> >
> > Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> > [jc: fixed misindented code]
>
> I noticed that you reverted my fix in this version, when this is
> compared with the one I sent last night.
>
> Comparing the result of applying all three with what I sent last
> night, this v7 looks worse (see below).  Let's discard this round
> and declare victory with what is already on 'seen'.
>
> Thanks.
>
>
> ---
>
> comparison between what these three patches would produce (preimage)
> and what is on 'seen' (postimage)is shown here.
>
> diff --git w/builtin/ls-files.c c/builtin/ls-files.c
> index fb9cf50d76..f6f9e483b2 100644
> --- w/builtin/ls-files.c
> +++ c/builtin/ls-files.c
> @@ -313,7 +313,8 @@ static void show_files(struct repository *repo, struct dir_struct *dir)
>                 if (show_killed)
>                         show_killed_files(repo->index, dir);
>         }
> -       if (! (show_cached || show_stage || show_deleted || show_modified))
> +
> +       if (!(show_cached || show_stage || show_deleted || show_modified))
>                 return;
>         for (i = 0; i < repo->index->cache_nr; i++) {
>                 const struct cache_entry *ce = repo->index->cache[i];
> @@ -328,15 +329,16 @@ static void show_files(struct repository *repo, struct dir_struct *dir)
>                 if (ce->ce_flags & CE_UPDATE)
>                         continue;
>                 if ((show_cached || show_stage) &&
> -                       (!show_unmerged || ce_stage(ce))) {
> -                               show_ce(repo, dir, ce, fullname.buf,
> -                                       ce_stage(ce) ? tag_unmerged :
> -                                       (ce_skip_worktree(ce) ? tag_skip_worktree :
> -                                               tag_cached));
> +                   (!show_unmerged || ce_stage(ce))) {
> +                       show_ce(repo, dir, ce, fullname.buf,
> +                               ce_stage(ce) ? tag_unmerged :
> +                               (ce_skip_worktree(ce) ? tag_skip_worktree :
> +                                tag_cached));
>                         if (skipping_duplicates)
>                                 goto skip_to_next_name;
>                 }
> -               if (!show_deleted && !show_modified)
> +
> +               if (!(show_deleted || show_modified))
>                         continue;
>                 if (ce_skip_worktree(ce))
>                         continue;
> @@ -349,12 +351,13 @@ static void show_files(struct repository *repo, struct dir_struct *dir)
>                                 goto skip_to_next_name;
>                 }
>                 if (show_modified &&
> -                       (stat_err || ie_modified(repo->index, ce, &st, 0))) {
> -                               show_ce(repo, dir, ce, fullname.buf, tag_modified);
> +                   (stat_err || ie_modified(repo->index, ce, &st, 0))) {
> +                       show_ce(repo, dir, ce, fullname.buf, tag_modified);
>                         if (skipping_duplicates)
>                                 goto skip_to_next_name;
>                 }
>                 continue;
> +
>  skip_to_next_name:
>                 {
>                         int j;
> @@ -362,7 +365,7 @@ static void show_files(struct repository *repo, struct dir_struct *dir)
>                         for (j = i + 1; j < repo->index->cache_nr; j++)
>                                 if (strcmp(ce->name, cache[j]->name))
>                                         break;
> -                       i = j - 1; /* compensate for outer for loop */
> +                       i = j - 1; /* compensate for the for loop */
>                 }
>         }
>
> @@ -590,7 +593,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
>                         N_("pretend that paths removed since <tree-ish> are still present")),
>                 OPT__ABBREV(&abbrev),
>                 OPT_BOOL(0, "debug", &debug_mode, N_("show debugging data")),
> -               OPT_BOOL(0,"deduplicate",&skipping_duplicates,N_("suppress duplicate entries")),
> +               OPT_BOOL(0, "deduplicate", &skipping_duplicates,
> +                        N_("suppress duplicate entries")),
>                 OPT_END()
>         };
>