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

Re: [PATCH] log --decorate: do not leak "commit" color into the next item

From
Jeff King <peff@peff.net>
Date
Feb 20, 2015, 01:42 UTC
Message-ID
<20150220014230.GA16124@peff.net>
In-Reply-To
<xmqqvbixzsnv.fsf@gitster.dls.corp.google.com>
On Thu, Feb 19, 2015 at 10:02:12AM -0800, Junio C Hamano wrote:
Show 13 quoted lines
> Jeff King <peff@peff.net> writes:
> 
> > Yeah, I think this is a good fix. I had a vague feeling that we may have
> > done this on purpose to let the decoration color "inherit" from the
> > existing colors for backwards compatibility, but I don't think that
> > could ever have worked (since color.decorate.* never defaulted to
> > "normal").
> 
> Hmph, but that $gmane/191118 talks about giving bold to commit-color
> and then expecting for decors to inherit the boldness, a wish I can
> understand.  But I do not necessarily agree with it---it relies on
> that after "<commit-color>(" and "<commit-color>, " there is no reset,
> which is not how everything else works.

I don't see anybody actually _wanting_ the inheritance. It is mentioned merely as an observation. So yeah, we would break anybody who does:

  [color "diff"]
  commit = blue
  [color "decorate"]
  branch = normal
  remoteBranch = normal
  tag = normal
  stash = normal
  HEAD = normal
and expects the "blue" to persist automatically.

But given that this behaves in the opposite way of every other part of git's color handling, I think we can call it a bug, and people doing that are crazy (they should s/normal/blue/ in the latter config).

> So this change at least needs to come with an explanation to people
> who are used to and took advantage of this color attribute leakage,
> definitely in the log message and preferrably to the documentation
> that covers all the color.*.<slot> settings, I think.

I'd agree it is worth a mention in the log (and possibly release notes), but I don't think it is worth polluting the documentation forever (though explaining that we never inherit might be worth doing, and that is perhaps what you meant).

-Peff
Previous: Junio C HamanoNext: Junio C Hamano
Message 5 of 16 in “[Q] should "color.*.<slot> = normal" emit nothing?”
  1. Junio C HamanoFeb 18, 2015
  2. log --decorate: do not leak "commit" color into the next itemJunio C Hamano, Feb 18, 2015
  3. Jeff KingFeb 18, 2015
  4. Junio C HamanoFeb 19, 2015
  5. Jeff KingFeb 20, 2015
  6. Junio C HamanoFeb 20, 2015
  7. Jeff KingFeb 21, 2015
  8. Junio C HamanoFeb 21, 2015
  9. 0/7 Fix leak of color/attributes in "git log --decorate"Junio C Hamano, Mar 4, 2015
  10. 1/7 Documentation/config.txt: avoid unnecessary negationJunio C Hamano, Mar 4, 2015
  11. 2/7 Documentation/config.txt: explain multi-valued variables onceJunio C Hamano, Mar 4, 2015
  12. 3/7 Documentation/config.txt: describe the structure first and then meaningJunio C Hamano, Mar 4, 2015
  13. 4/7 Documentation/config.txt: have a separate "Values" sectionJunio C Hamano, Mar 4, 2015
  14. 5/7 Documentation/config.txt: describe 'color' value type in the "Values" sectionJunio C Hamano, Mar 4, 2015
  15. 6/7 Documentation/config.txt: simplify boolean description in the syntax sectionJunio C Hamano, Mar 4, 2015
  16. 7/7 log --decorate: do not leak "commit" color into the next itemJunio C Hamano, Mar 4, 2015

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.