On Fri, May 24, 2019 at 5:08 PM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
> >> OK, I just had a look at the code in parse-options.c.
> >> Hmm, somewhat ugly! :-D
> >
> > Yeah it's a bit hackish. The problem is parse-options (and also config
> > parser) does not allow passing user data to the callback. Changing it
> > would affect all callsites at once. It's just too scary for me to do
> > it and I chose an uglier way but with smaller impact.
>
> As an aside because I've sometimes wanted to pass such arbitrary data to
> the callback.
>
> Do you mean it's painful to change "struct option" because of all the
> macros around it?
No, it's updating all the callback functions to take a new parameter.
> Instead of all callsites, wouldn't that just be a patch simliar to
> bf3ff338a2 ("parse-options: stop abusing 'callback' for lowlevel
> callbacks", 2019-01-27)?
>
> I.e. a special callback that'll be called to return a void* to arbitrary
> data, and then we'll call the callback with that.
That allows more gradual conversion, but we'll add a bunch new OPT_
macros. Not exactly nice.
If we do this, I'd rather go with converting all callback functions at
once. Though we probably want to avoid going through the same pain
again, by passing a single struct instead, e.g.
int some_callback(const struct option *opt, const struct optval *val);
"arg" and "unset" are in this "val", callback data is in it too. And
it may help reduce those -Wunused-paramters that Jeff's trying to
kill.
> Or, just passing it around in a global variable, since we know the flow
> of when we setup the data, and when parse_options() is called.
This might work fine with parse_options() at top level (e.g. cmd_xx
functions) but apply.c, diff.c and other should not really depend on
it. And we could almost avoid that with parse_opt_ctx_t.
--
Duy
