Matthieu Moy <matthieu....@grenoble-inp.fr> writes: > Junio C Hamano <gits...@pobox.com> writes: > >> After thinking about it a bit more, I think <file, line> support >> needs to be done not as a mere client of the generic, uncached >> git_config() API that we have had for forever, like the current >> iteration, but needs to know a bit more about the state the caller >> of the callback (namely, git_parse_source()), and we obviously do >> not want to expose that machinery to anybody other than the >> implementation of the config subsystem (to which the new facility >> this GSoC project adds belongs to), so in that sense you have to >> have your code in the same config.c file anyway. > > I do not buy the argument. Why would callers of the callback-style API > not be allowed to give <file, line> errors? > > To me, it is a weakness of the API that <file, line> information is not > available to callback functions, regardless of the config-cache.
I agree that it is good to allow scan-all-config-items-with-callback callers to learn <file, line>, but that is irrelevant. Perhaps you misread what I envision as the endgame of this thing to be and that is where our differences come from. One thing I think would be good to see in the endgame will be to give the benefit of the caching layer to the callback callers without having them change a single line of their code. One possible sequence of changes to make it happen would go like this, roughly in this order: - rename the current git_config() to git_config_raw(), and make it static to the config.c. - write a thin wrapper git_config() around git_config_raw() in config.c, until caching layer learns the iterator. - write caching layer to read from git_config_raw(). - teach git_config_raw() feed its callback functions to learn the <file, line> information. git_configset_get_<type> can then start using this in their error reporting. - implement iterator in the caching layer. - rewrite git_config() that was a thin wrapper around git_config_raw() by using the iterator over the cached values. - (optional) think about ways to give <file, line> information to the callback style callers. Perhaps we need git_config_2(). Perhaps we can rewrite all callers of git_config(). We do not have to decide it now. Between git_parse_source() and git_config_raw() we would need to pass the line-number information, but there is no reason for us to make public (all of these will be implementation details of the config system, including the config caching). My answer to "why shouldn't the callbacks have <file, line> information?" is "there is no reason why they shouldn't. It is a good addition in the long run". But the optionality of the last step in the above list makes it an irrelevant question. -- 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
