Re: [PATCH/RFC] Pro-Git thanks, Control-flow bug report
- From
Jeff King <peff@peff.net>
- Date
- Jul 25, 2011, 16:25 UTC
- Message-ID
- <20110725162548.GA7071@sigill.intra.peff.net>
- In-Reply-To
- <20110725125037.GA24198@sherwood.local>
On Mon, Jul 25, 2011 at 02:50:38PM +0200, Steffen Daode Nurpmeso wrote:
Show 15 quoted lines
> So while exploring git(1) i recently tried out colours (it's oh > so coloured for a two, since 2011 three colors vim(1) user - > fascinating) and found a control flow bug: > > ?0%0[steffen@sherwood git.git]$ ./git --version > git version 1.7.6.233.gd79bc.dirty > ?0%0[steffen@sherwood git.git]$ ./git -c color.ui=auto -c color.pager=false diff 2> AU; cat AU > git_config_colorbool(color.ui,auto,-1) > [pager_in_use(): spawned:0, GIT_PAGER_IN_USE:0] > auto_color:1 > color acc. 2 getenv(TERM) > git_default_config(color.pager,false): 0 > > So the pager is spawned after the color config setting has been > queried (and the latter is never updated).
Hmm. What's old is new again, I guess. I posted a patch to fix this almost exactly 3 years ago:
http://article.gmane.org/gmane.comp.version-control.git/90427
The patch is kind of an ugly special-case, and nobody else brought it up in the past 3 years, so it just got dropped. Maybe it's worth taking it now.
Show 11 quoted lines
> I'm not aware of the codebase, and so i can't offer a patch,
> unfortunately. I tried the following change in color.c first,
> but that's not a solution for the real problem:
>
> jauto_color:
> if (pager_in_use())
> stdout_is_tty = pager_use_color;
> else if (stdout_is_tty < 0)
> stdout_is_tty = isatty(1);
> if (stdout_is_tty) {
> ...You can't fix it strictly through color.c. The problem is that diff asks the color code about using colors, _then_ starts a pager. So the color code doesn't have enough information at the time it is asked to make the right decision.
A more elegant solution would be to push the query to color.c to happen at the time of color use, instead of during the startup sequence.
-Peff