{"thread":{"id":"31309","subject":"misleading diff-hunk header","startedAt":"2012-08-21T12:57:52Z","lastAt":"2012-08-26T18:53:27Z","messageCount":14,"participants":["Tim Chase","Thomas Rast","Johannes Sixt","Junio C Hamano","Jeff King","Stefano Lattarini"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"197530","messageId":"503385D0.5070605@tim.thechases.com","threadId":"31309","inReplyTo":null,"subject":"misleading diff-hunk header","fromName":"Tim Chase","fromEmail":"git@tim.thechases.com","sentAt":"2012-08-21T12:57:52Z","receivedAt":"2012-08-21T12:57:52Z","isPatch":false,"sender":{"key":"git@tim.thechases.com","avatar":null},"body":"[posted originally to git-users@ but advised this would be a better\nforum]\n\ndiff.{type}.xfuncname seems to start searching backwards in\nfrom the beginning of the hunk, not the first differing line.\n\nTo reproduce:\n\n $ mkdir tmp\n $ cd tmp\n $ git init\n $ cat > foo.c < writes:\n\n> diff.{type}.xfuncname seems to start searching backwards in\n> from the beginning of the hunk, not the first differing line.\n[...]\n> @@ -4,4 +4,5 @@ int call_me(int maybe)\n>\n> int main()\n> {\n> + return 0;\n> }\n>\n> misleadingly suggesting that the change occurred in the call_me()\n> function, rather than in main()\n\nI think that's intentional, and matches what 'diff -p' does. It gives\nyou the context before the hunk. After all, if a new function starts in\nthe leading context lines, you can see that in the usual diff data.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"197533","messageId":"5033AC55.8080405@tim.thechases.com","threadId":"31309","inReplyTo":"87a9xoi82i.fsf@thomas.inf.ethz.ch","subject":"Re: misleading diff-hunk header","fromName":"Tim Chase","fromEmail":"git@tim.thechases.com","sentAt":"2012-08-21T15:42:13Z","receivedAt":"2012-08-21T15:42:13Z","isPatch":false,"sender":{"key":"git@tim.thechases.com","avatar":null},"body":"On 08/21/12 10:22, Thomas Rast wrote:\n> Tim Chase writes:\n> \n>> diff.{type}.xfuncname seems to start searching backwards in\n>> from the beginning of the hunk, not the first differing line.\n> [...]\n>> @@ -4,4 +4,5 @@ int call_me(int maybe)\n>>\n>> int main()\n>> {\n>> + return 0;\n>> }\n>>\n>> misleadingly suggesting that the change occurred in the call_me()\n>> function, rather than in main()\n> \n> I think that's intentional, and matches what 'diff -p' does. It gives\n> you the context before the hunk. After all, if a new function starts in\n> the leading context lines, you can see that in the usual diff data.\n\nOkay...I tested \"diff -p\" and can't argue (much) with historical\nadherence. It just makes it hard for me to gather some stats on the\nfunctions that changed, and requires that I look in more than one\nplace (both in the header, and in the leading context) rather than\nhaving a single authoritative place to grep.\n\nThen again, \"diff -p\" only seems to support C functions, while git\nsupports bibtex, cpp, html, java, objc, pascal, php, python, ruby,\nand tex out-of-the-box, with the option to build your own\nfunction-finder, so pure adherence to history gets a little muddied.\n\nThanks for your thoughts,\n\n-tkc\n"},{"id":"197540","messageId":"5033C7D5.1030404@kdbg.org","threadId":"31309","inReplyTo":"5033AC55.8080405@tim.thechases.com","subject":"Re: misleading diff-hunk header","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-08-21T17:39:33Z","receivedAt":"2012-08-21T17:39:33Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 21.08.2012 17:42, schrieb Tim Chase:\n> On 08/21/12 10:22, Thomas Rast wrote:\n>>> misleadingly suggesting that the change occurred in the call_me()\n>>> function, rather than in main()\n>>\n>> I think that's intentional, and matches what 'diff -p' does...\n> \n> Okay...I tested \"diff -p\" and can't argue (much) with historical\n> adherence. It just makes it hard for me to gather some stats on the\n> functions that changed, and requires that I look in more than one\n> place (both in the header, and in the leading context) rather than\n> having a single authoritative place to grep.\n\nIf it's only for stats, why not just remove the context with -U0?\n\n-- Hannes\n"},{"id":"197541","messageId":"7vfw7gdtfg.fsf@alter.siamese.dyndns.org","threadId":"31309","inReplyTo":"5033AC55.8080405@tim.thechases.com","subject":"Re: misleading diff-hunk header","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-21T17:52:03Z","receivedAt":"2012-08-21T17:52:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tim Chase writes:\n\n> On 08/21/12 10:22, Thomas Rast wrote:\n>> Tim Chase writes:\n>> \n>>> diff.{type}.xfuncname seems to start searching backwards in\n>>> from the beginning of the hunk, not the first differing line.\n>> [...]\n>>> @@ -4,4 +4,5 @@ int call_me(int maybe)\n>>>\n>>> int main()\n>>> {\n>>> + return 0;\n>>> }\n>>>\n>>> misleadingly suggesting that the change occurred in the call_me()\n>>> function, rather than in main()\n>> \n>> I think that's intentional, and matches what 'diff -p' does. It gives\n>> you the context before the hunk. After all, if a new function starts in\n>> the leading context lines, you can see that in the usual diff data.\n\nCorrect. It is about \"give the user _more_ hint/clue on the context\nof the hunk\", in addition to what the user can see in the\npre-context of the hunk, so it is pointless to hoist \"int main()\"\nthere.\n\n> ... It just makes it hard for me to gather some stats on the\n> functions that changed, and requires that I look in more than one\n> place (both in the header, and in the leading context) rather than\n> having a single authoritative place to grep.\n\nThe right way to answer \"which functions were touched?\" question is\nto ignore what you see on the hunk header \"@@ .. @@\" lines and only\nlook at the patch text, running \"git diff\" with larger number of\ncontext lines as necessary.\n\nIf you have a large patch hunk that adds or removes two or more new\nfunctions, you would have to look at the patch text _anyway_ to\nlearn about these two or more names---they cannot possibly both\nappear on the hunk header lines, so looking at the context hint\nthere is pointless for the purpose for which you are using \"diff\"\noutput.\n"},{"id":"197565","messageId":"7vwr0rc217.fsf@alter.siamese.dyndns.org","threadId":"31309","inReplyTo":"5033C7D5.1030404@kdbg.org","subject":"Re: misleading diff-hunk header","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-21T22:29:08Z","receivedAt":"2012-08-21T22:29:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 21.08.2012 17:42, schrieb Tim Chase:\n>> On 08/21/12 10:22, Thomas Rast wrote:\n>>>> misleadingly suggesting that the change occurred in the call_me()\n>>>> function, rather than in main()\n>>>\n>>> I think that's intentional, and matches what 'diff -p' does...\n>> \n>> Okay...I tested \"diff -p\" and can't argue (much) with historical\n>> adherence. It just makes it hard for me to gather some stats on the\n>> functions that changed, and requires that I look in more than one\n>> place (both in the header, and in the leading context) rather than\n>> having a single authoritative place to grep.\n>\n> If it's only for stats, why not just remove the context with -U0?\n\nI actually think you want a way to say -U in\nthis case instead of unsightly -U99999.\n"},{"id":"197774","messageId":"20120824142908.GA15162@sigill.intra.peff.net","threadId":"31309","inReplyTo":"7vfw7gdtfg.fsf@alter.siamese.dyndns.org","subject":"Re: misleading diff-hunk header","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-08-24T14:29:09Z","receivedAt":"2012-08-24T14:29:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 21, 2012 at 10:52:03AM -0700, Junio C Hamano wrote:\n\n> >>> diff.{type}.xfuncname seems to start searching backwards in\n> >>> from the beginning of the hunk, not the first differing line.\n> >> [...]\n> >>> @@ -4,4 +4,5 @@ int call_me(int maybe)\n> >>>\n> >>> int main()\n> >>> {\n> >>> + return 0;\n> >>> }\n> >>>\n> >>> misleadingly suggesting that the change occurred in the call_me()\n> >>> function, rather than in main()\n> >> \n> >> I think that's intentional, and matches what 'diff -p' does. It gives\n> >> you the context before the hunk. After all, if a new function starts in\n> >> the leading context lines, you can see that in the usual diff data.\n> \n> Correct. It is about \"give the user _more_ hint/clue on the context\n> of the hunk\", in addition to what the user can see in the\n> pre-context of the hunk, so it is pointless to hoist \"int main()\"\n> there.\n\nI don't think it is pointless. If you are skimming a diff, then the hunk\nheaders stand out to easily show which functions were touched. Of\ncourse, as you mentioned later in your email, it is not an exhaustive\nlist, and I think for Tim's use case, he needs to actually read and\nparse the whole patch.\n\nBut mentioning call_me here _is_ pointless, because it is not relevant\ncontext at all (it was not modified; it just happens to be located near\nthe code in question). So I would argue that showing main() is more\nuseful to a reader.\n\nIt gets even more obvious as you increase the context. Imagine I have\ncode like this:\n\n int foo(void)\n {\n return 1;\n }\n\n int bar(void)\n {\n return 2;\n }\n\n int baz(void)\n {\n return 3;\n }\n\nand I modify \"baz\" to return \"4\" instead. With the regular diff\nsettings, the hunk header would claim that \"bar()\" is the context in the\nhunk header. But if I ask for -U7, then \"foo()\" is mentioned in the hunk\nheader. To me, that doesn't make sense; the modification is exactly the\nsame, so why would the hunk header differ?\n\nI suppose one could argue that the hunk header is not showing the\ncontext of the change, but rather the context of the surrounding context\nlines. But that doesn't seem useful to me.\n\nWe discussed this a while ago and you did a \"how about this\" patch:\n\n http://article.gmane.org/gmane.comp.version-control.git/181385\n\nGmane seems to be acting up this morning, so here is the patch (and your\ncomment) for reference:\n\n> Would this be sufficient? Instead of looking for the first line that\n> matches the \"beginning\" pattern going backwards starting from one line\n> before the displayed context, we start our examination at the first line\n> shown in the context.\n> \n> xdiff/xemit.c | 2 +-\n> 1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/xdiff/xemit.c b/xdiff/xemit.c\n> index 277e2ee..5f9c0e0 100644\n> --- a/xdiff/xemit.c\n> +++ b/xdiff/xemit.c\n> @@ -131,7 +131,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n> \n> \t\tif (xecfg->flags & XDL_EMIT_FUNCNAMES) {\n> \t\t\tlong l;\n> -\t\t\tfor (l = s1 - 1; l >= 0 && l > funclineprev; l--) {\n> +\t\t\tfor (l = s1; l >= 0 && l > funclineprev; l--) {\n> \t\t\t\tconst char *rec;\n> \t\t\t\tlong reclen = xdl_get_rec(&xe->xdf1, l, &rec);\n> \t\t\t\tlong newfunclen = ff(rec, reclen, funcbuf,\n\nIn the case we were discussing then, the modified function started on\nthe first line of context. But as Tim's example shows, it doesn't\nnecessarily have to. I think it would make more sense to start counting\nfrom the first modified line.\n\n-Peff\n"},{"id":"197775","messageId":"50379827.3010709@tim.thechases.com","threadId":"31309","inReplyTo":"20120824142908.GA15162@sigill.intra.peff.net","subject":"Re: misleading diff-hunk header","fromName":"Tim Chase","fromEmail":"git@tim.thechases.com","sentAt":"2012-08-24T15:05:11Z","receivedAt":"2012-08-24T15:05:11Z","isPatch":false,"sender":{"key":"git@tim.thechases.com","avatar":null},"body":"On 08/24/12 09:29, Jeff King wrote:\n> On Tue, Aug 21, 2012 at 10:52:03AM -0700, Junio C Hamano wrote:\n> \n>>>>> diff.{type}.xfuncname seems to start searching backwards in\n>>>>> from the beginning of the hunk, not the first differing line.\n>>>> [...]\n>>>>> @@ -4,4 +4,5 @@ int call_me(int maybe)\n>>>>>\n>>>>> int main()\n>>>>> {\n>>>>> + return 0;\n>>>>> }\n>>>>>\n>>>>> misleadingly suggesting that the change occurred in the call_me()\n>>>>> function, rather than in main()\n>>>>\n>>>> I think that's intentional, and matches what 'diff -p' does. \n...\n>> xdiff/xemit.c | 2 +-\n>> 1 files changed, 1 insertions(+), 1 deletions(-)\n>>\n>> diff --git a/xdiff/xemit.c b/xdiff/xemit.c\n>> index 277e2ee..5f9c0e0 100644\n>> --- a/xdiff/xemit.c\n>> +++ b/xdiff/xemit.c\n>> @@ -131,7 +131,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n>> \n>> \t\tif (xecfg->flags & XDL_EMIT_FUNCNAMES) {\n>> \t\t\tlong l;\n>> -\t\t\tfor (l = s1 - 1; l >= 0 && l > funclineprev; l--) {\n>> +\t\t\tfor (l = s1; l >= 0 && l > funclineprev; l--) {\n>> \t\t\t\tconst char *rec;\n>> \t\t\t\tlong reclen = xdl_get_rec(&xe->xdf1, l, &rec);\n>> \t\t\t\tlong newfunclen = ff(rec, reclen, funcbuf,\n> \n> In the case we were discussing then, the modified function started on\n> the first line of context. But as Tim's example shows, it doesn't\n> necessarily have to. I think it would make more sense to start counting\n> from the first modified line.\n\nJunio mentions that it matches the \"diff -p\" output, though I'd\nconsider that a bug in diff as well, since the diff(1) man/info\npages state \"-p Show which C function each change is in.\" In the\nabove (both with \"diff -p\" and with git), the change was clearly in\nmain() but it's not showing main(). Documented behavior and\nimplemented behavior conflict.\n\nStarting at the first differing line rather than the first line of\ncontext in the hunk would ameliorate this. It doesn't address what\nhappens if multiple functions were changed in the same hunk, but at\nleast it becomes correct for the first one. More complex code might\nbe doable to split hunks if an xfuncname match occurs between two\ndisjoint changes in the same hunk. But for my purposes here, the\nabove should suffice.\n\n-tkc\n"},{"id":"197783","messageId":"20120824164415.GA23262@sigill.intra.peff.net","threadId":"31309","inReplyTo":"20120824142908.GA15162@sigill.intra.peff.net","subject":"Re: misleading diff-hunk header","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-08-24T16:44:15Z","receivedAt":"2012-08-24T16:44:15Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 24, 2012 at 10:29:09AM -0400, Jeff King wrote:\n\n> > Would this be sufficient? Instead of looking for the first line that\n> > matches the \"beginning\" pattern going backwards starting from one line\n> > before the displayed context, we start our examination at the first line\n> > shown in the context.\n> > \n> > xdiff/xemit.c | 2 +-\n> > 1 files changed, 1 insertions(+), 1 deletions(-)\n> > \n> > diff --git a/xdiff/xemit.c b/xdiff/xemit.c\n> > index 277e2ee..5f9c0e0 100644\n> > --- a/xdiff/xemit.c\n> > +++ b/xdiff/xemit.c\n> > @@ -131,7 +131,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n> > \n> > \t\tif (xecfg->flags & XDL_EMIT_FUNCNAMES) {\n> > \t\t\tlong l;\n> > -\t\t\tfor (l = s1 - 1; l >= 0 && l > funclineprev; l--) {\n> > +\t\t\tfor (l = s1; l >= 0 && l > funclineprev; l--) {\n> > \t\t\t\tconst char *rec;\n> > \t\t\t\tlong reclen = xdl_get_rec(&xe->xdf1, l, &rec);\n> > \t\t\t\tlong newfunclen = ff(rec, reclen, funcbuf,\n> \n> In the case we were discussing then, the modified function started on\n> the first line of context. But as Tim's example shows, it doesn't\n> necessarily have to. I think it would make more sense to start counting\n> from the first modified line.\n\nThat patch would look something like this:\n\ndiff --git a/xdiff/xemit.c b/xdiff/xemit.c\nindex d11dbf9..441ecf5 100644\n--- a/xdiff/xemit.c\n+++ b/xdiff/xemit.c\n@@ -194,7 +194,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \n \t\tif (xecfg->flags & XDL_EMIT_FUNCNAMES) {\n \t\t\tget_func_line(xe, xecfg, &func_line,\n-\t\t\t\t s1 - 1, funclineprev);\n+\t\t\t\t xche->i1 - 1, funclineprev);\n \t\t\tfunclineprev = s1 - 1;\n \t\t}\n \t\tif (xdl_emit_hunk_hdr(s1 + 1, e1 - s1, s2 + 1, e2 - s2,\n\nNote that this breaks a ton of tests. Some of them are just noise (e.g.,\nt4042 changes line 2, so line 1 is the top of the context; before, we\nwould show no hunk header, since we were at the top of the file, but now\nwe will show line 1). Some of them are improved in the way that this\npatch intends (e.g., t4051).\n\nBut some I'm not sure of. For instance, the failure in t4018.38 is odd.\nI think it's because the pattern it is looking for is a somewhat odd toy\nexample (it's looking for a line with \"s\" in it, so naturally when we\nshift the start-point of our search, we are likely to find some other\nfalse positive). But it raises an interesting point: what if the pattern\nis just looking for lines in a list, and not an enclosing function?\n\nFor example, imagine you have a file a list of items, one per line.\nWith the old code, you'd get:\n\n\tdiff --git a/old b/new\n\tindex f384549..1066a25 100644\n\t--- a/old\n\t+++ b/new\n\t@@ -2,3 +2,3 @@ one\n\t two\n\t-three\n\t+three -- modified\n\t four\n\nSo the hunk header is showing you something useful; the element just\nabove your context. But with my patch, you'd see:\n\n\tdiff --git a/old b/new\n\tindex f384549..1066a25 100644\n\t--- a/old\n\t+++ b/new\n\t@@ -2,3 +2,3 @@ two\n\t two\n\t-three\n\t+three -- modified\n\t four\n\nI.e., it shows the element just before the change, which is already in\nthe context anyway. So it's actually less useful. Although note that the\ncurrent behavior is not all that useful, either; it is not really giving\nyou any information about the change, but rather just showing one extra\nline of context.\n\nSo I would say that which you would prefer might depend on exactly what\nyou are diffing. But I would also argue that in any case where the new\ncode produces a worse result, the hunk header was not all that useful to\nbegin with.\n\n-Peff\n"},{"id":"197805","messageId":"50381F52.9030007@tim.thechases.com","threadId":"31309","inReplyTo":"20120824164415.GA23262@sigill.intra.peff.net","subject":"Re: misleading diff-hunk header","fromName":"Tim Chase","fromEmail":"git@tim.thechases.com","sentAt":"2012-08-25T00:41:54Z","receivedAt":"2012-08-25T00:41:54Z","isPatch":false,"sender":{"key":"git@tim.thechases.com","avatar":null},"body":"On 08/24/12 11:44, Jeff King wrote:\n> With the old code, you'd get:\n> \n> \tdiff --git a/old b/new\n> \tindex f384549..1066a25 100644\n> \t--- a/old\n> \t+++ b/new\n> \t@@ -2,3 +2,3 @@ one\n> \t two\n> \t-three\n> \t+three -- modified\n> \t four\n> \n> So the hunk header is showing you something useful; the element just\n> above your context. But with my patch, you'd see:\n> \n> \tdiff --git a/old b/new\n> \tindex f384549..1066a25 100644\n> \t--- a/old\n> \t+++ b/new\n> \t@@ -2,3 +2,3 @@ two\n> \t two\n> \t-three\n> \t+three -- modified\n> \t four\n> \n> I.e., it shows the element just before the change, which is already in\n> the context anyway. So it's actually less useful. Although note that the\n> current behavior is not all that useful, either; it is not really giving\n> you any information about the change, but rather just showing one extra\n> line of context.\n> \n> So I would say that which you would prefer might depend on exactly what\n> you are diffing. But I would also argue that in any case where the new\n> code produces a worse result, the hunk header was not all that useful to\n> begin with.\n\nIf the documented purpose of \"diff -p\" (and by proxy\ndiff.{type}.xfuncname) is to show the name of the *function*\ncontaining the changed lines, and all you have is a list of lines\nwith no function names, it's pretty arbitrary to call either\nbehavior \"worse\". :-)\n\n-tkc\n"},{"id":"197807","messageId":"7va9xjy4or.fsf@alter.siamese.dyndns.org","threadId":"31309","inReplyTo":"50381F52.9030007@tim.thechases.com","subject":"Re: misleading diff-hunk header","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-25T04:29:56Z","receivedAt":"2012-08-25T04:29:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tim Chase writes:\n\n> If the documented purpose of \"diff -p\" (and by proxy\n> diff.{type}.xfuncname) is to show the name of the *function*\n> containing the changed lines,....\n\nYeah, the documentation is misleading, but I do not offhand think of\na better phrasing. Perhaps you could send in a patch to improve it.\n\nHow does GNU manual explain the option?\n"},{"id":"197835","messageId":"5038CB66.1040006@tim.thechases.com","threadId":"31309","inReplyTo":"7va9xjy4or.fsf@alter.siamese.dyndns.org","subject":"Re: misleading diff-hunk header","fromName":"Tim Chase","fromEmail":"git@tim.thechases.com","sentAt":"2012-08-25T12:56:06Z","receivedAt":"2012-08-25T12:56:06Z","isPatch":false,"sender":{"key":"git@tim.thechases.com","avatar":null},"body":"On 08/24/12 23:29, Junio C Hamano wrote:\n> Tim Chase writes:\n>> If the documented purpose of \"diff -p\" (and by proxy\n>> diff.{type}.xfuncname) is to show the name of the *function*\n>> containing the changed lines,....\n> \n> Yeah, the documentation is misleading, but I do not offhand think of\n> a better phrasing. Perhaps you could send in a patch to improve it.\n> \n> How does GNU manual explain the option?\n\nTersely. :-)\n\n -p --show-c-function\n Show which C function each change is in.\n\nAnd that's it. To describe the current behavior, it might be better\nwritten as \"Find and show the first function definition prior to the\nhunk\". The code in diff(1) actually just uses the regexp something\nlike \"^[a-z]\" which happens to find function definitions, but can\nalso find module-level variable definitions, structs, etc.\n\n-tkc\n"},{"id":"197859","messageId":"5039FDD7.2090800@gmail.com","threadId":"31309","inReplyTo":"5038CB66.1040006@tim.thechases.com","subject":"Re: misleading diff-hunk header","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-08-26T10:43:35Z","receivedAt":"2012-08-26T10:43:35Z","isPatch":false,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"On 08/25/2012 02:56 PM, Tim Chase wrote:\n> On 08/24/12 23:29, Junio C Hamano wrote:\n>> Tim Chase writes:\n>>> If the documented purpose of \"diff -p\" (and by proxy\n>>> diff.{type}.xfuncname) is to show the name of the *function*\n>>> containing the changed lines,....\n>>\n>> Yeah, the documentation is misleading, but I do not offhand think of\n>> a better phrasing. Perhaps you could send in a patch to improve it.\n>>\n>> How does GNU manual explain the option?\n> \n> Tersely. :-)\n> \n> -p --show-c-function\n> Show which C function each change is in.\n>\nThat's in the manpage, which is basically just a copy of the output from\n\"diff --help\". In the texinfo manual (which is the real documentation),\nthere are additional explanations, saying, among other things:\n\n To show in which functions differences occur for C and similar languages,\n you can use the --show-c-function (-p) option. This option automatically\n defaults to the context output format (see Context Format), with the\n default number of lines of context. You can override that number with\n -C lines elsewhere in the command line. You can override both the format\n and the number with -U lines elsewhere in the command line.\n The -p option is equivalent to -F '^[[:alpha:]$_]' if the unified format\n is specified, otherwise -c -F '^[[:alpha:]$_]' (see Specified Headings).\n GNU diff provides this option for the sake of convenience.\n ...\n The --show-function-line (-F) option finds the nearest unchanged line\n that precedes each hunk of differences and matches the given regular\n expression.\n\nYou can find more information in the on-line documentation:\n\n \n\nHTH,\n Stefano\n"},{"id":"197872","messageId":"7vzk5htrh4.fsf@alter.siamese.dyndns.org","threadId":"31309","inReplyTo":"5039FDD7.2090800@gmail.com","subject":"Re: misleading diff-hunk header","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-26T18:53:27Z","receivedAt":"2012-08-26T18:53:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefano Lattarini writes:\n\n> On 08/25/2012 02:56 PM, Tim Chase wrote:\n>> On 08/24/12 23:29, Junio C Hamano wrote:\n>>> Tim Chase writes:\n>>>> If the documented purpose of \"diff -p\" (and by proxy\n>>>> diff.{type}.xfuncname) is to show the name of the *function*\n>>>> containing the changed lines,....\n>>>\n>>> Yeah, the documentation is misleading, but I do not offhand think of\n>>> a better phrasing. Perhaps you could send in a patch to improve it.\n>>>\n>>> How does GNU manual explain the option?\n>> \n>> Tersely. :-)\n>> \n>> -p --show-c-function\n>> Show which C function each change is in.\n>>\n> That's in the manpage, which is basically just a copy of the output from\n> \"diff --help\". In the texinfo manual (which is the real documentation),\n> there are additional explanations, saying, among other things:\n>\n> To show in which functions differences occur for C and similar languages,\n> you can use the --show-c-function (-p) option. This option automatically\n> defaults to the context output format (see Context Format), with the\n> default number of lines of context. You can override that number with\n> -C lines elsewhere in the command line. You can override both the format\n> and the number with -U lines elsewhere in the command line.\n> The -p option is equivalent to -F '^[[:alpha:]$_]' if the unified format\n> is specified, otherwise -c -F '^[[:alpha:]$_]' (see Specified Headings).\n> GNU diff provides this option for the sake of convenience.\n> ...\n> The --show-function-line (-F) option finds the nearest unchanged line\n> that precedes each hunk of differences and matches the given regular\n> expression.\n\nSo in short, if we say \"Show which function each change is in\" in\nthe documentation, that is consistent with what GNU does and that is\ndescribed consistently with what GNU says, modulo that we obviously\ndo more than \"C\" via the diff..xfuncname mechanism.\n\nWe already document diff..xfuncname as determining \"the hunk\nheader\", and the documentation that is referred to (i.e. gitattributes)\nshows the shape of a hunk in the \"diff\" output:\n\n @@ -k,l +n,m @@ TEXT\n\n This is called a 'hunk header'. The \"TEXT\" portion is by default a line\n that begins with an alphabet, an underscore or a dollar sign; this\n matches what GNU 'diff -p' output uses.\n\nand then later says:\n\n Then, you would define a \"diff.tex.xfuncname\" configuration to\n specify a regular expression that matches a line that you would\n want to appear as the hunk header \"TEXT\".\n\nHonestly, I do not offhand see an obvious and possible room for vast\nimprovement over what we already have, so my RFH to Tim that appears\nat the beginning of what you quoted from my previous message still\nstands ;-).\n\nThanks.\n"}]}