Re: [PATCH 13/13] parse_opt_ref_sorting: always use with NONEG flag

From: Martin Ågren <martin.agren@gmail.com>
Date: Mar 20, 2019, 12:22 UTC
Message-ID: <CAN0heSrubOaXRbg=NBi0rCuzthUcOEU3HmUKGg+=t+N0CCJcbA@mail.gmail.com>
In-Reply-To: <20190320081650.GM10403@sigill.intra.peff.net>

On Wed, 20 Mar 2019 at 09:17, Jeff King <peff@peff.net> wrote:

>   - since this was cut-and-pasted to four different spots, let's define
>     a single OPT_REF_SORT() macro that we can use everywhere

Indeed, all four are identical. And FWIW I failed to find a fifth caller anywhere (I looked for "OPT_CALLBACK.*sort").

> -               OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
> -                            N_("field name to sort on"), &parse_opt_ref_sorting),
> +               OPT_REF_SORT(sorting_tail),

> +#define OPT_REF_SORT(var) \
> +       OPT_CALLBACK_F(0, "sort", (var), \
> +                      N_("key"), N_("field name to sort"), \
> +                      PARSE_OPT_NONEG, parse_opt_ref_sorting)

This one is not identical though. ;-) You drop the "on". I trust you to know which of these is (more) correct, but I was a bit surprised to see "on" disappear without any mention. Mistake, or intended?

Other than that surprise ending, the whole series was a nice read.

Martin