On Wed, May 17, 2017 at 10:34 PM, Junio C Hamano [off-list ref] wrote:

Samuel Lijin [off-list ref] writes:

quoted

@@ -932,7 +935,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)

      fill_directory(&dir, &pathspec);

-     for (i = 0; i < dir.nr; i++) {
+     for (k = i = 0; i < dir.nr; i++) {
              struct dir_entry *ent = dir.entries[i];
              int matches = 0;
              struct stat st;

@@ -954,10 +957,35 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
                  matches != MATCHED_EXACTLY)
                      continue;

+             // skip any dir.entries which contains a dir.ignored
+             for (; k < dir.ignored_nr; k++) {
+                     if (cmp_dir_entry(&dir.entries[i],
+                                             &dir.ignored[k]) < 0)
+                             break;

It is a bit unfortunate that we do not use the short-hand "ent" we
established for dir.entries[i] at the beginning of this loop here.

quoted

+             }
+             if ((k < dir.ignored_nr) &&
+                             check_dir_entry_contains(dir.entries[i], dir.ignored[k])) {
+                     continue;
+             }

The above logic is not needed when dir.entries[i] is a directory,
right?

Au contraire - this logic only matters when dir.entries[i] is a directory.

quoted

+
+             // current entry does not contain any ignored files

... or the entry may not have been a directory in the first place.

If it's not a directory, it can't contain ignored files ;)

quoted

              rel = relative_path(ent->name, prefix, &buf);
              string_list_append(&del_list, rel);
+
+             // skip untracked contents of an untracked dir
+             for (j = i + 1;
+                      j < dir.nr &&
+                          check_dir_entry_contains(dir.entries[i], dir.entries[j]);
+                      j++);
+             i = j - 1;
      }

+     for (i = 0; i < dir.nr; i++)
+             free(dir.entries[i]);
+
+     for (i = 0; i < dir.ignored_nr; i++)
+             free(dir.ignored[i]);
+

The above logic may not be incorrect per-se, but I have this
suspicion that the resulting code may become cleaner and easier to
understand if we did it in a new separate loop we call immediately
after fill_directory() returns.  Or it could even be a call to a
helper that "post processes" the "dir" thing that was polupated by
fill_directory() that removes elements in the entries[] array that
contains any element in ignored[] array.

Will try it out.

quoted

      if (interactive && del_list.nr > 0)
              interactive_main_loop();

Thanks.