Re: [PATCH 09/20] parse-options: add support for parsing subcommands
- From
SZEDER Gábor <szeder.dev@gmail.com>
- Date
- Aug 12, 2022, 15:15 UTC
- Message-ID
- <20220812151513.GB3790@szeder.dev>
- In-Reply-To
- <220725.86wnc1nf6c.gmgdl@evledraar.gmail.com>
On Mon, Jul 25, 2022 at 09:41:52PM +0200, Ævar Arnfjörð Bjarmason wrote:
Show 32 quoted lines
> >> > @@ -206,6 +217,11 @@ struct option {
> >> > #define OPT_ALIAS(s, l, source_long_name) \
> >> > { OPTION_ALIAS, (s), (l), (source_long_name) }
> >> >
> >> > +#define OPT_SUBCOMMAND(l, v, fn) { OPTION_SUBCOMMAND, 0, (l), (v), NULL, \
> >> > + NULL, 0, NULL, 0, NULL, 0, (fn) }
> >> > +#define OPT_SUBCOMMAND_F(l, v, fn, f) { OPTION_SUBCOMMAND, 0, (l), (v), NULL, \
> >> > + NULL, (f), NULL, 0, NULL, 0, (fn) }
> >>
> >> Nit, I know you're carrying forward existing patterns, but since that
> >> all pre-dated designated init perhaps we could just (untested):
> >>
> >> #define OPT_SUBCOMMAND_F(l, v, fn, f) { \
> >> .type = OPTION_SUBCOMMAND, \
> >> .long_name = (l), \
> >> .value = (v), \
> >> .ll_callback = (fn), \
> >> }
> >> #define OPT_SUBCOMMAND(l, v, fn) OPT_SUBCOMMAND_F((l), (v), (fn), 0)
> >>
> >> Which IMO is much nicer. I have some patches somewhere to convert these
> >> to saner patterns (I think not designated init, but the X() can be
> >> defined in terms of X_F() like that, but since this is new we can use
> >> designated init all the way...
> >
> > Oh, I love this idea! But are we there yet? I remember the weather
> > balloon about designated initializers, but I'm not sure whether we've
> > already made the decision to allow them.
>
> Yes, we've got a thoroughly hard dependency on that part of C99 for a
> while now, and it's OK to add new ones (especially in cases like these,
> where it makes thigs easier to read).Good. I updated this hunk to use designated initializers as you suggested, because all those unused 0/NULL fields in there are just... ugly.
> > If we do, then I'm inclined > > to volunteer to clean up all those OPT_* macros in 'parse-options.h' > > with designated initializers,
But I'll leave this for later, because it's awfully easy to make a mistake and assign a macro parameter to the wrong field, and I find the resulting diff very hard to review.