Re: [PATCH] git-apply: add --quiet flag

From: Jerry Zhang <jerry@skydio.com>
Date: Apr 28, 2021, 18:18 UTC
Message-ID: <CAMKO5CutTaW51GkkM_+viES3UbrOKA6VN1nOWNK+poRkxiO-sg@mail.gmail.com>
In-Reply-To: <xmqqsg3aahw7.fsf@gitster.g>

On Wed, Apr 28, 2021 at 2:16 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 25 quoted lines

>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Jerry Zhang <jerry@skydio.com> writes:
> >
> >> Replace OPT_VERBOSE with OPT_VERBOSITY.
> >
> > While it is not an incorrect statement, it is odd to have such an
> > implementation detail nobody cares as the first thing in the log
> > message, though.
> >
> >> This adds a --quiet flag to "git apply" so
> >> the user can turn down the verbosity.
> >
> > Sure, I think you can do "apply --no-verbose" to do the same thing
> > without any change, but we introduced VERBOSITY to replace VERBOSE
> > exactly so that --verbose can be countermanded with --quiet, and
> > this patch is a good example of the application of that feature.
> >
> > I wonder if this deserves a test.
>
> Oh, another thing.  "--quiet" with OPT_VERBOSITY is given negative
> values, whose magnitude may be used to express "even more quiet".
> This is different from "--no-verbose" that is supported by both
> OPT_VERBOSITY and OPT_VERBOSE that resets the variable to 0.

Ok I didn't realize every flag automatically came with a "no" version but --quiet is indeed what I'm looking for since I want to silence the status messages but still print out really critical errors.

Show 7 quoted lines

>
> So use of OPT_VERBOSITY() to support both --verbose and --quiet is
> good, but you'd need to audit the way the verbosity variable is used
> by the code.  "if (verbose) perform_verbosely()" would have to be
> rewritten as "if (verbose > verbosity_level) perform_verbosely()"
> or something like that, as the "verbose" variable can take a
> negative value to mean "less silent than the usual 0".

Yeah luckily apply.c uses verbosity correctly and consistently throughout.

> Also, does "git am" have an "--quiet" option (or "--verbose" for
> that matter), and if so, should it pass it down to underlying "git
> apply" (this is not a rhetorical suggestion --- it is a genuine
> question---I am not particularly interested in changing "am")?

am seems to have --quiet but not --verbose. In addition am does not seem to pass through those options into apply.c. I tested this using a patch that has whitespace errors and "git am" prints warning regardless of whether "--quiet" is used. However, interestingly "git am --3way" does not print warnings, due to these lines:

       /*
        * If we are allowed to fall back on 3-way merge, don't give false
        * errors during the initial attempt.
        */
       if (state->threeway && !index_file)
               apply_state.apply_verbosity = verbosity_silent;

Which no longer are relevant due to the previous changes making 3way happen first. So in conclusion I think it might be worthwhile to make the verbosity flags of "am" pass down to "apply", although it would be out of the scope of this change.