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

> @@ -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.

> +             }
> +             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?

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

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

>               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.

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

Thanks.

Reply via email to