Re: [PATCH v12 2/5] test-parse-options: print quiet as integer

From: Pranit Bauva <pranit.bauva@gmail.com>
Date: Apr 5, 2016, 15:39 UTC
Message-ID: <CAFZEwPORAer3=WmUqqhztv4kAOHdv+yB12tRyUYh_YTSmtcQsQ@mail.gmail.com>
In-Reply-To: <CAPig+cQpdr7A7GxKUN3tDs2rQ7z_S5b1WVsk01=BwkCwB2mp0A@mail.gmail.com>

[+cc:Duy Nguyen, Jonathan Nieder]

On Mon, Apr 4, 2016 at 3:00 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 8 quoted lines

> On Sat, Apr 2, 2016 at 7:33 PM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>> Current implementation of parse-options.c treats OPT__QUIET() as integer
>> and not boolean and thus it is more appropriate to print it as integer
>> to avoid confusion.
>
> I can buy this line of reasoning, however, it would be even easier to
> sell the change if you cited an existing client (a git command) which
> actually respects multiple quiet levels. Are there any?

I investigated into this. But I was unable to find any git commit which actually respects mulitple quiet levels. I first did a 'git grep OPT__QUIET' to find the commands which use this. Then I went through the documentation which covers it. None of them have any such mention of multiple quiet levels. But still I dug further and and went through all the individual source files. I followed the corresponding C source code for the header file included and also searched there for any trace of quiet. But I still didn't find any such use of multiple quiet levels. I have found that the commit id 212c0a6f (Duy Ngyuyen; 7 Dec, 2013; parse-options: remove OPT__BOOLEAN). Maybe he has something to say as to why this was introduced and OPT__QUIET which previously used OPT__BOOLEAN, now uses OPT_COUNTUP rather than OPT_BOOL. This commit In fact git-repack command has quiet but this is not mentioned in the documentation. If someone could include this it in the documentation. I would do it but I am not quite familiar with git-repack and haven't really used it anytime.

Show 6 quoted lines

> More importantly, though, this change implies that you should also add
> tests to ensure that the quiet level is indeed incremented with each
> --quiet, just as "-vv" and "--verbose --verbose" are already tested.
> You might be able to include such new tests directly in this patch as
> long as the commit message is clear about it, or add them in a
> separate patch.

I will include the tests for multiple level of quiet in the patch.

> By the way, I don't see any tests to ensure that --no-verbose and
> --no-quiet reset those respective values to 0. A separate patch which
> adds such tests would be nice (unless such tests already exist and I
> merely missed them).

There are no tests to ensure that --no-verbose and --no-quiet reset those respective values to 0 just before this patch. But I have covered the --no-verbose tests in the upcoming patch 3/5. I will include the tests for --no-quiet in this patch.