git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

From
Jeff King <peff@peff.net>
Date
Oct 12, 2017, 12:31 UTC
Message-ID
<20171012123153.i265nun6pklw7kjg@sigill.intra.peff.net>
In-Reply-To
<20171012021007.7441-2-gitster@pobox.com>
On Thu, Oct 12, 2017 at 11:10:06AM +0900, Junio C Hamano wrote:
Show 20 quoted lines
> From: Jeff King <peff@peff.net>
> 
> An earlier patch downgraded "always" that comes via the ui.color
> configuration variable to "auto", in order to work around an
> unfortunate regression to "git add -i".
> 
> That "fix" however regressed other third-party tools that depend on
> "git -c color.ui=always cmd" as the way to defeat any end-user
> configuration and force coloured output from git subcommand, even
> when the output does not go to a terminal.
> 
> It is a bit ugly to treat "-c color.ui=always" from the command line
> differently from a definition that comes from on-disk configuration
> files, but it is a moral equivalent of "--color=always" option given
> to the subcommand from the command line, i.e. a signal that tells us
> that the script writer knows what s/he is doing.  So let's take that
> route to unbreak this case while defeating a (now declared to be)
> misguided color.ui that is set to always in the configuration file.
> 
> NEEDS-SIGN-OFF-BY: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>

Thanks for picking this up. I meant to get to it yesterday but ran out of time. Your description looks good to me.

>  color.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

We should probably protect the command-line behavior with a test. Can you squash this in?

diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 25a9c65dc5..582cab5c8a 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -261,6 +261,17 @@ test_expect_success 'rev-list %C(auto,...) respects --color' '
 	test_cmp expect actual
 '
 
+test_expect_success "color.ui=always in config file same as auto" '
+	test_config color.ui always &&
+	git log --format=$COLOR -1 >actual &&
+	has_no_color actual
+'
+
+test_expect_success "color.ui=always on command-line is always" '
+	git -c color.ui=always log --format=$COLOR -1 >actual &&
+	has_color actual
+'
+
 iconv -f utf-8 -t $test_encoding > commit-msg <<EOF
 Test printing of complex bodies
 

Technically the first test is already covered by the "add -p" we added
elsewhere, but I think the sequence make sit easier to understand. Also
as an aside, I think this patch means that:

  git -c color.ui=always add -p

is broken (as would a hypothetical "git --default-color=always add -p").
That's sufficiently insane that I'm not sure we should care about it.

-Peff
Previous: Jeff KingNext: Junio C Hamano
Message 25 of 42 in “What happened to "git status --color=(always|auto|never)"?”
  1. Nazri RamliyOct 9, 2017
  2. Jonathan NiederOct 10, 2017
  3. Nazri RamliyOct 10, 2017
  4. Jonathan NiederOct 10, 2017
  5. Nazri RamliyOct 10, 2017
  6. Jeff KingOct 10, 2017
  7. Junio C HamanoOct 10, 2017
  8. Jeff KingOct 10, 2017
  9. Jonathan NiederOct 10, 2017
  10. Jeff KingOct 10, 2017
  11. Junio C HamanoOct 11, 2017
  12. 0/2 Piling more kludge on top of color.uiJunio C Hamano, Oct 12, 2017
  13. 2/2 color: discourage use of ui.color=alwaysJunio C Hamano, Oct 12, 2017
  14. Jonathan NiederOct 12, 2017
  15. Jeff KingOct 12, 2017
  16. Junio C HamanoOct 13, 2017
  17. 1/2 color: downgrade "always" to "auto" only for on-disk configurationJunio C Hamano, Oct 12, 2017
  18. Jonathan NiederOct 12, 2017
  19. Junio C HamanoOct 12, 2017
  20. Jonathan NiederOct 12, 2017
  21. Junio C HamanoOct 12, 2017
  22. Junio C HamanoOct 12, 2017
  23. Jeff KingOct 12, 2017
  24. Jeff KingOct 12, 2017
  25. Jeff KingOct 12, 2017
  26. Junio C HamanoOct 13, 2017
  27. Jeff KingOct 13, 2017
  28. Junio C HamanoOct 13, 2017
  29. Jeff KingOct 13, 2017
  30. 0/4 peeling back color.ui=always hacksJeff King, Oct 13, 2017
  31. 1/4 Revert "color: make "always" the same as "auto" in config"Jeff King, Oct 13, 2017
  32. 2/4 Revert "t6006: drop "always" color config tests"Jeff King, Oct 13, 2017
  33. 3/4 Revert "color: check color.ui in git_default_config()"Jeff King, Oct 13, 2017
  34. 4/4 tag: respect color.ui configJeff King, Oct 13, 2017
  35. Junio C HamanoOct 14, 2017
  36. Jeff KingOct 16, 2017
  37. Junio C HamanoOct 17, 2017
  38. Junio C HamanoOct 17, 2017
  39. Jeff KingOct 18, 2017
  40. Junio C HamanoOct 18, 2017
  41. Jonathan NiederOct 17, 2017
  42. Jeff KingOct 18, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.