Re: [PATCH 2/2] color: discourage use of ui.color=always

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> On Thu, Oct 12, 2017 at 11:10:07AM +0900, Junio C Hamano wrote:
>
>> Warn when we read such a configuration from a file, and nudge the
>> users to spell them 'auto' instead.
>
> Hmm. On the one hand, it is nice to make people aware that their config
> isn't doing what they might think.
>
> On the other hand, if "always" is no longer a problem for anybody, do we
> need to force users to take the step to eradicate it? I dunno. Were we
> planning to eventually remove it?
>
>> @@ -320,6 +322,11 @@ int git_config_colorbool(const char *var, const char *value)
>>  			 * Otherwise, we're looking at on-disk config;
>>  			 * downgrade to auto.
>>  			 */
>> +			if (!warn_once) {
>> +				warn_once++;
>> +				warning("setting '%s' to '%s' is no longer valid; "
>> +					"set it to 'auto' instead", var, value);
>> +			}
>
> This warn_once is sadly not enough to give non-annoying output to
> scripts that call many git commands. E.g.:
>
>   $ git config color.ui always
>   $ git add -p
>   warning: setting 'color.ui' to 'always' is no longer valid; set it to 'auto' instead
>   warning: setting 'color.ui' to 'always' is no longer valid; set it to 'auto' instead
>   warning: setting 'color.ui' to 'always' is no longer valid; set it to 'auto' instead
>   warning: setting 'color.ui' to 'always' is no longer valid; set it to 'auto' instead
>   warning: setting 'color.ui' to 'always' is no longer valid; set it to 'auto' instead
>   warning: setting 'color.ui' to 'always' is no longer valid; set it to 'auto' instead
>   warning: setting 'color.ui' to 'always' is no longer valid; set it to 'auto' instead
>   warning: setting 'color.ui' to 'always' is no longer valid; set it to 'auto' instead
>   warning: setting 'color.ui' to 'always' is no longer valid; set it to 'auto' instead
>   warning: setting 'color.ui' to 'always' is no longer valid; set it to 'auto' instead
>   diff --git a/file b/file
>   [...]

I am ambivalent.  

We (especially you) have kept saying that "always" is a mistake that
makes little sense, and wanted to force users to "fix" their
configuration.  But as you said, we made it not a mistake, so it is
OK to leave it as they are, I guess.

Let's drop the warning part of the change, at least.






[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux