Re: [PATCH 16/20] diff-parseopt: convert --quiet

From: Duy Nguyen <pclouds@gmail.com>
Date: Mar 6, 2019, 09:25 UTC
Message-ID: <CACsJy8CtewTfFUwJwRK4BzF+kBq1OvmDwgsWEzf8Y8o=_0_nZw@mail.gmail.com>
In-Reply-To: <CAPig+cR-efk5esa9=5j+Fu0eWxAkFnohkSL4eabrP44K_AkdCg@mail.gmail.com>

On Tue, Mar 5, 2019 at 10:42 PM Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 8 quoted lines

>
> On Tue, Mar 5, 2019 at 7:32 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> > diff --git a/diff.c b/diff.c
> > @@ -5299,6 +5299,8 @@ static void prep_parse_options(struct diff_options *options)
> > +               OPT_BOOL(0, "quiet", &options->flags.quick,
> > +                        N_("disable all output of the program")),
>
> As a reviewer, I was wondering why you didn't use OPT__QUIET() here, but...

It probably just didn't occur to me. After doing a couple conversions, you kinda get in a routine and forget to question if your choice is the right one.

Show 7 quoted lines

> > @@ -5348,9 +5350,7 @@ int diff_opt_parse(struct diff_options *options,
> > -       } else if (!strcmp(arg, "--quiet"))
> > -               options->flags.quick = 1;
> > -       else if (!strcmp(arg, "--ext-diff"))
>
> I guess the reason is that flags.quick isn't necessarily about
> verbosity/quietness.

Also OPT__QUIET() adds the short option -q. Adding that, even if helpful, should be done separately.

-- 
Duy