{"thread":{"id":"50815","subject":"[RFC PATCH 0/1] Fuzzy blame","startedAt":"2019-03-24T23:52:08Z","lastAt":"2019-04-03T21:49:54Z","messageCount":14,"participants":["michael@platin.gs","Junio C Hamano","Michael Platings","Barret Rhoden","Jeff King","Jacob Keller","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"372386","messageId":"20190324235020.49706-1-michael@platin.gs","threadId":"50815","inReplyTo":null,"subject":"[RFC PATCH 0/1] Fuzzy blame","fromName":"","fromEmail":"michael@platin.gs","sentAt":"2019-03-24T23:50:19Z","receivedAt":"2019-03-24T23:52:08Z","isPatch":true,"sender":{"key":"michael@platin.gs","avatar":"https://avatars.githubusercontent.com/u/1112348?v=4"},"body":"From: Michael Platings <michael@platin.gs>\n\nHi Git devs,\n\nSome of you may be familiar with the git-hyper-blame tool [1]. It's \"useful if\nyou have a commit that makes sweeping changes that are unlikely to be what you\nare looking for in a blame, such as mass reformatting or renaming.\"\n\ngit-hyper-blame is useful but (a) it's not convenient to install; (b) it's\nmissing functionality available in regular git blame; (c) it's method of\nmatching lines between chunks is too simplistic for many use cases; and\n(d) it's not Git so it doesn't integrate well with tools that expect Git\ne.g. vim plugins. Therefore I'm hoping to add similar and hopefully superior\nfunctionality to Git itself. I have a very rough patch so I'd like to get your\nthoughts on the general approach, particularly in terms of its user-visible\nbehaviour.\n\nMy initial idea was to lift the design directly from git-hyper-blame. However\nthe approach of picking single revisions to somehow ignore doesn't sit well\nwith the -w, -M & -C options, which have a similar intent but apply to all\nrevisions.\n\nI'd like to get your thoughts on whether we could allow applying the -M or -w\noptions to specific revisions. For example, imagine it was agreed that all\nthe #includes in a project should be reordered. In that case, it would be useful\nto be able to specify that the -M option should be used for blames on that\nrevision specifically, so that in future when someone wants to know why\na #include was added they don't have to run git blame twice to find out.\n\nOptions that are specific to a particular revision could be stored in a\n\".gitrevisions\" file or similar.\n\nIf the principle of allowing blame options to be applied per-revision is\nagreeable then I'd like to add a -F/--fuzzy option, to sit alongside -w, -M & -C.\n\nI've implemented a prototype \"fuzzy\" option, patch attached.\nThe option operates at the level of diff chunks. For each line in the \"after\"\nhalf of the chunk it uses a heuristic to choose which line in the \"before\" half\nof the chunk matches best. The heuristic I'm using at the moment is of matching\n\"bigrams\" as described in [2]. The initial pass typically gives reasonable\nresults, but can jumble up the lines. As in the reformatting/renaming use case\nthe content should stay in the same order, it's worth going to extra effort to\navoid jumbling lines. Therefore, after the initial pass, the line that can be\nmatched with the most confidence is used to partition the chunk into halves\nbefore and after it. The process is then repeated recursively on the halves\nabove and below the partition line.\nI feel like a similar algorithm has probably already been invented in a better\nform - if anyone knows of such a thing then please let me know!\n\nI look forward to hearing your thoughts.\nThanks,\n-Michael\n\n\n[1] https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-hyper-blame.html\n[2] https://en.wikipedia.org/wiki/S%C3%B8rensen%E2%80%93Dice_coefficient\n\nMichael Platings (1):\n  Add git blame --fuzzy option.\n\n blame.c                | 352 +++++++++++++++++++++++++++++++++++++++++++++++--\n blame.h                |   1 +\n builtin/blame.c        |   3 +\n t/t8020-blame-fuzzy.sh | 264 +++++++++++++++++++++++++++++++++++++\n 4 files changed, 609 insertions(+), 11 deletions(-)\n create mode 100755 t/t8020-blame-fuzzy.sh\n\n-- \n2.14.3 (Apple Git-98)\n\n"},{"id":"372387","messageId":"20190324235020.49706-2-michael@platin.gs","threadId":"50815","inReplyTo":"20190324235020.49706-1-michael@platin.gs","subject":"[RFC PATCH 1/1] Fuzzy blame","fromName":"","fromEmail":"michael@platin.gs","sentAt":"2019-03-24T23:50:20Z","receivedAt":"2019-03-24T23:52:17Z","isPatch":true,"sender":{"key":"michael@platin.gs","avatar":"https://avatars.githubusercontent.com/u/1112348?v=4"},"body":"From: Michael Platings <michael@platin.gs>\n\n---\n blame.c                | 352 +++++++++++++++++++++++++++++++++++++++++++++++--\n blame.h                |   1 +\n builtin/blame.c        |   3 +\n t/t8020-blame-fuzzy.sh | 264 +++++++++++++++++++++++++++++++++++++\n 4 files changed, 609 insertions(+), 11 deletions(-)\n create mode 100755 t/t8020-blame-fuzzy.sh\n\ndiff --git a/blame.c b/blame.c\nindex 5c07dec190..b5a40c8e9f 100644\n--- a/blame.c\n+++ b/blame.c\n@@ -997,6 +997,326 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb,\n \treturn;\n }\n \n+/* https://graphics.stanford.edu/~seander/bithacks.html#CountBitsSetParallel */\n+static int bitcount(uint32_t v) {\n+\tv = v - ((v >> 1) & 0x55555555u);\n+\tv = (v & 0x33333333u) + ((v >> 2) & 0x33333333u);\n+\treturn ((v + (v >> 4) & 0xf0f0f0fu) * 0x1010101u) >> 24;\n+}\n+\n+#define FINGERPRINT_LENGTH (8*256)\n+/* This is just a bitset indicating which byte pairs are present.\n+ e.g. the string \"good goo\" has pairs \"go\", \"oo\", \"od\", \"d \", \" g\"\n+ String similarity is calculated as a bitwise or and counting the set bits.\n+ TODO for the string lengths we typically deal with, this would probably be\n+ implemented more efficiently with a set data structure.\n+ */\n+struct fingerprint {\n+\tuint32_t bits[FINGERPRINT_LENGTH];\n+};\n+\n+static void get_fingerprint(struct fingerprint *result,\n+\t\t\t    const char *line_begin, const char *line_end) {\n+\tmemset(result, 0, sizeof(struct fingerprint));\n+\tfor (const char *p = line_begin; p + 1 < line_end; ++p) {\n+\t\tunsigned c = tolower(*p) | (tolower(*(p + 1)) << 8);\n+\t\tresult->bits[c >> 5] |= 1u << (c & 0x1f);\n+\t}\n+}\n+\n+static int fingerprint_similarity(const struct fingerprint *a,\n+\t\t\t\t  const struct fingerprint *b) {\n+\tint intersection = 0;\n+\tfor (int i = 0; i < FINGERPRINT_LENGTH; ++i) {\n+\t\tintersection += bitcount(a->bits[i] & b->bits[i]);\n+\t}\n+\treturn intersection;\n+}\n+\n+struct fuzzy_blame_parent_data {\n+\tstruct blame_origin *parent;\n+\tlong offset;\n+\tstruct blame_entry **processed_entries;\n+\tstruct blame_entry **target_entries;\n+\tconst char *parent_content;\n+\tconst char *target_content;\n+\tint *parent_line_starts;\n+\tint *target_line_starts;\n+\tint parent_line_count;\n+\tint target_line_count;\n+};\n+\n+static void get_chunk_fingerprints(struct fingerprint *fingerprints,\n+\t\t\t\t   const char *content,\n+\t\t\t\t   const int *line_starts,\n+\t\t\t\t   long chunk_start,\n+\t\t\t\t   long chunk_length) {\n+\tline_starts += chunk_start;\n+\tfor (int i = 0; i != chunk_length; ++i) {\n+\t\tconst char* linestart = content + line_starts[i];\n+\t\tconst char* lineend = content + line_starts[i + 1];\n+\t\tget_fingerprint(fingerprints + i, linestart, lineend);\n+\t}\n+}\n+\n+/* This finds the line that we can match with the most confidence, and\n+ uses it as a partition. It then calls itself on the lines on either side of\n+ that partition. In this way we avoid lines appearing out of order, and retain\n+ a sensible line ordering.\n+ TODO: so much optimisation. Currently this does the same work repeatedly.\n+ */\n+static void fuzzy_find_matching_lines_recurse(\n+\t\tconst char *content_a, const char *content_b,\n+\t\tconst int *line_starts_a, const int *line_starts_b,\n+\t\tint start_a, int start_b,\n+\t\tint length_a, int length_b,\n+\t\tint *result,\n+\t\tstruct fingerprint *fingerprints_a,\n+\t\tstruct fingerprint *fingerprints_b) {\n+\n+\tint most_certain_line = -1;\n+\tint most_certain_line_certainty = -1;\n+\n+\tfor (int i = 0; i < length_b; ++i) {\n+\t\tconst struct fingerprint *fingerprint_b = fingerprints_b + i;\n+\n+\t\tint closest_line_a = (i * 2 + 1) * length_a /\n+\t\t(length_b * 2);\n+\n+\t\t/* Limit range of search to a reasonable number of lines.\n+\t\t TODO consider scaling this up if length_a is greater than\n+\t\t length_b. */\n+\t\tconst int MAX_SEARCH_DISTANCE = 5;\n+\t\tint search_start = closest_line_a - (MAX_SEARCH_DISTANCE - 1);\n+\t\tint search_end = closest_line_a + MAX_SEARCH_DISTANCE;\n+\t\tif (search_start < 0) search_start = 0;\n+\t\tif (search_end > length_a) search_end = length_a;\n+\n+\t\t/* Find both the best and 2nd best matches. The match certainty\n+\t\t is the difference between these values. */\n+\t\tint best_similarity = 0, second_best_similarity = 0;\n+\t\tint best_similarity_index = 0;\n+\n+\t\tfor (int j = search_start; j < search_end; ++j) {\n+\t\t\tint similarity = fingerprint_similarity(\n+\t\t\t\t\t\t\t\tfingerprint_b,\n+\t\t\t\t\t\t\t\tfingerprints_a + j) *\n+\t\t\t\t(1000 - abs(j - closest_line_a));\n+\n+\t\t\tif (similarity > best_similarity) {\n+\t\t\t\tsecond_best_similarity = best_similarity;\n+\t\t\t\tbest_similarity = similarity;\n+\t\t\t\tbest_similarity_index = j;\n+\t\t\t}\n+\t\t\telse if (similarity > second_best_similarity) {\n+\t\t\t\tsecond_best_similarity = similarity;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (best_similarity == 0) {\n+\t\t\tresult[i] = -1;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tresult[i] = start_a + best_similarity_index;\n+\n+\t\tint certainty = best_similarity - second_best_similarity;\n+\t\tif (certainty > most_certain_line_certainty) {\n+\t\t\tmost_certain_line_certainty = certainty;\n+\t\t\tmost_certain_line = i;\n+\t\t}\n+\t}\n+\n+\tif (most_certain_line == -1) {\n+\t\treturn;\n+\t}\n+\n+\tif (most_certain_line > 0) {\n+\t\tfuzzy_find_matching_lines_recurse(content_a, content_b, line_starts_a, line_starts_b, start_a, start_b, result[most_certain_line] + 1 - start_a, most_certain_line, result, fingerprints_a, fingerprints_b);\n+\t}\n+\tif (most_certain_line + 1 < length_b) {\n+\t\tint second_half_start_a = result[most_certain_line];\n+\t\tint second_half_start_b = start_b + most_certain_line + 1;\n+\t\tint second_half_length_a = length_a + start_a - second_half_start_a;\n+\t\tint second_half_length_b = length_b + start_b - second_half_start_b;\n+\t\tfuzzy_find_matching_lines_recurse(content_a, content_b, line_starts_a, line_starts_b, second_half_start_a, second_half_start_b, second_half_length_a, second_half_length_b, result + most_certain_line + 1, fingerprints_a + second_half_start_a - start_a, fingerprints_b + most_certain_line + 1);\n+\t}\n+}\n+\n+/* Find line numbers in \"a\" that match with lines in \"b\"\n+ Returns an array of either line indices or -1 where no match is found.\n+ The returned array must be free()d after use.\n+ */\n+static int *fuzzy_find_matching_lines(\n+\tconst char *content_a, const char *content_b,\n+\tconst int *line_starts_a, const int *line_starts_b,\n+\tint start_a, int start_b,\n+\tint length_a, int length_b) {\n+\n+\tint *result = malloc(sizeof(int) * length_b);\n+\n+\tstruct fingerprint *fingerprints_a =\n+\t\tmalloc(sizeof(struct fingerprint) * length_a);\n+\tstruct fingerprint *fingerprints_b =\n+\t\tmalloc(sizeof(struct fingerprint) * length_b);\n+\n+\tget_chunk_fingerprints(fingerprints_a, content_a,\n+\t\t\t       line_starts_a,\n+\t\t\t       start_a, length_a);\n+\tget_chunk_fingerprints(fingerprints_b, content_b,\n+\t\t\t       line_starts_b,\n+\t\t\t       start_b, length_b);\n+\n+\tfuzzy_find_matching_lines_recurse(content_a, content_b,\n+\t\t\t\t\t    line_starts_a, line_starts_b,\n+\t\t\t\t\t    start_a, start_b,\n+\t\t\t\t\t    length_a, length_b,\n+\t\t\t\t\t    result,\n+\t\t\t\t\t    fingerprints_a,\n+\t\t\t\t\t    fingerprints_b);\n+\n+\tfree(fingerprints_a);\n+\tfree(fingerprints_b);\n+\n+\treturn result;\n+}\n+\n+static int blame_chunk_fuzzy(long parent_chunk_start,\n+\t\t\t\t  long parent_chunk_length,\n+\t\t\t\t  long target_chunk_start,\n+\t\t\t\t  long target_chunk_length,\n+\t\t\t\t  void *data)\n+{\n+\tstruct fuzzy_blame_parent_data *d = data;\n+\n+\tif (parent_chunk_start - target_chunk_start != d->offset)\n+\t\tdie(\"internal error in blame::blame_chunk_fuzzy\");\n+\n+\tint target_chunk_end = target_chunk_start + target_chunk_length;\n+\n+\tstruct blame_origin *parent = d->parent;\n+\tstruct blame_entry *e = *d->target_entries;\n+\tstruct blame_entry *parent_tail = NULL;\n+\tstruct blame_entry *target_tail = NULL;\n+\n+\tif (parent_chunk_length == 0) {\n+\t\t/* Don't try to blame parent for newly added lines */\n+\t\twhile (e && e->s_lno < target_chunk_end) {\n+\t\t\ttarget_tail = e;\n+\t\t\te = e->next;\n+\t\t}\n+\t\td->target_entries = &target_tail->next;\n+\t\tgoto finish;\n+\t}\n+\n+\tint *matched_lines = fuzzy_find_matching_lines(\n+\t\td->parent_content, d->target_content,\n+\t\td->parent_line_starts, d->target_line_starts,\n+\t\tparent_chunk_start, target_chunk_start,\n+\t\tparent_chunk_length, target_chunk_length);\n+\n+\twhile (e && e->s_lno < target_chunk_end) {\n+\t\tstruct blame_entry *next = e->next;\n+\n+\t\tfor (int i = 0; i < e->num_lines; ++i) {\n+\t\t\tstruct blame_entry *n =\n+\t\t\t\txcalloc(1, sizeof (struct blame_entry));\n+\t\t\tn->lno = e->lno + i;\n+\t\t\tn->num_lines = 1;\n+\t\t\tn->score = 0;\n+\n+\t\t\tint matched_line = matched_lines[i + e->s_lno -\n+\t\t\t\ttarget_chunk_start];\n+\n+\t\t\tif (matched_line != -1) {\n+\t\t\t\tn->suspect = blame_origin_incref(parent);\n+\t\t\t\tn->s_lno = matched_line;\n+\t\t\t\tn->next = parent_tail;\n+\t\t\t\tparent_tail = n;\n+\t\t\t} else {\n+\t\t\t\tn->suspect = blame_origin_incref(e->suspect);\n+\t\t\t\tn->s_lno = e->s_lno + i;\n+\t\t\t\tn->next = target_tail;\n+\t\t\t\ttarget_tail = n;\n+\t\t\t}\n+\t\t}\n+\n+\t\tblame_origin_decref(e->suspect);\n+\t\tfree(e);\n+\n+\t\te = next;\n+\t}\n+\n+\tif (parent_tail) {\n+\t\tparent_tail = llist_mergesort(parent_tail, get_next_blame,\n+\t\t\t\t\t      set_next_blame,\n+\t\t\t\t\t      compare_blame_suspect);\n+\t\t*d->processed_entries = parent_tail;\n+\t\twhile (parent_tail->next) parent_tail = parent_tail->next;\n+\t\td->processed_entries = &parent_tail->next;\n+\t}\n+\n+\tif (target_tail) {\n+\t\ttarget_tail = llist_mergesort(target_tail, get_next_blame,\n+\t\t\t\t\t      set_next_blame,\n+\t\t\t\t\t      compare_blame_suspect);\n+\t\t*d->target_entries = target_tail;\n+\t\twhile (target_tail->next) target_tail = target_tail->next;\n+\t\td->target_entries = &target_tail->next;\n+\t}\n+\n+\t*d->target_entries = e;\n+\n+\tfree(matched_lines);\n+\n+finish:\n+\td->offset = parent_chunk_start + parent_chunk_length -\n+\t\t(target_chunk_start + target_chunk_length);\n+\n+\treturn 0;\n+}\n+\n+static int find_line_starts(int **line_starts, const char *buf, unsigned long len);\n+\n+static void pass_blame_to_parent_fuzzy(struct blame_scoreboard *sb,\n+\t\t\t\t       struct blame_origin *target,\n+\t\t\t\t       struct blame_origin *parent)\n+{\n+\tmmfile_t file_p, file_o;\n+\tstruct fuzzy_blame_parent_data d;\n+\tstruct blame_entry *newdest = NULL;\n+\n+\tif (!target->suspects)\n+\t\treturn; /* nothing remains for this target */\n+\n+\td.parent = parent;\n+\td.offset = 0;\n+\td.processed_entries = &newdest;\n+\td.target_entries = &target->suspects;\n+\n+\tfill_origin_blob(&sb->revs->diffopt, parent, &file_p, &sb->num_read_blob);\n+\tfill_origin_blob(&sb->revs->diffopt, target, &file_o, &sb->num_read_blob);\n+\tsb->num_get_patch++;\n+\n+\td.parent_content = file_p.ptr;\n+\td.target_content = file_o.ptr;\n+\td.parent_line_count = find_line_starts(&d.parent_line_starts, file_p.ptr, file_p.size);\n+\td.target_line_count = find_line_starts(&d.target_line_starts, file_o.ptr, file_o.size);\n+\n+\tif (diff_hunks(&file_p, &file_o, blame_chunk_fuzzy, &d, sb->xdl_opts))\n+\t\tdie(\"unable to generate diff (%s -> %s)\",\n+\t\t\toid_to_hex(&parent->commit->object.oid),\n+\t\t\toid_to_hex(&target->commit->object.oid));\n+\n+\t*d.processed_entries = NULL;\n+\tqueue_blames(sb, parent, newdest);\n+\n+\tfree(d.target_line_starts);\n+\tfree(d.parent_line_starts);\n+\n+\treturn;\n+}\n+\n /*\n  * The lines in blame_entry after splitting blames many times can become\n  * very small and trivial, and at some point it becomes pointless to\n@@ -1433,7 +1753,7 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,\n \tstruct commit *commit = origin->commit;\n \tstruct commit_list *sg;\n \tstruct blame_origin *sg_buf[MAXSG];\n-\tstruct blame_origin *porigin, **sg_origin = sg_buf;\n+\tstruct blame_origin *porigin = NULL, **sg_origin = sg_buf;\n \tstruct blame_entry *toosmall = NULL;\n \tstruct blame_entry *blames, **blametail = &blames;\n \n@@ -1560,6 +1880,11 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,\n \t\t*tail = origin->suspects;\n \t\torigin->suspects = toosmall;\n \t}\n+\n+\tif (sb->fuzzy && porigin) {\n+\t\tpass_blame_to_parent_fuzzy(sb, origin, porigin);\n+\t}\n+\n \tfor (i = 0; i < num_sg; i++) {\n \t\tif (sg_origin[i]) {\n \t\t\tdrop_origin_blob(sg_origin[i]);\n@@ -1645,14 +1970,8 @@ static const char *get_next_line(const char *start, const char *end)\n \treturn nl ? nl + 1 : end;\n }\n \n-/*\n- * To allow quick access to the contents of nth line in the\n- * final image, prepare an index in the scoreboard.\n- */\n-static int prepare_lines(struct blame_scoreboard *sb)\n+static int find_line_starts(int **line_starts, const char *buf, unsigned long len)\n {\n-\tconst char *buf = sb->final_buf;\n-\tunsigned long len = sb->final_buf_size;\n \tconst char *end = buf + len;\n \tconst char *p;\n \tint *lineno;\n@@ -1661,15 +1980,26 @@ static int prepare_lines(struct blame_scoreboard *sb)\n \tfor (p = buf; p < end; p = get_next_line(p, end))\n \t\tnum++;\n \n-\tALLOC_ARRAY(sb->lineno, num + 1);\n-\tlineno = sb->lineno;\n+\tALLOC_ARRAY(*line_starts, num + 1);\n+\tlineno = *line_starts;\n \n \tfor (p = buf; p < end; p = get_next_line(p, end))\n \t\t*lineno++ = p - buf;\n \n \t*lineno = len;\n \n-\tsb->num_lines = num;\n+\treturn num;\n+}\n+\n+/*\n+ * To allow quick access to the contents of nth line in the\n+ * final image, prepare an index in the scoreboard.\n+ */\n+static int prepare_lines(struct blame_scoreboard *sb)\n+{\n+\tsb->num_lines = find_line_starts(&sb->lineno,\n+\t\t\t\t\t\t\t\t\t sb->final_buf,\n+\t\t\t\t\t\t\t\t\t sb->final_buf_size);\n \treturn sb->num_lines;\n }\n \ndiff --git a/blame.h b/blame.h\nindex be3a895043..eb528f9f80 100644\n--- a/blame.h\n+++ b/blame.h\n@@ -142,6 +142,7 @@ struct blame_scoreboard {\n \tint xdl_opts;\n \tint no_whole_file_rename;\n \tint debug;\n+\tint fuzzy;\n \n \t/* callbacks */\n \tvoid(*on_sanity_fail)(struct blame_scoreboard *, int);\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 177c1022a0..d0a0dfff79 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -57,6 +57,7 @@ static struct date_mode blame_date_mode = { DATE_ISO8601 };\n static size_t blame_date_width;\n \n static struct string_list mailmap = STRING_LIST_INIT_NODUP;\n+static int fuzzy;\n \n #ifndef DEBUG\n #define DEBUG 0\n@@ -808,6 +809,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('w', NULL, &xdl_opts, N_(\"Ignore whitespace differences\"), XDF_IGNORE_WHITESPACE),\n \t\tOPT_BIT(0, \"color-lines\", &output_option, N_(\"color redundant metadata from previous line differently\"), OUTPUT_COLOR_LINE),\n \t\tOPT_BIT(0, \"color-by-age\", &output_option, N_(\"color lines by age\"), OUTPUT_SHOW_AGE_WITH_COLOR),\n+\t\tOPT_BOOL('F', \"fuzzy\", &fuzzy, N_(\"Try to assign blame to similar lines in the parent\")),\n \n \t\t/*\n \t\t * The following two options are parsed by parse_revision_opt()\n@@ -996,6 +998,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \n \tinit_scoreboard(&sb);\n \tsb.revs = &revs;\n+\tsb.fuzzy = fuzzy;\n \tsb.contents_from = contents_from;\n \tsb.reverse = reverse;\n \tsb.repo = the_repository;\ndiff --git a/t/t8020-blame-fuzzy.sh b/t/t8020-blame-fuzzy.sh\nnew file mode 100755\nindex 0000000000..d26c945722\n--- /dev/null\n+++ b/t/t8020-blame-fuzzy.sh\n@@ -0,0 +1,264 @@\n+#!/bin/sh\n+\n+test_description='git blame ignore a specific revision'\n+. ./test-lib.sh\n+\n+pick_author='s/^[0-9a-f^]* *(\\([^ ]*\\) .*/\\1/'\n+\n+file_count=8\n+\n+# Each test is composed of 4 variables:\n+# titleN - the test name\n+# aN - the initial content\n+# bN - the final content\n+# expectedN - the line numbers from aN that we expect git blame\n+#             on bN to identify, or \"Final\" if bN itself should\n+#             be identified as the origin of that line.\n+\n+title1=\"Expand lines\"\n+cat <<EOF >a1\n+aaa\n+bbb\n+ccc\n+ddd\n+eee\n+EOF\n+cat <<EOF >b1\n+aaa\n+bbbx\n+bbbx\n+ccc\n+dddx\n+dddx\n+eee\n+EOF\n+cat <<EOF >expected1\n+1\n+2\n+2\n+3\n+4\n+4\n+5\n+EOF\n+\n+title2=\"Combine 3 lines into 2\"\n+cat <<EOF >a2\n+if ((maxgrow==0) ||\n+    ( single_line_field && (field->dcols < maxgrow)) ||\n+    (!single_line_field && (field->drows < maxgrow)))\n+EOF\n+cat <<EOF >b2\n+if ((maxgrow == 0) || (single_line_field && (field->dcols < maxgrow)) ||\n+    (!single_line_field && (field->drows < maxgrow))) {\n+EOF\n+cat <<EOF >expected2\n+2\n+3\n+EOF\n+\n+title3=\"Add curly brackets\"\n+cat <<EOF >a3\n+    if (rows) *rows = field->rows;\n+    if (cols) *cols = field->cols;\n+    if (frow) *frow = field->frow;\n+    if (fcol) *fcol = field->fcol;\n+EOF\n+cat <<EOF >b3\n+    if (rows) {\n+      *rows = field->rows;\n+    }\n+    if (cols) {\n+      *cols = field->cols;\n+    }\n+    if (frow) {\n+      *frow = field->frow;\n+    }\n+    if (fcol) {\n+      *fcol = field->fcol;\n+    }\n+EOF\n+cat <<EOF >expected3\n+1\n+1\n+Final\n+2\n+2\n+Final\n+3\n+3\n+Final\n+4\n+4\n+Final\n+EOF\n+\n+\n+title4=\"Combine many lines and change case\"\n+cat <<EOF >a4\n+for(row=0,pBuffer=field->buf;\n+    row<height;\n+    row++,pBuffer+=width )\n+  {\n+    if ((len = (int)( After_End_Of_Data( pBuffer, width ) - pBuffer )) > 0)\n+      {\n+        wmove( win, row, 0 );\n+        waddnstr( win, pBuffer, len );\n+EOF\n+cat <<EOF >b4\n+for (Row = 0, PBuffer = field->buf; Row < Height; Row++, PBuffer += Width) {\n+  if ((Len = (int)(afterEndOfData(PBuffer, Width) - PBuffer)) > 0) {\n+    wmove(win, Row, 0);\n+    waddnstr(win, PBuffer, Len);\n+EOF\n+cat <<EOF >expected4\n+1\n+5\n+7\n+8\n+EOF\n+\n+title5=\"Rename and combine lines\"\n+cat <<EOF >a5\n+bool need_visual_update = ((form != (FORM *)0)      &&\n+                           (form->status & _POSTED) &&\n+                           (form->current==field));\n+\n+if (need_visual_update)\n+  Synchronize_Buffer(form);\n+\n+if (single_line_field)\n+  {\n+    growth = field->cols * amount;\n+    if (field->maxgrow)\n+      growth = Minimum(field->maxgrow - field->dcols,growth);\n+    field->dcols += growth;\n+    if (field->dcols == field->maxgrow)\n+EOF\n+cat <<EOF >b5\n+bool NeedVisualUpdate = ((Form != (FORM *)0) && (Form->status & _POSTED) &&\n+                         (Form->current == field));\n+\n+if (NeedVisualUpdate) {\n+  synchronizeBuffer(Form);\n+}\n+\n+if (SingleLineField) {\n+  Growth = field->cols * amount;\n+  if (field->maxgrow) {\n+    Growth = Minimum(field->maxgrow - field->dcols, Growth);\n+  }\n+  field->dcols += Growth;\n+  if (field->dcols == field->maxgrow) {\n+EOF\n+cat <<EOF >expected5\n+1\n+3\n+4\n+5\n+6\n+Final\n+7\n+8\n+10\n+11\n+12\n+Final\n+13\n+14\n+EOF\n+\n+# Both lines match identically so position must be used to tie-break.\n+title6=\"Same line twice\"\n+cat <<EOF >a6\n+abc\n+abc\n+EOF\n+cat <<EOF >b6\n+abcd\n+abcd\n+EOF\n+cat <<EOF >expected6\n+1\n+2\n+EOF\n+\n+title7=\"Enforce line order\"\n+cat <<EOF >a7\n+abcdef\n+ghijkl\n+ab\n+EOF\n+cat <<EOF >b7\n+ghijk\n+abcd\n+EOF\n+cat <<EOF >expected7\n+2\n+3\n+EOF\n+\n+title8=\"Expand lines and rename variables\"\n+cat <<EOF >a8\n+int myFunction(int ArgumentOne, Thing *ArgTwo, Blah XuglyBug) {\n+  Squiggle FabulousResult = squargle(ArgumentOne, *ArgTwo,\n+    XuglyBug) + EwwwGlobalWithAReallyLongNameYepTooLong;\n+  return FabulousResult * 42;\n+}\n+EOF\n+cat <<EOF >b8\n+int myFunction(int argument_one, Thing *arg_asdfgh,\n+    Blah xugly_bug) {\n+  Squiggle fabulous_result = squargle(argument_one,\n+    *arg_asdfgh, xugly_bug)\n+    + g_ewww_global_with_a_really_long_name_yep_too_long;\n+  return fabulous_result * 42;\n+}\n+EOF\n+cat <<EOF >expected8\n+1\n+1\n+2\n+3\n+3\n+4\n+5\n+EOF\n+\n+test_expect_success setup '\n+\t{ for ((i=1;i<=$file_count;i++))\n+\tdo\n+\t\t# Append each line in a separate commit to make it easy to\n+\t\t# check which original line the blame output relates to.\n+\n+\t\tline_count=0 &&\n+\t\t{ while read line\n+\t\tdo\n+\t\t\tline_count=$((line_count+1)) &&\n+\t\t\techo $line >>\"$i\" &&\n+\t\t\tgit add \"$i\" &&\n+\t\t\ttest_tick &&\n+\t\t\tGIT_AUTHOR_NAME=\"$line_count\" git commit -m \"$line_count\"\n+\t\tdone } <\"a$i\"\n+\tdone } &&\n+\n+\t{ for ((i=1;i<=$file_count;i++))\n+\tdo\n+\t\t# Overwrite the files with the final content.\n+\t\tcp b$i $i &&\n+\t\tgit add $i &&\n+\t\ttest_tick\n+\tdone } &&\n+\n+\t# Commit the final content all at once so it can all be\n+\t# referred to with the same commit ID.\n+\tGIT_AUTHOR_NAME=Final git commit -m Final\n+'\n+\n+for ((i=1;i<=$file_count;i++)); do\n+\ttitle=\"title$i\"\n+\ttest_expect_success \"${!title}\" \\\n+\t\"git blame --fuzzy -- $i | sed -e \\\"$pick_author\\\" >actual && test_cmp expected$i actual\"\n+done\n+\n+test_done\n-- \n2.14.3 (Apple Git-98)\n\n"},{"id":"372394","messageId":"xmqq5zs7oexn.fsf@gitster-ct.c.googlers.com","threadId":"50815","inReplyTo":"20190324235020.49706-1-michael@platin.gs","subject":"Re: [RFC PATCH 0/1] Fuzzy blame","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-25T02:39:32Z","receivedAt":"2019-03-25T02:39:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"michael@platin.gs writes:\n\n> From: Michael Platings <michael@platin.gs>\n>\n> Hi Git devs,\n>\n> Some of you may be familiar with the git-hyper-blame tool [1]. It's \"useful if\n> you have a commit that makes sweeping changes that are unlikely to be what you\n> are looking for in a blame, such as mass reformatting or renaming.\"\n\nI recall a month or so ago brho@google (CC'ed) sent a \"let's allow\nblame to ignore some uninteresting commit\" topic, which was\nunfortunately not well reviewed (what I mean is *not* that it was\nreviewed thoroughly and found to be bad---not many reviewers found\ntime or inclination to review it well).  The topic is queued as\nbr/blame-ignore and its tip is at 43a290e3 (\"SQUASH???\", 2019-02-13)\nas of this writing.\n\nPerhaps you two can join forces?\n\nP.S. I expect to be offline for most of the week (packing, moving\nand unpacking.  Even though the places packing and unpacking happens\nare within 1 kilometer radius, that does not make it less hassle\nX-<).  See you guys next month.\n"},{"id":"372403","messageId":"CAJDYR9RWUmXzh9Pn3qGBXAxNf70-SMKUCB3wwXVYKRTKOy8F_g@mail.gmail.com","threadId":"50815","inReplyTo":"xmqq5zs7oexn.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH 0/1] Fuzzy blame","fromName":"Michael Platings","fromEmail":"michael@platin.gs","sentAt":"2019-03-25T09:32:22Z","receivedAt":"2019-03-25T09:32:38Z","isPatch":true,"sender":{"key":"michael@platin.gs","avatar":"https://avatars.githubusercontent.com/u/1112348?v=4"},"body":"(resending in plain text mode, sorry for the noise)\n\nThanks Junio, that's super helpful!\n\nA month or two ago I contacted the author of git-hyper-blame, Matt\nGiuca, asking whether anyone had looked into adding the feature to git\nblame. I didn't receive a response but maybe that prompted Barret\nRhoden's patch? Or maybe just a weird coincidence!\n\n@Barret I see your patches are a nice translation of git-hyper-blame.\nHowever could you give me your thoughts on the approach in my patch? A\ncomment in the git-hyper-blame source [1] says:\n# This doesn't work that well if there are a lot of line changes within the\n# hunk (demonstrated by GitHyperBlameLineMotionTest.testIntraHunkLineMotion).\n# A fuzzy heuristic that takes the text of the new line and tries to find a\n# deleted line within the hunk that mostly matches the new line could help.\n\nMy patch aims to implement this \"fuzzy heuristic\" so I'd love to get\nyour take on it.\n\nMany thanks,\n-Michael\n\nOn Mon, 25 Mar 2019 at 02:39, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> michael@platin.gs writes:\n>\n> > From: Michael Platings <michael@platin.gs>\n> >\n> > Hi Git devs,\n> >\n> > Some of you may be familiar with the git-hyper-blame tool [1]. It's \"useful if\n> > you have a commit that makes sweeping changes that are unlikely to be what you\n> > are looking for in a blame, such as mass reformatting or renaming.\"\n>\n> I recall a month or so ago brho@google (CC'ed) sent a \"let's allow\n> blame to ignore some uninteresting commit\" topic, which was\n> unfortunately not well reviewed (what I mean is *not* that it was\n> reviewed thoroughly and found to be bad---not many reviewers found\n> time or inclination to review it well).  The topic is queued as\n> br/blame-ignore and its tip is at 43a290e3 (\"SQUASH???\", 2019-02-13)\n> as of this writing.\n>\n> Perhaps you two can join forces?\n>\n> P.S. I expect to be offline for most of the week (packing, moving\n> and unpacking.  Even though the places packing and unpacking happens\n> are within 1 kilometer radius, that does not make it less hassle\n> X-<).  See you guys next month.\n"},{"id":"372437","messageId":"b077afed-d143-506e-977e-6edf2492f75f@google.com","threadId":"50815","inReplyTo":"CAJDYR9RWUmXzh9Pn3qGBXAxNf70-SMKUCB3wwXVYKRTKOy8F_g@mail.gmail.com","subject":"Re: [RFC PATCH 0/1] Fuzzy blame","fromName":"Barret Rhoden","fromEmail":"brho@google.com","sentAt":"2019-03-25T16:04:45Z","receivedAt":"2019-03-25T16:04:50Z","isPatch":true,"sender":{"key":"brho@google.com","avatar":null},"body":"Hi -\n\nOn 3/25/19 5:32 AM, Michael Platings wrote:\n> (resending in plain text mode, sorry for the noise)\n> \n> Thanks Junio, that's super helpful!\n> \n> A month or two ago I contacted the author of git-hyper-blame, Matt\n> Giuca, asking whether anyone had looked into adding the feature to git\n> blame. I didn't receive a response but maybe that prompted Barret\n> Rhoden's patch? Or maybe just a weird coincidence!\n\nWeird coincidence.  It's a big company.  =)\n\nI work on a project that needs a major reformatting, and one thing \ndelaying me was the lack of an ability to ignore commits during blame. \nhyper-blame does that, but I never liked the fact that it wasn't \ndirectly in git.\n\n> \n> @Barret I see your patches are a nice translation of git-hyper-blame.\n\nNot sure if you've seen my latest version, updated to v4 (2019-02-26) here:\n\nhttps://public-inbox.org/git/20190226170648.211847-1-brho@google.com/\n\nThe one Junio has (br/blame-ignore) hasn't been updated - not sure if \nthat's automated, or if it just fell through the cracks.\n\n> However could you give me your thoughts on the approach in my patch? A\n> comment in the git-hyper-blame source [1] says:\n> # This doesn't work that well if there are a lot of line changes within the\n> # hunk (demonstrated by GitHyperBlameLineMotionTest.testIntraHunkLineMotion).\n> # A fuzzy heuristic that takes the text of the new line and tries to find a\n> # deleted line within the hunk that mostly matches the new line could help.\n> \n> My patch aims to implement this \"fuzzy heuristic\" so I'd love to get\n> your take on it.\n\nThis is an interesting idea, and it sounds like it might be \ncomplimentary to the blame-ignore work.  Both have the flavor of \"user \nknows this commit is special and wants special processing.\"\n\nIn my patch, I didn't try to find the likely original version of a line \nin a diff hunk.  What I did amounted to finding blame based on the \nparent's image of the file.  Example in this message:\n\nhttps://public-inbox.org/git/20190226170648.211847-4-brho@google.com/\n\nOf note, line 12 was blamed on commit b, when it really came from commit a.\n\nFor any lines added beyond the size of the parent's image (e.g. the \nignored commit added more lines than it removed), those lines were \nremoved from blame contention - marked with all 0s.\n\nIn essence, my method did the following:\n\n\tfor all suspect/child lines 'i' in a diff hunk\n\t\tif i <= parent's hunk size\n\t\t\tassign to parent line i (+ offset)\n\t\telse\n\t\t\tmark umblamable\n\nDue to the two cases being contiguous, each hunk would be broken up into \nat most two blame entries.  (I actually short-circuit that for loop and \njust split at i == parent_size immediately).\n\nHaving a smart/fuzzy matcher sounds nicer.  My patch passes blame to the \nparent.  Yours finds the right *part* of the parent to blame, which \nmeans we have a better chance of finding the right original commit to blame.\n\nI think you're doing this:\n\n\tfor all suspect/child lines 'i' in a diff hunk\n\t\tif i matches parent line (say, x)\n\t\t\tassign to parent line x\n\t\telse\n\t\t\tassign to child line i\n\n\n From glancing at your code, it looks like you break up every blame \nentry into N entries, one for each line, which you need since each \nparent line X might not be adjacent to the matching lines in the child.\n\nOne question I have is under which circumstances do you find that you \ncannot match a suspect/child line to a parent?  One obvious case is a \ncommit that only adds lines - the parent's line set is the empty set.  I \nthink you catch that earlier in your code (parent_chunk_length == 0), \nthough I guess there are other cases where the parent has lines, but \nthey are below a certain match threshold?\n\nFor those cases where you can't find a match, I could imagine marking \nthem as unblamable.  The purpose of 'unblamable' in my patchset was to \nsignal to not bother looking up further commit info for a final blame \nentry.  It was largely so that the user (me!) wouldn't see a commit \nblamed when I explicitly told git to not tell me about that commit. \nBoth approaches sound fine though.\n\nAnother question was whether or not you wanted this to apply per commit \nor for an entire blame session.  It looks like your current code applies \nit overall, and not for a specific commit.  I'd much prefer it to be \nper-commit, though maybe the -F is just for showing it working in the RFC.\n\nThe first thing that comes to mind for me is to plug your fuzzy logic \ninto my patch set.  Basically, in my commit near \"These go to the \nparent\", we do your line-by-line matching.  We might not need my 'delta' \ncheck anymore, which was basically the 'if' part of my for loop (in text \nabove).  But maybe we need more info for your function.\n\nThat way, we'd get the per-commit control of when we ignore a commit \nfrom my patch, and we'd get the fuzzy-blaming brains of your patch, such \nthat we try to find the right *part* of the parent to blame.\n\nAnyway, let me know what your thoughts are.  We could try to change that \npart of my code so that it just calls some function that tells it where \nin the parent to blame, and otherwise marks unblamable.  Then your patch \ncan replace the logic?\n\nBarret\n\n"},{"id":"372484","messageId":"CAJDYR9R77_+gfOgLXX_Az8iODNRyDTHAT8BAubZeptEWJViYqA@mail.gmail.com","threadId":"50815","inReplyTo":"b077afed-d143-506e-977e-6edf2492f75f@google.com","subject":"Re: [RFC PATCH 0/1] Fuzzy blame","fromName":"Michael Platings","fromEmail":"michael@platin.gs","sentAt":"2019-03-25T23:21:19Z","receivedAt":"2019-03-25T23:21:33Z","isPatch":true,"sender":{"key":"michael@platin.gs","avatar":"https://avatars.githubusercontent.com/u/1112348?v=4"},"body":"Hi Barret,\n\n> I work on a project that needs a major reformatting, and one thing\n> delaying me was the lack of an ability to ignore commits during blame.\n\nI think we understand each other well then - I'm working on a plan to\nchange the variable naming rule in LLVM, and naturally other\ndevelopers aren't keen on making git blame less useful.\n\n> One question I have is under which circumstances do you find that you\n> cannot match a suspect/child line to a parent?  One obvious case is a\n> commit that only adds lines - the parent's line set is the empty set.  I\n> think you catch that earlier in your code (parent_chunk_length == 0),\n> though I guess there are other cases where the parent has lines, but\n> they are below a certain match threshold?\n\nYes, exactly. The threshold is currently 0 i.e. a single matching\nbigram is all that's required for two lines to be considered matching,\nbut in future the threshold could be configurable in the same manner\nas -M & -C options.\nIn the t8020-blame-fuzzy.sh test script in my patch, where it's\nexpected that a line will be attributed to the \"ignored\" commit you'll\nsee \"Final\". So far this is just \"}\" lines.\n\n> Another question was whether or not you wanted this to apply per commit\n> or for an entire blame session.  It looks like your current code applies\n> it overall, and not for a specific commit.\n\nThis is a really interesting question for this feature. Initially I\njust wanted to be able to say \"Hey, Git, ignore this revision please.\"\nBut then Git says \"OK, but how exactly? I can ignore whitespace and I\ncan detect moves & copies so do you want me to do those?\" And then I'm\nthinking, actually yes -M10 would be great because I know that this\nrevision also reordered a bunch of #includes and I still want people\nto be able to see where they came from. However other sets of options\nmight work better for other changes.\n\nOn looking at the problem this way it seems that fuzzy matching\nbelongs in the same class as -w, -M & -C. As these options apply for\nan entire blame session, it would be consistent to allow applying the\nfuzzy matching likewise. As a bonus, having the ability to apply the\n-F option for the entire blame session seems quite nice for other use\ncases.\n\n> I'd much prefer it to be per-commit\n\nYes, we definitely need a way to say \"fuzzy match this commit only\"\notherwise you lose the ability to detect small but significant changes\nin other commits.\nI haven't explored this fully, but I'm thinking that the revision\noptions file might look something like this:\n\n# Just use the defaults, whatever they may be\n6e8063eee1d30bc80c7802e94ed0caa8949c6323\n# This commit changed loads of tabs to spaces\n35ee755a8c43bcb3c2786522d423f006c23d32df -w\n# This commit reordered lots of short lines\nc5b679e14b761a7bfd6ae93cfffbf66e3c4e25a5 -M5\n# This commit copied some stuff and changed CamelCase to snake_case\n58b9cd43da695ee339b7679cf0c9f31e1f8ef67f -w -C15 -F\n\nFor the command-line, potentially we could make -w/-M/-C/-F specified\nafter --ignore-rev apply to only that revision e.g.:\ngit blame myfile --ignore-rev 35ee755a8c43bcb3c2786522d423f006c23d32df -M -F\n\nBut as I say, I haven't explored this fully.\n\n> For those cases where you can't find a match, I could imagine marking\n> them as unblamable.  The purpose of 'unblamable' in my patchset was to\n> signal to not bother looking up further commit info for a final blame\n> entry.  It was largely so that the user (me!) wouldn't see a commit\n> blamed when I explicitly told git to not tell me about that commit.\n\nI can see how the unblameable concept makes sense for the heuristic\nyou have right now. However, once you have a heuristic that can tell\nyou with a high degree of certainty that a line really did come from a\ncommit that you're merely not interested in, then I suggest that it's\nbetter to just point at that commit.\n\n> Both approaches sound fine though.\n\n:)\n\n> The first thing that comes to mind for me is to plug your fuzzy logic\n> into my patch set.\n\nPlease do! It should be easy to pluck fuzzy_find_matching_lines() and\nits dependencies out. Just to set your expectations, I have not yet\noptimised it and it is highly wasteful right now both in terms of time\nand memory.\n\n> But maybe we need more info for your function.\n\nThe extra info needed is the parent & child file content, plus the\nindices in the strings of where new lines start. This information is\nalready calculated in the course of doing the diff but it looks like a\nfair amount of plumbing work will be needed to make the information\navailable to the chunk callback. That was my reason for initially\nplonking the fuzzy matching in a separate pass.\n\nThanks,\n-Michael\n"},{"id":"372487","messageId":"20190325233516.GB23728@sigill.intra.peff.net","threadId":"50815","inReplyTo":"CAJDYR9R77_+gfOgLXX_Az8iODNRyDTHAT8BAubZeptEWJViYqA@mail.gmail.com","subject":"Re: [RFC PATCH 0/1] Fuzzy blame","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-25T23:35:16Z","receivedAt":"2019-03-25T23:35:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 25, 2019 at 11:21:19PM +0000, Michael Platings wrote:\n\n> > I work on a project that needs a major reformatting, and one thing\n> > delaying me was the lack of an ability to ignore commits during blame.\n> \n> I think we understand each other well then - I'm working on a plan to\n> change the variable naming rule in LLVM, and naturally other\n> developers aren't keen on making git blame less useful.\n\nThis is sort of a tangent to the thread, but have you looked into tools\nthat provide an interactive \"re-blame from the parent\" operation? I use\ntig for this.  Quite often my blame turns up on some boring line\n(whitespace fixing, minor tweaking of a function interface, etc), and\nthen I want to keep digging on the \"same\" line, as counted by line count\n(but it's OK if it's off by one or two lines, since I'm looking at a\nblame of the whole file).\n\nObviously this isn't as automated as saying \"ignore commit X, it's just\nvariable renaming\". But it also eliminates the need to a priori figure\nout all such X that affect the lines you care about. You get an answer,\nyour human mind says \"nope, that's not interesting\", and you press a\nbutton to dig further.\n\nI think there's room for both solutions to co-exist, but just suggesting\nyou to try out the one that's already been implemented if you haven't. ;)\n\n-Peff\n"},{"id":"372490","messageId":"CA+P7+xo-AHmB+Wv0Z+dpgshhmqSLEb41T-JP+NKJD8DAFARA5w@mail.gmail.com","threadId":"50815","inReplyTo":"20190325233516.GB23728@sigill.intra.peff.net","subject":"Re: [RFC PATCH 0/1] Fuzzy blame","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2019-03-26T03:07:53Z","receivedAt":"2019-03-26T03:08:09Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Mon, Mar 25, 2019 at 4:37 PM Jeff King <peff@peff.net> wrote:\n>\n> On Mon, Mar 25, 2019 at 11:21:19PM +0000, Michael Platings wrote:\n>\n> > > I work on a project that needs a major reformatting, and one thing\n> > > delaying me was the lack of an ability to ignore commits during blame.\n> >\n> > I think we understand each other well then - I'm working on a plan to\n> > change the variable naming rule in LLVM, and naturally other\n> > developers aren't keen on making git blame less useful.\n>\n> This is sort of a tangent to the thread, but have you looked into tools\n> that provide an interactive \"re-blame from the parent\" operation? I use\n> tig for this.  Quite often my blame turns up on some boring line\n> (whitespace fixing, minor tweaking of a function interface, etc), and\n> then I want to keep digging on the \"same\" line, as counted by line count\n> (but it's OK if it's off by one or two lines, since I'm looking at a\n> blame of the whole file).\n>\n\n+1 for the usefulness of this approach. It really helps figure things\nout in a way that doesn't require me to track all \"uninteresting\"\ncommits, and also works when I *am* trying to find that uninteresting\ncommit too.\n\n> Obviously this isn't as automated as saying \"ignore commit X, it's just\n> variable renaming\". But it also eliminates the need to a priori figure\n> out all such X that affect the lines you care about. You get an answer,\n> your human mind says \"nope, that's not interesting\", and you press a\n> button to dig further.\n>\n> I think there's room for both solutions to co-exist, but just suggesting\n> you to try out the one that's already been implemented if you haven't. ;)\n>\n> -Peff\n\nThat's also my sentiment.\n\nThanks,\nJake\n"},{"id":"372546","messageId":"CAJDYR9RVz6ZKQ-vdC8O3LYZnGeBcGHCRtL0m6UoRrKDBsUoFOw@mail.gmail.com","threadId":"50815","inReplyTo":"CA+P7+xo-AHmB+Wv0Z+dpgshhmqSLEb41T-JP+NKJD8DAFARA5w@mail.gmail.com","subject":"Re: [RFC PATCH 0/1] Fuzzy blame","fromName":"Michael Platings","fromEmail":"michael@platin.gs","sentAt":"2019-03-26T20:26:46Z","receivedAt":"2019-03-26T20:27:00Z","isPatch":true,"sender":{"key":"michael@platin.gs","avatar":"https://avatars.githubusercontent.com/u/1112348?v=4"},"body":"> Obviously this isn't as automated as saying \"ignore commit X, it's just\n> variable renaming\". But it also eliminates the need to a priori figure\n> out all such X that affect the lines you care about. You get an answer,\n> your human mind says \"nope, that's not interesting\", and you press a\n> button to dig further.\n\nHi Peff, for the use case you describe of someone stumbling across a\nrenaming commit, your approach is clearly better. However the use case\nBarret & I are facing is of deliberately choosing to make a large\nrefactoring/renaming commit, and not wanting everyone else working on\nthe project to have to press that extra button every time they run git\nblame.\n\nI think it's really important that we make this dead easy for everyone\nto use. The ultimate in ease of use would be for git blame to\nautomatically pick up ignore settings without the user having to even\nknow that it's happening. But that breaks the principle of least\nastonishment. The next simplest thing I can think of is to add a\nconfiguration option blame.ignoreRevs which would have the same\neffect, except the user has to opt in.\nBarret has implemented blame.ignoreRevsFile, but I think the world\nwill be a more consistent and happier place if we dictate the location\nthat the revisions are loaded from, in the same way as .gitignore.\nDeciding what that location should be is one of those bikeshed\narguments which is perhaps why Barret dodged it :)\n\n-Michael\n"},{"id":"372557","messageId":"CACsJy8D8yBK9p9Rgy+wk8cMfPLG7qanvGA-LcmmHmjbaMnvBLQ@mail.gmail.com","threadId":"50815","inReplyTo":"CAJDYR9RVz6ZKQ-vdC8O3LYZnGeBcGHCRtL0m6UoRrKDBsUoFOw@mail.gmail.com","subject":"Re: [RFC PATCH 0/1] Fuzzy blame","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-03-27T06:36:36Z","receivedAt":"2019-03-27T06:37:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 27, 2019 at 3:27 AM Michael Platings <michael@platin.gs> wrote:\n> I think it's really important that we make this dead easy for everyone\n> to use. The ultimate in ease of use would be for git blame to\n> automatically pick up ignore settings without the user having to even\n> know that it's happening. But that breaks the principle of least\n> astonishment. The next simplest thing I can think of is to add a\n> configuration option blame.ignoreRevs which would have the same\n> effect, except the user has to opt in.\n> Barret has implemented blame.ignoreRevsFile, but I think the world\n> will be a more consistent and happier place if we dictate the location\n> that the revisions are loaded from, in the same way as .gitignore.\n> Deciding what that location should be is one of those bikeshed\n> arguments which is perhaps why Barret dodged it :)\n\nAnd bikeshedding. Another good place to keep these revs is git-notes,\nwhich probably could result in faster lookups too and can be made\nvisible in git-log. But that's in addition to --ignoreRevsFile, not\nreplacing it.\n-- \nDuy\n"},{"id":"372558","messageId":"CAJDYR9T40xaSpmL_e00WWXzxEm_j0pcTqBhvr=5mu-fTpKodmQ@mail.gmail.com","threadId":"50815","inReplyTo":"CACsJy8D8yBK9p9Rgy+wk8cMfPLG7qanvGA-LcmmHmjbaMnvBLQ@mail.gmail.com","subject":"Re: [RFC PATCH 0/1] Fuzzy blame","fromName":"Michael Platings","fromEmail":"michael@platin.gs","sentAt":"2019-03-27T08:26:00Z","receivedAt":"2019-03-27T08:26:14Z","isPatch":true,"sender":{"key":"michael@platin.gs","avatar":"https://avatars.githubusercontent.com/u/1112348?v=4"},"body":"> Another good place to keep these revs is git-notes,\n> which probably could result in faster lookups too and can be made\n> visible in git-log.\n\nOh wow, I really like this. A major concern I had about the revisions\nfile was that you don't know what a revision ID will be until it's\nupstream. If you can specify *in the commit message itself* what\noptions should apply to git blame for that revision then that problem\nis solved. And if you change your mind later, or want to ignore a\npre-existing revision then git-notes solves that problem.\n\nSo I'm thinking you just have a commit message like this:\n\"\nMake all function names snake_case\ngit-blame-ignore: fuzzy\n\"\nAnd users who have blame.ignoreRevs set will have the -F/--fuzzy\noption applied to that commit.\n\n> But that's in addition to --ignoreRevsFile, not replacing it.\n\nI disagree. ignoreRevsFile has the major problem that the file will\nneed updating every time you rebase a commit to be ignored, and you'll\nneed to remember to edit it for cherry picks. Let's not have that\noption as I think it will add unhelpful complexity.\n\n-Michael\n"},{"id":"372559","messageId":"CACsJy8D-Nwyh0tXsOiqBUnkKVm5TQcVaszdLX1FMr7vDfQ8krg@mail.gmail.com","threadId":"50815","inReplyTo":"CAJDYR9T40xaSpmL_e00WWXzxEm_j0pcTqBhvr=5mu-fTpKodmQ@mail.gmail.com","subject":"Re: [RFC PATCH 0/1] Fuzzy blame","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-03-27T09:02:02Z","receivedAt":"2019-03-27T09:02:31Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 27, 2019 at 3:26 PM Michael Platings <michael@platin.gs> wrote:\n>\n> > Another good place to keep these revs is git-notes,\n> > which probably could result in faster lookups too and can be made\n> > visible in git-log.\n>\n> Oh wow, I really like this. A major concern I had about the revisions\n> file was that you don't know what a revision ID will be until it's\n> upstream. If you can specify *in the commit message itself* what\n> options should apply to git blame for that revision then that problem\n> is solved. And if you change your mind later, or want to ignore a\n> pre-existing revision then git-notes solves that problem.\n>\n> So I'm thinking you just have a commit message like this:\n> \"\n> Make all function names snake_case\n> git-blame-ignore: fuzzy\n> \"\n> And users who have blame.ignoreRevs set will have the -F/--fuzzy\n> option applied to that commit.\n\nYeah some trailer in the commit itself is also good if you know in\nadvance it should be treated differently. I think we have\ngit-interpret-trailers to help extract these info.\n\n> > But that's in addition to --ignoreRevsFile, not replacing it.\n>\n> I disagree. ignoreRevsFile has the major problem that the file will\n> need updating every time you rebase a commit to be ignored, and you'll\n> need to remember to edit it for cherry picks. Let's not have that\n> option as I think it will add unhelpful complexity.\n\nOK I was just trying to say I did not object any current suggestions\n(because I didn't know much in the first place). I'll just leave this\nfor other people to discuss :)\n-- \nDuy\n"},{"id":"373054","messageId":"7540d14b-f225-39ec-b37e-54cb157d4a72@google.com","threadId":"50815","inReplyTo":"CAJDYR9R77_+gfOgLXX_Az8iODNRyDTHAT8BAubZeptEWJViYqA@mail.gmail.com","subject":"Re: [RFC PATCH 0/1] Fuzzy blame","fromName":"Barret Rhoden","fromEmail":"brho@google.com","sentAt":"2019-04-03T15:25:43Z","receivedAt":"2019-04-03T15:25:49Z","isPatch":true,"sender":{"key":"brho@google.com","avatar":null},"body":"Hi -\n\nOn 3/25/19 7:21 PM, Michael Platings wrote:\n>> The first thing that comes to mind for me is to plug your fuzzy logic\n>> into my patch set.\n> Please do! It should be easy to pluck fuzzy_find_matching_lines() and\n> its dependencies out. Just to set your expectations, I have not yet\n> optimised it and it is highly wasteful right now both in terms of time\n> and memory.\n\nI edited my patch set to allow changing the heuristic.  I also made a \ncommit that uses your fingerprinting code to match target lines to \nparent lines.  I'll send it all out in another email and CC you.  The \nlast commit is still a work in progress.\n\nRegarding stuff like the name of the ignore file, in my first version, I \nwent with whatever git hyper-blame does.  That was shot down, rightly \nso, I think.  With a git-config setting, you can name the file whatever \nyou want, or add multiple files.  With my current patchset, you can \ndisable the file too with --ignore-revs-file=\"\".\n\nAs far as using notes or per-commit info, that might be nice, though \nit's not a huge burden to have a separate commit - you can wait til \nafter things get merged (so we have the final object name (hash)) and \nit's not hugely burdensome.  But I get that's just my opinion.  =)\n\nBarret\n\n\n\n"},{"id":"373072","messageId":"CAJDYR9TmRThj5J_4ecj619_-19VqzHX17dBpUScJvvv6c=5zjA@mail.gmail.com","threadId":"50815","inReplyTo":"7540d14b-f225-39ec-b37e-54cb157d4a72@google.com","subject":"Re: [RFC PATCH 0/1] Fuzzy blame","fromName":"Michael Platings","fromEmail":"michael@platin.gs","sentAt":"2019-04-03T21:49:39Z","receivedAt":"2019-04-03T21:49:54Z","isPatch":true,"sender":{"key":"michael@platin.gs","avatar":"https://avatars.githubusercontent.com/u/1112348?v=4"},"body":"Thanks Barret.\nI've cooled off on the git-notes idea since learning that notes\nbranches have to be pulled explicitly. And different people may have\ndifferent ideas about which types of commits they want to ignore, so\nnot predefining the name of the ignore file(s) does seem like the best\noption, even if it's not perfect. And maybe having --fuzzy as a\nseparate option would be nice, maybe not - either way it can wait. In\nshort, I've come full circle and wish to do what I can to assist your\nproposal (+ fuzzy matching).\nI've rewritten the fuzzy matching function so it's now much faster and\nmore modest in its memory use. I hope to share the patch tomorrow.\nFollowing that, I'll do what I can to assist with reviewing your\npatches.\nCheers,\n-Michael\n"}]}