{"thread":{"id":"23989","subject":"[RFC/PATCH 3/4] textconv: support for blame","startedAt":"2010-06-03T10:47:14Z","lastAt":"2010-06-06T21:51:59Z","messageCount":16,"participants":["Axel Bonnet","Johannes Sixt","Junio C Hamano","Matthieu Moy","Diane Gasselin","bonneta","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"142874","messageId":"1275562038-7468-1-git-send-email-axel.bonnet@ensimag.imag.fr","threadId":"23989","inReplyTo":null,"subject":"[RFC/PATCH 0/4] textconv support for blame","fromName":"Axel Bonnet","fromEmail":"axel.bonnet@ensimag.imag.fr","sentAt":"2010-06-03T10:47:14Z","receivedAt":"2010-06-03T10:47:14Z","isPatch":true,"sender":{"key":"axel.bonnet@ensimag.imag.fr","avatar":null},"body":"This is a patch series to implement textconv support for git blame. As textconv support has already been added to git diff, so we use textconv methods of diff.\nHere are the different changes:\n- make the diff textconv API public\n- add diff_options to blame (--textconv and --no-textconv)\n- perform textconv when we meet an object with textconv driver\n- t8006-blame-textconv.sh tests conversion works\n\nSigned-off-by: Diane Gasselin \nSigned-off-by: Clément Poulain \nSigned-off-by: Axel Bonnet \n\nAxel Bonnet (4):\n textconv: make the API public\n textconv: make diff_options accessible from blame\n textconv: support for blame\n t/t8006: test textconv support for blame\n\n builtin/blame.c | 92 ++++++++++++++++++++++++++++++++++--------\n diff.c | 12 ++----\n diff.h | 8 ++++\n t/t8006-blame-textconv.sh | 98 +++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 184 insertions(+), 26 deletions(-)\n create mode 100755 t/t8006-blame-textconv.sh\n"},{"id":"142875","messageId":"1275562038-7468-2-git-send-email-axel.bonnet@ensimag.imag.fr","threadId":"23989","inReplyTo":"1275562038-7468-1-git-send-email-axel.bonnet@ensimag.imag.fr","subject":"[RFC/PATCH 1/4] textconv: make the API public","fromName":"Axel Bonnet","fromEmail":"axel.bonnet@ensimag.imag.fr","sentAt":"2010-06-03T10:47:15Z","receivedAt":"2010-06-03T10:47:15Z","isPatch":true,"sender":{"key":"axel.bonnet@ensimag.imag.fr","avatar":null},"body":"The textconv functionality allows one to convert a file into text before\nrunning diff. But this functionality can be useful to other features\nsuch as blame.\n\nSigned-off-by: Diane Gasselin \nSigned-off-by: Clément Poulain \nSigned-off-by: Axel Bonnet \n---\n diff.c | 12 ++++--------\n diff.h | 8 ++++++++\n 2 files changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 494f560..b4a830f 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -43,10 +43,6 @@ static char diff_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_NORMAL,\t/* FUNCINFO */\n };\n \n-static void diff_filespec_load_driver(struct diff_filespec *one);\n-static size_t fill_textconv(struct userdiff_driver *driver,\n-\t\t\t struct diff_filespec *df, char **outbuf);\n-\n static int parse_diff_color_slot(const char *var, int ofs)\n {\n \tif (!strcasecmp(var+ofs, \"plain\"))\n@@ -1629,7 +1625,7 @@ void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const\n \t\toptions->b_prefix = b;\n }\n \n-static struct userdiff_driver *get_textconv(struct diff_filespec *one)\n+struct userdiff_driver *get_textconv(struct diff_filespec *one)\n {\n \tif (!DIFF_FILE_VALID(one))\n \t\treturn NULL;\n@@ -4002,9 +3998,9 @@ static char *run_textconv(const char *pgm, struct diff_filespec *spec,\n \treturn strbuf_detach(&buf, outsize);\n }\n \n-static size_t fill_textconv(struct userdiff_driver *driver,\n-\t\t\t struct diff_filespec *df,\n-\t\t\t char **outbuf)\n+size_t fill_textconv(struct userdiff_driver *driver,\n+\t\t struct diff_filespec *df,\n+\t\t char **outbuf)\n {\n \tsize_t size;\n \ndiff --git a/diff.h b/diff.h\nindex 9ace08c..2a0e36d 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -9,6 +9,8 @@\n struct rev_info;\n struct diff_options;\n struct diff_queue_struct;\n+struct diff_filespec;\n+struct userdiff_driver;\n \n typedef void (*change_fn_t)(struct diff_options *options,\n \t\t unsigned old_mode, unsigned new_mode,\n@@ -287,4 +289,10 @@ extern void diff_no_index(struct rev_info *, int, const char **, int, const char\n \n extern int index_differs_from(const char *def, int diff_flags);\n \n+extern size_t fill_textconv(struct userdiff_driver *driver,\n+\t\t\t struct diff_filespec *df,\n+\t\t\t char **outbuf);\n+\n+extern struct userdiff_driver *get_textconv(struct diff_filespec *one);\n+\n #endif /* DIFF_H */\n-- \n1.6.6.7.ga5fe3\n"},{"id":"142872","messageId":"1275562038-7468-3-git-send-email-axel.bonnet@ensimag.imag.fr","threadId":"23989","inReplyTo":"1275562038-7468-2-git-send-email-axel.bonnet@ensimag.imag.fr","subject":"[RFC/PATCH 2/4] textconv: make diff_options accessible from blame","fromName":"Axel Bonnet","fromEmail":"axel.bonnet@ensimag.imag.fr","sentAt":"2010-06-03T10:47:16Z","receivedAt":"2010-06-03T10:47:16Z","isPatch":true,"sender":{"key":"axel.bonnet@ensimag.imag.fr","avatar":null},"body":"Diff_options specify whether conversion is activated or not. Blame needs\nto access these options in order to concert files with external drivers\n\nSigned-off-by: Diane Gasselin \nSigned-off-by: Clément Poulain \nSigned-off-by: Axel Bonnet \n---\n builtin/blame.c | 18 +++++++++++-------\n 1 files changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex fc15863..63b497c 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -89,7 +89,8 @@ struct origin {\n * Given an origin, prepare mmfile_t structure to be used by the\n * diff machinery\n */\n-static void fill_origin_blob(struct origin *o, mmfile_t *file)\n+static void fill_origin_blob(struct diff_options opt,\n+\t\t\t struct origin *o, mmfile_t *file)\n {\n \tif (!o->file.ptr) {\n \t\tenum object_type type;\n@@ -741,8 +742,8 @@ static int pass_blame_to_parent(struct scoreboard *sb,\n \tif (last_in_target < 0)\n \t\treturn 1; /* nothing remains for this target */\n \n-\tfill_origin_blob(parent, &file_p);\n-\tfill_origin_blob(target, &file_o);\n+\tfill_origin_blob(sb->revs->diffopt, parent, &file_p);\n+\tfill_origin_blob(sb->revs->diffopt, target, &file_o);\n \tnum_get_patch++;\n \n \tmemset(&xpp, 0, sizeof(xpp));\n@@ -922,7 +923,7 @@ static int find_move_in_parent(struct scoreboard *sb,\n \tif (last_in_target < 0)\n \t\treturn 1; /* nothing remains for this target */\n \n-\tfill_origin_blob(parent, &file_p);\n+\tfill_origin_blob(sb->revs->diffopt, parent, &file_p);\n \tif (!file_p.ptr)\n \t\treturn 0;\n \n@@ -1063,7 +1064,7 @@ static int find_copy_in_parent(struct scoreboard *sb,\n \n \t\t\tnorigin = get_origin(sb, parent, p->one->path);\n \t\t\thashcpy(norigin->blob_sha1, p->one->sha1);\n-\t\t\tfill_origin_blob(norigin, &file_p);\n+\t\t\tfill_origin_blob(sb->revs->diffopt, norigin, &file_p);\n \t\t\tif (!file_p.ptr)\n \t\t\t\tcontinue;\n \n@@ -1990,7 +1991,9 @@ static int git_blame_config(const char *var, const char *value, void *cb)\n * Prepare a dummy commit that represents the work tree (or staged) item.\n * Note that annotating work tree item never works in the reverse.\n */\n-static struct commit *fake_working_tree_commit(const char *path, const char *contents_from)\n+static struct commit *fake_working_tree_commit(struct diff_options opt,\n+\t\t\t\t\t const char *path,\n+\t\t\t\t\t const char *contents_from)\n {\n \tstruct commit *commit;\n \tstruct origin *origin;\n@@ -2384,7 +2387,8 @@ parse_done:\n \t\t * or \"--contents\".\n \t\t */\n \t\tsetup_work_tree();\n-\t\tsb.final = fake_working_tree_commit(path, contents_from);\n+\t\tsb.final = fake_working_tree_commit(sb.revs->diffopt, path,\n+\t\t\t\t\t\t contents_from);\n \t\tadd_pending_object(&revs, &(sb.final->object), \":\");\n \t}\n \telse if (contents_from)\n-- \n1.6.6.7.ga5fe3\n"},{"id":"142870","messageId":"1275562038-7468-4-git-send-email-axel.bonnet@ensimag.imag.fr","threadId":"23989","inReplyTo":"1275562038-7468-3-git-send-email-axel.bonnet@ensimag.imag.fr","subject":"[RFC/PATCH 3/4] textconv: support for blame","fromName":"Axel Bonnet","fromEmail":"axel.bonnet@ensimag.imag.fr","sentAt":"2010-06-03T10:47:17Z","receivedAt":"2010-06-03T10:47:17Z","isPatch":true,"sender":{"key":"axel.bonnet@ensimag.imag.fr","avatar":null},"body":"This patches enables to perform textconv with blame if a textconv driver\nis available for the file.\n\nThe main task is performed by the textconv_object function which\nprepares diff_filespec and if possible converts the file using diff\ntextconv API.\n\nTextconv conversion is enabled by default (equivalent to the option\n--textconv), since blaming binary files is useless in most cases.\nThe option --no-textconv is used to disable textconv conversion.\n\nThe declaration of fill_blob_sha1 declaration is modified to get back\nthe mode the function was getting.\n\nSigned-off-by: Diane Gasselin \nSigned-off-by: Clément Poulain \nSigned-off-by: Axel Bonnet \n---\n builtin/blame.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++--------\n 1 files changed, 63 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 63b497c..4679fd9 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -20,6 +20,7 @@\n #include \"mailmap.h\"\n #include \"parse-options.h\"\n #include \"utf8.h\"\n+#include \"userdiff.h\"\n \n static char blame_usage[] = \"git blame [options] [rev-opts] [rev] [--] file\";\n \n@@ -86,17 +87,57 @@ struct origin {\n };\n \n /*\n+ * Prepare diff_filespec and convert it using diff textconv API\n+ * if the textconv driver exists.\n+ * Return 1 if the conversion succeeds, 0 otherwise.\n+ */\n+static int textconv_object(const char *path,\n+\t\t\t const unsigned char *sha1,\n+\t\t\t unsigned short mode,\n+\t\t\t struct strbuf *buf)\n+{\n+\tstruct diff_filespec *df;\n+\n+\tdf = alloc_filespec(path);\n+\tfill_filespec(df, sha1, mode);\n+\tget_textconv(df);\n+\n+\tif (!df->driver|| !df->driver->textconv) {\n+\t\tfree_filespec(df);\n+\t\treturn 0;\n+\t}\n+\n+\tbuf->len = fill_textconv(df->driver, df, &buf->buf);\n+\tbuf->alloc = 1;\n+\tfree_filespec(df);\n+\treturn 1;\n+}\n+\n+/*\n * Given an origin, prepare mmfile_t structure to be used by the\n * diff machinery\n */\n static void fill_origin_blob(struct diff_options opt,\n \t\t\t struct origin *o, mmfile_t *file)\n {\n+\tunsigned mode;\n+\n \tif (!o->file.ptr) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n \t\tenum object_type type;\n \t\tnum_read_blob++;\n-\t\tfile->ptr = read_sha1_file(o->blob_sha1, &type,\n-\t\t\t\t\t (unsigned long *)(&(file->size)));\n+\n+\t\tget_tree_entry(o->commit->object.sha1,\n+\t\t\t o->path,\n+\t\t\t o->blob_sha1, &mode);\n+\t\tif (DIFF_OPT_TST(&opt, ALLOW_TEXTCONV) &&\n+\t\t textconv_object(o->path, o->blob_sha1, mode, &buf))\n+\t\t\tfile->ptr = strbuf_detach(&buf, (size_t *) &file->size);\n+\t\telse\n+\t\t\tfile->ptr = read_sha1_file(o->blob_sha1, &type,\n+\t\t\t\t\t\t (unsigned long *)(&(file->size)));\n+\t\tstrbuf_release(&buf);\n+\n \t\tif (!file->ptr)\n \t\t\tdie(\"Cannot read blob %s for path %s\",\n \t\t\t sha1_to_hex(o->blob_sha1),\n@@ -280,15 +321,13 @@ static struct origin *get_origin(struct scoreboard *sb,\n * the parent to detect the case where a child's blob is identical to\n * that of its parent's.\n */\n-static int fill_blob_sha1(struct origin *origin)\n+static int fill_blob_sha1(struct origin *origin, unsigned *mode)\n {\n-\tunsigned mode;\n-\n \tif (!is_null_sha1(origin->blob_sha1))\n \t\treturn 0;\n \tif (get_tree_entry(origin->commit->object.sha1,\n \t\t\t origin->path,\n-\t\t\t origin->blob_sha1, &mode))\n+\t\t\t origin->blob_sha1, mode))\n \t\tgoto error_out;\n \tif (sha1_object_info(origin->blob_sha1, NULL) != OBJ_BLOB)\n \t\tgoto error_out;\n@@ -2033,10 +2072,13 @@ static struct commit *fake_working_tree_commit(struct diff_options opt,\n \t\t\tread_from = path;\n \t\t}\n \t\tmode = canon_mode(st.st_mode);\n+\n \t\tswitch (st.st_mode & S_IFMT) {\n \t\tcase S_IFREG:\n-\t\t\tif (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)\n-\t\t\t\tdie_errno(\"cannot open or read '%s'\", read_from);\n+\t\t\tif (!DIFF_OPT_TST(&opt, ALLOW_TEXTCONV) ||\n+\t\t\t !textconv_object(read_from, null_sha1, mode, &buf))\n+\t\t\t\tif (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)\n+\t\t\t\t\tdie_errno(\"cannot open or read '%s'\", read_from);\n \t\t\tbreak;\n \t\tcase S_IFLNK:\n \t\t\tif (strbuf_readlink(&buf, read_from, st.st_size) < 0)\n@@ -2249,8 +2291,10 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \tint cmd_is_annotate = !strcmp(argv[0], \"annotate\");\n \n \tgit_config(git_blame_config, NULL);\n+\tgit_config(git_diff_ui_config, NULL);\n \tinit_revisions(&revs, NULL);\n \trevs.date_mode = blame_date_mode;\n+\tDIFF_OPT_SET(&revs.diffopt, ALLOW_TEXTCONV);\n \n \tsave_commit_buffer = 0;\n \tdashdash_pos = 0;\n@@ -2411,12 +2455,20 @@ parse_done:\n \t\tsb.final_buf_size = o->file.size;\n \t}\n \telse {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tunsigned mode;\n \t\to = get_origin(&sb, sb.final, path);\n-\t\tif (fill_blob_sha1(o))\n+\t\tif (fill_blob_sha1(o, &mode))\n \t\t\tdie(\"no such path %s in %s\", path, final_commit_name);\n \n-\t\tsb.final_buf = read_sha1_file(o->blob_sha1, &type,\n-\t\t\t\t\t &sb.final_buf_size);\n+\t\tif (DIFF_OPT_TST(&sb.revs->diffopt, ALLOW_TEXTCONV) &&\n+\t\t textconv_object(path, o->blob_sha1, mode, &buf))\n+\t\t\tsb.final_buf= strbuf_detach(&buf, (size_t *) &sb.final_buf_size);\n+\t\telse\n+\t\t\tsb.final_buf = read_sha1_file(o->blob_sha1, &type,\n+\t\t\t\t\t\t &sb.final_buf_size);\n+\n+\t\tstrbuf_release(&buf);\n \t\tif (!sb.final_buf)\n \t\t\tdie(\"Cannot read blob %s for path %s\",\n \t\t\t sha1_to_hex(o->blob_sha1),\n-- \n1.6.6.7.ga5fe3\n"},{"id":"142873","messageId":"1275562038-7468-5-git-send-email-axel.bonnet@ensimag.imag.fr","threadId":"23989","inReplyTo":"1275562038-7468-4-git-send-email-axel.bonnet@ensimag.imag.fr","subject":"[RFC/PATCH 4/4] t/t8006: test textconv support for blame","fromName":"Axel Bonnet","fromEmail":"axel.bonnet@ensimag.imag.fr","sentAt":"2010-06-03T10:47:18Z","receivedAt":"2010-06-03T10:47:18Z","isPatch":true,"sender":{"key":"axel.bonnet@ensimag.imag.fr","avatar":null},"body":"Test the correct functionning of textconv with blame and\nblame HEAD^ .\nTest the case when no driver is specified.\n\nSigned-off-by: Diane Gasselin \nSigned-off-by: Clément Poulain \nSigned-off-by: Axel Bonnet \n---\n t/t8006-blame-textconv.sh | 98 +++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 98 insertions(+), 0 deletions(-)\n create mode 100755 t/t8006-blame-textconv.sh\n\ndiff --git a/t/t8006-blame-textconv.sh b/t/t8006-blame-textconv.sh\nnew file mode 100755\nindex 0000000..d3780ed\n--- /dev/null\n+++ b/t/t8006-blame-textconv.sh\n@@ -0,0 +1,98 @@\n+#!/bin/sh\n+\n+test_description='git blame textconv support'\n+. ./test-lib.sh\n+\n+find_blame() {\n+\tsed -e 's/^.*(/(/g'\n+}\n+\n+cat >helper <<'EOF'\n+#!/bin/sh\n+sed 's/^/converted: /' \"$@\" >helper.out\n+cat helper.out\n+EOF\n+chmod +x helper\n+\n+test_expect_success 'setup ' '\n+\techo test 1 >one.bin &&\n+\techo test number 2 >two.bin &&\n+\tln one.bin link.bin &&\n+\tgit add . &&\n+\tGIT_AUTHOR_NAME=Number1 git commit -a -m First --date=\"2010-01-01 18:00:00\" &&\n+\techo test 1 version 2 >one.bin &&\n+\techo test number 2 version 2 >>two.bin &&\n+\tGIT_AUTHOR_NAME=Number2 git commit -a -m Second --date=\"2010-01-01 20:00:00\"\n+'\n+\n+cat >expected <blame\n+\tfind_blame result\n+\ttest_cmp expected result\n+'\n+\n+test_expect_success 'setup textconv filters' '\n+\techo \"*.bin diff=test\" >.gitattributes &&\n+\tgit config diff.test.textconv ./helper &&\n+\tgit config diff.test.cachetextconv false\n+'\n+\n+test_expect_success 'blame with --no-textconv' '\n+\tgit blame --no-textconv one.bin | grep Number2 >blame\n+\tfind_blame result\n+\ttest_cmp expected result\n+'\n+\n+cat >expected <blame\n+\tfind_blame result\n+\ttest_cmp expected result\n+'\n+\n+cat >expected <blame\n+\tfind_blame result\n+\ttest_cmp expected result\n+'\n+\n+test_expect_success 'make a new commit' '\n+\techo \"test number 2 version 3\" >>two.bin &&\n+\tGIT_AUTHOR_NAME=Number3 git commit -a -m Third --date=\"2010-01-01 22:00:00\"\n+'\n+\n+test_expect_success 'textconv with blame from previous revision' '\n+\tgit blame HEAD^ two.bin >blame\n+\tfind_blame result\n+\ttest_cmp expected result\n+'\n+\n+cat >expected <>link.bin &&\n+\tGIT_AUTHOR_NAME=Number3 git commit -a -m Third --date=\"2010-01-01 20:00:00\"\n+'\n+\n+test_expect_success 'blame textconv on links' '\n+\tgit blame link.bin >blame\n+\tfind_blame result\n+\ttest_cmp expected result\n+'\n+\n+test_done\n-- \n1.6.6.7.ga5fe3\n"},{"id":"142901","messageId":"201006031744.07569.j6t@kdbg.org","threadId":"23989","inReplyTo":"1275562038-7468-5-git-send-email-axel.bonnet@ensimag.imag.fr","subject":"Re: [RFC/PATCH 4/4] t/t8006: test textconv support for blame","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-06-03T15:44:07Z","receivedAt":"2010-06-03T15:44:07Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Donnerstag, 3. Juni 2010, Axel Bonnet wrote:\n> +cat >helper <<'EOF'\n> +#!/bin/sh\n> +sed 's/^/converted: /' \"$@\" >helper.out\n> +cat helper.out\n> +EOF\n\nYou don't need an intermediate file here, do you? Without it, this textconv \nscript is a one-liner; now, isn't it possible to configure a shell command as \ntextconv command, i.e., without this helper script?\n\n> +test_expect_success 'setup ' '\n> +\techo test 1 >one.bin &&\n> +\techo test number 2 >two.bin &&\n> +\tln one.bin link.bin &&\n\nDo you need a hard link? Can't you just copy the file at the right time?\n\n> +test_expect_success 'blame with --no-textconv' '\n> +\tgit blame --no-textconv one.bin | grep Number2 >blame\n> +\tfind_blame result\n\nIt would be nice if you could write this like, e.g.,\n\n\tgit blame --no-textconv one.bin >blame &&\n\tfind_blame Number2 result\n\nso that the git command is not part of a pipeline (otherwise, unexpected exit \ncodes would go undetected).\n\nPlease look for missing '&&', you forgot it in many places.\n\n-- Hannes\n"},{"id":"142942","messageId":"7vvd9z5owr.fsf@alter.siamese.dyndns.org","threadId":"23989","inReplyTo":"1275562038-7468-3-git-send-email-axel.bonnet@ensimag.imag.fr","subject":"Re: [RFC/PATCH 2/4] textconv: make diff_options accessible from blame","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-06-04T05:48:20Z","receivedAt":"2010-06-04T05:48:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Axel Bonnet writes:\n\n> Diff_options specify whether conversion is activated or not. Blame needs\n> to access these options in order to concert files with external drivers\n>\n> Signed-off-by: Diane Gasselin \n> Signed-off-by: Clément Poulain \n> Signed-off-by: Axel Bonnet \n\nThe name of Clément is spelled correctly on the mail header while S-o-b\nline is corrupt. Perhaps you have recorded your commits in UTF-8 but\nallowed your MUA to send in 8859-1? This comment applies to all the\npatches in the series.\n\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index fc15863..63b497c 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -89,7 +89,8 @@ struct origin {\n> * Given an origin, prepare mmfile_t structure to be used by the\n> * diff machinery\n> */\n> -static void fill_origin_blob(struct origin *o, mmfile_t *file)\n> +static void fill_origin_blob(struct diff_options opt,\n> +\t\t\t struct origin *o, mmfile_t *file)\n> {\n> \tif (!o->file.ptr) {\n> \t\tenum object_type type;\n\nTwo points.\n\n * Generally we do not want to pass structures by value. It is especially\n true when the structure is bigger than one word, and accesses to the\n variable in the callee is read-only.\n\n * The callee does not seem to use the new parameter yet. You might want\n to defer this change until fill-origin-blob actually starts using it.\n"},{"id":"142943","messageId":"7veign5oc9.fsf@alter.siamese.dyndns.org","threadId":"23989","inReplyTo":"1275562038-7468-4-git-send-email-axel.bonnet@ensimag.imag.fr","subject":"Re: [RFC/PATCH 3/4] textconv: support for blame","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-06-04T06:00:38Z","receivedAt":"2010-06-04T06:00:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Axel Bonnet writes:\n\n> @@ -2033,10 +2072,13 @@ static struct commit *fake_working_tree_commit(struct diff_options opt,\n> \t\t\tread_from = path;\n> \t\t}\n> \t\tmode = canon_mode(st.st_mode);\n> +\n> \t\tswitch (st.st_mode & S_IFMT) {\n> \t\tcase S_IFREG:\n> -\t\t\tif (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)\n> -\t\t\t\tdie_errno(\"cannot open or read '%s'\", read_from);\n> +\t\t\tif (!DIFF_OPT_TST(&opt, ALLOW_TEXTCONV) ||\n> +\t\t\t !textconv_object(read_from, null_sha1, mode, &buf))\n> +\t\t\t\tif (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)\n> +\t\t\t\t\tdie_errno(\"cannot open or read '%s'\", read_from);\n\nThis is just a style thing but it would probably be easier to read if you\nstructured it like:\n\n\tif (! we are allowed to use textconv ||\n\t do textconv and we did get the converted data in the buffer)\n\t\t; /* happy */\n\telse if (! successfully read the blob into buffer)\n\t\tdie;\n\nBy the way, can't textconv_object() ever fail? I see the function has its\nown die() but it looks a bit funny to see one branch of an \"if\" statement\ncalls a function that lets the caller decide to die while the function\ncalled by the other branch unconditionally dies on failure at the API\ndesign level.\n\nAn alternative would be to encapsulate the whole of the above logic in one\nhelper function perhaps.\n\n> @@ -2249,8 +2291,10 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n> \tint cmd_is_annotate = !strcmp(argv[0], \"annotate\");\n> \n> \tgit_config(git_blame_config, NULL);\n> +\tgit_config(git_diff_ui_config, NULL);\n\nWhat configuration are we pulling into the system with this call? Would\nthey ever affect the internal diff machinery in a negative way? I am\nespecially wondering about \"diff.renames\" here.\n\n> \tinit_revisions(&revs, NULL);\n> \trevs.date_mode = blame_date_mode;\n> +\tDIFF_OPT_SET(&revs.diffopt, ALLOW_TEXTCONV);\n\nAs an RFC patch, I would have preferred if we didn't have this line to\nforce --textconv on by default, but instead you merely allowed the\nmechanism to be used by giving the option explicitly from the command\nline.\n\nOther than these points, the series looked quite sane to me.\n"},{"id":"142948","messageId":"vpqy6evut1o.fsf@bauges.imag.fr","threadId":"23989","inReplyTo":"7vvd9z5owr.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH 2/4] textconv: make diff_options accessible from blame","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-06-04T07:59:47Z","receivedAt":"2010-06-04T07:59:47Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Axel Bonnet writes:\n>\n>> Diff_options specify whether conversion is activated or not. Blame needs\n>> to access these options in order to concert files with external drivers\n>>\n>> Signed-off-by: Diane Gasselin \n>> Signed-off-by: Clément Poulain \n>> Signed-off-by: Axel Bonnet \n>\n> The name of Clément is spelled correctly on the mail header while S-o-b\n> line is corrupt.\n\nActually, it's valid UTF-8, but there's no header specifying the\nencoding in the email, therefore, the reader's default applies. My\nmailer displays it correctly, but yours doesn't.\n\n> Perhaps you have recorded your commits in UTF-8 but allowed your MUA\n> to send in 8859-1?\n\nThe MUA seems to be git-send-email. According to the source (I didn't\nfind it in the doc), git-send-email looks at the patch's headers to\nspecify the encoding.\n\nOn my machine, the patch applies well, and if I re-export it using\nformat-patch, I do get the headers:\n\nContent-Type: text/plain; charset=UTF-8\nContent-Transfer-Encoding: 8bit\n\nIf I send myself the patch with git-send-email, I also get the headers\nin the email (I tried from ensibm, which is the machine which sent the\npatch serie). So, it doesn't look like a bug in git, but rather a\nmiss-use.\n\nAxel, can you give us the exact command(s) you used to send the patch?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"142951","messageId":"vpq1vcnqj16.fsf@bauges.imag.fr","threadId":"23989","inReplyTo":"1275562038-7468-5-git-send-email-axel.bonnet@ensimag.imag.fr","subject":"Re: [RFC/PATCH 4/4] t/t8006: test textconv support for blame","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-06-04T08:49:41Z","receivedAt":"2010-06-04T08:49:41Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Axel Bonnet writes:\n\n> +test_expect_success 'blame with --no-textconv' '\n> +\tgit blame --no-textconv one.bin | grep Number2 >blame\n> +\tfind_blame result\n> +\ttest_cmp expected result\n> +'\n\nDon't you want to add && at each end of line, to make sure you catch\npotential failures of git blame on the first line (e.g. git blame\nproducing the correct output and then segfaulting for example)?\n\nActually, to really catch such failures, you should not run git on the\nleft hand side of a | (otherwise, you look for failures of the right\nhand side), and do this instead:\n\ntest_expect_success 'no filter specified' '\n\tgit blame one.bin >to-grep &&\n\tgrep Number2 to-grep >blame &&\n\tfind_blame result &&\n\ttest_cmp expected result\n'\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"142952","messageId":"AANLkTil7Y6PPkbmzuID4vY_fhEwvP4qa2UG5jC1qLtTG@mail.gmail.com","threadId":"23989","inReplyTo":"201006031744.07569.j6t@kdbg.org","subject":"Re: [RFC/PATCH 4/4] t/t8006: test textconv support for blame","fromName":"Diane Gasselin","fromEmail":"diane.gasselin@ensimag.imag.fr","sentAt":"2010-06-04T08:55:34Z","receivedAt":"2010-06-04T08:55:34Z","isPatch":true,"sender":{"key":"diane.gasselin@ensimag.imag.fr","avatar":null},"body":"Le 3 juin 2010 17:44, Johannes Sixt a écrit :\n> On Donnerstag, 3. Juni 2010, Axel Bonnet wrote:\n>> +cat >helper <<'EOF'\n>> +#!/bin/sh\n>> +sed 's/^/converted: /' \"$@\" >helper.out\n>> +cat helper.out\n>> +EOF\n>\n> You don't need an intermediate file here, do you? Without it, this textconv\n> script is a one-liner; now, isn't it possible to configure a shell command as\n> textconv command, i.e., without this helper script?\n>\n\nOk. We don't use the intermediate file anymore. Actually, we used what\nhas been done for textconv test for diff.\nI didn't find a way to directly specify the sed command as textconv\ncommand without using ./helper though.\n\ncat >helper <<'EOF'\n#!/bin/sh\nsed 's/^/converted: /' \"$@\"\nEOF\nchmod +x helper\n\n>> +test_expect_success 'setup ' '\n>> +     echo test 1 >one.bin &&\n>> +     echo test number 2 >two.bin &&\n>> +     ln one.bin link.bin &&\n>\n> Do you need a hard link? Can't you just copy the file at the right time?\n>\n\nAt first, we wanted to test how links handle textconv but it behaves\nas regular file so the test is not really relevant. It will be\ndeleted.\n\n>> +test_expect_success 'blame with --no-textconv' '\n>> +     git blame --no-textconv one.bin | grep Number2 >blame\n>> +     find_blame result\n>\n> It would be nice if you could write this like, e.g.,\n>\n>        git blame --no-textconv one.bin >blame &&\n>        find_blame Number2 result\n>\n> so that the git command is not part of a pipeline (otherwise, unexpected exit\n> codes would go undetected).\n>\n> Please look for missing '&&', you forgot it in many places.\n>\n\nWe did the appropriate changes.\nThanks a lot for your comments!\n\nDiane\n\n> -- Hannes\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"142957","messageId":"vpqzkzb9lml.fsf@bauges.imag.fr","threadId":"23989","inReplyTo":"AANLkTil7Y6PPkbmzuID4vY_fhEwvP4qa2UG5jC1qLtTG@mail.gmail.com","subject":"Re: [RFC/PATCH 4/4] t/t8006: test textconv support for blame","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-06-04T09:45:38Z","receivedAt":"2010-06-04T09:45:38Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Diane Gasselin writes:\n\n>>> +test_expect_success 'setup ' '\n>>> +     echo test 1 >one.bin &&\n>>> +     echo test number 2 >two.bin &&\n>>> +     ln one.bin link.bin &&\n>>\n>> Do you need a hard link? Can't you just copy the file at the right time?\n>>\n>\n> At first, we wanted to test how links handle textconv but it behaves\n> as regular file so the test is not really relevant. It will be\n> deleted.\n\nYou do want to test what happens for _symbolic_ links (stored as blob\nwhose content is the target of the link IIRC). You probably don't want\nto run the textconv filter on link targets. I guess you meant \"ln -s\"\nhere.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"142962","messageId":"57f94007bc6d4f34d1929a005110073f@ensimag.fr","threadId":"23989","inReplyTo":"vpqy6evut1o.fsf@bauges.imag.fr","subject":"Re: [RFC/PATCH 2/4] textconv: make diff_options accessible from blame","fromName":"bonneta","fromEmail":"bonneta@ensimag.fr","sentAt":"2010-06-04T10:21:07Z","receivedAt":"2010-06-04T10:21:07Z","isPatch":true,"sender":{"key":"bonneta@ensimag.fr","avatar":null},"body":"On Fri, 04 Jun 2010 09:59:47 +0200, Matthieu Moy\n wrote:\n> Junio C Hamano writes:\n> \n>> Axel Bonnet writes:\n>>\n>>> Diff_options specify whether conversion is activated or not. Blame\nneeds\n>>> to access these options in order to concert files with external\ndrivers\n>>>\n>>> Signed-off-by: Diane Gasselin \n>>> Signed-off-by: Clément Poulain \n>>> Signed-off-by: Axel Bonnet \n>>\n>> The name of Clément is spelled correctly on the mail header while S-o-b\n>> line is corrupt.\n> \n> Actually, it's valid UTF-8, but there's no header specifying the\n> encoding in the email, therefore, the reader's default applies. My\n> mailer displays it correctly, but yours doesn't.\n> \n>> Perhaps you have recorded your commits in UTF-8 but allowed your MUA\n>> to send in 8859-1?\n> \n> The MUA seems to be git-send-email. According to the source (I didn't\n> find it in the doc), git-send-email looks at the patch's headers to\n> specify the encoding.\n> \n> On my machine, the patch applies well, and if I re-export it using\n> format-patch, I do get the headers:\n> \n> Content-Type: text/plain; charset=UTF-8\n> Content-Transfer-Encoding: 8bit\n> \n> If I send myself the patch with git-send-email, I also get the headers\n> in the email (I tried from ensibm, which is the machine which sent the\n> patch serie). So, it doesn't look like a bug in git, but rather a\n> miss-use.\n> \n> Axel, can you give us the exact command(s) you used to send the patch?\n\nI made the patch with \"git send-email --cover --annotate\", and then edited\nthe messages with vim.\n\nI added the S-o-b lines by copy-pasting them from the test mail I\nhad send to Matthieu (from thunderbird). I saw there was a problem of\nencoding with the \"é\" of\nClément, so I modified it.\n\nI think I should have written the S-o-b lines directly in vim.\n"},{"id":"142961","messageId":"AANLkTinfYAFAX4Hcl9lZMspju_Yk5eGll2S-z9HPdkjh@mail.gmail.com","threadId":"23989","inReplyTo":"7veign5oc9.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH 3/4] textconv: support for blame","fromName":"Diane Gasselin","fromEmail":"diane.gasselin@ensimag.imag.fr","sentAt":"2010-06-04T10:34:25Z","receivedAt":"2010-06-04T10:34:25Z","isPatch":true,"sender":{"key":"diane.gasselin@ensimag.imag.fr","avatar":null},"body":"Le 4 juin 2010 08:00, Junio C Hamano a écrit :\n> Axel Bonnet writes:\n>\n>> @@ -2033,10 +2072,13 @@ static struct commit *fake_working_tree_commit(struct diff_options opt,\n>>                       read_from = path;\n>>               }\n>>               mode = canon_mode(st.st_mode);\n>> +\n>>               switch (st.st_mode & S_IFMT) {\n>>               case S_IFREG:\n>> -                     if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)\n>> -                             die_errno(\"cannot open or read '%s'\", read_from);\n>> +                     if (!DIFF_OPT_TST(&opt, ALLOW_TEXTCONV) ||\n>> +                         !textconv_object(read_from, null_sha1, mode, &buf))\n>> +                             if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)\n>> +                                     die_errno(\"cannot open or read '%s'\", read_from);\n>\n> This is just a style thing but it would probably be easier to read if you\n> structured it like:\n>\n>        if (! we are allowed to use textconv ||\n>            do textconv and we did get the converted data in the buffer)\n>                ; /* happy */\n>        else if (! successfully read the blob into buffer)\n>                die;\n>\n\nWe changed the structure, we now have something like:\n\ncase S_IFREG:\n\tif (DIFF_OPT_TST(&opt, ALLOW_TEXTCONV) &&\n\t textconv_object(read_from, null_sha1, mode, &buf))\n\t\t;\n\telse if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)\n\t\tdie_errno(\"cannot open or read '%s'\", read_from);\nbreak;\n\n> By the way, can't textconv_object() ever fail?  I see the function has its\n> own die()\n\nI don't really see which die you are talking about, there is no direct\ndie in textconv_object().\n\n> but it looks a bit funny to see one branch of an \"if\" statement\n> calls a function that lets the caller decide to die while the function\n> called by the other branch unconditionally dies on failure at the API\n> design level.\n>\n\nThe function fill_textconv() called by textconv_object() can fail if\nthere is a problem during the textconv conversion. But\ntextconv_object() tests before if we have to and if we can peform a\nconversion.\nWe did not add some die but only let the existing die.\n\n> An alternative would be to encapsulate the whole of the above logic in one\n> helper function perhaps.\n>\n>> @@ -2249,8 +2291,10 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n>>       int cmd_is_annotate = !strcmp(argv[0], \"annotate\");\n>>\n>>       git_config(git_blame_config, NULL);\n>> +     git_config(git_diff_ui_config, NULL);\n>\n> What configuration are we pulling into the system with this call?  Would\n> they ever affect the internal diff machinery in a negative way?  I am\n> especially wondering about \"diff.renames\" here.\n>\n\nActually, we call git_diff_ui_config in order to get the drivers. But\nwe could call a more specific configuration.\nHere are others solutions:\n1) We could add:\n\nswitch (userdiff_config(var, value)) {\n\tcase 0: break;\n\tcase -1: return -1;\n\tdefault: return 0;\n}\nto git_blame_config\n\n2) We could call\ngit_config(git_diff_basic_config, NULL);\nor git_config((config_fn_t)userdiff_config, NULL);\ninstead of git_config(git_diff_ui_config, NULL);\n\n>>       init_revisions(&revs, NULL);\n>>       revs.date_mode = blame_date_mode;\n>> +     DIFF_OPT_SET(&revs.diffopt, ALLOW_TEXTCONV);\n>\n> As an RFC patch, I would have preferred if we didn't have this line to\n> force --textconv on by default, but instead you merely allowed the\n> mechanism to be used by giving the option explicitly from the command\n> line.\n>\n> Other than these points, the series looked quite sane to me.\n>\n\nThanks a lot for your time and comments.\n"},{"id":"142994","messageId":"vpq1vcmaf0f.fsf@bauges.imag.fr","threadId":"23989","inReplyTo":"57f94007bc6d4f34d1929a005110073f@ensimag.fr","subject":"Re: [RFC/PATCH 2/4] textconv: make diff_options accessible from blame","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-06-04T17:23:12Z","receivedAt":"2010-06-04T17:23:12Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"bonneta writes:\n\n> On Fri, 04 Jun 2010 09:59:47 +0200, Matthieu Moy\n> wrote:\n>> Junio C Hamano writes:\n>> \n>>> Axel Bonnet writes:\n\n>>>> Signed-off-by: Clément Poulain \n>>>\n>>> The name of Clément is spelled correctly on the mail header while S-o-b\n>>> line is corrupt.\n>> \n>> Actually, it's valid UTF-8, but there's no header specifying the\n>> encoding in the email,\n[...]\n>> Axel, can you give us the exact command(s) you used to send the patch?\n>\n> I made the patch with \"git send-email --cover --annotate\", and then edited\n> the messages with vim.\n>\n> I added the S-o-b lines by copy-pasting them \n\nOK, I got it. You ran \"git format-patch\", and it didn't find any\nnon-ascii characters, so it didn't add any encoding header. Then you\nadded UTF-8, and the header still wasn't there.\n\nYou can use \"git rebase -i\" to edit the commit messages directly, and\nadd the s-o-b there, then git format-patch will DRT.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"143110","messageId":"20100606215159.GA6993@coredump.intra.peff.net","threadId":"23989","inReplyTo":"1275562038-7468-4-git-send-email-axel.bonnet@ensimag.imag.fr","subject":"Re: [RFC/PATCH 3/4] textconv: support for blame","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-06-06T21:51:59Z","receivedAt":"2010-06-06T21:51:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 03, 2010 at 12:47:17PM +0200, Axel Bonnet wrote:\n\n> /*\n> + * Prepare diff_filespec and convert it using diff textconv API\n> + * if the textconv driver exists.\n> + * Return 1 if the conversion succeeds, 0 otherwise.\n> + */\n> +static int textconv_object(const char *path,\n> +\t\t\t const unsigned char *sha1,\n> +\t\t\t unsigned short mode,\n> +\t\t\t struct strbuf *buf)\n> +{\n> +\tstruct diff_filespec *df;\n> +\n> +\tdf = alloc_filespec(path);\n> +\tfill_filespec(df, sha1, mode);\n> +\tget_textconv(df);\n> +\n> +\tif (!df->driver|| !df->driver->textconv) {\n> +\t\tfree_filespec(df);\n> +\t\treturn 0;\n> +\t}\n\ndf->driver is always non-NULL these days, isn't it? Also, why not just\nuse the return value of get_textconv, which does handles this\nconditional for you already (and avoids peeking directly at df->driver,\nwhich was what get_textconv was meant to abstract). I.e.:\n\n struct userdiff_driver *textconv;\n ...\n textconv = get_textconv(df);\n if (!textconv)\n ... free and return ...\n\n> +\tbuf->len = fill_textconv(df->driver, df, &buf->buf);\n> +\tbuf->alloc = 1;\n> +\tfree_filespec(df);\n> +\treturn 1;\n\nShoving the allocated buffer into a strbuf really feels like an abuse of\nstrbuf. I don't think there is any bug here (the original buffer\nactually comes from a strbuf, and by setting alloc to 1, you indicate\nthat any further appending would need to realloc), but it just seems\nlike violating the boundary of the strbuf API.\n\nAnd it isn't really necessary because:\n\n> +\t\tif (DIFF_OPT_TST(&opt, ALLOW_TEXTCONV) &&\n> +\t\t textconv_object(o->path, o->blob_sha1, mode, &buf))\n> +\t\t\tfile->ptr = strbuf_detach(&buf, (size_t *) &file->size);\n\nYou just end up pulling it out into an mmfile_t, anyway. So why involve\nstrbuf at all?\n\n> static void fill_origin_blob(struct diff_options opt,\n> \t\t\t struct origin *o, mmfile_t *file)\n> {\n> +\tunsigned mode;\n> +\n> \tif (!o->file.ptr) {\n> +\t\tstruct strbuf buf = STRBUF_INIT;\n> \t\tenum object_type type;\n> \t\tnum_read_blob++;\n> -\t\tfile->ptr = read_sha1_file(o->blob_sha1, &type,\n> -\t\t\t\t\t (unsigned long *)(&(file->size)));\n> +\n> +\t\tget_tree_entry(o->commit->object.sha1,\n> +\t\t\t o->path,\n> +\t\t\t o->blob_sha1, &mode);\n> +\t\tif (DIFF_OPT_TST(&opt, ALLOW_TEXTCONV) &&\n> +\t\t textconv_object(o->path, o->blob_sha1, mode, &buf))\n> +\t\t\tfile->ptr = strbuf_detach(&buf, (size_t *) &file->size);\n> +\t\telse\n> +\t\t\tfile->ptr = read_sha1_file(o->blob_sha1, &type,\n> +\t\t\t\t\t\t (unsigned long *)(&(file->size)));\n> +\t\tstrbuf_release(&buf);\n\nI don't understand why there's a get_tree_entry call here. Don't we\nalready have the path and blob_sha1 fields? Is the mode actually\nrelevant (i.e., can we just fake it for the purposes of the\ndiff_filespec we will create)?\n\nEven if we do need the get_tree_entry call, shouldn't it happen only if\nwe are allowing textconv, so non-textconv users don't pay for the call?\n\n> @@ -2249,8 +2291,10 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n> \tint cmd_is_annotate = !strcmp(argv[0], \"annotate\");\n> \n> \tgit_config(git_blame_config, NULL);\n> +\tgit_config(git_diff_ui_config, NULL);\n\nLike Junio, I am worried about the unintended consequences of\ngit_diff_ui_config. The userdiff drivers are parsed as part of\ngit_diff_basic_config, and you should use that.\n\nAlso, you are better to just add the call to git_blame_config (either\ngit_diff_basic_config, or looking directly to userdiff_config) instead\nof calling git_config again, which will avoid parsing the config files\ntwice.\n\n-Peff\n"}]}