Nicolas Morey-Chaisemartin <nmoreychaisemartin@xxxxxxxx> writes:
>> Of course you could also unify in the other direction and instead
>> of running git_config(git_defauilt_config, NULL), pick the exact
>> variables you care about (did you say askpass???).
>
> The only one we care about for this specific case if core.askpass
> as user gets prompted to authenticate on his IMAP server. So
> picking just this one would be simpler. However isn't the other
> way around cleaner if we happen to depend on another
> "generic/core" setting ?
Yes, "the other way around" is cleaner and more desirable exactly
for that reason.
There is an established way to ask another parser to handle
variables you do not handle yourself with the callback-style
configuration parsing. "git grep 'return git_default_config('"
shows places that are taking advantage of the technique. There is
an in-core collection of all the configuration (variable, value)
definitions, which is populated by reading the on-disk files just
once, and a call to git_config() iterates over this collection and
calls the callback-style parser for each (variable, value)
definition, resulting in a single pass.
The parser imap-send currently uses is based on a more recent style,
where each of the individual variables the caller is interested in
is looked up from the same in-core (variable, value) definitions.
It is easier to start writing, but does not have a good established
way to ask the more basic layer to grab things they care about,
without doing a full git_config() call if the basic layer only has
callback-style parser.
In the longer term (read: I do *not* think it is a good idea to do
this as part of this series; I am just pointing at a future course
to make it easier to use the config API in general, not just by the
imap-send command), we probably should come up with a way to add
another config helper that grabs the same set of variables as the
callback-style config helper at the same layer to help config
parsers like the ones in imap-send. E.g. git_default_config() is a
callback helper to grab all the basic configuration variable, and it
is suited for calling from a callback style configuration parser,
but it would be nicer to the current git_imap_config() and its
friends if there were a function they can call that is better than
"git_config(git_default_config)", which causes all the variables
that the default layer do not care about to be fed to the callback
only to be discarded.
In the far longer term (read: I do *not* think it is a good idea to
think about this for too long in the context of this series, and I
am not even sure what I speculate would be what I'd be convinced in
6 months myself), we may want to choose one style over the other.
My current inclination is to write off the targeted "what's the
value of this variable?" style as a failed experiment (because it
exactly has the "cannot chain easily" problem) and standardise on
the callback-style parsers, but at the same time I think it is
possible to establish a good convention and set of config parsing
helpers for subcommand specific config parsers that are not
callback-style to delegate parsing of configuration variables at the
more basic layer with enough engineering effort, and it would
probably be a more desirable outcome in the longer term, resulting
in removal of the callback-style parsers.
But for now, without such support, "the other way around" would
result in a cleaner solution that is futureproof until we solve the
"in the far longer term" issue.
Thanks.
