Jeff King wrote: > On Mon, May 17, 2021 at 11:27:23PM -0500, Felipe Contreras wrote: > > > > So in short, the color.pager is about "is the pager capable of > > > colors?" > > > > That's not the case. > > > > Even the documentation says so: > > > > color.pager:: > > A boolean to enable/disable colored output when the pager is in > > use (default is true). > > I think that documentation misses the reason you'd want to use it. > Likewise, the commit message introducing it (aa086eb813d) sucks, Well, it was 2006. Many of the best practices of today were not followed back then. > but the > motivation (from [0]) was: > > When I use a pager that escapes the escape character or highlights the > content itself the output of git diff without the pager should have > colors but not with the pager. For example using git diff with a > pathspec is quite short most of the time. For git diff I have to > enable paging manually and run git diff | $PAGER usually but git log > uses the pager automatically and should not use colors with it. This is aligned with what I said: the uswer wants to disable colors when using a pager. Yes, in this instance it's because the pager doesn't support colors, but that's not always necessarily so. A person with sight problems may use less (perfectly capable of colors), but yet not want to exercise that capability. It's still a preference. > For a more concrete example, my pager _does_ understand colors, and I > would not want to set pager.color to "false" (because then "git log", > etc, would not show me any colors). But I don't like the man colors you > are suggesting. You can change them in your environment. > I want to be able to turn them off by setting "color.man" or similar > to false, not by disabling color for everything that is paged. Sure, for that particular case it does make sense. I'll add that. > So color.pager being true is _necessary_ for showing colors in paged > outputs, but by itself is not sufficient. We have other per-context > color options (color.diff, color.branch, and so on). > > And so likewise, we would want to avoid turning on colors if the user > has set color.pager=false. Usually this is done automatically because > want_color() checks, which knows if we are using the pager or not. But > if we are going to call out to "man" which will invoke another pager, > that caller would have to check pager_use_color themselves (it's yet > another question of whether "the pager can handle color" applies equally > to the pager that Git will run versus the one that man will run). Yes, but we still need to check pager_use_color. Except... Maybe a user has GIT_PAGER set to a colorless pager, and MANPAGER to something fancier, in which case color.pager should be ignored. But that's probably a corner-case nobody is ever going to hit. Anyway, I've sent an update version with color.man. -- Felipe Contreras
