On Fri, Jan 18, 2013 at 12:53:32PM -0800, Junio C Hamano wrote:
... did you have any comment on
the struct config_key alternative I sent as a follow-up?
I did read it but I cannot say I did so very carefully. My gut
reaction was that the take the variable name and section name,
return the subsection name pointer and length, if there is any, and
the key made it readable enough. The proposed interface to make
and lend a copy to the caller does make it more readble, but I do
not know if that is worth doing. Neutral-to-slightly-in-favor, I
would say.
Now I re-read that struct config_key thing, I would have to say
that the idea of giving split and NUL-terminated strings to the
callers is good, but the cheat looks somewhat brittle for all the
reasons that come from using a static buffer (which you already
mentioned). As I do not offhand think of a better alternative, I'd
say we leave it for another day.
OK. I had the feeling if the config parser provided it to the caller
that more sites could take advantage of it (without adding too many
lines to call the parsing function). But looking again, there aren't
that many sites that would benefit. E.g., git_daemon_config in daemon.c
could use it to avoid using a constant offset. But the current code
there is not hard to read, and saving a few characters there is not
worth the complexity.
So I've re-rolled the original version, taking into account the comments
from you and Jonathan. I also clarified a few of the commit messages,
and modified two more sites to use the new function.
[1/8]: config: add helper function for parsing key names
Same as before, but now called parse_config_key.
[2/8]: archive-tar: use parse_config_key when parsing config
Same (rebased for new name, of course).
[3/8]: convert some config callbacks to parse_config_key
Tweaked confusing ep variable name. Fixed missing !name check in
userdiff code (which gets removed in the next patch anyway).
[4/8]: userdiff: drop parse_driver function
[5/8]: submodule: use parse_config_key when parsing config
[6/8]: submodule: simplify memory handling in config parsing
Same.
[7/8]: help: use parse_config_key for man config
[8/8]: reflog: use parse_config_key in config callback
Two new callsites. I split these out because unlike the ones in 3/8,
they do not benefit from a reduction in lines of code. However, I think
the results are still more readable. You can judge for yourself; drop
them if you disagree. Or feel free to squash them into 3/8 if that makes
more sense.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
