Samuel Lijin <sxli...@gmail.com> writes:

> +     for (j = i = 0; i < dir.nr;) {
> +             for (;
> +                  j < dir.ignored_nr &&
> +                    0 <= cmp_dir_entry(&dir.entries[i], &dir.ignored[j]);
> +                  j++);
> +
> +             if ((j < dir.ignored_nr) &&
> +                             check_dir_entry_contains(dir.entries[i], 
> dir.ignored[j])) {
> +                     /* skip any dir.entries which contains a dir.ignored */
> +                     free(dir.entries[i]);
> +                     dir.entries[i++] = NULL;
> +             } else {
> +                     /* prune the contents of a dir.entries which will be 
> removed */
> +                     struct dir_entry *ent = dir.entries[i++];
> +                     for (;
> +                          i < dir.nr &&
> +                            check_dir_entry_contains(ent, dir.entries[i]);
> +                          i++) {
> +                             free(dir.entries[i]);
> +                             dir.entries[i] = NULL;
> +                     }
> +             }
> +     }

The second loop in the else clause is a bit tricky, and the comment
"which will be removed" is not all that helpful to explain why the
loop is there.

But I think the code is correct.  Here is how I understood it.

    While looking at dir.entries[i], the code noticed that nothing
    in that directory is ignored.  But entries in dir.entries[] that
    come later may be contained in dir.entries[i] and we just want
    to show the top-level untracked one (e.g. "a/" and "a/b/" were
    in entries[], there is nothing in "a/", so naturally there is
    nothing in "a/b/", but we do not want to bother showing
    both---showing "a/" alone saying "the entire a/ is untracked" is
    what we want).

We may want to have a test to ensure "a/b/" is indeed omitted in
such a situation from the output, though.

By the way, instead of putting NULL, it may be easier to follow if
you used two pointers, src and dst, into dir.entries[], just like
you did in your latest version of [PATCH 4/6].  That way, you do not
have to change anything in the later loop that walks over elements
in the dir.entries[] array.  It would also help the logic easier to
follow if the above loop were its own helper function.

Putting them all together, here is what I came up with that can be
squashed into your patch.  I am undecided myself if this is easier
to follow than your version, but it seems to pass your test ;-)

Thanks.

 builtin/clean.c | 70 ++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 42 insertions(+), 28 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index dd3308a447..c8712e7ac8 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -851,9 +851,49 @@ static void interactive_main_loop(void)
        }
 }
 
+static void simplify_untracked(struct dir_struct *dir)
+{
+       int src, dst, ign;
+
+       for (src = dst = ign = 0; src < dir->nr; src++) {
+               /*
+                * Skip entries in ignored[] that cannot be inside
+                * entries[src]
+                */
+               while (ign < dir->ignored_nr &&
+                      0 <= cmp_dir_entry(&dir->entries[src], 
&dir->ignored[ign]))
+                       ign++;
+
+               if (dir->ignored_nr <= ign ||
+                   !check_dir_entry_contains(dir->entries[src], 
dir->ignored[ign])) {
+                       /*
+                        * entries[src] does not contain an ignored
+                        * path -- we need to keep it.  But we do not
+                        * want to show entries[] that are contained
+                        * in entries[src].
+                        */
+                       struct dir_entry *ent = dir->entries[src++];
+                       dir->entries[dst++] = ent;
+                       while (src < dir->nr &&
+                              check_dir_entry_contains(ent, 
dir->entries[src])) {
+                               free(dir->entries[src++]);
+                       }
+                       /* compensate for the outer loop's loop control */
+                       src--;
+               } else {
+                       /*
+                        * entries[src] contains an ignored path --
+                        * drop it.
+                        */
+                       free(dir->entries[src]);
+               }
+       }
+       dir->nr = dst;
+}
+
 int cmd_clean(int argc, const char **argv, const char *prefix)
 {
-       int i, j, res;
+       int i, res;
        int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;
        int ignored_only = 0, config_set = 0, errors = 0, gone = 1;
        int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
@@ -928,30 +968,7 @@ int cmd_clean(int argc, const char **argv, const char 
*prefix)
                       prefix, argv);
 
        fill_directory(&dir, &pathspec);
-
-       for (j = i = 0; i < dir.nr;) {
-               for (;
-                    j < dir.ignored_nr &&
-                      0 <= cmp_dir_entry(&dir.entries[i], &dir.ignored[j]);
-                    j++);
-
-               if ((j < dir.ignored_nr) &&
-                               check_dir_entry_contains(dir.entries[i], 
dir.ignored[j])) {
-                       /* skip any dir.entries which contains a dir.ignored */
-                       free(dir.entries[i]);
-                       dir.entries[i++] = NULL;
-               } else {
-                       /* prune the contents of a dir.entries which will be 
removed */
-                       struct dir_entry *ent = dir.entries[i++];
-                       for (;
-                            i < dir.nr &&
-                              check_dir_entry_contains(ent, dir.entries[i]);
-                            i++) {
-                               free(dir.entries[i]);
-                               dir.entries[i] = NULL;
-                       }
-               }
-       }
+       simplify_untracked(&dir);
 
        for (i = 0; i < dir.nr; i++) {
                struct dir_entry *ent = dir.entries[i];
@@ -959,9 +976,6 @@ int cmd_clean(int argc, const char **argv, const char 
*prefix)
                struct stat st;
                const char *rel;
 
-               if (!ent)
-                       continue;
-
                if (!cache_name_is_other(ent->name, ent->len))
                        continue;
 

Reply via email to