{"thread":{"id":"53839","subject":"--function-context shows wrong function in chunk","startedAt":"2020-07-10T18:38:45Z","lastAt":"2020-07-10T21:13:02Z","messageCount":4,"participants":["Zach Riggle","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"401361","messageId":"CAMP9c5k5kLSqhCh9400iLdZ=+-nKonavOYpBSs_NHdaVVJ_ycQ@mail.gmail.com","threadId":"53839","inReplyTo":null,"subject":"--function-context shows wrong function in chunk","fromName":"Zach Riggle","fromEmail":"zachriggle@gmail.com","sentAt":"2020-07-10T18:38:30Z","receivedAt":"2020-07-10T18:38:45Z","isPatch":false,"sender":{"key":"zachriggle@gmail.com","avatar":"https://gravatar.com/avatar/3eb62c0b142900b8c316fa0dcbf6f84079eb83e0d6fd6db79b44d6034c147604?d=mp&s=160"},"body":"When using --function-context, the function that is claimed at the top\nof the diff does not match the actual function.\n\nNote that the change exists in main, but the hunk header\n(terminology?) shows other_routine.\n\n$ git --version\ngit version 2.27.0\n\n$ git diff -b --function-context\ndiff --git i/example.c w/example.c\nindex d87b59b..346e2a7 100644\n--- i/example.c\n+++ w/example.c\n@@ -4,5 +4,5 @@ int other_routine() {\n }\n\n int main() {\n-\n+    puts(\"Hello, world!\");\n }\n\nZach Riggle\n"},{"id":"401373","messageId":"xmqqk0zbt9h3.fsf@gitster.c.googlers.com","threadId":"53839","inReplyTo":"CAMP9c5k5kLSqhCh9400iLdZ=+-nKonavOYpBSs_NHdaVVJ_ycQ@mail.gmail.com","subject":"Re: --function-context shows wrong function in chunk","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-10T20:12:56Z","receivedAt":"2020-07-10T20:13:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Zach Riggle <zachriggle@gmail.com> writes:\n\n> When using --function-context, the function that is claimed at the top\n> of the diff does not match the actual function.\n>\n> Note that the change exists in main, but the hunk header\n> (terminology?) shows other_routine.\n>\n> $ git --version\n> git version 2.27.0\n>\n> $ git diff -b --function-context\n> diff --git i/example.c w/example.c\n> index d87b59b..346e2a7 100644\n> --- i/example.c\n> +++ w/example.c\n> @@ -4,5 +4,5 @@ int other_routine() {\n>  }\n>\n>  int main() {\n> -\n> +    puts(\"Hello, world!\");\n>  }\n>\n> Zach Riggle\n\nI think it is possible to modify the \"find the line that match\nxfuncname pattern\" logic to start scanning backwards from the first\nactual change (i.e. the blank line in the preimage of the patch\ninside \"int main() {\" function in your example) and make the hunk\nheader say \"int main() {\" instead of \"int other_routine() {\".\n\nI however doubt that such a change makes any sense.  In fact, I find\nthe sample output above both quite logical and also even desirable.\n\nIt is logical because the first thing we see in the hunk, \"}\", is at\nthe end of \"int other_routine() {\" function; it does not conclude\nthe \"int main() {\" function.  Saying \"int main() {\" there on the\nhunk header line would be misleading and confusing.  It sends a\nwrong signal that there was such a line _before_ the first line we\nsee in this hunk, which is definitely not.\n\nIt is desirable because it gives more information than saying the\nillogical \"main\".  The reader can see what the routine before the\nbeginning of main function we see in the hunk is.  In the above\nexample it may not matter, but if we see \"return -1;\" at the end of\nthat function and a call to other_routine() from main(), e.g.\n\n        @@ ... @@ int other_routine() {\n            return -1;\n        }\n        int main() {\n        -   int i = other_routine();\n        +   int i = other_routine() ? 1 : 0;\n            printf(\"%d\\n\", i);\n        }\n\nit would be more informative than having \"int main() {\" there.\n"},{"id":"401375","messageId":"CAMP9c5kzRoVKEQMMVt_tnRMrayP+yT0c+Q2xxewbrZN8piCbHQ@mail.gmail.com","threadId":"53839","inReplyTo":"xmqqk0zbt9h3.fsf@gitster.c.googlers.com","subject":"Re: --function-context shows wrong function in chunk","fromName":"Zach Riggle","fromEmail":"zachriggle@gmail.com","sentAt":"2020-07-10T20:14:31Z","receivedAt":"2020-07-10T20:14:45Z","isPatch":false,"sender":{"key":"zachriggle@gmail.com","avatar":"https://gravatar.com/avatar/3eb62c0b142900b8c316fa0dcbf6f84079eb83e0d6fd6db79b44d6034c147604?d=mp&s=160"},"body":"This happens even with -U0 which does not include the e.g. trailing\nbit of other_routine.\n\n$ git diff -U0 -b --function-context\ndiff --git i/example.c w/example.c\nindex d87b59b..346e2a7 100644\n--- i/example.c\n+++ w/example.c\n@@ -6,3 +6,3 @@ int other_routine() {\n int main() {\n-\n+    puts(\"Hello, world!\");\n }\n\nZach Riggle\n\nOn Fri, Jul 10, 2020 at 3:12 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Zach Riggle <zachriggle@gmail.com> writes:\n>\n> > When using --function-context, the function that is claimed at the top\n> > of the diff does not match the actual function.\n> >\n> > Note that the change exists in main, but the hunk header\n> > (terminology?) shows other_routine.\n> >\n> > $ git --version\n> > git version 2.27.0\n> >\n> > $ git diff -b --function-context\n> > diff --git i/example.c w/example.c\n> > index d87b59b..346e2a7 100644\n> > --- i/example.c\n> > +++ w/example.c\n> > @@ -4,5 +4,5 @@ int other_routine() {\n> >  }\n> >\n> >  int main() {\n> > -\n> > +    puts(\"Hello, world!\");\n> >  }\n> >\n> > Zach Riggle\n>\n> I think it is possible to modify the \"find the line that match\n> xfuncname pattern\" logic to start scanning backwards from the first\n> actual change (i.e. the blank line in the preimage of the patch\n> inside \"int main() {\" function in your example) and make the hunk\n> header say \"int main() {\" instead of \"int other_routine() {\".\n>\n> I however doubt that such a change makes any sense.  In fact, I find\n> the sample output above both quite logical and also even desirable.\n>\n> It is logical because the first thing we see in the hunk, \"}\", is at\n> the end of \"int other_routine() {\" function; it does not conclude\n> the \"int main() {\" function.  Saying \"int main() {\" there on the\n> hunk header line would be misleading and confusing.  It sends a\n> wrong signal that there was such a line _before_ the first line we\n> see in this hunk, which is definitely not.\n>\n> It is desirable because it gives more information than saying the\n> illogical \"main\".  The reader can see what the routine before the\n> beginning of main function we see in the hunk is.  In the above\n> example it may not matter, but if we see \"return -1;\" at the end of\n> that function and a call to other_routine() from main(), e.g.\n>\n>         @@ ... @@ int other_routine() {\n>             return -1;\n>         }\n>         int main() {\n>         -   int i = other_routine();\n>         +   int i = other_routine() ? 1 : 0;\n>             printf(\"%d\\n\", i);\n>         }\n>\n> it would be more informative than having \"int main() {\" there.\n"},{"id":"401384","messageId":"xmqqimevrs4o.fsf@gitster.c.googlers.com","threadId":"53839","inReplyTo":"CAMP9c5kzRoVKEQMMVt_tnRMrayP+yT0c+Q2xxewbrZN8piCbHQ@mail.gmail.com","subject":"Re: --function-context shows wrong function in chunk","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-07-10T21:12:55Z","receivedAt":"2020-07-10T21:13:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Zach Riggle <zachriggle@gmail.com> writes:\n\n[administrivia: do not top-post]\n\n>> I think it is possible to modify the \"find the line that match\n>> xfuncname pattern\" logic to start scanning backwards from the first\n>> actual change (i.e. the blank line in the preimage of the patch\n>> inside \"int main() {\" function in your example) and make the hunk\n>> header say \"int main() {\" instead of \"int other_routine() {\".\n>>\n>> I however doubt that such a change makes any sense.  In fact, I find\n>> the sample output above both quite logical and also even desirable.\n\n> This happens even with -U0 which does not include the e.g. trailing\n> bit of other_routine.\n\nSure, it does not change the equation, though.  We are finding the\nline that matches the xfuncname pattern before the first line of the\nhunk, which is \"int other_routine() {\" in your example.\n\n> $ git diff -U0 -b --function-context\n> diff --git i/example.c w/example.c\n> index d87b59b..346e2a7 100644\n> --- i/example.c\n> +++ w/example.c\n> @@ -6,3 +6,3 @@ int other_routine() {\n>  int main() {\n> -\n> +    puts(\"Hello, world!\");\n>  }\n\nIn your updated example, the line \"int main() {\" is the first line\nof the hunk, and not a line before it.\n\nIf we said \"int main() {\" there, it would send a wrong signal that\nthere is another such line before the \"int main() {\" we see in the\nhunk.\n"}]}