Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes

Adam Spiers <git@adamspiers.org> writes:

Unless I was mistaken, I never objected to the EXC_CMDL, etc appearing in the text of the calling site of add_exclude().

The objection was about the .ary[0] bit. From the point of view of a caller of the API, it:

    - calls add_exclude_list() to declare "I now start adding new
      patterns that come from a new source of patterns"; then

    - calls add_exclude() repeatedly to add the patterns that come
      from that source.

no? Why does the latter has to keep repeating "Here is the new pattern for the EXC_CMDL group; it comes from the latest source I earlier declared, by the way", instead of just "Here is the new pattern for the EXC_CMDL group"? The ary[0] part always using "0" (not "4" or "ix") is what repeats that "by the way".

That is part of the "implementation of the machinery". If the API for the outside callers are to call add_exclude_list() to declare that patterns added by subsequent calls to add_exclude() are from one new source of the patterns (e.g. .gitignore file in a new directory level), and then call add_exclude() to add each pattern, then the callers to add_exclude() shouldn't have to care about the implementation detail that individual sources in exclude_list_group is implemented as an array in that sructure, and the latest ones should go to its ->array[0].

The implementation of the machinery may find it more convenient if they can add one or more "sources" to an exclude_list_group before starting to add patterns to ->array[0] or ->array[1] or ->array[2], and a finer grained internal API that lets the caller pass an instance of "struct exclude_list" regardless of where in an exclude_list_group's ary[] that instance sits may be necessary to do so.

But that does not mean other existing callers has to be aware of such inner detail. If the implementation of the machinery needs a helper function that adds an element to any struct exclude_list, not necessarily the one at the tip of an exclude_list_group, we can still do that by having the bulk of the logic in the internal, finer grained helper, say, add_pattern_to_exclude_list(), and keep the external API simpler by making it a thin wrapper around it, perhaps like:

   static void add_pattern_to_exclude_list(const char *pattern,
   		    const char *base, int baselen,
                    struct exclude_list *el);

   void add_exclude(const char *pattern,
   		    const char *base, int baselen,
                    struct exclude_list_group *group) {
	add_pattern_to_exclude_list(pattern, base, baselen, &group->ary[0]);
   }    

no?