Git development
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, win@wincent.com
Subject: Re: [BUG] git-diff-* --color oddness
Date: Fri, 04 Jan 2008 00:46:18 -0800	[thread overview]
Message-ID: <7vsl1ekmg5.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <20080104083252.GB3300@coredump.intra.peff.net> (Jeff King's message of "Fri, 4 Jan 2008 03:32:52 -0500")

Jeff King <peff@peff.net> writes:

> On Fri, Jan 04, 2008 at 12:26:55AM -0800, Junio C Hamano wrote:
>
>> > The first two lines of meta-info will be in the stock colors, but
>> > everything after will be in the custom colors. So we are actually
>> > reading the diff_ui options _during_ the diff. The culprit is
>> > funcname_pattern, which calls read_config_if_needed.
>> 
>> Yuck.  Why is funcname_pattern do ui-config stuff?  I know it
>> wants to get custom regexp crap, but that should belong to the
>> plumbing part, not Porcelain-only thing, shouldn't it?
>
> It's for user diff drivers, and it looks like the funcname pattern. Not
> sure why we want to demand-load that stuff at all...I wonder if it
> should just go into git_default_config (or a git_basic_diff_config).

Yeah, moving some to the diff-basic-config sounds sane.

 * I'd prefer to keep low-level produce consistent diff that can
   be reliably applied with git-apply without getting broken by
   user configuration while Porcelain level diff can be tweaked
   by the user to do whatever "human readable" crap they would
   want to see on the screen (such as "side by side"), and my
   gut feeling is that we should keep the user-level driver
   definition in ui-config, never to affect plumbing.

 * using or not-using colors should stay in ui-config;

 * funcname-pattern can go either way; that affects what appears
   at the end of @@ context @@ lines, and would not have risk to
   corrupt the patch for plumbing.

 * color choice can also go either way, but probably is better
   to be in the low-level.  Cnce color is used the output cannot
   be fed sanely to patch or git-apply _anyway_ so we can be
   sure that "git diff-plumbing --color" is run to produce human
   readable output.

  reply	other threads:[~2008-01-04  8:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-04  8:14 [BUG] git-diff-* --color oddness Jeff King
2008-01-04  8:24 ` Junio C Hamano
2008-01-04  8:28   ` Jeff King
2008-01-04  8:35     ` Junio C Hamano
2008-01-04  8:43       ` Jeff King
2008-01-04  8:26 ` Junio C Hamano
2008-01-04  8:32   ` Jeff King
2008-01-04  8:46     ` Junio C Hamano [this message]
2008-01-04  8:59       ` Jeff King
2008-01-04  9:25         ` Jeff King
2008-01-04  9:37           ` Junio C Hamano
2008-01-04  9:45             ` Jeff King

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=7vsl1ekmg5.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=win@wincent.com \
    /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