Re: [PATCH v2 16/17] ls: do not show duplicate cached entries

[Eric Sunshine <sunshine@sunshineco.com>]

On Wed, Mar 26, 2014 at 9:48 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> With the current show_files() "ls -tcm" will show
>
>   foo.c
> M foo.c
>
> The first item is redundant. If "foo.c" is modified, we know it's in
> the cache. Introduce show_files_compact to do that because ls-files is
> plumbing and scripts may already depend on current display behavior.
>
> Another difference in show_files_compact() is it does not show
> skip-worktree (aka outside sparse checkout) entries anymore, which
> makes sense in porcelain context.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/ls-files.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> index 709d8b1..cd8e35c 100644
> --- a/builtin/ls-files.c
> +++ b/builtin/ls-files.c
> @@ -337,6 +337,53 @@ static void show_files(struct dir_struct *dir)
>         }
>  }
>
> +static void show_files_compact(struct dir_struct *dir)
> +{
> +       int i;
> +
> +       /* For cached/deleted files we don't need to even do the readdir */
> +       if (show_others || show_killed) {
> +               if (!show_others)
> +                       dir->flags |= DIR_COLLECT_KILLED_ONLY;
> +               fill_directory(dir, &pathspec);
> +               if (show_others)
> +                       show_other_files(dir);
> +               if (show_killed)
> +                       show_killed_files(dir);
> +       }
> +       if (!(show_cached || show_stage || show_deleted || show_modified))
> +               return;
> +       for (i = 0; i < active_nr; i++) {
> +               const struct cache_entry *ce = active_cache[i];
> +               struct stat st;
> +               int err, shown = 0;
> +               if ((dir->flags & DIR_SHOW_IGNORED) &&
> +                   !ce_excluded(dir, ce))
> +                       continue;
> +               if (show_unmerged && !ce_stage(ce))
> +                       continue;
> +               if (ce->ce_flags & CE_UPDATE)
> +                       continue;
> +               if (ce_skip_worktree(ce))
> +                       continue;
> +               err = lstat(ce->name, &st);
> +               if (show_deleted && err) {
> +                       show_ce_entry(tag_removed, ce);
> +                       shown = 1;
> +               }
> +               if (show_modified && ce_modified(ce, &st, 0)) {

Is it possible for the lstat() to have failed for some reason when we
get here? If so, relying upon 'st' is unsafe, isn't it?

> +                       show_ce_entry(tag_modified, ce);
> +                       shown = 1;
> +               }
> +               if (ce_stage(ce)) {
> +                       show_ce_entry(tag_unmerged, ce);
> +                       shown = 1;
> +               }
> +               if (!shown && show_cached)
> +                       show_ce_entry(tag_cached, ce);
> +       }
> +}
> +
>  /*
>   * Prune the index to only contain stuff starting with "prefix"
>   */
> @@ -606,7 +653,10 @@ static int ls_files(const char **argv, const char *prefix)
>                 refresh_index(&the_index, REFRESH_QUIET, &pathspec, NULL, NULL);
>                 setup_pager();
>         }
> -       show_files(&dir);
> +       if (porcelain)
> +               show_files_compact(&dir);
> +       else
> +               show_files(&dir);
>         if (show_resolve_undo)
>                 show_ru_info();
>
> --
> 1.9.1.345.ga1a145c
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html