Re: [PATCH] fix color.pager = false with "git diff"

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 21, 2010, 19:02 UTC
Message-ID: <20101021190258.GA11759@burratino>
In-Reply-To: <20101021150244.GA18426@sigill.intra.peff.net>

[reordered for convenience] Jeff King wrote:

Show 15 quoted lines

> In favor of this patch:
>
>   1. It fixes a real bug.
>
>   2. Perfect is the enemy of the good, and I don't care enough about
>      this case to refactor the color code.
>
> Against:
>
>   1. It does nothing to fix other times when this ordering problem may
>      arise (I don't think there are others, but I didn't check very
>      thoroughly. And of course new ones may appear).
>
>   2. The bug was reported over 2 years ago, and hasn't come up since,
>      despite remaining unfixed.  Maybe nobody actually cares.

I'd say, go for it. At least one person (Benjamin Kudria, cc-ed) cared at some point. And including the code means that maybe some future hero refactoring "git diff" will notice and implement the proper solution.

Though to that end, wouldn't it make sense to include

>                                                         A more elegant
> solution would be totally rewriting the color code to check for the
> pager at first output.

in the commit message?