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

Jeff King [off-list ref] writes:

On Thu, Oct 12, 2017 at 11:10:07AM +0900, Junio C Hamano wrote:

quoted

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?

quoted

@@ -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.