git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Sixt <j.sixt@viscovery.net>,
	git@vger.kernel.org, Brandon Casey <drafnel@gmail.com>
Subject: Re: [PATCH] attr: map builtin userdiff drivers to well-known extensions
Date: Fri, 16 Dec 2011 20:17:51 -0500	[thread overview]
Message-ID: <20111217011751.GA20225@sigill.intra.peff.net> (raw)
In-Reply-To: <7vehw4ia5x.fsf@alter.siamese.dyndns.org>

On Fri, Dec 16, 2011 at 11:33:30AM -0800, Junio C Hamano wrote:

> I think we recently saw that the optional built-in one for C did not even
> understand a function that returns a pointer, and nobody complained about
> it for a long time,

Yeah. That implies to me that either people don't really care about
this feature, or that they are not actually using it because it requires
special configuration (we are not even using it in git.git, for
example).

> > And if it is bad on balance, is the right solution to avoid exposing
> > people to it, or is it to make our patterns better?
> 
> Can't we do both, by avoid exposing normal users to broken one while
> people who want to improve the pattern based one work on unbreak it?

Sure, we can do both. But if nobody is eating the dogfood, it will never
grow to taste better. Maybe we should start by using diff=c in git
itself?

> > I.e., is it fixable,
> > or is it simply too hard a problem to get right in the general case, and
> > we shouldn't turn it on by default?
> 
> I do not think that is the "either-or" question. My impression has been
> that even if it is fixable, it is too broken and produces worse result
> than the simple default in its current form.

What I meant by the either-or was: if it is fixable, then we should fix
it and consider turning it on as a default. If it's too hard to get
right, then we probably never want it on by default, and people who do
like it can turn it on (presumably because it works on their code
style).

-Peff

  reply	other threads:[~2011-12-17  1:17 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-16 11:00 [PATCH] attr: map builtin userdiff drivers to well-known extensions Jeff King
2011-12-16 14:00 ` Johannes Sixt
2011-12-16 17:01   ` Junio C Hamano
2011-12-16 19:21   ` Jeff King
2011-12-16 19:30     ` Jeff King
2011-12-16 19:33     ` Junio C Hamano
2011-12-17  1:17       ` Jeff King [this message]
2011-12-16 22:05     ` Johannes Sixt
2011-12-17  1:21       ` Jeff King
2011-12-17  3:38         ` Jonathan Nieder
2011-12-19 15:49           ` [PATCHv2 1/2] " Jeff King
2011-12-19 18:07             ` Jonathan Nieder
2011-12-19 18:55               ` Jeff King
2011-12-22  1:47             ` Ævar Arnfjörð Bjarmason
2011-12-19 15:57           ` [PATCHv2 2/2] attr: drop C/C++ default extension mapping Jeff King
2011-12-19 18:10             ` Jonathan Nieder
2011-12-19 20:51               ` Thomas Rast
2011-12-19 20:52         ` [PATCH] t4018: introduce test cases for the internal hunk header patterns Brandon Casey
2011-12-19 21:53           ` [PATCH] t4018: add a few more test cases for cpp hunk header matching Brandon Casey
2011-12-19 22:37           ` [PATCH] t4018: introduce test cases for the internal hunk header patterns Junio C Hamano
2011-12-19 22:57             ` Brandon Casey
2011-12-19 23:17               ` Junio C Hamano
2011-12-20  2:42                 ` [PATCH v2] " Brandon Casey
2011-12-20  8:25                   ` Jakub Narebski
2011-12-20 15:58                     ` Brandon Casey
2011-12-20  9:13                   ` Thomas Rast
2011-12-20 19:52                   ` Johannes Sixt
2011-12-20 20:08             ` [PATCH] " Junio C Hamano
2011-12-16 17:51 ` [PATCH] attr: map builtin userdiff drivers to well-known extensions Mark Levedahl
2011-12-16 19:28   ` Jeff King
2011-12-16 19:26 ` Philip Oakley
2011-12-16 19:32   ` Jeff King
2011-12-22  0:05     ` Philip Oakley
2011-12-23  5:47       ` Jeff King
2011-12-16 19:38   ` Junio C Hamano

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=20111217011751.GA20225@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=drafnel@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j.sixt@viscovery.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).