{"thread":{"id":"7155","subject":"Diff output from a rewrite of a function","startedAt":"2007-03-08T19:04:46Z","lastAt":"2007-03-09T19:47:54Z","messageCount":5,"participants":["Ilpo Järvinen","Robin Rosenberg","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"36648","messageId":"Pine.LNX.4.64.0703082049060.19974@kivilampi-30.cs.helsinki.fi","threadId":"7155","inReplyTo":null,"subject":"Diff output from a rewrite of a function","fromName":"Ilpo Järvinen","fromEmail":"ilpo.jarvinen@helsinki.fi","sentAt":"2007-03-08T19:04:46Z","receivedAt":"2007-03-08T19:04:46Z","isPatch":false,"sender":{"key":"ilpo.jarvinen@helsinki.fi","avatar":null},"body":"Hi,\n\nI have a C source file in which couple of functions are fully rewritten \n(only a part of the file), almost no real similarities (block closing \nparents will obviously match still and couple of other keyword lines here \nand there). I wonder if there is way to produce diff that does not get \nconfused by the empty lines / identical lines that are present in both \noriginal and the modified version. Default diff output is very bad looking \n(IMHO) because these identical lines cause a \"synchronization point\" to \noccur, that is, each identical line of the original is not considered as \n+/- but left as is. Thus I have something like 3-6 add+del blocks per \nfunction with a part of the change rather than e.g., one block per \nfunction or so... I found -B from man git-diff-files, but I guess \n\"complete rewrite changes\" means whole files as it did do anything.\n\nAny ideas?\n\nI'm currently using 1.4.2, if that's significant.\n\n-- \n i.\n\nps. Please cc me when replying.\n"},{"id":"36653","messageId":"200703082301.35774.robin.rosenberg.lists@dewire.com","threadId":"7155","inReplyTo":"Pine.LNX.4.64.0703082049060.19974@kivilampi-30.cs.helsinki.fi","subject":"Re: Diff output from a rewrite of a function","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2007-03-08T22:01:35Z","receivedAt":"2007-03-08T22:01:35Z","isPatch":false,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"torsdag 08 mars 2007 20:04 skrev Ilpo Järvinen:\n> Hi,\n> \n> I have a C source file in which couple of functions are fully rewritten \n> (only a part of the file), almost no real similarities (block closing \n> parents will obviously match still and couple of other keyword lines here \n> and there). I wonder if there is way to produce diff that does not get \n> confused by the empty lines / identical lines that are present in both \n> original and the modified version. Default diff output is very bad looking \n> (IMHO) because these identical lines cause a \"synchronization point\" to \n> occur, that is, each identical line of the original is not considered as \n> +/- but left as is. Thus I have something like 3-6 add+del blocks per \n> function with a part of the change rather than e.g., one block per \n> function or so... I found -B from man git-diff-files, but I guess \n> \"complete rewrite changes\" means whole files as it did do anything.\n> \n> Any ideas?\nIncrease the context size from the default three lines.  Something like\ndiff -U 7 old new will require larger chunks of unchanged code for diff\nbreak up a hunk. With git you can do \n\nGIT_DIFF_OPTS=-u7 git-diff-....\n\n> \n> I'm currently using 1.4.2, if that's significant.\n> \nThat was 1.5, but I think that part didn't change.\n\n-- robin\n"},{"id":"36655","messageId":"7vbqj38gkp.fsf@assigned-by-dhcp.cox.net","threadId":"7155","inReplyTo":"200703082301.35774.robin.rosenberg.lists@dewire.com","subject":"Re: Diff output from a rewrite of a function","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-08T23:24:06Z","receivedAt":"2007-03-08T23:24:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:\n\n> Increase the context size from the default three lines.  Something like\n> diff -U 7 old new will require larger chunks of unchanged code for diff\n> break up a hunk. With git you can do \n>\n> GIT_DIFF_OPTS=-u7 git-diff-....\n\nI think you can say \"git diff -U7\" to do what you are\ndescribing, but I do not think that does what you want.  I think\nwhat you want is a \"negative context\", which says \"do not\nconsider group of lines less than N lines as matching between\npreimage and postimage\".  What Ilpo wants is to see something\nlike this:\n\n-\tDeleted 1\n-\tDeleted 2\n-\tDeleted 3\n...\n-\tDeleted 400\n+\tAdded 1\n+\tAdded 2\n+\tAdded 3\n...\n+\tAdded 500\n\t/* happens to match because this was left intact,too */\n-\tDeleted 401\n-\tDeleted 402\n-\tDeleted 403\n+\tAdded 501\n\nas if the small context lines that happen to match are also part\nof the change, like this:\n\n-\tDeleted 1\n-\tDeleted 2\n-\tDeleted 3\n...\n-\tDeleted 400\n-\t/* happens to match because this was left intact,too */\n-\tDeleted 401\n-\tDeleted 402\n-\tDeleted 403\n+\tAdded 1\n+\tAdded 2\n+\tAdded 3\n...\n+\tAdded 500\n+\t/* happens to match because this was left intact,too */\n+\tAdded 501\n"},{"id":"36668","messageId":"Pine.LNX.4.64.0703090927310.22648@kivilampi-30.cs.helsinki.fi","threadId":"7155","inReplyTo":"7vbqj38gkp.fsf@assigned-by-dhcp.cox.net","subject":"Re: Diff output from a rewrite of a function","fromName":"Ilpo Järvinen","fromEmail":"ilpo.jarvinen@helsinki.fi","sentAt":"2007-03-09T08:39:41Z","receivedAt":"2007-03-09T08:39:41Z","isPatch":false,"sender":{"key":"ilpo.jarvinen@helsinki.fi","avatar":null},"body":"On Thu, 8 Mar 2007, Junio C Hamano wrote:\n\n> Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:\n> \n> > Increase the context size from the default three lines.  Something like\n> > diff -U 7 old new will require larger chunks of unchanged code for diff\n> > break up a hunk. With git you can do \n> >\n> > GIT_DIFF_OPTS=-u7 git-diff-....\n> \n> I think you can say \"git diff -U7\" to do what you are\n> describing, but I do not think that does what you want. \n\nTested both and as you predicted. It didn't do the trick... Tested even \nwith 70... :-)\n\n> I think what you want is a \"negative context\", which says \"do not\n> consider group of lines less than N lines as matching between\n> preimage and postimage\".  What Ilpo wants is to see something\n> like this:\n> \n> -\tDeleted 1\n> -\tDeleted 2\n> -\tDeleted 3\n> ...\n> -\tDeleted 400\n> +\tAdded 1\n> +\tAdded 2\n> +\tAdded 3\n> ...\n> +\tAdded 500\n> \t/* happens to match because this was left intact,too */\n> -\tDeleted 401\n> -\tDeleted 402\n> -\tDeleted 403\n> +\tAdded 501\n> \n> as if the small context lines that happen to match are also part\n> of the change, like this:\n> \n> -\tDeleted 1\n> -\tDeleted 2\n> -\tDeleted 3\n> ...\n> -\tDeleted 400\n> -\t/* happens to match because this was left intact,too */\n> -\tDeleted 401\n> -\tDeleted 402\n> -\tDeleted 403\n> +\tAdded 1\n> +\tAdded 2\n> +\tAdded 3\n> ...\n> +\tAdded 500\n> +\t/* happens to match because this was left intact,too */\n> +\tAdded 501\n\nYes, you are showing here exactly what I meant. IMHO the latter would be \neasier for everyone, especially for the review in mailinglists. It's very \nhard to find the correct part to comment from the messed up output, and in \nthe worst case that could be split to two blocks anyway and then there \nwill be deleted lines between the addition of the lines. Besides, the \nlatter would describe the nature of the change much better rather than \ntrying to be \"too intelligent\"... :-)\n\nThough I guess that when the change is included to any repository (don't \nknow enough about git internals to know for sure though), it will again \nperplex everyone looking it from the history because the diff tries again \nto be that intelligent :-)? So in the simplest format this is kind of \nhelper for review only but that's the most important thing as commit \nmessage could then include a note or so that anyone who really wants, can \ncreate a better diff manually with correct options or whatever is \nrequired for that.\n\nMain problem seems to be empty lines which you basically have in almost \nany code (or at least ought to have some of them in any non-trivial code) \nand they're always identical unless bad spacing exists. As I noted earlier \nalso block closing lines and keywords seem the create identical lines \neasily even when you don't have any relation between the code in the \ndifferent versions. It is quite common to say in any code:\n\t\t}\n\t}\nor:\n\t\tbreak;\nor:\n\t\t} else {\netc.\n\nFour lines seem to be the largest I have but that's in the end of a \nfunction so it wouldn't be a big deal (as there are no deleted lines after \nthat point). Here is a distribution of the synchronization (match) point \nsizes for a real change (the previous but quite similar version of the \ndiff is available on netdev of linux kernel if somebody is that interested \nabout it):\nijjarvin@glomgold-39:~$ sort tmp/syncpoints | uniq -c\n     11 1\n      5 2\n      1 3\n      1 4\nijjarvin@glomgold-39:~$ git-diff --stat HEAD^^ HEAD^ net/ipv4/tcp_input.c\n net/ipv4/tcp_input.c |  218 +++++++++++++++++++++++++++++++++++---------------\n 1 files changed, 153 insertions(+), 65 deletions(-)\nijjarvin@kivilampi-30:~/work/src/submit$\n\nTwo of the syncpoints with length of two (and the four lines closing \nthingie) might be considered as ok. The intention of the new and old code \nis to do the same thing but using a totally different algorithm, and those \ntwo twoline blocks actually do logically the same thing, others do not, \nbut due to language keywords and structure, they match. Though, I would \nnot mind if a better diff output couldn't then keep those two lines as \nfrom original.\n\nI could have tried to move the relevant functions to another place in the \nfile but that's sort of hackish approach to a problem I hope my tools \nwould be able to do if I ask it to (not sure if moving them would have \nworked anyway)...\n\nOr alternatively doing a script that creates garbage after each line in \nthe modified functions and then removes the garbage from the diff output \nor so... ;-)\n\n\nI assume it is not possible currently since you didn't suggest anything?\n\n\n-- \n i.\n"},{"id":"36688","messageId":"200703092047.54799.robin.rosenberg.lists@dewire.com","threadId":"7155","inReplyTo":"7vbqj38gkp.fsf@assigned-by-dhcp.cox.net","subject":"Re: Diff output from a rewrite of a function","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2007-03-09T19:47:54Z","receivedAt":"2007-03-09T19:47:54Z","isPatch":false,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"fredag 09 mars 2007 00:24 skrev Junio C Hamano:\n> Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:\n> \n> > Increase the context size from the default three lines.  Something like\n> > diff -U 7 old new will require larger chunks of unchanged code for diff\n> > break up a hunk. With git you can do \n> >\n> > GIT_DIFF_OPTS=-u7 git-diff-....\n> \n> I think you can say \"git diff -U7\" to do what you are\n> describing, but I do not think that does what you want.  \n\nTrue, it's not exactly what I want, but it is easier to read than a diff split \nup in  many small hunks with misleading context. With larger hunks I can \nignore the misleading matching lines in the middle without thinking too much, \nso it kind-o alleviates my problems a little.\n\n-- robin\n"}]}