Web lists-archives.com

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




Jeff King <peff@xxxxxxxx> writes:

> I do wonder, though, if we're digging ourselves a hole with the
> inconsistency between different --types that will bite us later. Given
> that it's not that hard to chomp the output (and as you noted, the shell
> does it fairly transparently), and given that the caller has to switch
> between "--get-color" and "--type=color", it's not that hard to handle
> the output differently if you know to do so.
>
> Mostly I was just surprised by the new behavior. Perhaps the right
> solution is not a patch to the code, but to the documentation. Something
> like:
>
> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> index 495bb57416..61f3a9cdd7 100644
> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt
> @@ -252,7 +252,9 @@ Valid `<type>`'s include:
>  	output.  The optional `default` parameter is used instead, if
>  	there is no color configured for `name`.
>  +
> -`--type=color [--default=<default>]` is preferred over `--get-color`.
> +`--type=color [--default=<default>]` is preferred over `--get-color`
> +(but note that `--get-color` will omit the trailing newline printed by
> +--type=color).
>  
>  -e::
>  --edit::

Yup, that would be a very sensible first step, regardless of what
the next step is.

After that, choices are

 (1) we'd introduce new inconsistency among --type=<type> by
     matching what --type=color does to what --get-color does, to
     allow us to revert that documentation update, or

 (2) we'd drop LF from all --type=<type>, that makes everything
     consistent and risk breaking a few existing scripts while doing
     so, and get yelled at by end users, or

 (3) we stop at this documentation update and do nothing else.