{"thread":{"id":"751","subject":"[PATCH] Add -B flag to diff-* brothers.","startedAt":"2005-05-28T19:33:44Z","lastAt":"2005-05-29T23:51:17Z","messageCount":4,"participants":["Junio C Hamano","Petr Baudis"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"4146","messageId":"7vbr6vrvqf.fsf@assigned-by-dhcp.cox.net","threadId":"751","inReplyTo":null,"subject":"[PATCH] Add -B flag to diff-* brothers.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-28T19:33:44Z","receivedAt":"2005-05-28T19:33:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This introduces diffcore-break.c, a new diffcore\ntransformation.\n\nWhen the -B flag is given, a patch that represents a complete\nrewrite is broken into a deletion followed by a creation.  This\nmakes it easier to review such a complete rewrite patch.\n\nThe -B flag takes the same syntax as the -M and -C flags to\nspecify the maximum amount of source material the resulting file\ncan still have to be considered a complete rewrite, and defaults\nto 15% if not specified.  For example, this detects the complete\nrewrite of ls-tree.c I sent earlier.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\nMakefile          |    3 -\ndiff-cache.c      |    7 +++\ndiff-files.c      |    5 ++\ndiff-tree.c       |    7 +++\ndiff.h            |    2 \ndiffcore-break.c  |  110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\ndiffcore-rename.c |    4 -\ndiffcore.h        |    3 -\n8 files changed, 137 insertions(+), 4 deletions(-)\nnew file (100644): diffcore-break.c\n\ndiff --git a/Makefile b/Makefile\n--- a/Makefile\n+++ b/Makefile\n@@ -48,7 +48,7 @@ LIB_OBJS += strbuf.o\n \n LIB_H += diff.h count-delta.h\n LIB_OBJS += diff.o diffcore-rename.o diffcore-pickaxe.o diffcore-pathspec.o \\\n-\tcount-delta.o diffcore-order.o\n+\tcount-delta.o diffcore-order.o diffcore-break.o\n \n LIB_OBJS += gitenv.o\n \n@@ -131,6 +131,7 @@ diffcore-rename.o : $(LIB_H) diffcore.h\n diffcore-pathspec.o : $(LIB_H) diffcore.h\n diffcore-pickaxe.o : $(LIB_H) diffcore.h\n diffcore-order.o : $(LIB_H) diffcore.h\n+diffcore-break.o : $(LIB_H) diffcore.h\n \n test: all\n \t$(MAKE) -C t/ all\ndiff --git a/diff-cache.c b/diff-cache.c\n--- a/diff-cache.c\n+++ b/diff-cache.c\n@@ -9,6 +9,7 @@ static int diff_setup_opt = 0;\n static int diff_score_opt = 0;\n static const char *pickaxe = NULL;\n static int pickaxe_opts = 0;\n+static int diff_break_opt = -1;\n static const char *orderfile = NULL;\n \n /* A file entry went away or appeared */\n@@ -189,6 +190,10 @@ int main(int argc, const char **argv)\n \t\t\tdiff_output_format = DIFF_FORMAT_PATCH;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strncmp(arg, \"-B\", 2)) {\n+\t\t\tdiff_break_opt = diff_scoreopt_parse(arg);\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strncmp(arg, \"-M\", 2)) {\n \t\t\tdetect_rename = DIFF_DETECT_RENAME;\n \t\t\tdiff_score_opt = diff_scoreopt_parse(arg);\n@@ -251,6 +256,8 @@ int main(int argc, const char **argv)\n \t\tdiffcore_rename(detect_rename, diff_score_opt);\n \tif (pickaxe)\n \t\tdiffcore_pickaxe(pickaxe, pickaxe_opts);\n+\tif (0 <= diff_break_opt)\n+\t\tdiffcore_break(diff_break_opt);\n \tif (orderfile)\n \t\tdiffcore_order(orderfile);\n \tdiff_flush(diff_output_format, 1);\ndiff --git a/diff-files.c b/diff-files.c\n--- a/diff-files.c\n+++ b/diff-files.c\n@@ -15,6 +15,7 @@ static int diff_setup_opt = 0;\n static int diff_score_opt = 0;\n static const char *pickaxe = NULL;\n static int pickaxe_opts = 0;\n+static int diff_break_opt = -1;\n static const char *orderfile = NULL;\n static int silent = 0;\n \n@@ -60,6 +61,8 @@ int main(int argc, const char **argv)\n \t\t\torderfile = argv[1] + 2;\n \t\telse if (!strcmp(argv[1], \"--pickaxe-all\"))\n \t\t\tpickaxe_opts = DIFF_PICKAXE_ALL;\n+\t\telse if (!strncmp(argv[1], \"-B\", 2))\n+\t\t\tdiff_break_opt = diff_scoreopt_parse(argv[1]);\n \t\telse if (!strncmp(argv[1], \"-M\", 2)) {\n \t\t\tdiff_score_opt = diff_scoreopt_parse(argv[1]);\n \t\t\tdetect_rename = DIFF_DETECT_RENAME;\n@@ -125,6 +128,8 @@ int main(int argc, const char **argv)\n \t\tdiffcore_rename(detect_rename, diff_score_opt);\n \tif (pickaxe)\n \t\tdiffcore_pickaxe(pickaxe, pickaxe_opts);\n+\tif (0 <= diff_break_opt)\n+\t\tdiffcore_break(diff_break_opt);\n \tif (orderfile)\n \t\tdiffcore_order(orderfile);\n \tdiff_flush(diff_output_format, 1);\ndiff --git a/diff-tree.c b/diff-tree.c\n--- a/diff-tree.c\n+++ b/diff-tree.c\n@@ -14,6 +14,7 @@ static int diff_setup_opt = 0;\n static int diff_score_opt = 0;\n static const char *pickaxe = NULL;\n static int pickaxe_opts = 0;\n+static int diff_break_opt = -1;\n static const char *orderfile = NULL;\n static const char *header = NULL;\n static const char *header_prefix = \"\";\n@@ -266,6 +267,8 @@ static int call_diff_flush(void)\n \t\tdiffcore_rename(detect_rename, diff_score_opt);\n \tif (pickaxe)\n \t\tdiffcore_pickaxe(pickaxe, pickaxe_opts);\n+\tif (0 <= diff_break_opt)\n+\t\tdiffcore_break(diff_break_opt);\n \tif (diff_queue_is_empty()) {\n \t\tdiff_flush(DIFF_FORMAT_NO_OUTPUT, 0);\n \t\treturn 0;\n@@ -531,6 +534,10 @@ int main(int argc, const char **argv)\n \t\t\tdiff_score_opt = diff_scoreopt_parse(arg);\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strncmp(arg, \"-B\", 2)) {\n+\t\t\tdiff_break_opt = diff_scoreopt_parse(arg);\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(arg, \"-z\")) {\n \t\t\tdiff_output_format = DIFF_FORMAT_MACHINE;\n \t\t\tcontinue;\ndiff --git a/diff.h b/diff.h\n--- a/diff.h\n+++ b/diff.h\n@@ -45,6 +45,8 @@ extern void diffcore_pathspec(const char\n \n extern void diffcore_order(const char *orderfile);\n \n+extern void diffcore_break(int max_score);\n+\n extern int diff_queue_is_empty(void);\n \n #define DIFF_FORMAT_HUMAN\t0\ndiff --git a/diffcore-break.c b/diffcore-break.c\nnew file mode 100644\n--- /dev/null\n+++ b/diffcore-break.c\n@@ -0,0 +1,110 @@\n+/*\n+ * Copyright (C) 2005 Junio C Hamano\n+ */\n+#include \"cache.h\"\n+#include \"diff.h\"\n+#include \"diffcore.h\"\n+#include \"delta.h\"\n+#include \"count-delta.h\"\n+\n+static int very_different(struct diff_filespec *src,\n+\t\t\t  struct diff_filespec *dst,\n+\t\t\t  int max_score)\n+{\n+\t/* dst is recorded as a modification of src.  Are they so\n+\t * different that we are better off recording this as a pair\n+\t * of delete and create?  max_score is maximum size that is\n+\t * common between src and dst for the pair to be still considered\n+\t * a change (not delete and create), and typically set to 5-10%.\n+\t */\n+\tvoid *delta;\n+\tunsigned long delta_size, base_size;\n+\n+\tif (!S_ISREG(src->mode) || !S_ISREG(dst->mode))\n+\t\treturn 0; /* leave symlink rename alone */\n+\n+\tif (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1))\n+\t\treturn 0; /* error but caught downstream */\n+\n+\tdelta_size = ((src->size < dst->size) ?\n+\t\t      (dst->size - src->size) : (src->size - dst->size));\n+\tbase_size = ((src->size < dst->size) ? src->size : dst->size);\n+\n+\t/*\n+\t * If delta size is larger than\n+\t * (MAX_SCORE-max_score)/MAX_SCORE * min(src->size, dst->size)\n+\t * then we declare this is too big a change to be a patch.\n+\t */\n+\tif (base_size * (MAX_SCORE-max_score) < delta_size * MAX_SCORE)\n+\t\treturn 1;\n+\n+\tif (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0))\n+\t\treturn 0; /* error but caught downstream */\n+\n+\tdelta = diff_delta(src->data, src->size,\n+\t\t\t   dst->data, dst->size,\n+\t\t\t   &delta_size);\n+\n+\t/* A delta that has a lot of literal additions would have\n+\t * big delta_size no matter what else it does.\n+\t */\n+\tif (base_size * (MAX_SCORE-max_score) < delta_size * MAX_SCORE)\n+\t\treturn 1;\n+\n+\t/* Estimate the edit size by interpreting delta. */\n+\tdelta_size = count_delta(delta, delta_size);\n+\tfree(delta);\n+\tif (delta_size == UINT_MAX)\n+\t\treturn 0; /* error in delta computation */\n+\t/*\n+\t * So how big is the edit?\n+\t */\n+\tif (base_size * (MAX_SCORE-max_score) < delta_size * MAX_SCORE)\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n+void diffcore_break(int max_score)\n+{\n+\tstruct diff_queue_struct *q = &diff_queued_diff;\n+\tstruct diff_queue_struct outq;\n+\tint i;\n+\n+\tif (!max_score)\n+\t\tmax_score = DEFAULT_MAXIMUM_SCORE;\n+\n+\toutq.nr = outq.alloc = 0;\n+\toutq.queue = NULL;\n+\n+\tfor (i = 0; i < q->nr; i++) {\n+\t\tstruct diff_filepair *p = q->queue[i];\n+\t\t/* We deal only with in-place edit of non directory */\n+\t\tif (DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two) &&\n+\t\t    !S_ISDIR(p->one->mode) && !S_ISDIR(p->two->mode) &&\n+\t\t    !strcmp(p->one->path, p->two->path) &&\n+\t\t    very_different(p->one, p->two, max_score)) {\n+\t\t\t/* Split this into delete and create */\n+\t\t\tstruct diff_filespec *one_pre, *one_post,\n+\t\t\t\t*two_pre, *two_post;\n+\n+\t\t\t/* deletion of one */\n+\t\t\tone_pre = alloc_filespec(p->one->path);\n+\t\t\tfill_filespec(one_pre, p->one->sha1, p->one->mode);\n+\t\t\tone_post = alloc_filespec(p->one->path);\n+\t\t\tdiff_queue(&outq, one_pre, one_post);\n+\n+\t\t\t/* creation of two */\n+\t\t\ttwo_pre = alloc_filespec(p->two->path);\n+\t\t\ttwo_post = alloc_filespec(p->two->path);\n+\t\t\tfill_filespec(two_post, p->two->sha1, p->two->mode);\n+\t\t\tdiff_queue(&outq, two_pre, two_post);\n+\t\t\tdiff_free_filepair(p);\n+\t\t}\n+\t\telse\n+\t\t\tdiff_q(&outq, p);\n+\t}\n+\tfree(q->queue);\n+\t*q = outq;\n+\n+\treturn;\n+}\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -208,8 +208,8 @@ static int score_compare(const void *a_,\n int diff_scoreopt_parse(const char *opt)\n {\n \tint diglen, num, scale, i;\n-\tif (opt[0] != '-' || (opt[1] != 'M' && opt[1] != 'C'))\n-\t\treturn -1; /* that is not a -M nor -C option */\n+\tif (opt[0] != '-' || (opt[1] != 'M' && opt[1] != 'C' && opt[1] != 'B'))\n+\t\treturn -1; /* that is not a -M, -C nor -B option */\n \tdiglen = strspn(opt+2, \"0123456789\");\n \tif (diglen == 0 || strlen(opt+2) != diglen)\n \t\treturn 0; /* use default */\ndiff --git a/diffcore.h b/diffcore.h\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -9,7 +9,8 @@\n  * in anything else.\n  */\n #define MAX_SCORE 10000\n-#define DEFAULT_MINIMUM_SCORE 5000\n+#define DEFAULT_MINIMUM_SCORE 5000 /* rename/copy similarity minimum */\n+#define DEFAULT_MAXIMUM_SCORE 1500 /* similarity maximum for break to happen */\n \n #define RENAME_DST_MATCHED 01\n \n------------------------------------------------\n\n"},{"id":"4161","messageId":"7vll5ypf5y.fsf@assigned-by-dhcp.cox.net","threadId":"751","inReplyTo":"7vbr6vrvqf.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] diff: move diffcore-break before diffcore-rename.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-29T09:14:33Z","receivedAt":"2005-05-29T09:14:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The order diffcore-break is applied has changed.  Earlier it\ncame fairly late in the diffcore chain, but now it comes before\ndiffcore-rename, which makes diffcore-break far more useful than\nbefore.\n\nAs the new test t4008-diff-break-rewrite demonstrates, if a file\nis a complete rewrite, it is broken and then can be subjected to\nthe usual rename detection.  For example, if file0 gets\ncompletely rewritten to make it as if it was rather based on\nfile1 which disappeared, diff-* -B -M would do the following:\n\n    The original change would look like this:\n\n\tfile0     --> file0' (quite different from file0)\n\tfile1     --> /dev/null\n\n    After diffcore-break, it would become this:\n\n\tfile0     --> /dev/null\n\t/dev/null --> file0'\n\tfile1     --> /dev/null\n\n    Then diffcore-rename matches them up:\n\n\tfile1     --> file0'\n\nThe meaning of the parameter the -B option takes has changed\nfrom the previous one.  Earlier it meant the amount of source\nmaterials permitted to remain for the file and still be\nconsidered a rewrite, which meant that the lower the number, the\ntighter the detection criteria.  This was changed to match the\nway the parameters to the -C and the -M options are used, where\nlarger parameter value means tighter detection criteria.  The\nparameter for the -B option now means how much new material must\nbe in the resulting file for the filepair to be considered a\nrewrite, and defaults to 99%.\n\nThe internal score values are finer grained now.  Earlier\nmaximum of 10000 has been raised to 60000; there is no user\nvisible changes but there is no reason to waste available bits.\n\nDocumentation now mentions the -B option.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\nDocumentation/git-diff-cache.txt |    5 -\nDocumentation/git-diff-files.txt |    5 -\nDocumentation/git-diff-tree.txt  |    5 -\ndiff.c                           |   20 +++-\ndiffcore-break.c                 |  101 +++++++++++++----------\ndiffcore-rename.c                |   64 ++++++++++++--\ndiffcore.h                       |   13 ++-\nt/t4008-diff-break-rewrite.sh    |  169 +++++++++++++++++++++++++++++++++++++++\n8 files changed, 317 insertions(+), 65 deletions(-)\nnew file (100755): t/t4008-diff-break-rewrite.sh\n\ndiff --git a/Documentation/git-diff-cache.txt b/Documentation/git-diff-cache.txt\n--- a/Documentation/git-diff-cache.txt\n+++ b/Documentation/git-diff-cache.txt\n@@ -9,7 +9,7 @@ git-diff-cache - Compares content and mo\n \n SYNOPSIS\n --------\n-'git-diff-cache' [-p] [-r] [-z] [-m] [-M] [-R] [-C] [-O<orderfile>] [-S<string>] [--pickaxe-all] [--cached] <tree-ish> [<path>...]\n+'git-diff-cache' [-p] [-r] [-z] [-m] [-B] [-M] [-R] [-C] [-O<orderfile>] [-S<string>] [--pickaxe-all] [--cached] <tree-ish> [<path>...]\n \n DESCRIPTION\n -----------\n@@ -35,6 +35,9 @@ OPTIONS\n -z::\n \t\\0 line termination on output\n \n+-B::\n+\tBreak complete rewrite changes into pairs of delete and create.\n+\n -M::\n \tDetect renames.\n \ndiff --git a/Documentation/git-diff-files.txt b/Documentation/git-diff-files.txt\n--- a/Documentation/git-diff-files.txt\n+++ b/Documentation/git-diff-files.txt\n@@ -9,7 +9,7 @@ git-diff-files - Compares files in the w\n \n SYNOPSIS\n --------\n-'git-diff-files' [-p] [-q] [-r] [-z] [-M] [-C] [-R] [-O<orderfile>] [-S<string>] [--pickaxe-all] [<pattern>...]\n+'git-diff-files' [-p] [-q] [-r] [-z] [-B] [-M] [-C] [-R] [-O<orderfile>] [-S<string>] [--pickaxe-all] [<pattern>...]\n \n DESCRIPTION\n -----------\n@@ -29,6 +29,9 @@ OPTIONS\n -R::\n \tOutput diff in reverse.\n \n+-B::\n+\tBreak complete rewrite changes into pairs of delete and create.\n+\n -M::\n \tDetect renames.\n \ndiff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt\n--- a/Documentation/git-diff-tree.txt\n+++ b/Documentation/git-diff-tree.txt\n@@ -9,7 +9,7 @@ git-diff-tree - Compares the content and\n \n SYNOPSIS\n --------\n-'git-diff-tree' [-p] [-r] [-z] [--stdin] [-M] [-R] [-C] [-O<orderfile>] [-S<string>] [--pickaxe-all] [-m] [-s] [-v] [-t] <tree-ish> <tree-ish> [<pattern>]\\*\n+'git-diff-tree' [-p] [-r] [-z] [--stdin] [-B] [-M] [-R] [-C] [-O<orderfile>] [-S<string>] [--pickaxe-all] [-m] [-s] [-v] [-t] <tree-ish> <tree-ish> [<pattern>]\\*\n \n DESCRIPTION\n -----------\n@@ -33,6 +33,9 @@ OPTIONS\n \tgenerate patch (see section on generating patches).  For\n \tgit-diff-tree, this flag implies '-r' as well.\n \n+-B::\n+\tBreak complete rewrite changes into pairs of delete and create.\n+\n -M::\n \tDetect renames.\n \ndiff --git a/diff.c b/diff.c\n--- a/diff.c\n+++ b/diff.c\n@@ -609,6 +609,7 @@ struct diff_filepair *diff_queue(struct \n \tdp->two = two;\n \tdp->score = 0;\n \tdp->source_stays = 0;\n+\tdp->broken_pair = 0;\n \tdiff_q(queue, dp);\n \treturn dp;\n }\n@@ -643,6 +644,16 @@ static void diff_flush_raw(struct diff_f\n \t\tsprintf(status, \"%c%03d\", p->status,\n \t\t\t(int)(0.5 + p->score * 100.0/MAX_SCORE));\n \t\tbreak;\n+\tcase 'N': case 'D':\n+\t\ttwo_paths = 0;\n+\t\tif (p->score)\n+\t\t\tsprintf(status, \"%c%03d\", p->status,\n+\t\t\t\t(int)(0.5 + p->score * 100.0/MAX_SCORE));\n+\t\telse {\n+\t\t\tstatus[0] = p->status;\n+\t\t\tstatus[1] = 0;\n+\t\t}\n+\t\tbreak;\n \tdefault:\n \t\ttwo_paths = 0;\n \t\tstatus[0] = p->status;\n@@ -799,8 +810,9 @@ void diff_debug_filepair(const struct di\n {\n \tdiff_debug_filespec(p->one, i, \"one\");\n \tdiff_debug_filespec(p->two, i, \"two\");\n-\tfprintf(stderr, \"score %d, status %c source_stays %d\\n\",\n-\t\tp->score, p->status ? : '?', p->source_stays);\n+\tfprintf(stderr, \"score %d, status %c stays %d broken %d\\n\",\n+\t\tp->score, p->status ? : '?',\n+\t\tp->source_stays, p->broken_pair);\n }\n \n void diff_debug_queue(const char *msg, struct diff_queue_struct *q)\n@@ -939,12 +951,12 @@ void diffcore_std(const char **paths,\n {\n \tif (paths)\n \t\tdiffcore_pathspec(paths);\n+\tif (0 <= break_opt)\n+\t\tdiffcore_break(break_opt);\n \tif (detect_rename)\n \t\tdiffcore_rename(detect_rename, rename_score);\n \tif (pickaxe)\n \t\tdiffcore_pickaxe(pickaxe, pickaxe_opts);\n-\tif (0 <= break_opt)\n-\t\tdiffcore_break(break_opt);\n \tif (orderfile)\n \t\tdiffcore_order(orderfile);\n }\ndiff --git a/diffcore-break.c b/diffcore-break.c\n--- a/diffcore-break.c\n+++ b/diffcore-break.c\n@@ -9,13 +9,18 @@\n \n static int very_different(struct diff_filespec *src,\n \t\t\t  struct diff_filespec *dst,\n-\t\t\t  int max_score)\n+\t\t\t  int min_score)\n {\n \t/* dst is recorded as a modification of src.  Are they so\n \t * different that we are better off recording this as a pair\n-\t * of delete and create?  max_score is maximum size that is\n-\t * common between src and dst for the pair to be still considered\n-\t * a change (not delete and create), and typically set to 5-10%.\n+\t * of delete and create?  min_score is the minimum amount of\n+\t * new material that must exist in the dst and not in src for\n+\t * the pair to be considered a complete rewrite, and recommended\n+\t * to be set to a very high value, 99% or so.\n+\t *\n+\t * The value we return represents the amount of new material\n+\t * that is in dst and not in src.  We return 0 when we do not\n+\t * want to get the filepair broken.\n \t */\n \tvoid *delta;\n \tunsigned long delta_size, base_size;\n@@ -28,15 +33,19 @@ static int very_different(struct diff_fi\n \n \tdelta_size = ((src->size < dst->size) ?\n \t\t      (dst->size - src->size) : (src->size - dst->size));\n-\tbase_size = ((src->size < dst->size) ? src->size : dst->size);\n+\n+\t/* Notice that we use max of src and dst as the base size,\n+\t * unlike rename similarity detection.  This is so that we do\n+\t * not mistake a large addition as a complete rewrite.\n+\t */\n+\tbase_size = ((src->size < dst->size) ? dst->size : src->size);\n \n \t/*\n-\t * If delta size is larger than\n-\t * (MAX_SCORE-max_score)/MAX_SCORE * min(src->size, dst->size)\n-\t * then we declare this is too big a change to be a patch.\n+\t * If file size difference is too big compared to the\n+\t * base_size, we declare this a complete rewrite.\n \t */\n-\tif (base_size * (MAX_SCORE-max_score) < delta_size * MAX_SCORE)\n-\t\treturn 1;\n+\tif (base_size * min_score < delta_size * MAX_SCORE)\n+\t\treturn MAX_SCORE;\n \n \tif (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0))\n \t\treturn 0; /* error but caught downstream */\n@@ -48,60 +57,68 @@ static int very_different(struct diff_fi\n \t/* A delta that has a lot of literal additions would have\n \t * big delta_size no matter what else it does.\n \t */\n-\tif (base_size * (MAX_SCORE-max_score) < delta_size * MAX_SCORE)\n-\t\treturn 1;\n+\tif (base_size * min_score < delta_size * MAX_SCORE)\n+\t\treturn MAX_SCORE;\n \n \t/* Estimate the edit size by interpreting delta. */\n \tdelta_size = count_delta(delta, delta_size);\n \tfree(delta);\n \tif (delta_size == UINT_MAX)\n \t\treturn 0; /* error in delta computation */\n-\t/*\n-\t * So how big is the edit?\n-\t */\n-\tif (base_size * (MAX_SCORE-max_score) < delta_size * MAX_SCORE)\n-\t\treturn 1;\n-\treturn 0;\n+\n+\tif (base_size < delta_size)\n+\t\treturn MAX_SCORE;\n+\n+\treturn delta_size * MAX_SCORE / base_size; \n }\n \n-void diffcore_break(int max_score)\n+void diffcore_break(int min_score)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tstruct diff_queue_struct outq;\n \tint i;\n \n-\tif (!max_score)\n-\t\tmax_score = DEFAULT_MAXIMUM_SCORE;\n+\tif (!min_score)\n+\t\tmin_score = DEFAULT_BREAK_SCORE;\n \n \toutq.nr = outq.alloc = 0;\n \toutq.queue = NULL;\n \n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filepair *p = q->queue[i];\n-\t\t/* We deal only with in-place edit of non directory */\n+\t\tint score;\n+\n+\t\t/* We deal only with in-place edit of non directory.\n+\t\t * We do not break anything else.\n+\t\t */\n \t\tif (DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two) &&\n \t\t    !S_ISDIR(p->one->mode) && !S_ISDIR(p->two->mode) &&\n-\t\t    !strcmp(p->one->path, p->two->path) &&\n-\t\t    very_different(p->one, p->two, max_score)) {\n-\t\t\t/* Split this into delete and create */\n-\t\t\tstruct diff_filespec *one_pre, *one_post,\n-\t\t\t\t*two_pre, *two_post;\n-\n-\t\t\t/* deletion of one */\n-\t\t\tone_pre = alloc_filespec(p->one->path);\n-\t\t\tfill_filespec(one_pre, p->one->sha1, p->one->mode);\n-\t\t\tone_post = alloc_filespec(p->one->path);\n-\t\t\tdiff_queue(&outq, one_pre, one_post);\n-\n-\t\t\t/* creation of two */\n-\t\t\ttwo_pre = alloc_filespec(p->two->path);\n-\t\t\ttwo_post = alloc_filespec(p->two->path);\n-\t\t\tfill_filespec(two_post, p->two->sha1, p->two->mode);\n-\t\t\tdiff_queue(&outq, two_pre, two_post);\n-\t\t\tdiff_free_filepair(p);\n+\t\t    !strcmp(p->one->path, p->two->path)) {\n+\t\t\tscore = very_different(p->one, p->two, min_score);\n+\t\t\tif (min_score <= score) {\n+\t\t\t\t/* Split this into delete and create */\n+\t\t\t\tstruct diff_filespec *null_one, *null_two;\n+\t\t\t\tstruct diff_filepair *dp;\n+\n+\t\t\t\t/* deletion of one */\n+\t\t\t\tnull_one = alloc_filespec(p->one->path);\n+\t\t\t\tdp = diff_queue(&outq, p->one, null_one);\n+\t\t\t\tdp->score = score;\n+\t\t\t\tdp->broken_pair = 1;\n+\n+\t\t\t\t/* creation of two */\n+\t\t\t\tnull_two = alloc_filespec(p->two->path);\n+\t\t\t\tdp = diff_queue(&outq, null_two, p->two);\n+\t\t\t\tdp->score = score;\n+\t\t\t\tdp->broken_pair = 1;\n+\n+\t\t\t\tfree(p); /* not diff_free_filepair(), we are\n+\t\t\t\t\t  * reusing one and two here.\n+\t\t\t\t\t  */\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t}\n-\t\telse\n-\t\t\tdiff_q(&outq, p);\n+\t\tdiff_q(&outq, p);\n \t}\n \tfree(q->queue);\n \t*q = outq;\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -232,7 +232,7 @@ void diffcore_rename(int detect_rename, \n \tint num_create, num_src, dst_cnt;\n \n \tif (!minimum_score)\n-\t\tminimum_score = DEFAULT_MINIMUM_SCORE;\n+\t\tminimum_score = DEFAULT_RENAME_SCORE;\n \trenq.queue = NULL;\n \trenq.nr = renq.alloc = 0;\n \n@@ -311,26 +311,66 @@ void diffcore_rename(int detect_rename, \n \toutq.nr = outq.alloc = 0;\n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filepair *p = q->queue[i];\n-\t\tstruct diff_rename_dst *dst = locate_rename_dst(p->two, 0);\n \t\tstruct diff_filepair *pair_to_free = NULL;\n \n-\t\tif (dst) {\n-\t\t\t/* creation */\n-\t\t\tif (dst->pair) {\n-\t\t\t\t/* renq has rename/copy to produce\n-\t\t\t\t * this file already, so we do not\n-\t\t\t\t * emit the creation record in the\n-\t\t\t\t * output.\n-\t\t\t\t */\n+\t\tif (!DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two)) {\n+\t\t\t/*\n+\t\t\t * Creation\n+\t\t\t *\n+\t\t\t * We would output this create record if it has\n+\t\t\t * not been turned into a rename/copy already.\n+\t\t\t */\n+\t\t\tstruct diff_rename_dst *dst =\n+\t\t\t\tlocate_rename_dst(p->two, 0);\n+\t\t\tif (dst && dst->pair) {\n \t\t\t\tdiff_q(&outq, dst->pair);\n \t\t\t\tpair_to_free = p;\n \t\t\t}\n \t\t\telse\n-\t\t\t\t/* no matching rename/copy source, so record\n-\t\t\t\t * this as a creation.\n+\t\t\t\t/* no matching rename/copy source, so\n+\t\t\t\t * record this as a creation.\n \t\t\t\t */\n \t\t\t\tdiff_q(&outq, p);\n \t\t}\n+\t\telse if (DIFF_FILE_VALID(p->one) && !DIFF_FILE_VALID(p->two)) {\n+\t\t\t/*\n+\t\t\t * Deletion\n+\t\t\t *\n+\t\t\t * We would output this delete record if:\n+\t\t\t *\n+\t\t\t * (1) this is a broken delete and the counterpart\n+\t\t\t *     broken create remains in the output; or\n+\t\t\t * (2) this is not a broken delete, and renq does\n+\t\t\t *     not have a rename/copy to create p->two->path.\n+\t\t\t *\n+\t\t\t * Otherwise, the counterpart broken create\n+\t\t\t * has been turned into a rename-edit; or\n+\t\t\t * delete did not have a matching create to\n+\t\t\t * begin with.\n+\t\t\t */\n+\t\t\tif (DIFF_PAIR_BROKEN(p)) {\n+\t\t\t\t/* broken delete */\n+\t\t\t\tstruct diff_rename_dst *dst =\n+\t\t\t\t\tlocate_rename_dst(p->one, 0);\n+\t\t\t\tif (dst && dst->pair)\n+\t\t\t\t\t/* counterpart is now rename/copy */\n+\t\t\t\t\tpair_to_free = p;\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tfor (j = 0; j < renq.nr; j++)\n+\t\t\t\t\tif (!strcmp(renq.queue[j]->two->path,\n+\t\t\t\t\t\t    p->one->path))\n+\t\t\t\t\t\tbreak;\n+\t\t\t\tif (j < renq.nr)\n+\t\t\t\t\t/* this path remains */\n+\t\t\t\t\tpair_to_free = p;\n+\t\t\t}\n+\n+\t\t\tif (pair_to_free)\n+\t\t\t\t;\n+\t\t\telse\n+\t\t\t\tdiff_q(&outq, p);\n+\t\t}\n \t\telse if (!diff_unmodified_pair(p))\n \t\t\t/* all the usual ones need to be kept */\n \t\t\tdiff_q(&outq, p);\ndiff --git a/diffcore.h b/diffcore.h\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -8,9 +8,9 @@\n  * (e.g. diffcore-rename, diffcore-pickaxe).  Never include this header\n  * in anything else.\n  */\n-#define MAX_SCORE 10000\n-#define DEFAULT_MINIMUM_SCORE 5000 /* rename/copy similarity minimum */\n-#define DEFAULT_MAXIMUM_SCORE 1500 /* similarity maximum for break to happen */\n+#define MAX_SCORE 60000\n+#define DEFAULT_RENAME_SCORE 30000 /* rename/copy similarity minimum (50%) */\n+#define DEFAULT_BREAK_SCORE  59400 /* minimum for break to happen (99%)*/\n \n #define RENAME_DST_MATCHED 01\n \n@@ -43,14 +43,19 @@ struct diff_filepair {\n \tstruct diff_filespec *one;\n \tstruct diff_filespec *two;\n \tunsigned short int score;\n-\tchar source_stays; /* all of R/C are copies */\n \tchar status; /* M C R N D U (see Documentation/diff-format.txt) */\n+\tunsigned source_stays : 1; /* all of R/C are copies */\n+\tunsigned broken_pair : 1;\n };\n #define DIFF_PAIR_UNMERGED(p) \\\n \t(!DIFF_FILE_VALID((p)->one) && !DIFF_FILE_VALID((p)->two))\n \n #define DIFF_PAIR_RENAME(p) (strcmp((p)->one->path, (p)->two->path))\n \n+#define DIFF_PAIR_BROKEN(p) \\\n+\t( (!DIFF_FILE_VALID((p)->one) != !DIFF_FILE_VALID((p)->two)) && \\\n+\t  ((p)->broken_pair != 0) )\n+\n #define DIFF_PAIR_TYPE_CHANGED(p) \\\n \t((S_IFMT & (p)->one->mode) != (S_IFMT & (p)->two->mode))\n \ndiff --git a/t/t4008-diff-break-rewrite.sh b/t/t4008-diff-break-rewrite.sh\nnew file mode 100755\n--- /dev/null\n+++ b/t/t4008-diff-break-rewrite.sh\n@@ -0,0 +1,169 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2005 Junio C Hamano\n+#\n+\n+test_description='Break and then rename\n+\n+We have two very different files, file0 and file1, registered in a tree.\n+\n+We update file1 so drastically that it is more similar to file0, and\n+then remove file0.  With -B, changes to file1 should be broken into\n+separate delete and create, resulting in removal of file0, removal of\n+original file1 and creation of completely rewritten file1.\n+\n+Further, with -B and -M together, these three modifications should\n+turn into rename-edit of file0 into file1.\n+\n+Starting from the same two files in the tree, we swap file0 and file1.\n+With -B, this should be detected as two complete rewrites, resulting in\n+four changes in total.\n+\n+Further, with -B and -M together, these should turn into two renames.\n+'\n+. ./test-lib.sh\n+\n+_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'\n+_x40=\"$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40\"\n+sanitize_diff_raw='s/ '\"$_x40\"' '\"$_x40\"' \\([CDNR]\\)[0-9]*\t/ X X \\1#\t/'\n+compare_diff_raw () {\n+    # When heuristics are improved, the score numbers would change.\n+    # Ignore them while comparing.\n+    # Also we do not check SHA1 hash generation in this test, which\n+    # is a job for t0000-basic.sh\n+\n+    sed -e \"$sanitize_diff_raw\" <\"$1\" >.tmp-1\n+    sed -e \"$sanitize_diff_raw\" <\"$2\" >.tmp-2\n+    diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2\n+}\n+\n+test_expect_success \\\n+    setup \\\n+    'cat ../../README >file0 &&\n+     cat ../../COPYING >file1 &&\n+    git-update-cache --add file0 file1 &&\n+    tree=$(git-write-tree) &&\n+    echo \"$tree\"'\n+\n+test_expect_success \\\n+    'change file1 with copy-edit of file0 and remove file0' \\\n+    'sed -e \"s/git/GIT/\" file0 >file1 &&\n+     rm -f file0 &&\n+    git-update-cache --remove file0 file1'\n+\n+test_expect_success \\\n+    'run diff with -B' \\\n+    'git-diff-cache -B --cached \"$tree\" >current'\n+\n+cat >expected <<\\EOF\n+:100644 000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 0000000000000000000000000000000000000000 D\tfile0\n+:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100\tfile1\n+:000000 100644 0000000000000000000000000000000000000000 11e331465a89c394dc25c780de230043750c1ec8 N100\tfile1\n+EOF\n+\n+test_expect_success \\\n+    'validate result of -B (#1)' \\\n+    'compare_diff_raw current expected'\n+\n+test_expect_success \\\n+    'run diff with -B and -M' \\\n+    'git-diff-cache -B -M \"$tree\" >current'\n+\n+cat >expected <<\\EOF\n+:100644 100644 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 08bb2fb671deff4c03a4d4a0a1315dff98d5732c R100\tfile0\tfile1\n+EOF\n+\n+test_expect_success \\\n+    'validate result of -B -M (#2)' \\\n+    'compare_diff_raw current expected'\n+\n+test_expect_success \\\n+    'swap file0 and file1' \\\n+    'rm -f file0 file1 &&\n+     git-read-tree -m $tree &&\n+     git-checkout-cache -f -u -a &&\n+     mv file0 tmp &&\n+     mv file1 file0 &&\n+     mv tmp file1 &&\n+     git-update-cache file0 file1'\n+\n+test_expect_success \\\n+    'run diff with -B' \\\n+    'git-diff-cache -B \"$tree\" >current'\n+\n+cat >expected <<\\EOF\n+:100644 000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 0000000000000000000000000000000000000000 D100\tfile0\n+:000000 100644 0000000000000000000000000000000000000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 N100\tfile0\n+:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100\tfile1\n+:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N100\tfile1\n+EOF\n+\n+test_expect_success \\\n+    'validate result of -B (#3)' \\\n+    'compare_diff_raw current expected'\n+\n+test_expect_success \\\n+    'run diff with -B and -M' \\\n+    'git-diff-cache -B -M \"$tree\" >current'\n+\n+cat >expected <<\\EOF\n+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 R100\tfile1\tfile0\n+:100644 100644 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 R100\tfile0\tfile1\n+EOF\n+\n+test_expect_success \\\n+    'validate result of -B -M (#4)' \\\n+    'compare_diff_raw current expected'\n+\n+test_expect_success \\\n+    'make file0 into something completely different' \\\n+    'rm -f file0 &&\n+     ln -s frotz file0 &&\n+     git-update-cache file0 file1'\n+\n+test_expect_success \\\n+    'run diff with -B' \\\n+    'git-diff-cache -B \"$tree\" >current'\n+\n+cat >expected <<\\EOF\n+:100644 120000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 67be421f88824578857624f7b3dc75e99a8a1481 T\tfile0\n+:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100\tfile1\n+:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N100\tfile1\n+EOF\n+\n+test_expect_success \\\n+    'validate result of -B (#5)' \\\n+    'compare_diff_raw current expected'\n+\n+test_expect_success \\\n+    'run diff with -B' \\\n+    'git-diff-cache -B -M \"$tree\" >current'\n+\n+# This should not mistake file0 as the copy source of new file1\n+# due to type differences.\n+cat >expected <<\\EOF\n+:100644 120000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 67be421f88824578857624f7b3dc75e99a8a1481 T\tfile0\n+:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100\tfile1\n+:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N100\tfile1\n+EOF\n+\n+test_expect_success \\\n+    'validate result of -B -M (#6)' \\\n+    'compare_diff_raw current expected'\n+\n+test_expect_success \\\n+    'run diff with -M' \\\n+    'git-diff-cache -M \"$tree\" >current'\n+\n+# This should not mistake file0 as the copy source of new file1\n+# due to type differences.\n+cat >expected <<\\EOF\n+:100644 120000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 67be421f88824578857624f7b3dc75e99a8a1481 T\tfile0\n+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 M\tfile1\n+EOF\n+\n+test_expect_success \\\n+    'validate result of -M (#7)' \\\n+    'compare_diff_raw current expected'\n+\n+test_done\n\n"},{"id":"4200","messageId":"20050529215617.GT1036@pasky.ji.cz","threadId":"751","inReplyTo":"7vbr6vrvqf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add -B flag to diff-* brothers.","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-05-29T21:56:17Z","receivedAt":"2005-05-29T21:56:17Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sat, May 28, 2005 at 09:33:44PM CEST, I got a letter\nwhere Junio C Hamano <junkio@cox.net> told me that...\n> This introduces diffcore-break.c, a new diffcore\n> transformation.\n> \n> When the -B flag is given, a patch that represents a complete\n> rewrite is broken into a deletion followed by a creation.  This\n> makes it easier to review such a complete rewrite patch.\n> \n> The -B flag takes the same syntax as the -M and -C flags to\n> specify the maximum amount of source material the resulting file\n> can still have to be considered a complete rewrite, and defaults\n> to 15% if not specified.  For example, this detects the complete\n> rewrite of ls-tree.c I sent earlier.\n> \n> Signed-off-by: Junio C Hamano <junkio@cox.net>\n\nActually, I like this one - contrary to -O I can see how this could be\nquite useful - I have wished for this many times when people would send\nme some \"complete rewrite\" patches which I actually wanted to review.\n\nPlease don't give up on it. :-)\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nC++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor\n"},{"id":"4215","messageId":"7vpsv9h9qi.fsf@assigned-by-dhcp.cox.net","threadId":"751","inReplyTo":"20050529215617.GT1036@pasky.ji.cz","subject":"Re: [PATCH] Add -B flag to diff-* brothers.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-29T23:51:17Z","receivedAt":"2005-05-29T23:51:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"PB\" == Petr Baudis <pasky@ucw.cz> writes:\n\nPB> Actually, I like this one - contrary to -O I can see how this could be\nPB> quite useful - I have wished for this many times when people would send\nPB> me some \"complete rewrite\" patches which I actually wanted to review.\n\nPB> Please don't give up on it. :-)\n\nWell, although I do not do Porcelain ;-), I do want to have -O\nto help my use pattern.  I envision that Porcelain noticing the\nexistence of ${GIT-.git}/patch-order file and adding -O to its\ndiff-* argument would make the world a better place.\n\nAnd I am not giving up on -B yet, but as you can imagine, it has\ninteresting interaction with -M/-C, since rename detection code\nmust be prepared to deal with broken pairs (earlier it did not\nhave to worry about the same path that is a regular file\nappearing twice in its input).  Since I have tested -B only in\nthe context of full set of patches I sent out, and have not\ntested it with only the subset Linus decided to swallow, I would\nrecommend against applying it on top of the Linus tip as is.\nI'll be rebasing it to his tip, test it again and then resubmit\nlater.  I'll do the same for -O changes.\n\n"}]}