{"thread":{"id":"41923","subject":"[PATCH] diff: run arguments through precompose_argv","startedAt":"2016-04-04T20:38:44Z","lastAt":"2016-05-12T15:39:11Z","messageCount":12,"participants":["Alexander Rinass","Junio C Hamano","Johannes Sixt","Torsten Bögershausen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"282646","messageId":"1459802325-22056-1-git-send-email-alex@fournova.com","threadId":"41923","inReplyTo":null,"subject":"[PATCH] diff: run arguments through precompose_argv","fromName":"Alexander Rinass","fromEmail":"alex@fournova.com","sentAt":"2016-04-04T20:38:44Z","receivedAt":"2016-04-04T20:38:44Z","isPatch":true,"sender":{"key":"alex@fournova.com","avatar":"https://avatars.githubusercontent.com/u/116053?v=4"},"body":"I found an issue where file paths containing decomposed unicode chars\nare not converted to precomposed unicode form when passed to the diff\ncommand family.\n\nAs a result, no diff is displayed.\n\nWith the help of Torsten Bögershausen and Junio C Hamano I came up \nwith the following patch to fix the issue.\n\nAlexander Rinass (1):\n  diff: run arguments through precompose_argv\n\n builtin/diff-files.c         |  1 +\n builtin/diff-index.c         |  1 +\n builtin/diff-tree.c          |  2 ++\n builtin/diff.c               |  1 +\n t/t3910-mac-os-precompose.sh | 42 ++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 47 insertions(+)\n\n-- \n2.7.2\n"},{"id":"282647","messageId":"1459802325-22056-2-git-send-email-alex@fournova.com","threadId":"41923","inReplyTo":"1459802325-22056-1-git-send-email-alex@fournova.com","subject":"[PATCH] diff: run arguments through precompose_argv","fromName":"Alexander Rinass","fromEmail":"alex@fournova.com","sentAt":"2016-04-04T20:38:45Z","receivedAt":"2016-04-04T20:38:45Z","isPatch":true,"sender":{"key":"alex@fournova.com","avatar":"https://avatars.githubusercontent.com/u/116053?v=4"},"body":"File paths containing decomposed unicode chars passed to diff\ncommand are not converted to precomposed unicode form.\n\nAs a result, no diff is displayed when feeding such a file path to the\ndiff command.\n\nOpposite to most builtin commands, the diff builtin is missing the\nparse_options call, which internally runs arguments through the\nprecompose_argv call, which ensures all arguments are in precomposed\nunicode form.\n\nFix the problem by adding a precompose_argv call directly, as a call to\nparse_options would require additional work to call it.\n\nAlso applies to diff-index, diff-files and diff-tree.\n\nSigned-off-by: Alexander Rinass <alex@fournova.com>\nThanks-to: Torsten Bögershausen <tboegi@web.de>\nThanks-to: Junio C Hamano <gitster@pobox.com>\n---\n builtin/diff-files.c         |  1 +\n builtin/diff-index.c         |  1 +\n builtin/diff-tree.c          |  2 ++\n builtin/diff.c               |  1 +\n t/t3910-mac-os-precompose.sh | 42 ++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 47 insertions(+)\n\ndiff --git a/builtin/diff-files.c b/builtin/diff-files.c\nindex 8ed2eb8..15c61fd 100644\n--- a/builtin/diff-files.c\n+++ b/builtin/diff-files.c\n@@ -24,6 +24,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)\n \tgitmodules_config();\n \tgit_config(git_diff_basic_config, NULL); /* no \"diff\" UI options */\n \trev.abbrev = 0;\n+\tprecompose_argv(argc, argv);\n \n \targc = setup_revisions(argc, argv, &rev, NULL);\n \twhile (1 < argc && argv[1][0] == '-') {\ndiff --git a/builtin/diff-index.c b/builtin/diff-index.c\nindex d979824..1af373d 100644\n--- a/builtin/diff-index.c\n+++ b/builtin/diff-index.c\n@@ -21,6 +21,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)\n \tgitmodules_config();\n \tgit_config(git_diff_basic_config, NULL); /* no \"diff\" UI options */\n \trev.abbrev = 0;\n+\tprecompose_argv(argc, argv);\n \n \targc = setup_revisions(argc, argv, &rev, NULL);\n \tfor (i = 1; i < argc; i++) {\ndiff --git a/builtin/diff-tree.c b/builtin/diff-tree.c\nindex 2a12b81..806dd7a 100644\n--- a/builtin/diff-tree.c\n+++ b/builtin/diff-tree.c\n@@ -114,6 +114,8 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n \topt->disable_stdin = 1;\n \tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n \ts_r_opt.tweak = diff_tree_tweak_rev;\n+\n+\tprecompose_argv(argc, argv);\n \targc = setup_revisions(argc, argv, opt, &s_r_opt);\n \n \twhile (--argc > 0) {\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 52c98a9..d6b8f98 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -319,6 +319,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \tif (!no_index)\n \t\tgitmodules_config();\n \tgit_config(git_diff_ui_config, NULL);\n+\tprecompose_argv(argc, argv);\n \n \tinit_revisions(&rev, prefix);\n \ndiff --git a/t/t3910-mac-os-precompose.sh b/t/t3910-mac-os-precompose.sh\nindex 8319356..26dd5b7 100755\n--- a/t/t3910-mac-os-precompose.sh\n+++ b/t/t3910-mac-os-precompose.sh\n@@ -49,12 +49,54 @@ test_expect_success \"setup\" '\n test_expect_success \"setup case mac\" '\n \tgit checkout -b mac_os\n '\n+# This will test nfd2nfc in git diff\n+test_expect_success \"git diff f.Adiar\" '\n+\ttouch f.$Adiarnfc &&\n+\tgit add f.$Adiarnfc &&\n+\techo f.Adiarnfc >f.$Adiarnfc &&\n+\tgit diff f.$Adiarnfd >expect &&\n+\tgit diff f.$Adiarnfc >actual &&\n+\ttest_cmp expect actual &&\n+\tgit reset HEAD f.Adiarnfc &&\n+\trm f.$Adiarnfc expect actual\n+'\n+# This will test nfd2nfc in git diff-files\n+test_expect_success \"git diff-files f.Adiar\" '\n+\ttouch f.$Adiarnfc &&\n+\tgit add f.$Adiarnfc &&\n+\techo f.Adiarnfc >f.$Adiarnfc &&\n+\tgit diff-files f.$Adiarnfd >expect &&\n+\tgit diff-files f.$Adiarnfc >actual &&\n+\ttest_cmp expect actual &&\n+\tgit reset HEAD f.Adiarnfc &&\n+\trm f.$Adiarnfc expect actual\n+'\n+# This will test nfd2nfc in git diff-index\n+test_expect_success \"git diff-index f.Adiar\" '\n+\ttouch f.$Adiarnfc &&\n+\tgit add f.$Adiarnfc &&\n+\techo f.Adiarnfc >f.$Adiarnfc &&\n+\tgit diff-index HEAD f.$Adiarnfd >expect &&\n+\tgit diff-index HEAD f.$Adiarnfc >actual &&\n+\ttest_cmp expect actual &&\n+\tgit reset HEAD f.Adiarnfc &&\n+\trm f.$Adiarnfc expect actual\n+'\n # This will test nfd2nfc in readdir()\n test_expect_success \"add file Adiarnfc\" '\n \techo f.Adiarnfc >f.$Adiarnfc &&\n \tgit add f.$Adiarnfc &&\n \tgit commit -m \"add f.$Adiarnfc\"\n '\n+# This will test nfd2nfc in git diff-tree\n+test_expect_success \"git diff-tree f.Adiar\" '\n+\techo f.Adiarnfc >>f.$Adiarnfc &&\n+\tgit diff-tree HEAD f.$Adiarnfd >expect &&\n+\tgit diff-tree HEAD f.$Adiarnfc >actual &&\n+\ttest_cmp expect actual &&\n+\tgit checkout f.$Adiarnfc &&\n+\trm expect actual\n+'\n # This will test nfd2nfc in git stage()\n test_expect_success \"stage file d.Adiarnfd/f.Adiarnfd\" '\n \tmkdir d.$Adiarnfd &&\n-- \n2.7.2\n"},{"id":"282722","messageId":"xmqqzit80yov.fsf@gitster.mtv.corp.google.com","threadId":"41923","inReplyTo":"1459802325-22056-2-git-send-email-alex@fournova.com","subject":"Re: [PATCH] diff: run arguments through precompose_argv","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-05T17:09:52Z","receivedAt":"2016-04-05T17:09:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexander Rinass <alex@fournova.com> writes:\n\n> File paths containing decomposed unicode chars passed to diff\n> command are not converted to precomposed unicode form.\n>\n> As a result, no diff is displayed when feeding such a file path to the\n> diff command.\n>\n> Opposite to most builtin commands, the diff builtin is missing the\n> parse_options call, which internally runs arguments through the\n> precompose_argv call, which ensures all arguments are in precomposed\n> unicode form.\n>\n> Fix the problem by adding a precompose_argv call directly, as a call to\n> parse_options would require additional work to call it.\n>\n> Also applies to diff-index, diff-files and diff-tree.\n\nThanks.  The log message talks about \"such a file path\", but\nprecompose_argv() applies indiscriminately on any command line\narguments, so things like -G<string> would also get the same\ntreatment, which I think is what most users would want).\n\nWill queue.\n\n> Signed-off-by: Alexander Rinass <alex@fournova.com>\n> Thanks-to: Torsten Bögershausen <tboegi@web.de>\n> Thanks-to: Junio C Hamano <gitster@pobox.com>\n> ---\n>  builtin/diff-files.c         |  1 +\n>  builtin/diff-index.c         |  1 +\n>  builtin/diff-tree.c          |  2 ++\n>  builtin/diff.c               |  1 +\n>  t/t3910-mac-os-precompose.sh | 42 ++++++++++++++++++++++++++++++++++++++++++\n>  5 files changed, 47 insertions(+)\n>\n> diff --git a/builtin/diff-files.c b/builtin/diff-files.c\n> index 8ed2eb8..15c61fd 100644\n> --- a/builtin/diff-files.c\n> +++ b/builtin/diff-files.c\n> @@ -24,6 +24,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)\n>  \tgitmodules_config();\n>  \tgit_config(git_diff_basic_config, NULL); /* no \"diff\" UI options */\n>  \trev.abbrev = 0;\n> +\tprecompose_argv(argc, argv);\n>  \n>  \targc = setup_revisions(argc, argv, &rev, NULL);\n>  \twhile (1 < argc && argv[1][0] == '-') {\n> diff --git a/builtin/diff-index.c b/builtin/diff-index.c\n> index d979824..1af373d 100644\n> --- a/builtin/diff-index.c\n> +++ b/builtin/diff-index.c\n> @@ -21,6 +21,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)\n>  \tgitmodules_config();\n>  \tgit_config(git_diff_basic_config, NULL); /* no \"diff\" UI options */\n>  \trev.abbrev = 0;\n> +\tprecompose_argv(argc, argv);\n>  \n>  \targc = setup_revisions(argc, argv, &rev, NULL);\n>  \tfor (i = 1; i < argc; i++) {\n> diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c\n> index 2a12b81..806dd7a 100644\n> --- a/builtin/diff-tree.c\n> +++ b/builtin/diff-tree.c\n> @@ -114,6 +114,8 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n>  \topt->disable_stdin = 1;\n>  \tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n>  \ts_r_opt.tweak = diff_tree_tweak_rev;\n> +\n> +\tprecompose_argv(argc, argv);\n>  \targc = setup_revisions(argc, argv, opt, &s_r_opt);\n>  \n>  \twhile (--argc > 0) {\n> diff --git a/builtin/diff.c b/builtin/diff.c\n> index 52c98a9..d6b8f98 100644\n> --- a/builtin/diff.c\n> +++ b/builtin/diff.c\n> @@ -319,6 +319,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n>  \tif (!no_index)\n>  \t\tgitmodules_config();\n>  \tgit_config(git_diff_ui_config, NULL);\n> +\tprecompose_argv(argc, argv);\n>  \n>  \tinit_revisions(&rev, prefix);\n>  \n> diff --git a/t/t3910-mac-os-precompose.sh b/t/t3910-mac-os-precompose.sh\n> index 8319356..26dd5b7 100755\n> --- a/t/t3910-mac-os-precompose.sh\n> +++ b/t/t3910-mac-os-precompose.sh\n> @@ -49,12 +49,54 @@ test_expect_success \"setup\" '\n>  test_expect_success \"setup case mac\" '\n>  \tgit checkout -b mac_os\n>  '\n> +# This will test nfd2nfc in git diff\n> +test_expect_success \"git diff f.Adiar\" '\n> +\ttouch f.$Adiarnfc &&\n> +\tgit add f.$Adiarnfc &&\n> +\techo f.Adiarnfc >f.$Adiarnfc &&\n> +\tgit diff f.$Adiarnfd >expect &&\n> +\tgit diff f.$Adiarnfc >actual &&\n> +\ttest_cmp expect actual &&\n> +\tgit reset HEAD f.Adiarnfc &&\n> +\trm f.$Adiarnfc expect actual\n> +'\n> +# This will test nfd2nfc in git diff-files\n> +test_expect_success \"git diff-files f.Adiar\" '\n> +\ttouch f.$Adiarnfc &&\n> +\tgit add f.$Adiarnfc &&\n> +\techo f.Adiarnfc >f.$Adiarnfc &&\n> +\tgit diff-files f.$Adiarnfd >expect &&\n> +\tgit diff-files f.$Adiarnfc >actual &&\n> +\ttest_cmp expect actual &&\n> +\tgit reset HEAD f.Adiarnfc &&\n> +\trm f.$Adiarnfc expect actual\n> +'\n> +# This will test nfd2nfc in git diff-index\n> +test_expect_success \"git diff-index f.Adiar\" '\n> +\ttouch f.$Adiarnfc &&\n> +\tgit add f.$Adiarnfc &&\n> +\techo f.Adiarnfc >f.$Adiarnfc &&\n> +\tgit diff-index HEAD f.$Adiarnfd >expect &&\n> +\tgit diff-index HEAD f.$Adiarnfc >actual &&\n> +\ttest_cmp expect actual &&\n> +\tgit reset HEAD f.Adiarnfc &&\n> +\trm f.$Adiarnfc expect actual\n> +'\n>  # This will test nfd2nfc in readdir()\n>  test_expect_success \"add file Adiarnfc\" '\n>  \techo f.Adiarnfc >f.$Adiarnfc &&\n>  \tgit add f.$Adiarnfc &&\n>  \tgit commit -m \"add f.$Adiarnfc\"\n>  '\n> +# This will test nfd2nfc in git diff-tree\n> +test_expect_success \"git diff-tree f.Adiar\" '\n> +\techo f.Adiarnfc >>f.$Adiarnfc &&\n> +\tgit diff-tree HEAD f.$Adiarnfd >expect &&\n> +\tgit diff-tree HEAD f.$Adiarnfc >actual &&\n> +\ttest_cmp expect actual &&\n> +\tgit checkout f.$Adiarnfc &&\n> +\trm expect actual\n> +'\n>  # This will test nfd2nfc in git stage()\n>  test_expect_success \"stage file d.Adiarnfd/f.Adiarnfd\" '\n>  \tmkdir d.$Adiarnfd &&\n"},{"id":"282725","messageId":"57040EE0.40503@kdbg.org","threadId":"41923","inReplyTo":"xmqqzit80yov.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff: run arguments through precompose_argv","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-04-05T19:15:44Z","receivedAt":"2016-04-05T19:15:44Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 05.04.2016 um 19:09 schrieb Junio C Hamano:\n>> Thanks-to: Torsten Bögershausen <tboegi@web.de>\n\nI sense NFD disease: The combining diaresis should combine with the o, \nnot the g. Here is a correct line to copy-and-paste if you like:\n\nThanks-to: Torsten Bögershausen <tboegi@web.de>\n\n-- Hannes\n"},{"id":"282727","messageId":"570411A6.6060208@web.de","threadId":"41923","inReplyTo":"57040EE0.40503@kdbg.org","subject":"Re: [PATCH] diff: run arguments through precompose_argv","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-04-05T19:27:34Z","receivedAt":"2016-04-05T19:27:34Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 05.04.16 21:15, Johannes Sixt wrote:\n> Am 05.04.2016 um 19:09 schrieb Junio C Hamano:\n>>> Thanks-to: Torsten Bögershausen <tboegi@web.de>\n> \n> I sense NFD disease: The combining diaresis should combine with the o, not the g. Here is a correct line to copy-and-paste if you like:\n> \n> Thanks-to: Torsten Bögershausen <tboegi@web.de>\n> \n> -- Hannes\n\nGood eyes.\n\nAnd thanks to Alexander for doing this patch\n"},{"id":"282737","messageId":"xmqq7fgb20n4.fsf@gitster.mtv.corp.google.com","threadId":"41923","inReplyTo":"570411A6.6060208@web.de","subject":"Re: [PATCH] diff: run arguments through precompose_argv","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-05T21:42:23Z","receivedAt":"2016-04-05T21:42:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> On 05.04.16 21:15, Johannes Sixt wrote:\n>> Am 05.04.2016 um 19:09 schrieb Junio C Hamano:\n>>>> Thanks-to: Torsten Bögershausen <tboegi@web.de>\n>> \n>> I sense NFD disease: The combining diaresis should combine with the o, not the g. Here is a correct line to copy-and-paste if you like:\n>> \n>> Thanks-to: Torsten Bögershausen <tboegi@web.de>\n>> \n>> -- Hannes\n>\n> Good eyes.\n>\n> And thanks to Alexander for doing this patch\n\nYup.  It is funny that I wrote:\n\n    Thanks.  The log message talks about \"such a file path\", but\n    precompose_argv() applies indiscriminately on any command line\n    arguments, so things like -G<string> would also get the same\n    treatment, which I think is what most users would want.\n\nbut \"git log --grep=\" with your name would not have found the one\nthat would have resulted from the original e-mail message applied.\n"},{"id":"282751","messageId":"C52E38F0-B0F2-4769-A2C7-798D0CD99B47@fournova.com","threadId":"41923","inReplyTo":"57040EE0.40503@kdbg.org","subject":"Re: [PATCH] diff: run arguments through precompose_argv","fromName":"Alexander Rinass","fromEmail":"alex@fournova.com","sentAt":"2016-04-06T06:51:28Z","receivedAt":"2016-04-06T06:51:28Z","isPatch":true,"sender":{"key":"alex@fournova.com","avatar":"https://avatars.githubusercontent.com/u/116053?v=4"},"body":"\n> On 05 Apr 2016, at 21:15, Johannes Sixt <j6t@kdbg.org> wrote:\n> \n> Am 05.04.2016 um 19:09 schrieb Junio C Hamano:\n>>> Thanks-to: Torsten Bögershausen <tboegi@web.de>\n> \n> I sense NFD disease: The combining diaresis should combine with the o, not the g. Here is a correct line to copy-and-paste if you like:\n> \n> Thanks-to: Torsten Bögershausen <tboegi@web.de>\n> \n> -- Hannes\n\nThanks for reviewing and catching the NFD encoding error.\n\nI will send in a patch v2 with the correct NFC encoding.\n\nWould you also like me to alter the commit message as mentioned by Junio?\n\nI could rewrite the sentence:\n\n“As a result, no diff is displayed when feeding such a file path to the\ndiff command.”\n\ninto simply saying:\n\n“As a result, no diff is displayed.”\n\nHowever, I don't read the original message as it would imply that only\nfile paths are affected by the precompose_argv call. \n\nAre there other suggestions on improving the commit message?\n"},{"id":"282756","messageId":"5704BF14.9060100@web.de","threadId":"41923","inReplyTo":"C52E38F0-B0F2-4769-A2C7-798D0CD99B47@fournova.com","subject":"Re: [PATCH] diff: run arguments through precompose_argv","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-04-06T07:47:32Z","receivedAt":"2016-04-06T07:47:32Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 06.04.16 08:51, Alexander Rinass wrote:\n> \n>> On 05 Apr 2016, at 21:15, Johannes Sixt <j6t@kdbg.org> wrote:\n>>\n>> Am 05.04.2016 um 19:09 schrieb Junio C Hamano:\n>>>> Thanks-to: Torsten Bögershausen <tboegi@web.de>\n>>\n>> I sense NFD disease: The combining diaresis should combine with the o, not the g. Here is a correct line to copy-and-paste if you like:\n>>\n>> Thanks-to: Torsten Bögershausen <tboegi@web.de>\n>>\n>> -- Hannes\n> \n> Thanks for reviewing and catching the NFD encoding error.\n> \n> I will send in a patch v2 with the correct NFC encoding.\n> \n> Would you also like me to alter the commit message as mentioned by Junio?\n> \n> I could rewrite the sentence:\n> \n> “As a result, no diff is displayed when feeding such a file path to the\n> diff command.”\n> \n> into simply saying:\n> \n> “As a result, no diff is displayed.”\n> \n> However, I don't read the original message as it would imply that only\n> file paths are affected by the precompose_argv call. \n> \n> Are there other suggestions on improving the commit message?\nMay be something like this, (but this is highly a personal taste question)\n\nWhen running diff commands, file paths containing decomposed unicode code points\nare not converted to precomposed unicode form under Mac OS X.\n\nAs a result, no diff is displayed.\n\nOpposite to most builtin commands, the diff builtin is missing the\nparse_options call, which internally runs arguments through the\nprecompose_argv call, which ensures all arguments are in precomposed\nunicode form.\n\nFix the problem by adding a precompose_argv call to diff, diff-index, diff-files and diff-tree.\n\nSigned-off-by: Alexander Rinass <alex@fournova.com>\nHelped-By: Torsten Bögershausen <tboegi@web.de>\nHelped-By: Junio C Hamano <gitster@pobox.com>\n"},{"id":"282780","messageId":"xmqq60vuy7ek.fsf@gitster.mtv.corp.google.com","threadId":"41923","inReplyTo":"5704BF14.9060100@web.de","subject":"Re: [PATCH] diff: run arguments through precompose_argv","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-06T17:27:31Z","receivedAt":"2016-04-06T17:27:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> May be something like this, (but this is highly a personal taste question)\n\nMy observation was not a suggestion to rewrite the log message, but\nprimarily about describing the change in behaviour.\n\nI didn't mean to nitpick the phrasing but let me think aloud about\nit, using this as a template.  I rewrapped overlong lines from the\noriginal.\n\n> When running diff commands, file paths containing decomposed\n> unicode code points are not converted to precomposed unicode form\n> under Mac OS X.\n>\n> As a result, no diff is displayed.\n\nAdd \", but we normalize the paths in the index and the history to\nprecomposed form on that platform.\" at the end of the first sentence, to\nmake sure you mention both causes of \"As a result\" that follows.\nAlso, these are better to be a single paragraph.  I.e.\n\n    When running diff commands, a pathspec containing decomposed\n    unicode code points is not converted to precomposed unicode form\n    under Mac OS X, but we normalize the paths in the index and the\n    history to precomposed form on that platform.  As a result, the\n    pathspec would not match and no diff is shown.\n\n> Opposite to most builtin commands, the diff builtin is missing the\n> parse_options call, which internally runs arguments through the\n> precompose_argv call, which ensures all arguments are in precomposed\n> unicode form.\n>\n> Fix the problem by adding a precompose_argv call to diff,\n> diff-index, diff-files and diff-tree.\n\nPerhaps it is just me but I found \"Opposite to most\" harder to grok\nfor some reason.\n\nAnd the \"backward compatibility note\" is missing.  I'd write these\ntwo paragraphs like this if I were doing this patch.\n\n    Unlike many builtin commands, the \"diff\" family of commands do\n    not use parse_options(), which is how other builtin commands\n    indirectly call precompose_argv() to normalize argv[] into\n    precomposed form on Mac OSX.  Teach these commands to call\n    precompose_argv() themselves.\n\n    Note that precomopose_argv() normalizes not just paths but all\n    command line arguments, so things like \"git diff -G $string\"\n    when $string has the decomposed form would first normalized into\n    the precomposed form and would stop hitting the same string in\n    the decomposed form in the diff output with this change.  It is\n    not a problem per-se, as \"log\" family of commands already use\n    parse_options() and call precompose_argv()--we can think of it\n    as making the \"diff\" family of commands behave in a similar way\n    as the commands in the \"log\" family.\n"},{"id":"286294","messageId":"xmqqy47gi6xz.fsf@gitster.mtv.corp.google.com","threadId":"41923","inReplyTo":"C52E38F0-B0F2-4769-A2C7-798D0CD99B47@fournova.com","subject":"Re: [PATCH] diff: run arguments through precompose_argv","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-11T22:08:24Z","receivedAt":"2016-05-11T22:08:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexander Rinass <alex@fournova.com> writes:\n\n>> On 05 Apr 2016, at 21:15, Johannes Sixt <j6t@kdbg.org> wrote:\n>> \n>> Am 05.04.2016 um 19:09 schrieb Junio C Hamano:\n>>>> Thanks-to: Torsten Bögershausen <tboegi@web.de>\n>> \n>> I sense NFD disease: The combining diaresis should combine with the o, not the g. Here is a correct line to copy-and-paste if you like:\n>> \n>> Thanks-to: Torsten Bögershausen <tboegi@web.de>\n>> \n>> -- Hannes\n>\n> Thanks for reviewing and catching the NFD encoding error.\n>\n> I will send in a patch v2 with the correct NFC encoding.\n>\n> Would you also like me to alter the commit message as mentioned by Junio?\n>\n> I could rewrite the sentence:\n>\n> “As a result, no diff is displayed when feeding such a file path to the\n> diff command.”\n>\n> into simply saying:\n>\n> “As a result, no diff is displayed.”\n>\n> However, I don't read the original message as it would imply that only\n> file paths are affected by the precompose_argv call. \n>\n> Are there other suggestions on improving the commit message?\n\nI think after this message there were a few suggestions, and then we\nheard nothing.  Should we still be waiting for a response from you?\n\nThanks.\n"},{"id":"286327","messageId":"D97D1033-70B9-46D5-A672-042DE0283CCA@fournova.com","threadId":"41923","inReplyTo":"xmqqy47gi6xz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff: run arguments through precompose_argv","fromName":"Alexander Rinass","fromEmail":"alex@fournova.com","sentAt":"2016-05-12T11:16:06Z","receivedAt":"2016-05-12T11:16:06Z","isPatch":true,"sender":{"key":"alex@fournova.com","avatar":"https://avatars.githubusercontent.com/u/116053?v=4"},"body":"> On 12 May 2016, at 00:08, Junio C Hamano <gitster@pobox.com> wrote:\n> \n> Alexander Rinass <alex@fournova.com> writes:\n> \n>>> On 05 Apr 2016, at 21:15, Johannes Sixt <j6t@kdbg.org> wrote:\n>>> \n>>> Am 05.04.2016 um 19:09 schrieb Junio C Hamano:\n>>>>> Thanks-to: Torsten Bögershausen <tboegi@web.de>\n>>> \n>>> I sense NFD disease: The combining diaresis should combine with the o, not the g. Here is a correct line to copy-and-paste if you like:\n>>> \n>>> Thanks-to: Torsten Bögershausen <tboegi@web.de>\n>>> \n>>> -- Hannes\n>> \n>> Thanks for reviewing and catching the NFD encoding error.\n>> \n>> I will send in a patch v2 with the correct NFC encoding.\n>> \n>> Would you also like me to alter the commit message as mentioned by Junio?\n>> \n>> I could rewrite the sentence:\n>> \n>> “As a result, no diff is displayed when feeding such a file path to the\n>> diff command.”\n>> \n>> into simply saying:\n>> \n>> “As a result, no diff is displayed.”\n>> \n>> However, I don't read the original message as it would imply that only\n>> file paths are affected by the precompose_argv call. \n>> \n>> Are there other suggestions on improving the commit message?\n> \n> I think after this message there were a few suggestions, and then we\n> heard nothing.  Should we still be waiting for a response from you?\n> \n> Thanks.\n\nSorry for not replying earlier. \n\nI will create a v2 patch until the weekend and send it to the mailing list.\n"},{"id":"286330","messageId":"xmqqvb2jguao.fsf@gitster.mtv.corp.google.com","threadId":"41923","inReplyTo":"D97D1033-70B9-46D5-A672-042DE0283CCA@fournova.com","subject":"Re: [PATCH] diff: run arguments through precompose_argv","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-12T15:39:11Z","receivedAt":"2016-05-12T15:39:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexander Rinass <alex@fournova.com> writes:\n\n> I will create a v2 patch until the weekend and send it to the mailing list.\n\nThanks.\n"}]}