Re: [PATCH v4 05/10] userdiff: add and use for_each_userdiff_driver()

On Wed, Mar 24, 2021 at 12:57:15PM -0700, Junio C Hamano wrote:

And while "type" could also be made a part of this structure and
have the API always iterate over all entries regardless of "type",
i.e. the callback function could be the one responsible for finding
the entries in the table for one particular type, it is understandable
that potential callers can be helped by having the pre-filtering
based on the "type" thing on the API side.

Yeah. We _could_ also have the for-each function filter by name (if
present). And then quit when it finds a matching name (because it knows
the names are supposed to be unique).

That is the opposite of:

Or a single for_each_driver() that gives <name, length, type> tuple
to the callback function, iterating over all drivers regardless of
the type.

I'd be fine with that, too. I don't have a huge preference either way,
but it does feel like it would fix the weird asymmetry (though as I
said, I don't think the asymmetry is _wrong_, and it may not be worth
over-engineering this tiny corner of the interface).

Thanks for spelling out both of these directions clearly. My response to
Ævar was a little muddled as I was having trouble figuring out just what
it was that left me so puzzled by the patch. :)


By the way, one thing I wondered about all of this: could we have an
entry in both the custom and builtin lists? I think the answer is "no",
because any custom config for a builtin type would get placed inside the
existing struct in the builtin_drivers array (which is sadly a reason we
must leak any old string values when we parse diff.*.funcname, etc; we
don't know if they are string literals or heap values from previously
parsed config).

I also notice that the "builtin" for-each does not count the boolean
"diff" or "!diff" attribute structs. That is perhaps reasonable, as they
do not have interesting names nor values in the first place. I do still
wonder whether "builtin vs custom" is even a useful distinction (i.e.,
those builtins are less builtin_drivers[] than drivers[]).

-Peff