Re: [PATCH v3 6/8] clean: teach clean -d to skip dirs containing ignored files
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 18, 2017, 02:34 UTC
- Message-ID
- <xmqq4lwix58s.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170516073423.25762-7-sxlijin@gmail.com>
Samuel Lijin <sxlijin@gmail.com> writes:
Show 18 quoted lines
> @@ -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.
Show 5 quoted lines
> + }
> + 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.
Show 17 quoted lines
> 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.