Re: [PATCH 1/2] docs: add a basic description of the config API

[Nguyen Thai Ngoc Duy <pclouds@gmail.com>]

On Wed, Feb 8, 2012 at 1:40 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Your excuse that "the toplevel callback in my callchain never returns 1,
> so overall the nd/columns series is ok" just muddies the water.  It means
> if later somebody wanted to use inner callback functions you use from the
> git_column_config() callchain chain as a toplevel callback for whatever
> reason, that will violate the "0 for success, or -1 for error" convention.
> More importantly, if somebody wants to turn a top-level callback that
> currently returns 0 into a sub callback used by his callback callchain, he
> cannot change that existing callback to return 1 to tell him to short
> circuit, because for other callers returning 1 would be a violation.
>
> What I was getting at is that we probably should officially declare that
> returning 1 to signal success is perfectly acceptable (and it probably
> should mean the caller who called the callback function as a sub callback
> should return immediately, taking it as a signal that the key has already
> been handled), as the primary purpose of this thread to discuss Peff's
> patch is to write these rules down.

Or allow multiple callbacks from git_config(). The problem with is it
adds another common set of config vars. Now it's common to chain var
sets by doing

int config_callback(...)
{
    ....
    return yet_another_callback(...);
}

int main()
{
   git_config(config_callback, ...)
   ...

where yet_another_callback() hard codes another callback (usually
git_default_config). This is inflexible. If we allow multiple
callbacks, git_column_config() could be changed to return just 0 or -1
(i.e. 1 remains unsuccessful error code).

On second thought, I think calling git_config() multiple times, each
time with one callback, may work too. We may want to cache config
content to avoid accessing files too many times though.

> Of course, all that relies on the audit of the git_config() machinery. I
> think it is written to accept non-negative as success, and that is why I
> said "I too think this should be acceptable" in the first place.
-- 
Duy