{"thread":{"id":"8705","subject":"[PATCH] diff: round down similarity index","startedAt":"2007-06-24T22:23:34Z","lastAt":"2007-06-25T10:34:59Z","messageCount":8,"participants":["René Scharfe","Johannes Schindelin","David Kastrup","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"45707","messageId":"467EEEE6.3090100@lsrfire.ath.cx","threadId":"8705","inReplyTo":null,"subject":"[PATCH] diff: round down similarity index","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2007-06-24T22:23:34Z","receivedAt":"2007-06-24T22:23:34Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Rounding down the printed (dis)similarity index allows us to use\n\"100%\" as a special value that indicates complete rewrites and\nfully equal file contents, respectively.\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n\nThe added documentation is only correct if you apply the\ndiffcore-rename patch, too.\n\n Documentation/diff-format.txt |    7 +++++++\n diff.c                        |   30 +++++++++++++++++++-----------\n 2 files changed, 26 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt\nindex 18d49d2..0015032 100644\n--- a/Documentation/diff-format.txt\n+++ b/Documentation/diff-format.txt\n@@ -126,6 +126,13 @@ the file that rename/copy produces, respectively.\n     If there is need for such substitution then the whole\n     pathname is put in double quotes.\n \n+The similarity index is the percentage of unchanged lines, and\n+the dissimilarity index is the percentage of changed lines.  It\n+is a rounded down integer, followed by a percent sign.  The\n+similarity index value of 100% is thus reserved for two equal\n+files, while 100% dissimilarity means that no line from the old\n+file made it into the new one.\n+\n \n combined diff format\n --------------------\ndiff --git a/diff.c b/diff.c\nindex 4aa9bbc..f1db55d 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1813,6 +1813,19 @@ static void diff_fill_sha1_info(struct diff_filespec *one)\n \t\thashclr(one->sha1);\n }\n \n+static int similarity_index(struct diff_filepair *p)\n+{\n+\tint result = p->score * 100.0 / MAX_SCORE;\n+\n+\t/* Paranoia: guard against floating point rounding errors. */\n+\tif (p->score == MAX_SCORE)\n+\t\tresult = 100;\n+\telse if (result == 100)\n+\t\tresult = 99;\n+\n+\treturn result;\n+}\n+\n static void run_diff(struct diff_filepair *p, struct diff_options *o)\n {\n \tconst char *pgm = external_diff();\n@@ -1847,23 +1860,20 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)\n \t\t\t\t\"similarity index %d%%\\n\"\n \t\t\t\t\"copy from %s\\n\"\n \t\t\t\t\"copy to %s\\n\",\n-\t\t\t\t(int)(0.5 + p->score * 100.0/MAX_SCORE),\n-\t\t\t\tname_munged, other_munged);\n+\t\t\t\tsimilarity_index(p), name_munged, other_munged);\n \t\tbreak;\n \tcase DIFF_STATUS_RENAMED:\n \t\tlen += snprintf(msg + len, sizeof(msg) - len,\n \t\t\t\t\"similarity index %d%%\\n\"\n \t\t\t\t\"rename from %s\\n\"\n \t\t\t\t\"rename to %s\\n\",\n-\t\t\t\t(int)(0.5 + p->score * 100.0/MAX_SCORE),\n-\t\t\t\tname_munged, other_munged);\n+\t\t\t\tsimilarity_index(p), name_munged, other_munged);\n \t\tbreak;\n \tcase DIFF_STATUS_MODIFIED:\n \t\tif (p->score) {\n \t\t\tlen += snprintf(msg + len, sizeof(msg) - len,\n \t\t\t\t\t\"dissimilarity index %d%%\\n\",\n-\t\t\t\t\t(int)(0.5 + p->score *\n-\t\t\t\t\t      100.0/MAX_SCORE));\n+\t\t\t\t\tsimilarity_index(p));\n \t\t\tcomplete_rewrite = 1;\n \t\t\tbreak;\n \t\t}\n@@ -2385,8 +2395,7 @@ static void diff_flush_raw(struct diff_filepair *p,\n \t}\n \n \tif (p->score)\n-\t\tsprintf(status, \"%c%03d\", p->status,\n-\t\t\t(int)(0.5 + p->score * 100.0/MAX_SCORE));\n+\t\tsprintf(status, \"%c%03d\", p->status, similarity_index(p));\n \telse {\n \t\tstatus[0] = p->status;\n \t\tstatus[1] = 0;\n@@ -2668,8 +2677,7 @@ static void show_rename_copy(const char *renamecopy, struct diff_filepair *p)\n {\n \tchar *names = pprint_rename(p->one->path, p->two->path);\n \n-\tprintf(\" %s %s (%d%%)\\n\", renamecopy, names,\n-\t       (int)(0.5 + p->score * 100.0/MAX_SCORE));\n+\tprintf(\" %s %s (%d%%)\\n\", renamecopy, names, similarity_index(p));\n \tfree(names);\n \tshow_mode_change(p, 0);\n }\n@@ -2693,7 +2701,7 @@ static void diff_summary(struct diff_filepair *p)\n \t\tif (p->score) {\n \t\t\tchar *name = quote_one(p->two->path);\n \t\t\tprintf(\" rewrite %s (%d%%)\\n\", name,\n-\t\t\t\t(int)(0.5 + p->score * 100.0/MAX_SCORE));\n+\t\t\t       similarity_index(p));\n \t\t\tfree(name);\n \t\t\tshow_mode_change(p, 0);\n \t\t} else\tshow_mode_change(p, 1);\n"},{"id":"45715","messageId":"Pine.LNX.4.64.0706250021250.4059@racer.site","threadId":"8705","inReplyTo":"467EEEE6.3090100@lsrfire.ath.cx","subject":"Re: [PATCH] diff: round down similarity index","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-24T23:25:43Z","receivedAt":"2007-06-24T23:25:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 25 Jun 2007, René Scharfe wrote:\n\n> Rounding down the printed (dis)similarity index allows us to use\n> \"100%\" as a special value that indicates complete rewrites and\n> fully equal file contents, respectively.\n>\n> [...]\n> \n> +static int similarity_index(struct diff_filepair *p)\n> +{\n> +\tint result = p->score * 100.0 / MAX_SCORE;\n> +\n> +\t/* Paranoia: guard against floating point rounding errors. */\n> +\tif (p->score == MAX_SCORE)\n> +\t\tresult = 100;\n> +\telse if (result == 100)\n> +\t\tresult = 99;\n> +\n> +\treturn result;\n> +}\n\nThat's not even properly rounding down. The correct formula (correct in \nthe sense for \"what you want\") would be\n\n\tp->score * 100.0 / MAX_SCORE\n\nif p->score == MAX_SCORE, iff the files are identical. And yes, that is \nthe old formula.\n\nBesides, AFAIR p->score is not even calculated if the files are identical, \nbecause that hits a different code path.\n\nCiao,\nDscho\n"},{"id":"45716","messageId":"Pine.LNX.4.64.0706250026400.4059@racer.site","threadId":"8705","inReplyTo":"Pine.LNX.4.64.0706250021250.4059@racer.site","subject":"Re: [PATCH] diff: round down similarity index","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-24T23:27:17Z","receivedAt":"2007-06-24T23:27:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 25 Jun 2007, Johannes Schindelin wrote:\n\n> On Mon, 25 Jun 2007, Ren� Scharfe wrote:\n> \n> > Rounding down the printed (dis)similarity index allows us to use\n> > \"100%\" as a special value that indicates complete rewrites and\n> > fully equal file contents, respectively.\n> >\n> > [...]\n> > \n> > +static int similarity_index(struct diff_filepair *p)\n> > +{\n> > +\tint result = p->score * 100.0 / MAX_SCORE;\n> > +\n> > +\t/* Paranoia: guard against floating point rounding errors. */\n> > +\tif (p->score == MAX_SCORE)\n> > +\t\tresult = 100;\n> > +\telse if (result == 100)\n> > +\t\tresult = 99;\n> > +\n> > +\treturn result;\n> > +}\n> \n> That's not even properly rounding down. The correct formula (correct in \n> the sense for \"what you want\") would be\n> \n> \tp->score * 100.0 / MAX_SCORE\n> \n> if p->score == MAX_SCORE, iff the files are identical. And yes, that is \n> the old formula.\n\nJust ignore that, please.\n\n> Besides, AFAIR p->score is not even calculated if the files are identical, \n> because that hits a different code path.\n\nBut this still holds true.\n\nCiao,\nDscho\n"},{"id":"45726","messageId":"467F1A77.3030209@lsrfire.ath.cx","threadId":"8705","inReplyTo":"Pine.LNX.4.64.0706250026400.4059@racer.site","subject":"Re: [PATCH] diff: round down similarity index","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2007-06-25T01:29:27Z","receivedAt":"2007-06-25T01:29:27Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Johannes Schindelin schrieb:\n> Hi,\n> \n> On Mon, 25 Jun 2007, Johannes Schindelin wrote:\n> \n>> On Mon, 25 Jun 2007, Ren� Scharfe wrote:\n>>\n>>> Rounding down the printed (dis)similarity index allows us to use\n>>> \"100%\" as a special value that indicates complete rewrites and\n>>> fully equal file contents, respectively.\n>>>\n>>> [...]\n>>>\n>>> +static int similarity_index(struct diff_filepair *p)\n>>> +{\n>>> +\tint result = p->score * 100.0 / MAX_SCORE;\n>>> +\n>>> +\t/* Paranoia: guard against floating point rounding errors. */\n>>> +\tif (p->score == MAX_SCORE)\n>>> +\t\tresult = 100;\n>>> +\telse if (result == 100)\n>>> +\t\tresult = 99;\n>>> +\n>>> +\treturn result;\n>>> +}\n\n>> Besides, AFAIR p->score is not even calculated if the files are identical, \n>> because that hits a different code path.\n\nTrue, ->score is set to MAX_SCORE for identical files by a different\ncode path than the one that actually compares the contents and\ncalculates a score.  That doesn't matter for printing the \"similarity\nindex nn%\" line etc., though.  Or is there a way for identical files to\nend up with a ->score of 0 (or some other value != MAX_SCORE) that I missed?\n\nIn any case, the patch doesn't change the way the score is calculated,\ni.e. its value is the same as before.  It only changes how it is displayed.\n\nRené\n"},{"id":"45731","messageId":"85y7i8boyh.fsf@lola.goethe.zz","threadId":"8705","inReplyTo":"467EEEE6.3090100@lsrfire.ath.cx","subject":"Re: [PATCH] diff: round down similarity index","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-06-25T05:08:54Z","receivedAt":"2007-06-25T05:08:54Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n\n> +static int similarity_index(struct diff_filepair *p)\n> +{\n> +\tint result = p->score * 100.0 / MAX_SCORE;\n\nUse floor(p->score ... MAX_SCORE) here: I don't think that C otherwise\nspecifies a preferred way of rounding on float->int conversions.\n\n> +\t/* Paranoia: guard against floating point rounding errors. */\n> +\tif (p->score == MAX_SCORE)\n> +\t\tresult = 100;\n> +\telse if (result == 100)\n> +\t\tresult = 99;\n\nAlso, p->score itself must be calculated by truncating.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"45734","messageId":"7vir9co8ue.fsf@assigned-by-dhcp.cox.net","threadId":"8705","inReplyTo":"467EEEE6.3090100@lsrfire.ath.cx","subject":"Re: [PATCH] diff: round down similarity index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-25T06:18:33Z","receivedAt":"2007-06-25T06:18:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n\n> +static int similarity_index(struct diff_filepair *p)\n> +{\n> +\tint result = p->score * 100.0 / MAX_SCORE;\n> +\n> +\t/* Paranoia: guard against floating point rounding errors. */\n> +\tif (p->score == MAX_SCORE)\n> +\t\tresult = 100;\n> +\telse if (result == 100)\n> +\t\tresult = 99;\n> +\n> +\treturn result;\n> +}\n\nWhy not simply do this?\n\n        static int similarity_index(struct diff_filepair *p)\n        {\n                if (p->score == MAX_SCORE)\n                        return 100;\n                return p->score * 100 / MAX_SCORE;\n        }\n\nMAX_SCORE and p->score run up to 60000 and we assume int is at\nleast 32-bit, so I do not think there is no risk of overflowing.\n"},{"id":"45759","messageId":"467F9A30.7040201@lsrfire.ath.cx","threadId":"8705","inReplyTo":"86k5tsh761.fsf@lola.quinscape.zz","subject":"Re: [PATCH] diff: round down similarity index","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2007-06-25T10:34:24Z","receivedAt":"2007-06-25T10:34:24Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"David Kastrup schrieb:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n>> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n>>\n>>> +static int similarity_index(struct diff_filepair *p)\n>>> +{\n>>> +\tint result = p->score * 100.0 / MAX_SCORE;\n>>> +\n>>> +\t/* Paranoia: guard against floating point rounding errors. */\n>>> +\tif (p->score == MAX_SCORE)\n>>> +\t\tresult = 100;\n>>> +\telse if (result == 100)\n>>> +\t\tresult = 99;\n>>> +\n>>> +\treturn result;\n>>> +}\n>> Why not simply do this?\n>>\n>>         static int similarity_index(struct diff_filepair *p)\n>>         {\n>>                 if (p->score == MAX_SCORE)\n>>                         return 100;\n>>                 return p->score * 100 / MAX_SCORE;\n>>         }\n>>\n>> MAX_SCORE and p->score run up to 60000 and we assume int is at\n>> least 32-bit, so I do not think there is no risk of overflowing.\n\nGood idea!  *smacks himself against the forehead*\n\nI just copied the old code (sans the addition of 0.5) and then tried\nto work around it's rounding problem (which I'm not even certain\nexists and matters).  Look out for an updated patch..\n\n> You bet me to it on the way to work.  Anyway, when working with\n> integers, the first two lines are definitely not required (I am\n> assuming that p->score is integer as well but have not checked).\n\nRight.\n\n> What probably requires more thought is the calculation of p->score\n> itself: if one has large numbers as a ratio (original vs changes\n> lines), multiplying by 60000 before dividing by the second large\n> numbers might exceed the size of the integers.\n\nMAX_SCORE is defined as 60000.0, which makes it a double.  AFAICS all\nthe multiplications with MAX_SCORE are done using floating point\narithmetic for that reason.\n\nRené\n"},{"id":"45760","messageId":"467F9A53.5000405@lsrfire.ath.cx","threadId":"8705","inReplyTo":"7vir9co8ue.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] diff: round down similarity index, take 2","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2007-06-25T10:34:59Z","receivedAt":"2007-06-25T10:34:59Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Rounding down the printed (dis)similarity index allows us to use\n\"100%\" as a special value that indicates complete rewrites and\nfully equal file contents, respectively.\n\nWhile at it, switch index calculation to integer arithmetic as\nsuggested by Junio and David Kastrup.\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n\nThe added documentation is only correct if you apply the\ndiffcore-rename patch, too.\n\n Documentation/diff-format.txt |    7 +++++++\n diff.c                        |   22 +++++++++++-----------\n 2 files changed, 18 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt\nindex 18d49d2..0015032 100644\n--- a/Documentation/diff-format.txt\n+++ b/Documentation/diff-format.txt\n@@ -126,6 +126,13 @@ the file that rename/copy produces, respectively.\n     If there is need for such substitution then the whole\n     pathname is put in double quotes.\n \n+The similarity index is the percentage of unchanged lines, and\n+the dissimilarity index is the percentage of changed lines.  It\n+is a rounded down integer, followed by a percent sign.  The\n+similarity index value of 100% is thus reserved for two equal\n+files, while 100% dissimilarity means that no line from the old\n+file made it into the new one.\n+\n \n combined diff format\n --------------------\ndiff --git a/diff.c b/diff.c\nindex 9938969..81527bf 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1813,6 +1813,11 @@ static void diff_fill_sha1_info(struct diff_filespec *one)\n \t\thashclr(one->sha1);\n }\n \n+static int similarity_index(struct diff_filepair *p)\n+{\n+\treturn p->score * 100 / (int)MAX_SCORE;\n+}\n+\n static void run_diff(struct diff_filepair *p, struct diff_options *o)\n {\n \tconst char *pgm = external_diff();\n@@ -1847,23 +1852,20 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)\n \t\t\t\t\"similarity index %d%%\\n\"\n \t\t\t\t\"copy from %s\\n\"\n \t\t\t\t\"copy to %s\\n\",\n-\t\t\t\t(int)(0.5 + p->score * 100.0/MAX_SCORE),\n-\t\t\t\tname_munged, other_munged);\n+\t\t\t\tsimilarity_index(p), name_munged, other_munged);\n \t\tbreak;\n \tcase DIFF_STATUS_RENAMED:\n \t\tlen += snprintf(msg + len, sizeof(msg) - len,\n \t\t\t\t\"similarity index %d%%\\n\"\n \t\t\t\t\"rename from %s\\n\"\n \t\t\t\t\"rename to %s\\n\",\n-\t\t\t\t(int)(0.5 + p->score * 100.0/MAX_SCORE),\n-\t\t\t\tname_munged, other_munged);\n+\t\t\t\tsimilarity_index(p), name_munged, other_munged);\n \t\tbreak;\n \tcase DIFF_STATUS_MODIFIED:\n \t\tif (p->score) {\n \t\t\tlen += snprintf(msg + len, sizeof(msg) - len,\n \t\t\t\t\t\"dissimilarity index %d%%\\n\",\n-\t\t\t\t\t(int)(0.5 + p->score *\n-\t\t\t\t\t      100.0/MAX_SCORE));\n+\t\t\t\t\tsimilarity_index(p));\n \t\t\tcomplete_rewrite = 1;\n \t\t\tbreak;\n \t\t}\n@@ -2387,8 +2389,7 @@ static void diff_flush_raw(struct diff_filepair *p,\n \t}\n \n \tif (p->score)\n-\t\tsprintf(status, \"%c%03d\", p->status,\n-\t\t\t(int)(0.5 + p->score * 100.0/MAX_SCORE));\n+\t\tsprintf(status, \"%c%03d\", p->status, similarity_index(p));\n \telse {\n \t\tstatus[0] = p->status;\n \t\tstatus[1] = 0;\n@@ -2670,8 +2671,7 @@ static void show_rename_copy(const char *renamecopy, struct diff_filepair *p)\n {\n \tchar *names = pprint_rename(p->one->path, p->two->path);\n \n-\tprintf(\" %s %s (%d%%)\\n\", renamecopy, names,\n-\t       (int)(0.5 + p->score * 100.0/MAX_SCORE));\n+\tprintf(\" %s %s (%d%%)\\n\", renamecopy, names, similarity_index(p));\n \tfree(names);\n \tshow_mode_change(p, 0);\n }\n@@ -2695,7 +2695,7 @@ static void diff_summary(struct diff_filepair *p)\n \t\tif (p->score) {\n \t\t\tchar *name = quote_one(p->two->path);\n \t\t\tprintf(\" rewrite %s (%d%%)\\n\", name,\n-\t\t\t\t(int)(0.5 + p->score * 100.0/MAX_SCORE));\n+\t\t\t       similarity_index(p));\n \t\t\tfree(name);\n \t\t\tshow_mode_change(p, 0);\n \t\t} else\tshow_mode_change(p, 1);\n"}]}