Re: [PATCH] Colourise git-branch output

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

 



Linus Torvalds <torvalds@xxxxxxxx> writes:

> I wish you'd learn to use the proper syntax ;)

Ok, ok, I'm an old timer.  But [color.diff] syntax is also
accepted and I have it in my ~/.gitconfig:

	$ cat ~/.gitconfig
	[diff.color]
        	old = red reverse
	$ git repo-config --global diff.color.old
        red reverse
        $ exit

> ... what's nice about it is that you can also do
>
> 	[color "diff"]
> 		auto
> 		old = red
> 		new = green
>
> and the config file rules for booleans are such that a config variable 
> without the "= val" part parses the same as "= true", so you can now do
>
> 	git repo-config --bool color.diff.auto
>
> and it will say "true".

I find that your version of "auto" above to be utterly
confusing.

The existing configuration variable diff.color as an extended
boolean takes true, false or "auto".

I do agree this is a nice way to say that I pretend to be color
blind to git repo-config:

	$ git repo-config --global --bool color.diff false
        $ cat ~/.gitconfig
        [color]
        	diff = false

and it is also nice to be able to say my preference is "auto"
with (you can have --bool with this example and still use
"auto", by the way, when setting):

	$ git repo-config --global color.diff auto
        $ cat ~/.gitconfig
        [color]
        	diff = auto

I also agree it is nice that the "boolean magic" kicks in when
we hand-craft it:

	$ cat ~/.gitconfig
        [color]
        	diff
	$ git repo-config --global --bool color.diff
        true

But your handcrafted

	[color "diff"]
        	auto

simply feels a very confusing syntax.  How would you even
express my preference is always-color (that is, "true")?

If we are changing color.diff to color.diff.usecolor which is
bool + auto, then I would understand it.

	[color "diff"]
        	usecolor
                # usecolor = auto
                # usecolor = no

but that would not let you query with --bool ("auto" would
barf).

I guess we should at least extend --bool so that scripts can
easily query extended bools; if the value looks like a bool (or
there is no value), it would return it normalized to "true" or
"false" so they do not have to say:

	case `repo-config --bool the.variable` in
        true | yes | on | 1)
        	echo ah you mean yes ;;
	esac

but if the value is not either boolean true or false, then
return the string as is (such as "auto").  I am inclined to just
extend --bool itself to do so (which means we would lose the
error checking form the script for truly boolean fields) rather
than adding a new --bool-or-string option.

In any case,

	[color]
        	diff
                # diff = auto
                # diff = no

would work already if we only talk about built-in accesses.  We
would need to extend repo-config --bool to make it easier for
scripts to work with this extended bool, but that is a minor
detail.

So it seems to me that the only reason to have
color.diff.usecolor is so that we can have "diff" related color
settings in one place.  Which is fine.

You could do the same thing with your "auto"

	[color "diff"]
        	auto
		# auto = always
                # auto = no

but what that means is that color.diff.auto _is_ the extended
boolean that tells us to:

	* use color on terminal when set to true
        * use color unconditionally when set to a non-bool 'always'
        * never use color when set to false

which sounds rather, eh, unusual.


	

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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]