Samuel Lijin <sxlijin@xxxxxxxxx> 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.