On Fri, Jan 04, 2013 at 11:54:34PM -0800, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > > Adam Spiers <git@xxxxxxxxxxxxxx> writes: > > > >> diff --git a/builtin/clean.c b/builtin/clean.c > >> index 0c7b3d0..bd18b88 100644 > >> --- a/builtin/clean.c > >> +++ b/builtin/clean.c > >> @@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix) > >> if (!ignored) > >> setup_standard_excludes(&dir); > >> > >> + add_exclude_list(&dir, EXC_CMDL); > >> for (i = 0; i < exclude_list.nr; i++) > >> add_exclude(exclude_list.items[i].string, "", 0, > >> - &dir.exclude_list[EXC_CMDL]); > >> + &dir.exclude_list_groups[EXC_CMDL].ary[0]); > > > > This looks somewhat ugly for two reasons. > > > > * The abstraction add_exclude() offers to its callers is just to > > let them add one pattern to the list of patterns for the kind > > (here, EXC_CMDL); why should they care about .ary[0] part? Are > > there cases any sane caller (not the implementation of the > > exclude_list_group machinery e.g. add_excludes_from_... function) > > may want to call it with .ary[1]? I do not think of any. > > Shouldn't the public API function add_exclude() take a pointer to > > the list group itself? > > > > * When naming an array of things, we tend to prefer naming it > > > > type thing[count] > > > > so that the second element can be called "thing[2]" and not > > "things[2]". dir.exclude_list_group[EXC_CMDL] reads better. > > Also, "ary[]" is a bad name, even as an implementation detail, for > two reasons: it is naming it after its type (being an "array") not > after what it is (if it holds the patterns from the same information > source, e.g. file, togeter, "src" might be a better name), and it > uses rather unusual abbreviation (I haven't seen "array" shortened > to "ary" anywhere else). OK, well in that case Documentation/technical/api-allocation-growing.txt needs to be fixed, because I copied it from that. I would never normally shorten "array" to "ary" either, but I did it in an attempt to conform to the stated guidelines. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html