Re: Incorrect diff-parseopt conversion?

From: Duy Nguyen <pclouds@gmail.com>
Date: May 22, 2019, 00:11 UTC
Message-ID: <CACsJy8BHeVDDnEYXaOAPtEoesTVNCiPGSgnJYjtL6aYSU0-J5A@mail.gmail.com>
In-Reply-To: <82f79cd9-5273-b9a4-744d-f5fa95dd1f47@ramsayjones.plus.com>

On Wed, May 22, 2019 at 2:56 AM Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:

Show 19 quoted lines

>
> 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!).

OPTION_CALLBACK can take two different callback types, the "lowlevel" one allows more access to parseopt context data while still does common parsing for us. OPTION_LOWLEVEL_CALLBACK, on the other hand, calls ll_callback directly and leaves it to the callback to handle everything. We want OPTION_CALLBACK here so we will not have to do all the work, we just need extra info (the prefix to be specific).

Show 9 quoted lines

>
> ATB,
> Ramsay Jones
>
> [1] Yes, the reason my branch is long running is because
> we keep changing the same files! We have both defined new
> OPT_() macros, some with the same name ... ;-)
>
>

-- 
Duy