Jeff King wrote: > The current documentation for color.pager is technically correct, but > slightly misleading and doesn't really clarify the purpose of the > variable. As explained in the original thread which added it: > > https://lore.kernel.org/git/E1G6zPH-00062L-Je@xxxxxxxxxxxx/ > > the point is deal with pagers that don't understand colors. Missing "to" before "deal". More importantly, I think I'd find a reference to the commit or a quotation from the affected user more helpful than a reference to the mailing list archive, since that would make this a bit more self-contained. (Especially since I think this is un-subtle enough that chasing through the mailing list thread doesn't add much to my life. :)) [...] > --- a/Documentation/config/color.txt > +++ b/Documentation/config/color.txt > @@ -127,8 +127,9 @@ color.interactive.<slot>:: > interactive commands. > > color.pager:: > - A boolean to enable/disable colored output when the pager is in > - use (default is true). > + A boolean to specify whether `auto` color modes should colorize > + output going to the pager. Defaults to true; set this to false > + if your pager does not understand ANSI color codes. I quite like the "set this to false if your pager does not understand ANSI color codes" part --- short and to the point. The sentence before takes me long enough to understand that I don't think we've gotten the wording right yet. Before I suggest some wording, let's make sure I understand the behavior correctly: - unlike other color.* settings, this can only be "true" or "false". It cannot be "auto". - in other color.* settings, "auto" means "colors are used only when stderr goes to a terminal". A pager typically ultimately writes to a terminal, but (1) it's not guaranteed to (e.g., xless writes to its own window instead) and (2) more importantly for us, it's not guaranteed to write terminal escapes as is. - so this setting can be used to answer "for the sake of evaluating color settings, should we treat output that is going to a pager as going to a terminal?" If I understood correctly, how about some text like the following? A boolean to specify whether `auto` color modes should colorize output going to a pager, in addition to their behavior of colorizing output going to a terminal. Defaults to true; [etc] Side note, not about this patch: we treat pager.color as a synonym for color.pager. Is that something we want to document, or is that an instance of being extra friendly when the user makes a typo? Thanks, Jonathan
