On Fri, Dec 29, 2017 at 06:16:31PM -0500, Todd Zullinger wrote: > Ævar Arnfjörð Bjarmason wrote: > > No idea how to test this, in particular trying to pipe the output of > > color.ui=never v.s. color.ui=auto to a file as "auto" will disable > > coloring when it detects a pipe, but this fixes the issue. > > You might be able to use similar methods as those Jeff used > in the series merged from jk/ui-color-always-to-auto: > > https://github.com/gitster/git/tree/jk/ui-color-always-to-auto Yeah, test_terminal is the solution to testing. But... > He may also have some ideas about this issue in general. > (Or they could be tramatic memories, depending on how > painful it was to dig into the color code.) Yep. If we make diff-tree support color.ui, it's going to break a bunch of other stuff (like add--interactive) for people who set color.ui=always. I know this empirically, because we did that in v2.13, and a bunch of people complained. ;) The root of the problem is that the plumbing diff-tree defaults its internal color variable to "auto" in the first place. In theory the best way forward is fixing that, but it's likely to have a bunch of fallouts itself (scripts which use plumbing and where the user _does_ want color will stop showing it). This bug has been around since v1.8.4, I think, so it's hard to say how many people are depending on it at this point. A hackier option which would probably make most people happy would be to have plumbing respect "color.ui=never", but not any other values. I think the history of the back and forth is: - 4c7f1819b3 (make color.ui default to 'auto', 2013-06-10) introduced the problem of plumbing defaulting to "auto". This was in v1.8.4. - we did something similar to Ævar's patch in 136c8c8b8f (color: check color.ui in git_default_config(), 2017-07-13). That shipped in v2.14.2, and people with color.ui=always complained, because things like add--interactive broke for them. - we tried fixing it with 6be4595edb (color: make "always" the same as "auto" in config, 2017-10-03), but that broke people doing "git -c color.ui=always" as an equivalent of "--color". We talked about making the "-c" config behave differently from on-disk config, but got pretty disgusted at the weird hacks. And so... - we ended up with 33c643bb08 (Revert "color: check color.ui in git_default_config()", 2017-10-13), which just reverts the whole mess back to the pre-v2.14 state. This shipped in v2.15. So I don't think we want to go down that road again. If anything, we want to either fix the original sin from 4c7f1819b3, or we want to do the "respect only never" hack. -Peff
