Re: [PATCH/RFC] parse-options.c: make OPTION__COUNTUP consider negative values

From: Pranit Bauva <pranit.bauva@gmail.com>
Date: Mar 18, 2016, 11:23 UTC
Message-ID: <CAFZEwPOib-3JJQ+ufAbmNf1HYb5003LJw_paF2s9L8OK59N0PQ@mail.gmail.com>
In-Reply-To: <20160317015044.GB12830@sigill.intra.peff.net>

Sorry for a little late reply. I was under the impression that I had replied before.

On Thu, Mar 17, 2016 at 7:20 AM, Jeff King <peff@peff.net> wrote:

Show 8 quoted lines

> That aside, this patch does mean that one can no longer use
> OPT_COUNTUP() for negative values (i.e., the caller must start it at
> either 0 or 1, and it must always go up from there).
>
>
> And we would need to verify that all of the existing callers are OK with
> this. Did you check that that (not rhetorical; I suspect they are all
> OK, but somebody needs to check)?

I did a grep on parse-options.h and saw that only "verbose", "quiet" and "force" use OPT_COUNTUP(). I then did a git grep for "verbose = -1", "quiet = -1" But with "force = -1" showed that "builtin/clean.c" uses it. On a bit careful examination, this patch would not make difference to it.

> We are also changing semantics without changing the interface, which
> means any topics in flight (that you _cannot_ review, because you have
> not seen them yet) may be subtly broken. To me that is not an absolute
> deal-breaker, but something to weigh against the utility of the change.

As I am new here, I don't really know how to go about with this. Could you describe in a little detail so that I can work on it?

Show 7 quoted lines

> When looking more carefully at builtin/commit.c for the other thread, it
> occurred to me that OPT_BOOL might be a better fit for commit's "-v". It
> really is a boolean "show the diff or not" and thus unlike the other
> "make me more verbose". And OPT_BOOL already has the behavior you want,
> I think.
>
> -Peff