git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthias Kestenholz <mk@spinlock.ch>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>,
	git@vger.kernel.org, Jakub Narebski <jnareb@gmail.com>
Subject: Re: [PATCH/resend] add--interactive: allow diff colors without interactive colors
Date: Sat, 05 Jan 2008 15:10:26 +0100	[thread overview]
Message-ID: <1199542226.6209.34.camel@futex> (raw)
In-Reply-To: <7vd4sga5n6.fsf@gitster.siamese.dyndns.org>

On Sat, 2008-01-05 at 03:11 -0800, Junio C Hamano wrote:
> Matthias Kestenholz <mk@spinlock.ch> writes:
> 
> > I managed to throw something together which works and passes all
> > the tests. Documentation included. :-)
> 
> Is it because we do not usually test colours and the tests run
> without terminals to make sure "color.* = auto" does not kick
> in?
> 

Probably, yes. I've to confess that I have not thought too much about
how color output could be tested. 

> > I would be happy for feedback and suggestions.
> 
>  * Shouldn't "color.git = true" with "color.diff = false" mean
>    "I want colour for everything by default but I do not want to
>    see coloured diff"?
> 

This works with the new patch below.

>  * git_foo_config() callback from git_config() returns 0 on
>    success; the API change needs to be documented to warn
>    others.
> 

I've changed the config reader. It behaves like the others now.

> I haven't studied your patch very deeply so I may have misread
> what you tried to do, regarding the first point, though.

Still missing are the updates for git add -i and git svn.


I have added a new function git_color_config() which should be called
after git_config() has finished its work. The function needs to run when
all config reading has been done, because otherwise the color setup
would be sensitive to the order in which the variables are placed in the
config file, which I'd rather avoid.

I've also unified the colorbool variables, because otherwise the
individual colorbool variables would need to be exported into other C
files, and I don't see the point of multiple colorbools anyway.

The patch got somewhat big, therefore I've splitted it up into several
pieces:

[PATCH 1/4] Add infrastructure for a single color config variable
[PATCH 2/4] git branch: Use color configuration infrastructure
[PATCH 3/4] status and commit: Use color configuration infrastructure
[PATCH 4/4] diff and log: Use color configuration infrastructure

  reply	other threads:[~2008-01-05 14:11 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-04  8:35 [PATCH] add--interactive: allow diff colors without interactive colors Jeff King
2008-01-05  0:20 ` Junio C Hamano
2008-01-05  3:37   ` Jeff King
2008-01-05  8:01     ` Junio C Hamano
2008-01-05  8:51       ` Jeff King
2008-01-05  9:28         ` Junio C Hamano
2008-01-05  9:57           ` Jeff King
2008-01-05 10:58   ` [PATCH/resend] " Matthias Kestenholz
2008-01-05 11:11     ` Junio C Hamano
2008-01-05 14:10       ` Matthias Kestenholz [this message]
2008-01-05 14:11         ` [PATCH 1/4] Add infrastructure for a single color config variable Matthias Kestenholz
2008-01-05 14:11           ` [PATCH 2/4] git branch: Use color configuration infrastructure Matthias Kestenholz
2008-01-05 14:11             ` [PATCH 3/4] status and commit: " Matthias Kestenholz
2008-01-05 14:11               ` [PATCH 4/4] diff and log: " Matthias Kestenholz
2008-01-08 11:23             ` [PATCH 2/4] git branch: " Jeff King
2008-01-08 12:52               ` Matthias Kestenholz
2008-01-05 11:37     ` [PATCH/resend] add--interactive: allow diff colors without interactive colors Jakub Narebski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1199542226.6209.34.camel@futex \
    --to=mk@spinlock.ch \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jnareb@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).