Re: [PATCH] builtin/config.c: don't print a newline with --color

[Jeff King <peff@peff.net>]

On Sun, Mar 03, 2019 at 10:18:11AM +0900, Junio C Hamano wrote:

> An interesting aspect of the above is that this is *NOT* limited to
> colors.  Regardless of the type you are reading, be it an int or a
> bool, VAR=$(git config ...) will strip the trailing LF, and existing
> scripts people have do rely on that, i.e. when people write
> 
> 	VAR=$(git config ...)
> 	echo "var setting is $VAR"
> 
> they rely on VAR=$(...) assignment to strip trailing LF and echo to
> add a final LF to the string.
> 
> So if we are going to change anything, the change MUST NOT single
> out "color".  IOW, the title of the patch already tells us that it
> is giving a wrong solution.

I'm not sure I agree. Colors have always been special, and
"--type=color" was advertised as a replacement for "--get-color". And
"--get-color" never output the newline.

Philosophically speaking, I'd make the argument that the "color" type is
a bit special because unlike other output, it is unreadable binary gunk.
But I dunno. It does suck for the output to be dependent on "--type",
because it means that anything consuming it has to understand the
various types. It also creates potential complications if we ever
implement a "--stdin" mode to grab the (type-interpreted) values of
several keys, where presumably we'd have to have newlines as part of the
protocol.

> Needless to say, "using chop in Perl is wrong to begin with" misses
> the point from two directions---(1) 'chop in Perl' is a mere
> example---scripts not written in Perl using chop may still rely on
> the existing behaviour that the output always has the final LF, and
> (2) even if we agree that using chop in Perl is a bad idea, such a
> script has happily been working, and suddenly breaking it is a
> regression no matter what.

With respect to backwards compatibility, my thinking on the matter was
basically:

  1. Since --type=color was supposed to be a drop-in replacement for
     --get-color, it's a bug that they don't behave the same.

  2. It's a fairly recent feature, so nobody really noticed the bug
     until recently (and it was in fact me who noticed and got annoyed
     by it). If it were an ancient behavior, we might think about
     retaining even bug compatibility, but that's not the case here.

But I do admit your argument about consistency is giving me second
thoughts.

-Peff