Re: Incorrect diff-parseopt conversion?

[Duy Nguyen <pclouds@gmail.com>]

On Wed, May 22, 2019 at 7:35 AM Ramsay Jones
<ramsay@ramsayjones.plus.com> wrote:
>
>
>
> On 22/05/2019 01:11, Duy Nguyen wrote:
> > On Wed, May 22, 2019 at 2:56 AM Ramsay Jones
> > <ramsay@ramsayjones.plus.com> wrote:
> >>
> >> Hi Duy,
> >>
> >> I am in the middle of rebasing a long running branch onto
> >> current master (v2.22.0-rc1) and noticed something odd with
> >> commit af2f368091 ("diff-parseopt: convert --output-*",
> >> 2019-02-21).
> >>
> >> As part of the branch I am rebasing, I have defined a new
> >> OPT_LL_CALLBACK() macro[1], which I had intended to apply to
> >> the 'output' option to diff. However, commit af2f368091
> >> defines that option thus:
> >>
> >> +               { OPTION_CALLBACK, 0, "output", options, N_("<file>"),
> >> +                 N_("Output to a specific file"),
> >> +                 PARSE_OPT_NONEG, NULL, 0, diff_opt_output },
> >>
> >> Note that the 'option type' is given as OPTION_CALLBACK, not
> >> as OPTION_LOWLEVEL_CALLBACK. Is this intended?
> >
> > Yeah I think this is correct (phew!).
>
> 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.
-- 
Duy