Re: [PATCHv2 1/5] revision.h: turn rev_info.early_output back into an unsigned int

From: Jeff King <peff@peff.net>
Date: Jun 10, 2017, 06:41 UTC
Message-ID: <20170610064102.w4kp6omxdznfe7fa@sigill.intra.peff.net>
In-Reply-To: <20170609181733.6793-2-szeder.dev@gmail.com>

On Fri, Jun 09, 2017 at 08:17:29PM +0200, SZEDER Gábor wrote:

Show 9 quoted lines

> rev_info.early_output started out as an unsigned int in cdcefbc97 (Add
> "--early-output" log flag for interactive GUI use, 2007-11-03), but
> later it was turned into a single bit in a bit field in cc243c3ce
> (show: --ignore-missing, 2011-05-18) without explanation, though its
> users still expect it to be a regular integer type.  Consequently, any
> even number given via '--early-output=<N>' effectively disabled the
> feature.
> 
> Turn it back into an unsigned int, restoring its original data type.

This confused me for a moment, as on my first read it seems like the obvious solution is to normalize the input to a bit-field, like:

  revs->early_output = !!atoi(optarg);

But the "users still expect" bit was a bit subtle to me, as I thought you meant users of Git. But you mean that the feature itself is not a boolean, but rather an integer count of how much early output to show.

I'm not sure if I was just being thick or if that point (and the fact that --early-output has basically been a noop since 2011!) should be made more explicit.

Given that nobody noticed, I kind of wonder if we should consider ripping the feature out entirely.

-Peff