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

2014-03-27 Thread Eric Sunshine
On Wed, Mar 26, 2014 at 9:48 AM, Nguyễn Thái Ngọc Duy pclo...@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 pclo...@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 majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-03-26 Thread Nguyễn Thái Ngọc Duy
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 pclo...@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)) {
+   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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html