{"thread":{"id":"28219","subject":"git diff annoyance / feature request","startedAt":"2011-08-25T19:14:24Z","lastAt":"2011-08-27T05:14:05Z","messageCount":23,"participants":["Boaz Harrosh","Jeff King","Junio C Hamano","Eric Sunshine","Brandon Casey","Miles Bader","Thomas Rast","René Scharfe","Alexey Shumkin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"174252","messageId":"4E569F10.8060808@panasas.com","threadId":"28219","inReplyTo":null,"subject":"git diff annoyance / feature request","fromName":"Boaz Harrosh","fromEmail":"bharrosh@panasas.com","sentAt":"2011-08-25T19:14:24Z","receivedAt":"2011-08-25T19:14:24Z","isPatch":false,"sender":{"key":"bharrosh@panasas.com","avatar":"https://gravatar.com/avatar/347e426f8ca0409f6891ceeefd4c3c2d8769608382323057efeaa362b4c3dbf3?d=mp&s=160"},"body":"\ngit diff has this very annoying miss-fixture where it will state\nas hunk header the closest label instead of the function name.\n\nSo I get:\n@@ -675,9 +670,23 @@ try_again:\n \t}\n \n \tif (flag) {\n-\t\tfoo();\n+\t\tbazz();\n \t}\n \n \nInstead of what I'd like:\n@@ -563,12 +563,7 @@ static int write_exec(struct page_collect *pcol)\n \t}\n \n \tif (flag) {\n-\t\tfoo();\n+\t\tbazz();\n \t}\n \n\nI mean. The label \"try_again\" is not at all unique in my file. As a\nreader I would like to see where is that code going to. The function\nname is a unique file identifier that tells me exactly where the change\nis going. The label is not. (It's not freaking BASIC)\n\nI bet all this was just inherited from diff. Would it be accepted if\nI send a patch to fix it? What you guys think a goto label makes any\nsense at all?\n\nThanks\nBoaz\n"},{"id":"174256","messageId":"20110825200001.GA6165@sigill.intra.peff.net","threadId":"28219","inReplyTo":"4E569F10.8060808@panasas.com","subject":"Re: git diff annoyance / feature request","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-08-25T20:00:01Z","receivedAt":"2011-08-25T20:00:01Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 25, 2011 at 12:14:24PM -0700, Boaz Harrosh wrote:\n\n> git diff has this very annoying miss-fixture where it will state\n> as hunk header the closest label instead of the function name.\n> [...]\n>\n> I bet all this was just inherited from diff. Would it be accepted if\n> I send a patch to fix it? What you guys think a goto label makes any\n> sense at all?\n\nUnless you tell git what type of content is in your file, it uses the same\nbasic heuristics for finding a hunk-header line that diff does. Namely,\nthe most recent line that starts with an alphabetic character,\nunderscore, or dollar sign.\n\nIf you want language-specific hunk headers, you can use gitattributes to\ntell git what's in your files. We already have a builtin C driver that\nwill do what you want. You just need to do[1]:\n\n echo '*.c diff=cpp' >.gitattributes\n\nNote that it handles both C and C++, hence the name. See \"git help\ngitattributes\" for details (the section \"Defining a custom hunk-header\"\nis what you want).\n\nIf your upstream (which looks like linux-2.6) doesn't want\n.gitattributes files in the repository, you can also put the entry into\n.git/info/attributes.\n\n-Peff\n\n[1] Since we have builtin funcname patterns for many types, we arguably\ncould also have a builtin mapping of common extensions to diff drivers.\n"},{"id":"174260","messageId":"7vippljkxs.fsf@alter.siamese.dyndns.org","threadId":"28219","inReplyTo":"4E569F10.8060808@panasas.com","subject":"Re: git diff annoyance / feature request","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-25T20:27:43Z","receivedAt":"2011-08-25T20:27:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Boaz Harrosh writes:\n\n> I mean. The label \"try_again\" is not at all unique in my file. As a\n> reader I would like to see where is that code going to. The function\n> name is a unique file identifier that tells me exactly where the change\n> is going. The label is not. (It's not freaking BASIC)\n>\n> I bet all this was just inherited from diff. Would it be accepted if\n> I send a patch to fix it? What you guys think a goto label makes any\n> sense at all?\n\nThe default tries to mimic what GNU used to do when we added the feature.\n\nThe diff.*.xfuncname configuration variable is there exactly for people\nlike you to tweak what we use for hunk headers. Please experiment with it\nand if you come up with a better set of patterns, people may want to copy\nit and use it themselves. we may even consider updating the built-in\ndefault with your patterns, once they got adopted by wider audiences.\n\nPersonally, I would have to say that the source wouldn't be using too many\nlabels with the same name for this behaviour to be problematic, especially\nif it is not freaking BASIC ;-), so...\n"},{"id":"174262","messageId":"20110825204047.GA9948@sigill.intra.peff.net","threadId":"28219","inReplyTo":"20110825200001.GA6165@sigill.intra.peff.net","subject":"[RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-08-25T20:40:47Z","receivedAt":"2011-08-25T20:40:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We already provide sane hunk-header patterns for specific\nlanguages. However, the user has to manually map common\nextensions to use them. It's not that hard to do, but it's\nan extra step that the user might not even know is an\noption. Let's be nice and do it automatically.\n\nSigned-off-by: Jeff King \n---\nI tried to think of negative side effects.\n\nThe userdiff drivers we have are pretty conservative; they just specify\nhunk headers. So if you have a binary file named \"foo.c\", we still do\nthe regular binary detection.\n\nIf you have any matching attribute line in your own files, it should\noverride. So:\n\n foo/* -diff\n\nwill still mark foo/bar.c as binary, even with this change.\n\nCan anyone think of other possible side effects?\n\nAlso, any other extensions that would go into such a list? I have no\nidea what the common extension is for something like pascal or csharp.\n\n attr.c | 12 ++++++++++++\n 1 files changed, 12 insertions(+), 0 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex da29c8e..5118a14 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -294,6 +294,18 @@ static void free_attr_elem(struct attr_stack *e)\n \n static const char *builtin_attr[] = {\n \t\"[attr]binary -diff -text\",\n+\t\"*.html diff=html\",\n+\t\"*.java diff=java\",\n+\t\"*.perl diff=perl\",\n+\t\"*.pl diff=perl\",\n+\t\"*.php diff=php\",\n+\t\"*.py diff=python\",\n+\t\"*.rb diff=ruby\",\n+\t\"*.bib diff=bibtex\",\n+\t\"*.tex diff=tex\",\n+\t\"*.c diff=cpp\",\n+\t\"*.cc diff=cpp\",\n+\t\"*.cxx diff=cpp\",\n \tNULL,\n };\n \n-- \n1.7.6.10.g62f04\n"},{"id":"174265","messageId":"CAPig+cQ33PESWC5fzN8enLFRwNPx8o+PgRUTeCva4dSJ_EdwOw@mail.gmail.com","threadId":"28219","inReplyTo":"20110825204047.GA9948@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2011-08-25T21:00:51Z","receivedAt":"2011-08-25T21:00:51Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 25, 2011 at 4:40 PM, Jeff King wrote:\n> We already provide sane hunk-header patterns for specific\n> languages. However, the user has to manually map common\n> extensions to use them. It's not that hard to do, but it's\n> an extra step that the user might not even know is an\n> option. Let's be nice and do it automatically.\n>\n> Also, any other extensions that would go into such a list? I have no\n> idea what the common extension is for something like pascal or csharp.\n\nC# uses extension \".cs\".\n\n\".cpp\" is common, in fact often required, by Windows compilers.\n\nWhat about \".h\" and \".hpp\"?\n\n-- ES\n"},{"id":"174266","messageId":"20110825210654.GA11077@sigill.intra.peff.net","threadId":"28219","inReplyTo":"CAPig+cQ33PESWC5fzN8enLFRwNPx8o+PgRUTeCva4dSJ_EdwOw@mail.gmail.com","subject":"Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-08-25T21:06:54Z","receivedAt":"2011-08-25T21:06:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 25, 2011 at 05:00:51PM -0400, Eric Sunshine wrote:\n\n> > Also, any other extensions that would go into such a list? I have no\n> > idea what the common extension is for something like pascal or csharp.\n> \n> C# uses extension \".cs\".\n> \n> \".cpp\" is common, in fact often required, by Windows compilers.\n\nThanks, added both to my list.\n\n> What about \".h\" and \".hpp\"?\n\nHow well do our cpp patterns do with header files? I imagine they're\nbetter than the default, but I don't think I've ever really tried\nanything tricky.\n\n-Peff\n"},{"id":"174277","messageId":"4E56C58E.4080905@panasas.com","threadId":"28219","inReplyTo":"7vippljkxs.fsf@alter.siamese.dyndns.org","subject":"Re: git diff annoyance / feature request","fromName":"Boaz Harrosh","fromEmail":"bharrosh@panasas.com","sentAt":"2011-08-25T21:58:38Z","receivedAt":"2011-08-25T21:58:38Z","isPatch":false,"sender":{"key":"bharrosh@panasas.com","avatar":"https://gravatar.com/avatar/347e426f8ca0409f6891ceeefd4c3c2d8769608382323057efeaa362b4c3dbf3?d=mp&s=160"},"body":"On 08/25/2011 01:27 PM, Junio C Hamano wrote:\n> Boaz Harrosh writes:\n> \n>> I mean. The label \"try_again\" is not at all unique in my file. As a\n>> reader I would like to see where is that code going to. The function\n>> name is a unique file identifier that tells me exactly where the change\n>> is going. The label is not. (It's not freaking BASIC)\n>>\n>> I bet all this was just inherited from diff. Would it be accepted if\n>> I send a patch to fix it? What you guys think a goto label makes any\n>> sense at all?\n> \n> The default tries to mimic what GNU used to do when we added the feature.\n> \n> The diff.*.xfuncname configuration variable is there exactly for people\n> like you to tweak what we use for hunk headers. Please experiment with it\n> and if you come up with a better set of patterns, people may want to copy\n> it and use it themselves. we may even consider updating the built-in\n> default with your patterns, once they got adopted by wider audiences.\n> \n\nThanks, I'll investigate it sounds very interesting.\n\n> Personally, I would have to say that the source wouldn't be using too many\n> labels with the same name for this behaviour to be problematic, especially\n> if it is not freaking BASIC ;-), so...\n\nThe Linux Kernel is full of \"goto out\" or \"goto err\" its a common error handling\npractice. I actually like it because it taps onto a known pattern.\n\nNow the patch tell me @@@ lable out: !! that's not very useful I would say\n\nThanks I'm sure I can shape it up the way I like it\nBoaz\n"},{"id":"174278","messageId":"4E56C62F.1000403@panasas.com","threadId":"28219","inReplyTo":"20110825210654.GA11077@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Boaz Harrosh","fromEmail":"bharrosh@panasas.com","sentAt":"2011-08-25T22:01:19Z","receivedAt":"2011-08-25T22:01:19Z","isPatch":true,"sender":{"key":"bharrosh@panasas.com","avatar":"https://gravatar.com/avatar/347e426f8ca0409f6891ceeefd4c3c2d8769608382323057efeaa362b4c3dbf3?d=mp&s=160"},"body":"On 08/25/2011 02:06 PM, Jeff King wrote:\n> On Thu, Aug 25, 2011 at 05:00:51PM -0400, Eric Sunshine wrote:\n> \n>>> Also, any other extensions that would go into such a list? I have no\n>>> idea what the common extension is for something like pascal or csharp.\n>>\n>> C# uses extension \".cs\".\n>>\n>> \".cpp\" is common, in fact often required, by Windows compilers.\n> \n> Thanks, added both to my list.\n> \n>> What about \".h\" and \".hpp\"?\n> \n> How well do our cpp patterns do with header files? I imagine they're\n> better than the default, but I don't think I've ever really tried\n> anything tricky.\n> \n> -Peff\n\nThanks Jeff, thanks everyone! This looks very promising. Specially that\nit's all already there and I don't have to code it up.\n\nRTFM time for me now\nBoaz\n"},{"id":"174281","messageId":"5qgbkjmEZ8jSRkpVNieElg1bcVbuEStD525CFu1hZPQ7F03R3EzjXwQdDKQBOnR1zWDiZBsGu53K20rbOGpYd6rmp2-e-ZI3Z42BKT01TVI@cipher.nrlssc.navy.mil","threadId":"28219","inReplyTo":"20110825204047.GA9948@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2011-08-25T22:29:36Z","receivedAt":"2011-08-25T22:29:36Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"On 08/25/2011 03:40 PM, Jeff King wrote:\n> We already provide sane hunk-header patterns for specific\n> languages. However, the user has to manually map common\n> extensions to use them. It's not that hard to do, but it's\n> an extra step that the user might not even know is an\n> option. Let's be nice and do it automatically.\n> \n> Signed-off-by: Jeff King \n> ---\n> I tried to think of negative side effects.\n\nThat's what I worried about when I last touched this code. Now I'm\nthinking \"what took us so long to do this!!??\".\n\n> Also, any other extensions that would go into such a list?\n\n*.bib diff=bibtex\n*.tex diff=tex\n\n*.[Ff] diff=fortran\n*.[Ff][0-9][0-9] diff=fortran\n\nGNU fortran currently recognizes .fXX where XX is 90, 95, 03 and 08\nand probably enables/disables features based on the respective standard.\n[0-9][0-9] would future proof against fortran f13 and f25 as long as\nthere aren't other extensions that would conflict.\n\nWikipedia says that .for is an extension for fortran, but I've never\nseen that in the wild. Maybe it's a windows thing (3-char ext).\n\n-Brandon\n"},{"id":"174283","messageId":"7v8vqhhzgd.fsf@alter.siamese.dyndns.org","threadId":"28219","inReplyTo":"20110825204047.GA9948@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-25T22:57:06Z","receivedAt":"2011-08-25T22:57:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> If you have any matching attribute line in your own files, it should\n> override. So:\n>\n> foo/* -diff\n>\n> will still mark foo/bar.c as binary, even with this change.\n>\n> Can anyone think of other possible side effects?\n>\n> Also, any other extensions that would go into such a list? I have no\n> idea what the common extension is for something like pascal or csharp.\n\nAs long as the builtin ones are the lowest priority fallback, we should be\nOk.\n\nDo we say anywhere that \"Ah, this has 'diff' attribute defined, so it must\nbe text\"? If so, we should fix _that_. In other words, having this one\nextra entry\n\n\t\"* diff=default\"\n\nin the builtin_attr[] array should be a no-op, I think.\n\n>\n> attr.c | 12 ++++++++++++\n> 1 files changed, 12 insertions(+), 0 deletions(-)\n>\n> diff --git a/attr.c b/attr.c\n> index da29c8e..5118a14 100644\n> --- a/attr.c\n> +++ b/attr.c\n> @@ -294,6 +294,18 @@ static void free_attr_elem(struct attr_stack *e)\n> \n> static const char *builtin_attr[] = {\n> \t\"[attr]binary -diff -text\",\n> +\t\"*.html diff=html\",\n> +\t\"*.java diff=java\",\n> +\t\"*.perl diff=perl\",\n> +\t\"*.pl diff=perl\",\n> +\t\"*.php diff=php\",\n> +\t\"*.py diff=python\",\n> +\t\"*.rb diff=ruby\",\n> +\t\"*.bib diff=bibtex\",\n> +\t\"*.tex diff=tex\",\n> +\t\"*.c diff=cpp\",\n> +\t\"*.cc diff=cpp\",\n> +\t\"*.cxx diff=cpp\",\n> \tNULL,\n> };\n"},{"id":"174288","messageId":"4E56DE59.5050601@sunshineco.com","threadId":"28219","inReplyTo":"20110825210654.GA11077@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2011-08-25T23:44:25Z","receivedAt":"2011-08-25T23:44:25Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On 08/25/2011 05:06 PM, Jeff King wrote:\n> On Thu, Aug 25, 2011 at 05:00:51PM -0400, Eric Sunshine wrote:\n>\n>>> Also, any other extensions that would go into such a list? I have no\n>>> idea what the common extension is for something like pascal or csharp.\n>>\n>> C# uses extension \".cs\".\n>>\n>> \".cpp\" is common, in fact often required, by Windows compilers.\n>\n> Thanks, added both to my list.\n\nTo clarify, I meant to say that for C++, .cpp is common/required on Windows.\n\n>> What about \".h\" and \".hpp\"?\n>\n> How well do our cpp patterns do with header files? I imagine they're\n> better than the default, but I don't think I've ever really tried\n> anything tricky.\n\nI scanned through a number of revisions for one of my long-running C++ \nprojects comparing the diff of header files with and without \"*.h \ndiff=cpp\". In some header files in this project, the oft-used C++ \nkeywords public:, protected:, and private: appear at start-of-line. In \nsuch cases, the default diff emits a less-than-useful hunk header:\n\n @@ -19,8 +19,8 @@ public:\n\nwhereas, \"diff=cpp\" emits:\n\n @@ -19,8 +19,8 @@ class Foobar\n\n-- ES\n"},{"id":"174296","messageId":"20110826023951.GA17625@sigill.intra.peff.net","threadId":"28219","inReplyTo":"4E56DE59.5050601@sunshineco.com","subject":"Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-08-26T02:39:51Z","receivedAt":"2011-08-26T02:39:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 25, 2011 at 07:44:25PM -0400, Eric Sunshine wrote:\n\n> >How well do our cpp patterns do with header files? I imagine they're\n> >better than the default, but I don't think I've ever really tried\n> >anything tricky.\n> \n> I scanned through a number of revisions for one of my long-running\n> C++ projects comparing the diff of header files with and without \"*.h\n> diff=cpp\". In some header files in this project, the oft-used C++\n> keywords public:, protected:, and private: appear at start-of-line.\n> In such cases, the default diff emits a less-than-useful hunk header:\n> \n> @@ -19,8 +19,8 @@ public:\n> \n> whereas, \"diff=cpp\" emits:\n> \n> @@ -19,8 +19,8 @@ class Foobar\n\nThanks. My C++ is so rusty that I didn't think immediately of how often\nthose keywords appear in header files. Also, code in inline\nfunctions in either C or C++ will be found in header files. So I think\ndefaulting *.h and *.hpp to cpp is sensible.\n\n-Peff\n"},{"id":"174297","messageId":"20110826024533.GB17625@sigill.intra.peff.net","threadId":"28219","inReplyTo":"5qgbkjmEZ8jSRkpVNieElg1bcVbuEStD525CFu1hZPQ7F03R3EzjXwQdDKQBOnR1zWDiZBsGu53K20rbOGpYd6rmp2-e-ZI3Z42BKT01TVI@cipher.nrlssc.navy.mil","subject":"Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-08-26T02:45:33Z","receivedAt":"2011-08-26T02:45:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 25, 2011 at 05:29:36PM -0500, Brandon Casey wrote:\n\n> > Also, any other extensions that would go into such a list?\n> \n> *.bib diff=bibtex\n> *.tex diff=tex\n\nI had those ones already. ;P\n\n> *.[Ff] diff=fortran\n> *.[Ff][0-9][0-9] diff=fortran\n\nThanks, I'll add those. I don't see a big problem with generalizing\nf[0-9][0-9] to always be fortran, even though many of those numbers\naren't used. I don't think I've ever seen one used for anything else.\n\nShould all of our matches be case-insensitive? That is, should we be\nmatching both .HTML and .html? Clearly lowercase is the One True Way,\nbut I don't know what kind of junk people with case-insensitive\nfilesystems have, or whether we should even worry about it.\n\n> Wikipedia says that .for is an extension for fortran, but I've never\n> seen that in the wild. Maybe it's a windows thing (3-char ext).\n\nWe can leave it out. It's easy enough for somebody to add their own\ngitattribute if they want, or to even complain that it should be in the\ndefault set. I was trying to keep this list to the utterly common, not\nbecome a catalogue of obscure fortran customs. :)\n\n-Peff\n"},{"id":"174298","messageId":"20110826025913.GC17625@sigill.intra.peff.net","threadId":"28219","inReplyTo":"7v8vqhhzgd.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-08-26T02:59:13Z","receivedAt":"2011-08-26T02:59:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 25, 2011 at 03:57:06PM -0700, Junio C Hamano wrote:\n\n> > If you have any matching attribute line in your own files, it should\n> > override. So:\n> >\n> > foo/* -diff\n> >\n> > will still mark foo/bar.c as binary, even with this change.\n> >\n> > Can anyone think of other possible side effects?\n> >\n> > Also, any other extensions that would go into such a list? I have no\n> > idea what the common extension is for something like pascal or csharp.\n> \n> As long as the builtin ones are the lowest priority fallback, we should be\n> Ok.\n> \n> Do we say anywhere that \"Ah, this has 'diff' attribute defined, so it must\n> be text\"? If so, we should fix _that_. In other words, having this one\n> extra entry\n> \n> \t\"* diff=default\"\n> \n> in the builtin_attr[] array should be a no-op, I think.\n\nNo, certainly not since 122aa6f (diff: introduce diff..binary,\n2008-10-05). That commit's message claims that we did before it, but\nlooking at the patch, I am not so sure. But I'm not about to start\ntesting a 3-year-old patch to see if it really was the source of the\nfix; the point is that it is correct now. :)\n\nI think it could be a problem in the future if the builtin userdiff\ndrivers started growing more invasive options, like automatically\nclaiming to be non-binary (i.e., setting diff.cpp.binary = false by\ndefault). In other words, I think we have two options:\n\n 1. Builtin drivers like \"cpp\" can stay minimal, only setting funcname\n and color-words headers that aren't going to produce terrible\n results if we are wrong about detecting by extension.\n\n 2. We force the user to identify file types manually, so we can't be\n wrong. The \"cpp\" diff driver means \"you are a text C file\", and if\n a user mis-marks a binary file with that diff driver, they are the\n one who is wrong.\n\nSo if it's an either/or situation, we should decide not only that\nextension auto-detection is a good feature, but that it trumps adding\nmore advanced features to the builtin drivers in the future.\n\nOr we could decide that the extensions really are good enough, and if\nyou really do have binary files named \"foo.c\", it's your problem to\noverride the defaults with \"*.c -diff\".\n\n-Peff\n"},{"id":"174299","messageId":"4E5719FA.9060603@sunshineco.com","threadId":"28219","inReplyTo":"20110826024533.GB17625@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2011-08-26T03:58:50Z","receivedAt":"2011-08-26T03:58:50Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On 08/25/2011 10:45 PM, Jeff King wrote:\n> Should all of our matches be case-insensitive? That is, should we be\n> matching both .HTML and .html? Clearly lowercase is the One True Way,\n> but I don't know what kind of junk people with case-insensitive\n> filesystems have, or whether we should even worry about it.\n\nIn the Windows world, uppercase extensions are common. Also, one often \nfinds .htm on Windows rather than .html.\n\nSpeaking of other platforms, on Mac OS X:\n\nObjective-C is .m\nObjective-C++ is .mm (and long-deprecated .M is probably not relevant)\n\n-- ES\n"},{"id":"174301","messageId":"7vty94g1oe.fsf@alter.siamese.dyndns.org","threadId":"28219","inReplyTo":"20110826025913.GC17625@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-26T05:52:01Z","receivedAt":"2011-08-26T05:52:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> No, certainly not since 122aa6f (diff: introduce diff..binary,\n> 2008-10-05). That commit's message claims that we did before it, but\n> looking at the patch, I am not so sure. But I'm not about to start\n> testing a 3-year-old patch to see if it really was the source of the\n> fix; the point is that it is correct now. :)\n\nViolently agreed ;-)\n\n> I think it could be a problem in the future if the builtin userdiff\n> drivers started growing more invasive options, like automatically\n> claiming to be non-binary (i.e., setting diff.cpp.binary = false by\n> default).\n\nWell, I think we can be careful when we start thinking about doing\nsomething complex like that, then. Mentioning the above consideration in\nthe commit message of the final version of this patch would probably be a\ngood idea, I presume.\n\nThanks.\n"},{"id":"174307","messageId":"buo4o148rq9.fsf@dhlpc061.dev.necel.com","threadId":"28219","inReplyTo":"4E56C58E.4080905@panasas.com","subject":"Re: git diff annoyance / feature request","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2011-08-26T09:08:46Z","receivedAt":"2011-08-26T09:08:46Z","isPatch":false,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Boaz Harrosh writes:\n>> Personally, I would have to say that the source wouldn't be using too many\n>> labels with the same name for this behaviour to be problematic, especially\n>> if it is not freaking BASIC ;-), so...\n>\n> The Linux Kernel is full of \"goto out\" or \"goto err\" its a common error handling\n> practice. I actually like it because it taps onto a known pattern.\n>\n> Now the patch tell me @@@ lable out: !! that's not very useful I would say\n>\n> Thanks I'm sure I can shape it up the way I like it\n\nIncidentally, if these annoyances were inherited from GNU diff, it would\nbe good to send a bug report there too,... I doubt anybody likes the\nbehavior any better with diff!\n\n[I know I've been annoyed by such things before...]\n\nThanks,\n\n-Miles\n\n-- \nInnards, n. pl. The stomach, heart, soul, and other bowels.\n"},{"id":"174309","messageId":"201108261144.35888.trast@student.ethz.ch","threadId":"28219","inReplyTo":"20110825204047.GA9948@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-08-26T09:44:35Z","receivedAt":"2011-08-26T09:44:35Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King wrote:\n> We already provide sane hunk-header patterns for specific\n> languages. However, the user has to manually map common\n> extensions to use them. It's not that hard to do, but it's\n> an extra step that the user might not even know is an\n> option. Let's be nice and do it automatically.\n> \n> Signed-off-by: Jeff King \n> ---\n> I tried to think of negative side effects.\n> \n> The userdiff drivers we have are pretty conservative; they just specify\n> hunk headers.\n\nAnd word-diff regexes.\n\nIn my book this is a plus for your patch, but I'm just saying. It\nwill trigger the slightly slower mode of word-diffing that splits at\nfar more places than the default.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"174323","messageId":"49Jb0B80dYohdvQHEk2HW7bFvbaGqrVei6JPtUczcIQObOhHK_bRVKFymHMrH2zujgbzsGeCGMdeqznOy2_QjqRtEmq6i-OlPkXcT1B76QA@cipher.nrlssc.navy.mil","threadId":"28219","inReplyTo":"20110826024533.GB17625@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2011-08-26T15:33:38Z","receivedAt":"2011-08-26T15:33:38Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"On 08/25/2011 09:45 PM, Jeff King wrote:\n> On Thu, Aug 25, 2011 at 05:29:36PM -0500, Brandon Casey wrote:\n> \n>>> Also, any other extensions that would go into such a list?\n>>\n>> *.bib diff=bibtex\n>> *.tex diff=tex\n> \n> I had those ones already. ;P\n\nIndeed. I must be blind, I skipped right over them.\n\n>> *.[Ff] diff=fortran\n>> *.[Ff][0-9][0-9] diff=fortran\n> \n> Thanks, I'll add those. I don't see a big problem with generalizing\n> f[0-9][0-9] to always be fortran, even though many of those numbers\n> aren't used. I don't think I've ever seen one used for anything else.\n> \n> Should all of our matches be case-insensitive? That is, should we be\n> matching both .HTML and .html? Clearly lowercase is the One True Way,\n> but I don't know what kind of junk people with case-insensitive\n> filesystems have, or whether we should even worry about it.\n\nFor the fortran case, Gnu fortran actually processes the files differently\ndepending on whether the f is capitalized (it preprocesses or not). So\nthere is a functional reason for using a capital letter.\n\nFor the others, I don't know. Do people still create files named .HTML\nor is that just a relic of the past? I can't really think of a strong\nargument for or against matching insensitively.\n\n-Brandon\n"},{"id":"174350","messageId":"4E580D49.1070006@lsrfire.ath.cx","threadId":"28219","inReplyTo":"4E569F10.8060808@panasas.com","subject":"Re: git diff annoyance / feature request","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-08-26T21:16:57Z","receivedAt":"2011-08-26T21:16:57Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 25.08.2011 21:14, schrieb Boaz Harrosh:\n> \n> git diff has this very annoying miss-fixture where it will state\n> as hunk header the closest label instead of the function name.\n> \n> So I get:\n> @@ -675,9 +670,23 @@ try_again:\n> \t}\n> \n> \tif (flag) {\n> -\t\tfoo();\n> +\t\tbazz();\n> \t}\n> \n> \n> Instead of what I'd like:\n> @@ -563,12 +563,7 @@ static int write_exec(struct page_collect *pcol)\n> \t}\n> \n> \tif (flag) {\n> -\t\tfoo();\n> +\t\tbazz();\n> \t}\n\nCheap trick: change your coding style to place a single space before\nlabels instead of having them start right at the beginning of a line.\n\nRené\n"},{"id":"174351","messageId":"4E581213.6070304@panasas.com","threadId":"28219","inReplyTo":"4E580D49.1070006@lsrfire.ath.cx","subject":"Re: git diff annoyance / feature request","fromName":"Boaz Harrosh","fromEmail":"bharrosh@panasas.com","sentAt":"2011-08-26T21:37:23Z","receivedAt":"2011-08-26T21:37:23Z","isPatch":false,"sender":{"key":"bharrosh@panasas.com","avatar":"https://gravatar.com/avatar/347e426f8ca0409f6891ceeefd4c3c2d8769608382323057efeaa362b4c3dbf3?d=mp&s=160"},"body":"On 08/26/2011 02:16 PM, René Scharfe wrote:\n> Am 25.08.2011 21:14, schrieb Boaz Harrosh:\n> \n> Cheap trick: change your coding style to place a single space before\n> labels instead of having them start right at the beginning of a line.\n> \n> René\n> \n\nNope, does not work! and I have no choice about it, it's Linux coding\nstyle\n\nBoaz\n"},{"id":"174353","messageId":"7v4o13dene.fsf@alter.siamese.dyndns.org","threadId":"28219","inReplyTo":"4E581213.6070304@panasas.com","subject":"Re: git diff annoyance / feature request","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-26T21:52:21Z","receivedAt":"2011-08-26T21:52:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Boaz Harrosh writes:\n\n> On 08/26/2011 02:16 PM, René Scharfe wrote:\n>> Am 25.08.2011 21:14, schrieb Boaz Harrosh:\n>> \n>> Cheap trick: change your coding style to place a single space before\n>> labels instead of having them start right at the beginning of a line.\n>> \n>> René\n>> \n>\n> Nope, does not work! and I have no choice about it, it's Linux coding\n> style\n\nI too thought it was in the Documentation/CodingStyle, but I don't find it\nin my copy. \"Chapter 7\" has an example of using goto and it does have\nlabel at the left edge of the page without indentation, but does not seem\nto say it should not be indented.\n\nTaken together with the output from \"grep '#goto label' scripts/checkpatch.pl\"\nI concluded that \"it's Linux coding style\" was a myth.\n"},{"id":"174370","messageId":"loom.20110827T070650-93@post.gmane.org","threadId":"28219","inReplyTo":"20110825204047.GA9948@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions","fromName":"Alexey Shumkin","fromEmail":"zapped@mail.ru","sentAt":"2011-08-27T05:14:05Z","receivedAt":"2011-08-27T05:14:05Z","isPatch":true,"sender":{"key":"alex.crezoff@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1183752?v=4"},"body":"> Also, any other extensions that would go into such a list? I have no\n> idea what the common extension is for something like pascal or csharp.\n\n# [Object] Pascal unit files\n*.pas diff=pascal\n\n# + project files (they rarely contain procedures/functions\n# but it is not forbidden in specification)\n\n*.dpr diff=pascal\n\n# for Lazarus - pp and .lpr respectivly\n*.pp diff=pascal\n*.lpr diff=pascal\n"}]}