{"thread":{"id":"35104","subject":"[PATCH] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","startedAt":"2013-10-11T11:24:19Z","lastAt":"2013-10-22T20:26:01Z","messageCount":31,"participants":["Yoshioka Tsuneo","Sam Vilain","Keshav Kini","Thomas Rast","Duy Nguyen","Felipe Contreras","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"228729","messageId":"38848735-7CFA-404E-AE51-4F445F813266@gmail.com","threadId":"35104","inReplyTo":null,"subject":"[PATCH] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-11T11:24:19Z","receivedAt":"2013-10-11T11:24:19Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"\"git diff -M --stat\" can detect rename and show renamed file name like\n\"foofoofoo => barbarbar\", but if destination filename is long the line\nis shortened like \"...barbarbar\" so there is no way to know whether the\nfile is renamed or existed in the source commit.\nThis commit makes it visible like \"...foo => ...bar\".\n\nSigned-off-by: Tsuneo Yoshioka \n---\n diff.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++-------\n 1 file changed, 47 insertions(+), 7 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a04a34d..9b55546 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1643,13 +1643,53 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tlen = name_width;\n \t\tname_len = strlen(name);\n \t\tif (name_width < name_len) {\n-\t\t\tchar *slash;\n-\t\t\tprefix = \"...\";\n-\t\t\tlen -= 3;\n-\t\t\tname += name_len - len;\n-\t\t\tslash = strchr(name, '/');\n-\t\t\tif (slash)\n-\t\t\t\tname = slash;\n+\t\t\tchar *arrow = strstr(name, \" => \");\n+\t\t\tif(arrow){\n+\t\t\t\tint prefix_len = (name_width- 4) / 2;\n+\t\t\t\tint f_omit;\n+\t\t\t\tchar *pre_arrow = alloca(name_width + 10);\n+\t\t\t\tchar *post_arrow = arrow + 4;\n+\t\t\t\tchar *prefix_buf = alloca(name_width + 10);\n+\t\t\t\tchar *pre_arrow_slash = NULL;\n+\n+\t\t\t\tif(arrow - name < prefix_len){\n+\t\t\t\t\tprefix_len = (int)(arrow - name);\n+\t\t\t\t\tf_omit = 0;\n+\t\t\t\t}else{\n+\t\t\t\t\tprefix_len -= 3;\n+\t\t\t\t\tf_omit = 1;\n+\t\t\t\t}\n+\t\t\t\tstrncpy(pre_arrow, arrow - prefix_len, prefix_len);\n+\t\t\t\tpre_arrow[prefix_len] = '¥0';\n+\t\t\t\tpre_arrow_slash = strchr(pre_arrow, '/');\n+\t\t\t\tif(f_omit && pre_arrow_slash){\n+\t\t\t\t\tpre_arrow = pre_arrow_slash;\n+\t\t\t\t}\n+\t\t\t\tsprintf(prefix_buf, \"%s%s => \", (f_omit ? \"...\" : \"\"), pre_arrow);\n+\t\t\t\tprefix = prefix_buf;\n+\n+\t\t\t\tif(strlen(post_arrow) > name_width - strlen(prefix)){\n+\t\t\t\t\tchar *post_arrow_slash = NULL;\n+\n+\t\t\t\t\tpost_arrow += strlen(post_arrow) - (name_width - strlen(prefix) - 3);\n+\t\t\t\t\tstrcat(prefix_buf, \"...\");\n+\t\t\t\t\tpost_arrow_slash = strchr(post_arrow, '/');\n+\t\t\t\t\tif(post_arrow_slash){\n+\t\t\t\t\t\tpost_arrow = post_arrow_slash;\n+\t\t\t\t\t}\n+\t\t\t\t\tname = post_arrow;\n+\t\t\t\t\tname_len = (int) (name_width - strlen(prefix));\n+\t\t\t\t}\n+\t\t\t\tlen -= strlen(prefix);\n+\t\t\t}else{\n+\t\t\t\tchar *slash = NULL;\n+\t\t\t\tprefix = \"...\";\n+\t\t\t\tlen -= 3;\n+\t\t\t\tname += name_len - len;\n+\t\t\t\tslash = strchr(name, '/');\n+\t\t\t\tif (slash)\n+\t\t\t\t\tname = slash;\n+\t\t\t}\n \t\t}\n \n \t\tif (file->is_binary) {\n-- \n1.8.4.475.g867697c\n"},{"id":"228736","messageId":"A15CCF08-83FD-4F3C-9773-C26DEE38FD33@gmail.com","threadId":"35104","inReplyTo":"38848735-7CFA-404E-AE51-4F445F813266@gmail.com","subject":"[PATCH v2] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-11T13:07:30Z","receivedAt":"2013-10-11T13:07:30Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"\"git diff -M --stat\" can detect rename and show renamed file name like\n\"foofoofoo => barbarbar\", but if destination filename is long the line\nis shortened like \"...barbarbar\" so there is no way to know whether the\nfile is renamed or existed in the source commit.\nThis commit makes it visible like \"...foo => ...bar\".\n\nSigned-off-by: Tsuneo Yoshioka \n---\n diff.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++-------\n 1 file changed, 51 insertions(+), 7 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a04a34d..3aeaf3e 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1643,13 +1643,57 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tlen = name_width;\n \t\tname_len = strlen(name);\n \t\tif (name_width < name_len) {\n-\t\t\tchar *slash;\n-\t\t\tprefix = \"...\";\n-\t\t\tlen -= 3;\n-\t\t\tname += name_len - len;\n-\t\t\tslash = strchr(name, '/');\n-\t\t\tif (slash)\n-\t\t\t\tname = slash;\n+\t\t\tchar *arrow = strstr(name, \" => \");\n+\t\t\tif (arrow) {\n+\t\t\t\tint prefix_len = (name_width - 4) / 2;\n+\t\t\t\tint f_omit;\n+\t\t\t\tint f_brace = 0;\n+\t\t\t\tchar *pre_arrow = alloca(name_width + 10);\n+\t\t\t\tchar *post_arrow = arrow + 4;\n+\t\t\t\tchar *prefix_buf = alloca(name_width + 10);\n+\t\t\t\tchar *pre_arrow_slash = NULL;\n+\n+\t\t\t\tif (arrow - name < prefix_len) {\n+\t\t\t\t\tprefix_len = (int)(arrow - name);\n+\t\t\t\t\tf_omit = 0;\n+\t\t\t\t} else {\n+\t\t\t\t\tprefix_len -= 3;\n+\t\t\t\t\tf_omit = 1;\n+\t\t\t\t\tif (name[0] == '{') {\n+\t\t\t\t\t\tprefix_len -= 1;\n+\t\t\t\t\t\tf_brace = 1;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t\tprefix_len = ((prefix_len >= 0) ? prefix_len : 0);\n+\t\t\t\tstrncpy(pre_arrow, arrow - prefix_len, prefix_len);\n+\t\t\t\tpre_arrow[prefix_len] = '¥0';\n+\t\t\t\tpre_arrow_slash = strchr(pre_arrow, '/');\n+\t\t\t\tif (f_omit && pre_arrow_slash)\n+\t\t\t\t\tpre_arrow = pre_arrow_slash;\n+\t\t\t\tsprintf(prefix_buf, \"%s%s%s => \", (f_brace ? \"{\" : \"\"), (f_omit ? \"...\" : \"\"), pre_arrow);\n+\t\t\t\tprefix = prefix_buf;\n+\n+\t\t\t\tif (strlen(post_arrow) > name_width - strlen(prefix)) {\n+\t\t\t\t\tchar *post_arrow_slash = NULL;\n+\n+\t\t\t\t\tpost_arrow += strlen(post_arrow) - (name_width - strlen(prefix) - 3);\n+\t\t\t\t\tstrcat(prefix_buf, \"...\");\n+\t\t\t\t\tpost_arrow_slash = strchr(post_arrow, '/');\n+\t\t\t\t\tif (post_arrow_slash)\n+\t\t\t\t\t\tpost_arrow = post_arrow_slash;\n+\t\t\t\t\tname = post_arrow;\n+\t\t\t\t\tname_len = (int) (name_width - strlen(prefix));\n+\t\t\t\t}\n+\t\t\t\tlen -= strlen(prefix);\n+\t\t\t} else {\n+\t\t\t\tchar *slash = NULL;\n+\t\t\t\tprefix = \"...\";\n+\t\t\t\tlen -= 3;\n+\t\t\t\tname += name_len - len;\n+\t\t\t\tslash = strchr(name, '/');\n+\t\t\t\tif (slash)\n+\t\t\t\t\tname = slash;\n+\t\t\t}\n \t\t}\n \n \t\tif (file->is_binary) {\n-- \n1.8.4.475.g867697c\n"},{"id":"228748","messageId":"52584125.1090706@vilain.net","threadId":"35104","inReplyTo":"A15CCF08-83FD-4F3C-9773-C26DEE38FD33@gmail.com","subject":"Re: [spf:guess,mismatch] [PATCH v2] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2013-10-11T18:19:17Z","receivedAt":"2013-10-11T18:19:17Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"On 10/11/2013 06:07 AM, Yoshioka Tsuneo wrote:\n> +\t\t\t\tprefix_len = ((prefix_len >= 0) ? prefix_len : 0);\n> +\t\t\t\tstrncpy(pre_arrow, arrow - prefix_len, prefix_len);\n> +\t\t\t\tpre_arrow[prefix_len] = '¥0';\n\n\nThis seems to be an encoding mistake; was this supposed to be an ASCII\narrow?\n\nSam\n"},{"id":"228773","messageId":"87mwmfyru4.fsf@gmail.com","threadId":"35104","inReplyTo":"52584125.1090706@vilain.net","subject":"Re: [spf:guess,mismatch] [PATCH v2] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Keshav Kini","fromEmail":"keshav.kini@gmail.com","sentAt":"2013-10-12T05:35:15Z","receivedAt":"2013-10-12T05:35:15Z","isPatch":true,"sender":{"key":"keshav.kini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/691290?v=4"},"body":"Sam Vilain writes:\n\n> On 10/11/2013 06:07 AM, Yoshioka Tsuneo wrote:\n>> +\t\t\t\tprefix_len = ((prefix_len >= 0) ? prefix_len : 0);\n>> +\t\t\t\tstrncpy(pre_arrow, arrow - prefix_len, prefix_len);\n>> +\t\t\t\tpre_arrow[prefix_len] = '¥0';\n>\n>\n> This seems to be an encoding mistake; was this supposed to be an ASCII\n> arrow?\n\nThat's supposed to be a null terminator character, '\\0'. See\nhttps://en.wikipedia.org/wiki/Yen_symbol#Code_page_932\n\n-Keshav\n"},{"id":"228885","messageId":"660A536D-9993-4B81-B6FF-A113F9111570@gmail.com","threadId":"35104","inReplyTo":"A15CCF08-83FD-4F3C-9773-C26DEE38FD33@gmail.com","subject":"[PATCH v3] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-12T20:48:16Z","receivedAt":"2013-10-12T20:48:16Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"\"git diff -M --stat\" can detect rename and show renamed file name like\n\"foofoofoo => barbarbar\", but if destination filename is long the line\nis shortened like \"...barbarbar\" so there is no way to know whether the\nfile is renamed or existed in the source commit.\nThis commit makes it visible like \"...foo => ...bar\".\n\nSigned-off-by: Tsuneo Yoshioka \n---\n diff.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++-------\n 1 file changed, 51 insertions(+), 7 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a04a34d..3aeaf3e 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1643,13 +1643,57 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tlen = name_width;\n \t\tname_len = strlen(name);\n \t\tif (name_width < name_len) {\n-\t\t\tchar *slash;\n-\t\t\tprefix = \"...\";\n-\t\t\tlen -= 3;\n-\t\t\tname += name_len - len;\n-\t\t\tslash = strchr(name, '/');\n-\t\t\tif (slash)\n-\t\t\t\tname = slash;\n+\t\t\tchar *arrow = strstr(name, \" => \");\n+\t\t\tif (arrow) {\n+\t\t\t\tint prefix_len = (name_width - 4) / 2;\n+\t\t\t\tint f_omit;\n+\t\t\t\tint f_brace = 0;\n+\t\t\t\tchar *pre_arrow = alloca(name_width + 10);\n+\t\t\t\tchar *post_arrow = arrow + 4;\n+\t\t\t\tchar *prefix_buf = alloca(name_width + 10);\n+\t\t\t\tchar *pre_arrow_slash = NULL;\n+\n+\t\t\t\tif (arrow - name < prefix_len) {\n+\t\t\t\t\tprefix_len = (int)(arrow - name);\n+\t\t\t\t\tf_omit = 0;\n+\t\t\t\t} else {\n+\t\t\t\t\tprefix_len -= 3;\n+\t\t\t\t\tf_omit = 1;\n+\t\t\t\t\tif (name[0] == '{') {\n+\t\t\t\t\t\tprefix_len -= 1;\n+\t\t\t\t\t\tf_brace = 1;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t\tprefix_len = ((prefix_len >= 0) ? prefix_len : 0);\n+\t\t\t\tstrncpy(pre_arrow, arrow - prefix_len, prefix_len);\n+\t\t\t\tpre_arrow[prefix_len] = '\\0';\n+\t\t\t\tpre_arrow_slash = strchr(pre_arrow, '/');\n+\t\t\t\tif (f_omit && pre_arrow_slash)\n+\t\t\t\t\tpre_arrow = pre_arrow_slash;\n+\t\t\t\tsprintf(prefix_buf, \"%s%s%s => \", (f_brace ? \"{\" : \"\"), (f_omit ? \"...\" : \"\"), pre_arrow);\n+\t\t\t\tprefix = prefix_buf;\n+\n+\t\t\t\tif (strlen(post_arrow) > name_width - strlen(prefix)) {\n+\t\t\t\t\tchar *post_arrow_slash = NULL;\n+\n+\t\t\t\t\tpost_arrow += strlen(post_arrow) - (name_width - strlen(prefix) - 3);\n+\t\t\t\t\tstrcat(prefix_buf, \"...\");\n+\t\t\t\t\tpost_arrow_slash = strchr(post_arrow, '/');\n+\t\t\t\t\tif (post_arrow_slash)\n+\t\t\t\t\t\tpost_arrow = post_arrow_slash;\n+\t\t\t\t\tname = post_arrow;\n+\t\t\t\t\tname_len = (int) (name_width - strlen(prefix));\n+\t\t\t\t}\n+\t\t\t\tlen -= strlen(prefix);\n+\t\t\t} else {\n+\t\t\t\tchar *slash = NULL;\n+\t\t\t\tprefix = \"...\";\n+\t\t\t\tlen -= 3;\n+\t\t\t\tname += name_len - len;\n+\t\t\t\tslash = strchr(name, '/');\n+\t\t\t\tif (slash)\n+\t\t\t\t\tname = slash;\n+\t\t\t}\n \t\t}\n \n \t\tif (file->is_binary) {\n-- \n1.8.4.475.g867697c\n"},{"id":"228886","messageId":"2FC653A3-91B5-46C4-9332-E4F51EB86146@gmail.com","threadId":"35104","inReplyTo":"A15CCF08-83FD-4F3C-9773-C26DEE38FD33@gmail.com","subject":"[PATCH v3] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-12T20:48:17Z","receivedAt":"2013-10-12T20:48:17Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"\"git diff -M --stat\" can detect rename and show renamed file name like\n\"foofoofoo => barbarbar\", but if destination filename is long the line\nis shortened like \"...barbarbar\" so there is no way to know whether the\nfile is renamed or existed in the source commit.\nThis commit makes it visible like \"...foo => ...bar\".\n\nSigned-off-by: Tsuneo Yoshioka \n---\n diff.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++-------\n 1 file changed, 51 insertions(+), 7 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a04a34d..3aeaf3e 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1643,13 +1643,57 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tlen = name_width;\n \t\tname_len = strlen(name);\n \t\tif (name_width < name_len) {\n-\t\t\tchar *slash;\n-\t\t\tprefix = \"...\";\n-\t\t\tlen -= 3;\n-\t\t\tname += name_len - len;\n-\t\t\tslash = strchr(name, '/');\n-\t\t\tif (slash)\n-\t\t\t\tname = slash;\n+\t\t\tchar *arrow = strstr(name, \" => \");\n+\t\t\tif (arrow) {\n+\t\t\t\tint prefix_len = (name_width - 4) / 2;\n+\t\t\t\tint f_omit;\n+\t\t\t\tint f_brace = 0;\n+\t\t\t\tchar *pre_arrow = alloca(name_width + 10);\n+\t\t\t\tchar *post_arrow = arrow + 4;\n+\t\t\t\tchar *prefix_buf = alloca(name_width + 10);\n+\t\t\t\tchar *pre_arrow_slash = NULL;\n+\n+\t\t\t\tif (arrow - name < prefix_len) {\n+\t\t\t\t\tprefix_len = (int)(arrow - name);\n+\t\t\t\t\tf_omit = 0;\n+\t\t\t\t} else {\n+\t\t\t\t\tprefix_len -= 3;\n+\t\t\t\t\tf_omit = 1;\n+\t\t\t\t\tif (name[0] == '{') {\n+\t\t\t\t\t\tprefix_len -= 1;\n+\t\t\t\t\t\tf_brace = 1;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t\tprefix_len = ((prefix_len >= 0) ? prefix_len : 0);\n+\t\t\t\tstrncpy(pre_arrow, arrow - prefix_len, prefix_len);\n+\t\t\t\tpre_arrow[prefix_len] = '\\0';\n+\t\t\t\tpre_arrow_slash = strchr(pre_arrow, '/');\n+\t\t\t\tif (f_omit && pre_arrow_slash)\n+\t\t\t\t\tpre_arrow = pre_arrow_slash;\n+\t\t\t\tsprintf(prefix_buf, \"%s%s%s => \", (f_brace ? \"{\" : \"\"), (f_omit ? \"...\" : \"\"), pre_arrow);\n+\t\t\t\tprefix = prefix_buf;\n+\n+\t\t\t\tif (strlen(post_arrow) > name_width - strlen(prefix)) {\n+\t\t\t\t\tchar *post_arrow_slash = NULL;\n+\n+\t\t\t\t\tpost_arrow += strlen(post_arrow) - (name_width - strlen(prefix) - 3);\n+\t\t\t\t\tstrcat(prefix_buf, \"...\");\n+\t\t\t\t\tpost_arrow_slash = strchr(post_arrow, '/');\n+\t\t\t\t\tif (post_arrow_slash)\n+\t\t\t\t\t\tpost_arrow = post_arrow_slash;\n+\t\t\t\t\tname = post_arrow;\n+\t\t\t\t\tname_len = (int) (name_width - strlen(prefix));\n+\t\t\t\t}\n+\t\t\t\tlen -= strlen(prefix);\n+\t\t\t} else {\n+\t\t\t\tchar *slash = NULL;\n+\t\t\t\tprefix = \"...\";\n+\t\t\t\tlen -= 3;\n+\t\t\t\tname += name_len - len;\n+\t\t\t\tslash = strchr(name, '/');\n+\t\t\t\tif (slash)\n+\t\t\t\t\tname = slash;\n+\t\t\t}\n \t\t}\n \n \t\tif (file->is_binary) {\n-- \n1.8.4.475.g867697c\n"},{"id":"228887","messageId":"D89D1A4D-B9B0-483C-BDBB-BC6CAF6A4D2E@gmail.com","threadId":"35104","inReplyTo":"87mwmfyru4.fsf@gmail.com","subject":"Re: [spf:guess,mismatch] [PATCH v2] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-12T20:52:35Z","receivedAt":"2013-10-12T20:52:35Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"Hello\n\nOn Oct 12, 2013, at 8:35 AM, Keshav Kini wrote:\n\n> Sam Vilain writes:\n> \n>> On 10/11/2013 06:07 AM, Yoshioka Tsuneo wrote:\n>>> +\t\t\t\tprefix_len = ((prefix_len >= 0) ? prefix_len : 0);\n>>> +\t\t\t\tstrncpy(pre_arrow, arrow - prefix_len, prefix_len);\n>>> +\t\t\t\tpre_arrow[prefix_len] = '¥0';\n>> \n>> \n>> This seems to be an encoding mistake; was this supposed to be an ASCII\n>> arrow?\n> \n> That's supposed to be a null terminator character, '\\0'. See\n> https://en.wikipedia.org/wiki/Yen_symbol#Code_page_932\nThank you for pointing out the encoding issue.\nIt looks, I need to change encoding of \"pbcopy\" command to\n\"en_US.UTF-8\" from \"C\" on Mac OS X.\nI just sent updated patch as \"PATCH v3\".\n\nThanks !\n\n---\nTsuneo Yoshioka (吉岡 恒夫)\nyoshiokatsuneo@gmail.com\n\n\n\n\nOn Oct 12, 2013, at 8:35 AM, Keshav Kini wrote:\n\n> Sam Vilain writes:\n> \n>> On 10/11/2013 06:07 AM, Yoshioka Tsuneo wrote:\n>>> +\t\t\t\tprefix_len = ((prefix_len >= 0) ? prefix_len : 0);\n>>> +\t\t\t\tstrncpy(pre_arrow, arrow - prefix_len, prefix_len);\n>>> +\t\t\t\tpre_arrow[prefix_len] = '¥0';\n>> \n>> \n>> This seems to be an encoding mistake; was this supposed to be an ASCII\n>> arrow?\n> \n> That's supposed to be a null terminator character, '\\0'. See\n> https://en.wikipedia.org/wiki/Yen_symbol#Code_page_932\n> \n> -Keshav\n> \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"},{"id":"228908","messageId":"877gdg7w46.fsf@linux-k42r.v.cablecom.net","threadId":"35104","inReplyTo":"660A536D-9993-4B81-B6FF-A113F9111570@gmail.com","subject":"Re: [PATCH v3] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2013-10-13T20:29:29Z","receivedAt":"2013-10-13T20:29:29Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Hi,\n\nYoshioka Tsuneo writes:\n\n> \"git diff -M --stat\" can detect rename and show renamed file name like\n> \"foofoofoo => barbarbar\", but if destination filename is long the line\n> is shortened like \"...barbarbar\" so there is no way to know whether the\n> file is renamed or existed in the source commit.\n\nThanks for your patch! I think this is indeed something that should be\nfixed.\n\nCan you explain the algorithm chosen in the commit message or a block\ncomment in the code? I find it much easier to follow large code blocks\n(like the one you added) with a prior notion of what it tries to do.\n\n [As an aside, Documentation/SubmittingPatches says\n\n The body should provide a meaningful commit message, which:\n\n . explains the problem the change tries to solve, iow, what is wrong\n with the current code without the change.\n\n . justifies the way the change solves the problem, iow, why the\n result with the change is better.\n\n . alternate solutions considered but discarded, if any.\n\n Observe that you explained the first item very well, but not the\n others.]\n\n> This commit makes it visible like \"...foo => ...bar\".\n\nAlso, you should rewrite this to be in the imperative mood:\n\n Make sure there is always an arrow, e.g., \"...foo => ...bar\".\n\nor some such.\n\n [Again from SubmittingPatches:\n\n Describe your changes in imperative mood, e.g. \"make xyzzy do frotz\"\n instead of \"[This patch] makes xyzzy do frotz\" or \"[I] changed xyzzy\n to do frotz\", as if you are giving orders to the codebase to change\n its behaviour.]\n\n> Signed-off-by: Tsuneo Yoshioka \n> ---\n> diff.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++-------\n> 1 file changed, 51 insertions(+), 7 deletions(-)\n\nCan you add a test? Perhaps like the one below. (You can squash it\ninto your commit if you like it.)\n\nNote that in the test, the generated line looks like this:\n\n {..._does_not_fit_in_a_single_line => .../path1 | 0\n\nI don't want to go all bikesheddey, but I think it's somewhat\nunfortunate that the elided parts do not correspond to each other. In\nparticular, I think the closing brace should not be omitted. Perhaps\nsomething like this would be ideal (making it up on the spot, don't\ncount characters):\n\n {...a_single_line => ..._as_the_first}/path1 | 0\n\ndiff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh\nindex 2f327b7..03d6371 100755\n--- a/t/t4001-diff-rename.sh\n+++ b/t/t4001-diff-rename.sh\n@@ -156,4 +156,16 @@ test_expect_success 'rename pretty print common prefix and suffix overlap' '\n \ttest_i18ngrep \" d/f/{ => f}/e \" output\n '\n \n+test_expect_success 'rename of very long path shows =>' '\n+\tmkdir long_dirname_that_does_not_fit_in_a_single_line &&\n+\tmkdir another_extremely_long_path_but_not_the_same_as_the_first &&\n+\tcp path1 long_dirname*/ &&\n+\tgit add long_dirname*/path1 &&\n+\ttest_commit add_long_pathname &&\n+\tgit mv long_dirname*/path1 another_extremely_*/ &&\n+\ttest_commit move_long_pathname &&\n+\tgit diff -M --stat HEAD^ HEAD >output &&\n+\ttest_i18ngrep \"=>.*path1\" output\n+'\n+\n test_done\n\n-- \nThomas Rast\ntr@thomasrast.ch\n"},{"id":"228940","messageId":"CACsJy8Cg5M8dHSMW+giwYB2016jZhryyxET0kxkLDX7xk=B47w@mail.gmail.com","threadId":"35104","inReplyTo":"660A536D-9993-4B81-B6FF-A113F9111570@gmail.com","subject":"Re: [PATCH v3] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-10-14T19:04:38Z","receivedAt":"2013-10-14T19:04:38Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Oct 13, 2013 at 3:48 AM, Yoshioka Tsuneo\n wrote:\n> \"git diff -M --stat\" can detect rename and show renamed file name like\n> \"foofoofoo => barbarbar\", but if destination filename is long the line\n> is shortened like \"...barbarbar\" so there is no way to know whether the\n> file is renamed or existed in the source commit.\n> This commit makes it visible like \"...foo => ...bar\".\n>\n> Signed-off-by: Tsuneo Yoshioka \n> ---\n> diff.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++-------\n> 1 file changed, 51 insertions(+), 7 deletions(-)\n>\n> diff --git a/diff.c b/diff.c\n> index a04a34d..3aeaf3e 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1643,13 +1643,57 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> len = name_width;\n> name_len = strlen(name);\n> if (name_width < name_len) {\n> - char *slash;\n> - prefix = \"...\";\n> - len -= 3;\n> - name += name_len - len;\n> - slash = strchr(name, '/');\n> - if (slash)\n> - name = slash;\n> + char *arrow = strstr(name, \" => \");\n> + if (arrow) {\n\nThis looks iffy. What if \" => \" is part of the path name?\nfile->is_renamed would be a more reliable sign. In that case I think\nyou just need an ellipsis version of pprint_rename() (i.e. drop the\nresult of previous pprint_rename() on the floor and create a new\nstring with \"...\" and \" => \" in your pprint_ellipsis_rename or\nsomething)\n\n> + int prefix_len = (name_width - 4) / 2;\n> + int f_omit;\n> + int f_brace = 0;\n> + char *pre_arrow = alloca(name_width + 10);\n> + char *post_arrow = arrow + 4;\n> + char *prefix_buf = alloca(name_width + 10);\n> + char *pre_arrow_slash = NULL;\n> +\n> + if (arrow - name < prefix_len) {\n> + prefix_len = (int)(arrow - name);\n> + f_omit = 0;\n> + } else {\n> + prefix_len -= 3;\n> + f_omit = 1;\n> + if (name[0] == '{') {\n> + prefix_len -= 1;\n> + f_brace = 1;\n> + }\n> + }\n> + prefix_len = ((prefix_len >= 0) ? prefix_len : 0);\n> + strncpy(pre_arrow, arrow - prefix_len, prefix_len);\n> + pre_arrow[prefix_len] = '\\0';\n> + pre_arrow_slash = strchr(pre_arrow, '/');\n> + if (f_omit && pre_arrow_slash)\n> + pre_arrow = pre_arrow_slash;\n> + sprintf(prefix_buf, \"%s%s%s => \", (f_brace ? \"{\" : \"\"), (f_omit ? \"...\" : \"\"), pre_arrow);\n> + prefix = prefix_buf;\n> +\n> + if (strlen(post_arrow) > name_width - strlen(prefix)) {\n> + char *post_arrow_slash = NULL;\n> +\n> + post_arrow += strlen(post_arrow) - (name_width - strlen(prefix) - 3);\n> + strcat(prefix_buf, \"...\");\n> + post_arrow_slash = strchr(post_arrow, '/');\n> + if (post_arrow_slash)\n> + post_arrow = post_arrow_slash;\n> + name = post_arrow;\n> + name_len = (int) (name_width - strlen(prefix));\n> + }\n> + len -= strlen(prefix);\n> + } else {\n> + char *slash = NULL;\n> + prefix = \"...\";\n> + len -= 3;\n> + name += name_len - len;\n> + slash = strchr(name, '/');\n> + if (slash)\n> + name = slash;\n> + }\n> }\n>\n> if (file->is_binary) {\n> --\n> 1.8.4.475.g867697c\n>\n>\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\n\n-- \nDuy\n"},{"id":"228996","messageId":"AFC93704-D6C5-49AF-9A66-C5EA81348FFA@gmail.com","threadId":"35104","inReplyTo":"660A536D-9993-4B81-B6FF-A113F9111570@gmail.com","subject":"[PATCH v4] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-15T09:45:59Z","receivedAt":"2013-10-15T09:45:59Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"\n\"git diff -M --stat\" can detect rename and show renamed file name like\n\"foofoofoo => barbarbar\". But if destination filename is long, the line\nis shortened like \"...barbarbar\" so there is no way to know whether the\nfile is renamed or existed in the source commit.\nMake sure there is always an arrow, like \"...foo => ...bar\".\nThe output can contains curly braces('{','}') for grouping.\nSo, in general, the outpu format is \"{ => }\"\nTo keep arrow(\"=>\"), try to omit as long as possible at first\nbecause later part or changing part will be the more important part.\nIf it is not enough, shorten , , and trying to\nhave the maximum length the same because those will be equaly important.\n\nSigned-off-by: Tsuneo Yoshioka \nTest-added-by: Thomas Rast \n---\n diff.c | 183 +++++++++++++++++++++++++++++++++++++++++++------\n t/t4001-diff-rename.sh | 12 ++++\n 2 files changed, 173 insertions(+), 22 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a04a34d..7f907ed 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1258,11 +1258,10 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t}\n }\n \n-static char *pprint_rename(const char *a, const char *b)\n+static void pprint_rename_find_common_prefix_suffix(const char *a, const char *b, struct strbuf *pfx, struct strbuf *a_mid, struct strbuf *b_mid, struct strbuf *sfx)\n {\n \tconst char *old = a;\n \tconst char *new = b;\n-\tstruct strbuf name = STRBUF_INIT;\n \tint pfx_length, sfx_length;\n \tint pfx_adjust_for_slash;\n \tint len_a = strlen(a);\n@@ -1272,10 +1271,9 @@ static char *pprint_rename(const char *a, const char *b)\n \tint qlen_b = quote_c_style(b, NULL, NULL, 0);\n \n \tif (qlen_a || qlen_b) {\n-\t\tquote_c_style(a, &name, NULL, 0);\n-\t\tstrbuf_addstr(&name, \" => \");\n-\t\tquote_c_style(b, &name, NULL, 0);\n-\t\treturn strbuf_detach(&name, NULL);\n+\t\tquote_c_style(a, a_mid, NULL, 0);\n+\t\tquote_c_style(b, b_mid, NULL, 0);\n+\t\treturn;\n \t}\n \n \t/* Find common prefix */\n@@ -1321,18 +1319,149 @@ static char *pprint_rename(const char *a, const char *b)\n \t\ta_midlen = 0;\n \tif (b_midlen < 0)\n \t\tb_midlen = 0;\n+\t\n+\tstrbuf_add(pfx, a, pfx_length);\n+\tstrbuf_add(a_mid, a + pfx_length, a_midlen);\n+\tstrbuf_add(b_mid, b + pfx_length, b_midlen);\n+\tstrbuf_add(sfx, a + len_a - sfx_length, sfx_length);\n+}\n \n-\tstrbuf_grow(&name, pfx_length + a_midlen + b_midlen + sfx_length + 7);\n-\tif (pfx_length + sfx_length) {\n-\t\tstrbuf_add(&name, a, pfx_length);\n+/*\n+ * Omit each parts to fix in name_width.\n+ * Formatted string is \"{ => }\".\n+ * At first, omit as long as possible.\n+ * If it is not enough, omit , , by tring to set the length of\n+ * those 3 parts(including \"...\") to the same.\n+ * Ex:\n+ * \"foofoofoo => barbarbar\"\n+ * will be like\n+ * \"...foo => ...bar\".\n+ * \"long_parent{foofoofoo => barbarbar}longfilename\"\n+ * will be like\n+ * \"...parent{...foofoo => ...barbar}...lename\"\n+ */\n+static void pprint_rename_omit(struct strbuf *pfx, struct strbuf *a_mid, struct strbuf *b_mid, struct strbuf *sfx, int name_width)\n+{\n+\n+#define ARROW \" => \"\n+#define ELLIPSIS \"...\"\n+#define swap(a,b) myswap((a),(b),sizeof(a))\n+\t\n+#define myswap(a, b, size) do {\t\t\\\n+unsigned char mytmp[size];\t\\\n+memcpy(mytmp, &a, size);\t\t\\\n+memcpy(&a, &b, size);\t\t\\\n+memcpy(&b, mytmp, size);\t\t\\\n+} while (0)\n+\n+\tint use_curly_braces = (pfx->len > 0) || (sfx->len > 0);\n+\tsize_t name_len;\n+\tsize_t len;\n+\tsize_t part_lengths[4];\n+\tsize_t max_part_len = 0;\n+\tsize_t remainder_part_len = 0;\n+\tint i, j;\n+\n+\tname_len = pfx->len + a_mid->len + b_mid->len + sfx->len + strlen(ARROW) + (use_curly_braces?2:0);\n+\t\n+\tif (name_len <= name_width){\n+\t\t/* Everthing fits in name_width */\n+\t\treturn;\n+\t}\n+\t\n+\tif(use_curly_braces){\n+\t\tif(strlen(ELLIPSIS) + (name_len - pfx->len) <= name_width){\n+\t\t\t/*\n+\t\t\t Just omitting left of '{' is enough\n+\t\t\t Ex: ...aaa{foofoofoo => bar}file\n+\t\t\t */\n+\t\t\tstrbuf_splice(pfx, name_len - pfx->len, name_width - (name_len - pfx->len), ELLIPSIS, strlen(ELLIPSIS));\n+\t\t\treturn;\n+\t\t}else{\n+\t\t\tif (pfx->len > strlen(ELLIPSIS)) {\n+\t\t\t\t/*\n+\t\t\t\t Just omitting left of '{' is not enough\n+\t\t\t\t name will be \"...{SOMETHING}SOMETHING\"\n+\t\t\t\t */\n+\t\t\t\tstrbuf_reset(pfx);\n+\t\t\t\tstrbuf_addstr(pfx, ELLIPSIS);\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t/* available length for a_mid, b_mid and sfx */\n+\tlen = name_width - strlen(ARROW) - (use_curly_braces?2:0);\n+\t\n+\t/* a_mid, b_mid, sfx will be have the same max, including ellipsis(\"...\"). */\n+\tpart_lengths[0] = (int)a_mid->len;\n+\tpart_lengths[1] = (int)b_mid->len;\n+\tpart_lengths[2] = (int)sfx->len;\n+\t\n+\t/* bubble sort of part_lengths, descending order */\n+\tfor(i=0;i<3;i++){\n+\t\tfor(j= i+1; j<3; j++){\n+\t\t\tif(part_lengths[j] > part_lengths[i]){\n+\t\t\t\tswap(part_lengths[i], part_lengths[j]);\n+\t\t\t}\n+\t\t}\n+\t}\n+\t\n+\tif (part_lengths[1] + part_lengths[1] + part_lengths[2] <= len) {\n+\t\t/*\n+\t\t * \"{...foofoo => barbar}file\"\n+\t\t * There is only one omitting part.\n+\t\t */\n+\t\tmax_part_len = len - part_lengths[1] - part_lengths[2];\n+\t} else if (part_lengths[2] + part_lengths[2] + part_lengths[2] <= len){\n+\t\t/*\n+\t\t * \"{...foofoo => ...barbar}file\"\n+\t\t * There are 2 omitting part.\n+\t\t */\n+\t\tmax_part_len = (len - part_lengths[2])/2;\n+\t\tremainder_part_len = (len - part_lengths[2]) - max_part_len * 2;\n+\t} else {\n+\t\t/*\n+\t\t * \"{...ofoo => ...rbar}...file\"\n+\t\t * There are 3 omitting part.\n+\t\t */\n+\t\tmax_part_len = len / 3;\n+\t\tremainder_part_len = len - (max_part_len) * 3;\n+\t}\n+\t\n+\tif (max_part_len < strlen(ELLIPSIS))\n+\t\tmax_part_len = strlen(ELLIPSIS);\n+\t\n+\tif (sfx->len > max_part_len)\n+\t\tstrbuf_splice(sfx, 0, sfx->len - max_part_len + strlen(ELLIPSIS), ELLIPSIS, strlen(ELLIPSIS));\n+\tif (remainder_part_len==2)\n+\t\tmax_part_len++;\n+\tif (a_mid->len > max_part_len)\n+\t\tstrbuf_splice(a_mid, 0, a_mid->len - max_part_len + strlen(ELLIPSIS), ELLIPSIS, strlen(ELLIPSIS));\n+\tif (remainder_part_len==1)\n+\t\tmax_part_len++;\n+\tif (b_mid->len > max_part_len)\n+\t\tstrbuf_splice(b_mid, 0, b_mid->len - max_part_len + strlen(ELLIPSIS), ELLIPSIS, strlen(ELLIPSIS));\n+}\n+\n+static char *pprint_rename(const char *a, const char *b, int name_width)\n+{\n+\tstruct strbuf pfx = STRBUF_INIT, a_mid = STRBUF_INIT, b_mid = STRBUF_INIT, sfx = STRBUF_INIT;\n+\tstruct strbuf name = STRBUF_INIT;\n+\t\n+\tpprint_rename_find_common_prefix_suffix(a, b, &pfx, &a_mid, &b_mid, &sfx);\n+\tpprint_rename_omit(&pfx, &a_mid, &b_mid, &sfx, name_width);\n+\t\n+\tstrbuf_grow(&name, pfx.len + a_mid.len + b_mid.len + sfx.len + 7);\n+\tif (pfx.len + sfx.len) {\n+\t\tstrbuf_addbuf(&name, &pfx);\n \t\tstrbuf_addch(&name, '{');\n \t}\n-\tstrbuf_add(&name, a + pfx_length, a_midlen);\n+\tstrbuf_addbuf(&name, &a_mid);\n \tstrbuf_addstr(&name, \" => \");\n-\tstrbuf_add(&name, b + pfx_length, b_midlen);\n-\tif (pfx_length + sfx_length) {\n+\tstrbuf_addbuf(&name, &b_mid);\n+\tif (pfx.len + sfx.len) {\n \t\tstrbuf_addch(&name, '}');\n-\t\tstrbuf_add(&name, a + len_a - sfx_length, sfx_length);\n+\t\tstrbuf_addbuf(&name, &sfx);\n \t}\n \treturn strbuf_detach(&name, NULL);\n }\n@@ -1418,23 +1547,31 @@ static void show_graph(FILE *file, char ch, int cnt, const char *set, const char\n \tfprintf(file, \"%s\", reset);\n }\n \n-static void fill_print_name(struct diffstat_file *file)\n+static void fill_print_name(struct diffstat_file *file, int name_width)\n {\n \tchar *pname;\n \n-\tif (file->print_name)\n-\t\treturn;\n-\n \tif (!file->is_renamed) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tif (file->print_name)\n+\t\t\treturn;\n \t\tif (quote_c_style(file->name, &buf, NULL, 0)) {\n \t\t\tpname = strbuf_detach(&buf, NULL);\n \t\t} else {\n \t\t\tpname = file->name;\n \t\t\tstrbuf_release(&buf);\n \t\t}\n+\t\tif(strlen(pname) > name_width){\n+\t\t\tstruct strbuf buf2 = STRBUF_INIT;\n+\t\t\tstrbuf_addstr(&buf2, \"...\");\n+\t\t\tstrbuf_addstr(&buf2, pname + strlen(pname) - name_width - 3);\n+\t\t}\n \t} else {\n-\t\tpname = pprint_rename(file->from_name, file->name);\n+\t\tif (file->print_name){\n+\t\t\tfree(file->print_name);\n+\t\t\tfile->print_name = NULL;\n+\t\t}\n+\t\tpname = pprint_rename(file->from_name, file->name, name_width);\n \t}\n \tfile->print_name = pname;\n }\n@@ -1517,7 +1654,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t\tcount++; /* not shown == room for one more */\n \t\t\tcontinue;\n \t\t}\n-\t\tfill_print_name(file);\n+\t\tfill_print_name(file, INT_MAX);\n \t\tlen = strlen(file->print_name);\n \t\tif (max_len < len)\n \t\t\tmax_len = len;\n@@ -1629,7 +1766,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tfor (i = 0; i < count; i++) {\n \t\tconst char *prefix = \"\";\n \t\tstruct diffstat_file *file = data->files[i];\n-\t\tchar *name = file->print_name;\n+\t\tchar *name;\n \t\tuintmax_t added = file->added;\n \t\tuintmax_t deleted = file->deleted;\n \t\tint name_len;\n@@ -1637,6 +1774,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tif (!file->is_interesting && (added + deleted == 0))\n \t\t\tcontinue;\n \n+\t\tfill_print_name(file, name_width);\n+\t\tname = file->print_name;\n \t\t/*\n \t\t * \"scale\" the filename\n \t\t */\n@@ -1772,7 +1911,7 @@ static void show_numstat(struct diffstat_t *data, struct diff_options *options)\n \t\t\t\t\"%\"PRIuMAX\"\\t%\"PRIuMAX\"\\t\",\n \t\t\t\tfile->added, file->deleted);\n \t\tif (options->line_termination) {\n-\t\t\tfill_print_name(file);\n+\t\t\tfill_print_name(file, INT_MAX);\n \t\t\tif (!file->is_renamed)\n \t\t\t\twrite_name_quoted(file->name, options->file,\n \t\t\t\t\t\t options->line_termination);\n@@ -4258,7 +4397,7 @@ static void show_mode_change(FILE *file, struct diff_filepair *p, int show_name,\n static void show_rename_copy(FILE *file, const char *renamecopy, struct diff_filepair *p,\n \t\t\tconst char *line_prefix)\n {\n-\tchar *names = pprint_rename(p->one->path, p->two->path);\n+\tchar *names = pprint_rename(p->one->path, p->two->path, INT_MAX);\n \n \tfprintf(file, \" %s %s (%d%%)\\n\", renamecopy, names, similarity_index(p));\n \tfree(names);\ndiff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh\nindex 2f327b7..03d6371 100755\n--- a/t/t4001-diff-rename.sh\n+++ b/t/t4001-diff-rename.sh\n@@ -156,4 +156,16 @@ test_expect_success 'rename pretty print common prefix and suffix overlap' '\n \ttest_i18ngrep \" d/f/{ => f}/e \" output\n '\n \n+test_expect_success 'rename of very long path shows =>' '\n+\tmkdir long_dirname_that_does_not_fit_in_a_single_line &&\n+\tmkdir another_extremely_long_path_but_not_the_same_as_the_first &&\n+\tcp path1 long_dirname*/ &&\n+\tgit add long_dirname*/path1 &&\n+\ttest_commit add_long_pathname &&\n+\tgit mv long_dirname*/path1 another_extremely_*/ &&\n+\ttest_commit move_long_pathname &&\n+\tgit diff -M --stat HEAD^ HEAD >output &&\n+\ttest_i18ngrep \"=>.*path1\" output\n+'\n+\n test_done\n-- \n1.8.4.475.g867697c\n"},{"id":"228997","messageId":"E3435C7B-9673-40F5-96EA-AD59B20EBC4A@gmail.com","threadId":"35104","inReplyTo":"877gdg7w46.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH v3] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-15T09:46:07Z","receivedAt":"2013-10-15T09:46:07Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"Hello Thomas\n\nThank you very much for your kind review.\nNow, I just posted \"PATCH v4\" that will include your suggestion like keeping \"{\", \"}\"\nwhile omitting, improving commit message and comment, and test.\n\nThanks!\n\n---\nTsuneo Yoshioka (吉岡 恒夫)\nyoshiokatsuneo@gmail.com\n\n\n\n\nOn Oct 13, 2013, at 11:29 PM, Thomas Rast wrote:\n\n> Hi,\n> \n> Yoshioka Tsuneo writes:\n> \n>> \"git diff -M --stat\" can detect rename and show renamed file name like\n>> \"foofoofoo => barbarbar\", but if destination filename is long the line\n>> is shortened like \"...barbarbar\" so there is no way to know whether the\n>> file is renamed or existed in the source commit.\n> \n> Thanks for your patch! I think this is indeed something that should be\n> fixed.\n> \n> Can you explain the algorithm chosen in the commit message or a block\n> comment in the code? I find it much easier to follow large code blocks\n> (like the one you added) with a prior notion of what it tries to do.\n> \n> [As an aside, Documentation/SubmittingPatches says\n> \n> The body should provide a meaningful commit message, which:\n> \n> . explains the problem the change tries to solve, iow, what is wrong\n> with the current code without the change.\n> \n> . justifies the way the change solves the problem, iow, why the\n> result with the change is better.\n> \n> . alternate solutions considered but discarded, if any.\n> \n> Observe that you explained the first item very well, but not the\n> others.]\n> \n>> This commit makes it visible like \"...foo => ...bar\".\n> \n> Also, you should rewrite this to be in the imperative mood:\n> \n> Make sure there is always an arrow, e.g., \"...foo => ...bar\".\n> \n> or some such.\n> \n> [Again from SubmittingPatches:\n> \n> Describe your changes in imperative mood, e.g. \"make xyzzy do frotz\"\n> instead of \"[This patch] makes xyzzy do frotz\" or \"[I] changed xyzzy\n> to do frotz\", as if you are giving orders to the codebase to change\n> its behaviour.]\n> \n>> Signed-off-by: Tsuneo Yoshioka \n>> ---\n>> diff.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++-------\n>> 1 file changed, 51 insertions(+), 7 deletions(-)\n> \n> Can you add a test? Perhaps like the one below. (You can squash it\n> into your commit if you like it.)\n> \n> Note that in the test, the generated line looks like this:\n> \n> {..._does_not_fit_in_a_single_line => .../path1 | 0\n> \n> I don't want to go all bikesheddey, but I think it's somewhat\n> unfortunate that the elided parts do not correspond to each other. In\n> particular, I think the closing brace should not be omitted. Perhaps\n> something like this would be ideal (making it up on the spot, don't\n> count characters):\n> \n> {...a_single_line => ..._as_the_first}/path1 | 0\n> \n> diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh\n> index 2f327b7..03d6371 100755\n> --- a/t/t4001-diff-rename.sh\n> +++ b/t/t4001-diff-rename.sh\n> @@ -156,4 +156,16 @@ test_expect_success 'rename pretty print common prefix and suffix overlap' '\n> \ttest_i18ngrep \" d/f/{ => f}/e \" output\n> '\n> \n> +test_expect_success 'rename of very long path shows =>' '\n> +\tmkdir long_dirname_that_does_not_fit_in_a_single_line &&\n> +\tmkdir another_extremely_long_path_but_not_the_same_as_the_first &&\n> +\tcp path1 long_dirname*/ &&\n> +\tgit add long_dirname*/path1 &&\n> +\ttest_commit add_long_pathname &&\n> +\tgit mv long_dirname*/path1 another_extremely_*/ &&\n> +\ttest_commit move_long_pathname &&\n> +\tgit diff -M --stat HEAD^ HEAD >output &&\n> +\ttest_i18ngrep \"=>.*path1\" output\n> +'\n> +\n> test_done\n> \n> -- \n> Thomas Rast\n> tr@thomasrast.ch\n"},{"id":"228998","messageId":"09553ADF-AE8C-460C-AC7B-1B4C26F6F6B2@gmail.com","threadId":"35104","inReplyTo":"CACsJy8Cg5M8dHSMW+giwYB2016jZhryyxET0kxkLDX7xk=B47w@mail.gmail.com","subject":"Re: [PATCH v3] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-15T09:46:11Z","receivedAt":"2013-10-15T09:46:11Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"Hello Duy\n\nThank you very much your suggestion.\nAs you suggested, I tried to reuse intermediate result of pprint_rename(), instead of\nparsing the output again.\nI just posted the new patch as \"PATCH v4\"\n\nThanks !\n\n---\nTsuneo Yoshioka (吉岡 恒夫)\nyoshiokatsuneo@gmail.com\n\n\n\n\nOn Oct 14, 2013, at 10:04 PM, Duy Nguyen wrote:\n\n> On Sun, Oct 13, 2013 at 3:48 AM, Yoshioka Tsuneo\n> wrote:\n>> \"git diff -M --stat\" can detect rename and show renamed file name like\n>> \"foofoofoo => barbarbar\", but if destination filename is long the line\n>> is shortened like \"...barbarbar\" so there is no way to know whether the\n>> file is renamed or existed in the source commit.\n>> This commit makes it visible like \"...foo => ...bar\".\n>> \n>> Signed-off-by: Tsuneo Yoshioka \n>> ---\n>> diff.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++-------\n>> 1 file changed, 51 insertions(+), 7 deletions(-)\n>> \n>> diff --git a/diff.c b/diff.c\n>> index a04a34d..3aeaf3e 100644\n>> --- a/diff.c\n>> +++ b/diff.c\n>> @@ -1643,13 +1643,57 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n>> len = name_width;\n>> name_len = strlen(name);\n>> if (name_width < name_len) {\n>> - char *slash;\n>> - prefix = \"...\";\n>> - len -= 3;\n>> - name += name_len - len;\n>> - slash = strchr(name, '/');\n>> - if (slash)\n>> - name = slash;\n>> + char *arrow = strstr(name, \" => \");\n>> + if (arrow) {\n> \n> This looks iffy. What if \" => \" is part of the path name?\n> file->is_renamed would be a more reliable sign. In that case I think\n> you just need an ellipsis version of pprint_rename() (i.e. drop the\n> result of previous pprint_rename() on the floor and create a new\n> string with \"...\" and \" => \" in your pprint_ellipsis_rename or\n> something)\n> \n>> + int prefix_len = (name_width - 4) / 2;\n>> + int f_omit;\n>> + int f_brace = 0;\n>> + char *pre_arrow = alloca(name_width + 10);\n>> + char *post_arrow = arrow + 4;\n>> + char *prefix_buf = alloca(name_width + 10);\n>> + char *pre_arrow_slash = NULL;\n>> +\n>> + if (arrow - name < prefix_len) {\n>> + prefix_len = (int)(arrow - name);\n>> + f_omit = 0;\n>> + } else {\n>> + prefix_len -= 3;\n>> + f_omit = 1;\n>> + if (name[0] == '{') {\n>> + prefix_len -= 1;\n>> + f_brace = 1;\n>> + }\n>> + }\n>> + prefix_len = ((prefix_len >= 0) ? prefix_len : 0);\n>> + strncpy(pre_arrow, arrow - prefix_len, prefix_len);\n>> + pre_arrow[prefix_len] = '\\0';\n>> + pre_arrow_slash = strchr(pre_arrow, '/');\n>> + if (f_omit && pre_arrow_slash)\n>> + pre_arrow = pre_arrow_slash;\n>> + sprintf(prefix_buf, \"%s%s%s => \", (f_brace ? \"{\" : \"\"), (f_omit ? \"...\" : \"\"), pre_arrow);\n>> + prefix = prefix_buf;\n>> +\n>> + if (strlen(post_arrow) > name_width - strlen(prefix)) {\n>> + char *post_arrow_slash = NULL;\n>> +\n>> + post_arrow += strlen(post_arrow) - (name_width - strlen(prefix) - 3);\n>> + strcat(prefix_buf, \"...\");\n>> + post_arrow_slash = strchr(post_arrow, '/');\n>> + if (post_arrow_slash)\n>> + post_arrow = post_arrow_slash;\n>> + name = post_arrow;\n>> + name_len = (int) (name_width - strlen(prefix));\n>> + }\n>> + len -= strlen(prefix);\n>> + } else {\n>> + char *slash = NULL;\n>> + prefix = \"...\";\n>> + len -= 3;\n>> + name += name_len - len;\n>> + slash = strchr(name, '/');\n>> + if (slash)\n>> + name = slash;\n>> + }\n>> }\n>> \n>> if (file->is_binary) {\n>> --\n>> 1.8.4.475.g867697c\n>> \n>> \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> \n> \n> -- \n> Duy\n"},{"id":"228999","messageId":"CAMP44s0mkcocVCekrgYnnwcS9op2KBu69JQ2kDrmT48a3HBp6A@mail.gmail.com","threadId":"35104","inReplyTo":"AFC93704-D6C5-49AF-9A66-C5EA81348FFA@gmail.com","subject":"Re: [PATCH v4] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-15T10:07:22Z","receivedAt":"2013-10-15T10:07:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Oct 15, 2013 at 4:45 AM, Yoshioka Tsuneo\n wrote:\n>\n> \"git diff -M --stat\" can detect rename and show renamed file name like\n> \"foofoofoo => barbarbar\". But if destination filename is long, the line\n> is shortened like \"...barbarbar\" so there is no way to know whether the\n> file is renamed or existed in the source commit.\n> Make sure there is always an arrow, like \"...foo => ...bar\".\n> The output can contains curly braces('{','}') for grouping.\n> So, in general, the outpu format is \"{ => }\"\n> To keep arrow(\"=>\"), try to omit as long as possible at first\n> because later part or changing part will be the more important part.\n> If it is not enough, shorten , , and trying to\n> have the maximum length the same because those will be equaly important.\n\nThis has similar style issues as v1.\n\n> -static char *pprint_rename(const char *a, const char *b)\n> +static void pprint_rename_find_common_prefix_suffix(const char *a, const char *b, struct strbuf *pfx, struct strbuf *a_mid, struct strbuf *b_mid, struct strbuf *sfx)\n> {\n> const char *old = a;\n> const char *new = b;\n> - struct strbuf name = STRBUF_INIT;\n> int pfx_length, sfx_length;\n> int pfx_adjust_for_slash;\n> int len_a = strlen(a);\n> @@ -1272,10 +1271,9 @@ static char *pprint_rename(const char *a, const char *b)\n> int qlen_b = quote_c_style(b, NULL, NULL, 0);\n>\n> if (qlen_a || qlen_b) {\n> - quote_c_style(a, &name, NULL, 0);\n> - strbuf_addstr(&name, \" => \");\n> - quote_c_style(b, &name, NULL, 0);\n> - return strbuf_detach(&name, NULL);\n> + quote_c_style(a, a_mid, NULL, 0);\n> + quote_c_style(b, b_mid, NULL, 0);\n> + return;\n> }\n>\n> /* Find common prefix */\n> @@ -1321,18 +1319,149 @@ static char *pprint_rename(const char *a, const char *b)\n> a_midlen = 0;\n> if (b_midlen < 0)\n> b_midlen = 0;\n> +\n> + strbuf_add(pfx, a, pfx_length);\n> + strbuf_add(a_mid, a + pfx_length, a_midlen);\n> + strbuf_add(b_mid, b + pfx_length, b_midlen);\n> + strbuf_add(sfx, a + len_a - sfx_length, sfx_length);\n> +}\n>\n> - strbuf_grow(&name, pfx_length + a_midlen + b_midlen + sfx_length + 7);\n> - if (pfx_length + sfx_length) {\n> - strbuf_add(&name, a, pfx_length);\n> +/*\n> + * Omit each parts to fix in name_width.\n> + * Formatted string is \"{ => }\".\n> + * At first, omit as long as possible.\n> + * If it is not enough, omit , , by tring to set the length of\n> + * those 3 parts(including \"...\") to the same.\n> + * Ex:\n> + * \"foofoofoo => barbarbar\"\n> + * will be like\n> + * \"...foo => ...bar\".\n> + * \"long_parent{foofoofoo => barbarbar}longfilename\"\n> + * will be like\n> + * \"...parent{...foofoo => ...barbar}...lename\"\n> + */\n> +static void pprint_rename_omit(struct strbuf *pfx, struct strbuf *a_mid, struct strbuf *b_mid, struct strbuf *sfx, int name_width)\n\nSeems like this line needs to be broken.\n\n> +{\n> +\n> +#define ARROW \" => \"\n> +#define ELLIPSIS \"...\"\n> +#define swap(a,b) myswap((a),(b),sizeof(a))\n\nI'm not entirely sure, but I think this should be:\n\n#define swap(a, b) myswap((a), (b), sizeof(a))\n\n> +\n> +#define myswap(a, b, size) do { \\\n> +unsigned char mytmp[size]; \\\n> +memcpy(mytmp, &a, size); \\\n> +memcpy(&a, &b, size); \\\n> +memcpy(&b, mytmp, size); \\\n> +} while (0)\n> +\n> + int use_curly_braces = (pfx->len > 0) || (sfx->len > 0);\n> + size_t name_len;\n> + size_t len;\n> + size_t part_lengths[4];\n> + size_t max_part_len = 0;\n> + size_t remainder_part_len = 0;\n> + int i, j;\n> +\n> + name_len = pfx->len + a_mid->len + b_mid->len + sfx->len + strlen(ARROW) + (use_curly_braces?2:0);\n> +\n> + if (name_len <= name_width){\n\nif () {\n\n> + /* Everthing fits in name_width */\n> + return;\n> + }\n> +\n> + if(use_curly_braces){\n\nDitto.\n\n> + if(strlen(ELLIPSIS) + (name_len - pfx->len) <= name_width){\n\nDitto.\n\n> + /*\n> + Just omitting left of '{' is enough\n> + Ex: ...aaa{foofoofoo => bar}file\n> + */\n> + strbuf_splice(pfx, name_len - pfx->len, name_width - (name_len - pfx->len), ELLIPSIS, strlen(ELLIPSIS));\n> + return;\n> + }else{\n\n} else {\n\n> + if (pfx->len > strlen(ELLIPSIS)) {\n> + /*\n> + Just omitting left of '{' is not enough\n> + name will be \"...{SOMETHING}SOMETHING\"\n> + */\n> + strbuf_reset(pfx);\n> + strbuf_addstr(pfx, ELLIPSIS);\n> + }\n> + }\n> + }\n> +\n> + /* available length for a_mid, b_mid and sfx */\n> + len = name_width - strlen(ARROW) - (use_curly_braces?2:0);\n\nuse_curly_braces ? 2 : 0\n\n> +\n> + /* a_mid, b_mid, sfx will be have the same max, including ellipsis(\"...\"). */\n> + part_lengths[0] = (int)a_mid->len;\n> + part_lengths[1] = (int)b_mid->len;\n> + part_lengths[2] = (int)sfx->len;\n> +\n> + /* bubble sort of part_lengths, descending order */\n> + for(i=0;i<3;i++){\n\nfor (i = 0; i < 3; i++) {\n\n> + for(j= i+1; j<3; j++){\n\nDitto.\n\n> + if(part_lengths[j] > part_lengths[i]){\n\nif ()\n foo;\n\n(it's a single line, no need for braces, In fact all the fors could\nget rid of them, but not really required.)\n\n> + swap(part_lengths[i], part_lengths[j]);\n> + }\n> + }\n> + }\n> +\n> + if (part_lengths[1] + part_lengths[1] + part_lengths[2] <= len) {\n> + /*\n> + * \"{...foofoo => barbar}file\"\n> + * There is only one omitting part.\n> + */\n> + max_part_len = len - part_lengths[1] - part_lengths[2];\n> + } else if (part_lengths[2] + part_lengths[2] + part_lengths[2] <= len){\n\n} else if () {\n\n> + /*\n> + * \"{...foofoo => ...barbar}file\"\n> + * There are 2 omitting part.\n> + */\n> + max_part_len = (len - part_lengths[2])/2;\n\n(len - part_lengths[2]) / 2\n\n> + remainder_part_len = (len - part_lengths[2]) - max_part_len * 2;\n> + } else {\n> + /*\n> + * \"{...ofoo => ...rbar}...file\"\n> + * There are 3 omitting part.\n> + */\n> + max_part_len = len / 3;\n> + remainder_part_len = len - (max_part_len) * 3;\n> + }\n> +\n> + if (max_part_len < strlen(ELLIPSIS))\n> + max_part_len = strlen(ELLIPSIS);\n> +\n> + if (sfx->len > max_part_len)\n> + strbuf_splice(sfx, 0, sfx->len - max_part_len + strlen(ELLIPSIS), ELLIPSIS, strlen(ELLIPSIS));\n> + if (remainder_part_len==2)\n\nremainder_part_len == 2\n\n> + max_part_len++;\n> + if (a_mid->len > max_part_len)\n> + strbuf_splice(a_mid, 0, a_mid->len - max_part_len + strlen(ELLIPSIS), ELLIPSIS, strlen(ELLIPSIS));\n> + if (remainder_part_len==1)\n\nDitto.\n\n> + max_part_len++;\n> + if (b_mid->len > max_part_len)\n> + strbuf_splice(b_mid, 0, b_mid->len - max_part_len + strlen(ELLIPSIS), ELLIPSIS, strlen(ELLIPSIS));\n> +}\n> +\n> +static char *pprint_rename(const char *a, const char *b, int name_width)\n> +{\n> + struct strbuf pfx = STRBUF_INIT, a_mid = STRBUF_INIT, b_mid = STRBUF_INIT, sfx = STRBUF_INIT;\n> + struct strbuf name = STRBUF_INIT;\n> +\n> + pprint_rename_find_common_prefix_suffix(a, b, &pfx, &a_mid, &b_mid, &sfx);\n> + pprint_rename_omit(&pfx, &a_mid, &b_mid, &sfx, name_width);\n> +\n> + strbuf_grow(&name, pfx.len + a_mid.len + b_mid.len + sfx.len + 7);\n\n> + if (pfx.len + sfx.len) {\n> + strbuf_addbuf(&name, &pfx);\n> strbuf_addch(&name, '{');\n> }\n> - strbuf_add(&name, a + pfx_length, a_midlen);\n> + strbuf_addbuf(&name, &a_mid);\n> strbuf_addstr(&name, \" => \");\n> - strbuf_add(&name, b + pfx_length, b_midlen);\n> - if (pfx_length + sfx_length) {\n> + strbuf_addbuf(&name, &b_mid);\n> + if (pfx.len + sfx.len) {\n> strbuf_addch(&name, '}');\n> - strbuf_add(&name, a + len_a - sfx_length, sfx_length);\n> + strbuf_addbuf(&name, &sfx);\n> }\n> return strbuf_detach(&name, NULL);\n> }\n> @@ -1418,23 +1547,31 @@ static void show_graph(FILE *file, char ch, int cnt, const char *set, const char\n> fprintf(file, \"%s\", reset);\n> }\n>\n> -static void fill_print_name(struct diffstat_file *file)\n> +static void fill_print_name(struct diffstat_file *file, int name_width)\n> {\n> char *pname;\n>\n> - if (file->print_name)\n> - return;\n> -\n> if (!file->is_renamed) {\n> struct strbuf buf = STRBUF_INIT;\n> + if (file->print_name)\n> + return;\n> if (quote_c_style(file->name, &buf, NULL, 0)) {\n> pname = strbuf_detach(&buf, NULL);\n> } else {\n> pname = file->name;\n> strbuf_release(&buf);\n> }\n> + if(strlen(pname) > name_width){\n\nif () {\n\n> + struct strbuf buf2 = STRBUF_INIT;\n> + strbuf_addstr(&buf2, \"...\");\n> + strbuf_addstr(&buf2, pname + strlen(pname) - name_width - 3);\n> + }\n> } else {\n> - pname = pprint_rename(file->from_name, file->name);\n> + if (file->print_name){\n\nDitto\n\n> + free(file->print_name);\n> + file->print_name = NULL;\n> + }\n> + pname = pprint_rename(file->from_name, file->name, name_width);\n> }\n> file->print_name = pname;\n> }\n> @@ -1517,7 +1654,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> count++; /* not shown == room for one more */\n> continue;\n> }\n> - fill_print_name(file);\n> + fill_print_name(file, INT_MAX);\n> len = strlen(file->print_name);\n> if (max_len < len)\n> max_len = len;\n> @@ -1629,7 +1766,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> for (i = 0; i < count; i++) {\n> const char *prefix = \"\";\n> struct diffstat_file *file = data->files[i];\n> - char *name = file->print_name;\n> + char *name;\n> uintmax_t added = file->added;\n> uintmax_t deleted = file->deleted;\n> int name_len;\n> @@ -1637,6 +1774,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> if (!file->is_interesting && (added + deleted == 0))\n> continue;\n>\n> + fill_print_name(file, name_width);\n> + name = file->print_name;\n> /*\n> * \"scale\" the filename\n> */\n> @@ -1772,7 +1911,7 @@ static void show_numstat(struct diffstat_t *data, struct diff_options *options)\n> \"%\"PRIuMAX\"\\t%\"PRIuMAX\"\\t\",\n> file->added, file->deleted);\n> if (options->line_termination) {\n> - fill_print_name(file);\n> + fill_print_name(file, INT_MAX);\n> if (!file->is_renamed)\n> write_name_quoted(file->name, options->file,\n> options->line_termination);\n> @@ -4258,7 +4397,7 @@ static void show_mode_change(FILE *file, struct diff_filepair *p, int show_name,\n> static void show_rename_copy(FILE *file, const char *renamecopy, struct diff_filepair *p,\n> const char *line_prefix)\n> {\n> - char *names = pprint_rename(p->one->path, p->two->path);\n> + char *names = pprint_rename(p->one->path, p->two->path, INT_MAX);\n>\n> fprintf(file, \" %s %s (%d%%)\\n\", renamecopy, names, similarity_index(p));\n> free(names);\n> diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh\n> index 2f327b7..03d6371 100755\n> --- a/t/t4001-diff-rename.sh\n> +++ b/t/t4001-diff-rename.sh\n> @@ -156,4 +156,16 @@ test_expect_success 'rename pretty print common prefix and suffix overlap' '\n> test_i18ngrep \" d/f/{ => f}/e \" output\n> '\n>\n> +test_expect_success 'rename of very long path shows =>' '\n> + mkdir long_dirname_that_does_not_fit_in_a_single_line &&\n> + mkdir another_extremely_long_path_but_not_the_same_as_the_first &&\n> + cp path1 long_dirname*/ &&\n> + git add long_dirname*/path1 &&\n> + test_commit add_long_pathname &&\n> + git mv long_dirname*/path1 another_extremely_*/ &&\n> + test_commit move_long_pathname &&\n> + git diff -M --stat HEAD^ HEAD >output &&\n> + test_i18ngrep \"=>.*path1\" output\n> +'\n> +\n> test_done\n\n-- \nFelipe Contreras\n"},{"id":"229000","messageId":"79A13931-694C-4DDC-BEDF-71A0DBA0ECA1@gmail.com","threadId":"35104","inReplyTo":"AFC93704-D6C5-49AF-9A66-C5EA81348FFA@gmail.com","subject":"[PATCH v5] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-15T10:24:06Z","receivedAt":"2013-10-15T10:24:06Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"\n\"git diff -M --stat\" can detect rename and show renamed file name like\n\"foofoofoo => barbarbar\". But if destination filename is long, the line\nis shortened like \"...barbarbar\" so there is no way to know whether the\nfile is renamed or existed in the source commit.\nMake sure there is always an arrow, like \"...foo => ...bar\".\nThe output can contains curly braces('{','}') for grouping.\nSo, in general, the outpu format is \"{ => }\"\nTo keep arrow(\"=>\"), try to omit as long as possible at first\nbecause later part or changing part will be the more important part.\nIf it is not enough, shorten , , and trying to\nhave the maximum length the same because those will be equaly important.\n\nSigned-off-by: Tsuneo Yoshioka \nTest-added-by: Thomas Rast \n---\n diff.c | 187 +++++++++++++++++++++++++++++++++++++++++++------\n t/t4001-diff-rename.sh | 12 ++++\n 2 files changed, 177 insertions(+), 22 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a04a34d..cf50807 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1258,11 +1258,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t}\n }\n \n-static char *pprint_rename(const char *a, const char *b)\n+static void pprint_rename_find_common_prefix_suffix(const char *a, const char *b\n+\t\t\t\t\t\t\t\t\t\t\t\t\t, struct strbuf *pfx, struct strbuf *a_mid\n+\t\t\t\t\t\t\t\t\t\t\t\t\t, struct strbuf *b_mid, struct strbuf *sfx)\n {\n \tconst char *old = a;\n \tconst char *new = b;\n-\tstruct strbuf name = STRBUF_INIT;\n \tint pfx_length, sfx_length;\n \tint pfx_adjust_for_slash;\n \tint len_a = strlen(a);\n@@ -1272,10 +1273,9 @@ static char *pprint_rename(const char *a, const char *b)\n \tint qlen_b = quote_c_style(b, NULL, NULL, 0);\n \n \tif (qlen_a || qlen_b) {\n-\t\tquote_c_style(a, &name, NULL, 0);\n-\t\tstrbuf_addstr(&name, \" => \");\n-\t\tquote_c_style(b, &name, NULL, 0);\n-\t\treturn strbuf_detach(&name, NULL);\n+\t\tquote_c_style(a, a_mid, NULL, 0);\n+\t\tquote_c_style(b, b_mid, NULL, 0);\n+\t\treturn;\n \t}\n \n \t/* Find common prefix */\n@@ -1321,18 +1321,151 @@ static char *pprint_rename(const char *a, const char *b)\n \t\ta_midlen = 0;\n \tif (b_midlen < 0)\n \t\tb_midlen = 0;\n+\t\n+\tstrbuf_add(pfx, a, pfx_length);\n+\tstrbuf_add(a_mid, a + pfx_length, a_midlen);\n+\tstrbuf_add(b_mid, b + pfx_length, b_midlen);\n+\tstrbuf_add(sfx, a + len_a - sfx_length, sfx_length);\n+}\n+\n+/*\n+ * Omit each parts to fix in name_width.\n+ * Formatted string is \"{ => }\".\n+ * At first, omit as long as possible.\n+ * If it is not enough, omit , , by tring to set the length of\n+ * those 3 parts(including \"...\") to the same.\n+ * Ex:\n+ * \"foofoofoo => barbarbar\"\n+ * will be like\n+ * \"...foo => ...bar\".\n+ * \"long_parent{foofoofoo => barbarbar}longfilename\"\n+ * will be like\n+ * \"...parent{...foofoo => ...barbar}...lename\"\n+ */\n+static void pprint_rename_omit(struct strbuf *pfx, struct strbuf *a_mid, struct strbuf *b_mid\n+\t\t\t\t\t\t\t , struct strbuf *sfx, int name_width)\n+{\n+\n+#define ARROW \" => \"\n+#define ELLIPSIS \"...\"\n+#define swap(a, b) myswap((a), (b), sizeof(a))\n+\t\n+#define myswap(a, b, size) do {\t\t\\\n+unsigned char mytmp[size];\t\\\n+memcpy(mytmp, &a, size);\t\t\\\n+memcpy(&a, &b, size);\t\t\\\n+memcpy(&b, mytmp, size);\t\t\\\n+} while (0)\n+\n+\tint use_curly_braces = (pfx->len > 0) || (sfx->len > 0);\n+\tsize_t name_len;\n+\tsize_t len;\n+\tsize_t part_lengths[4];\n+\tsize_t max_part_len = 0;\n+\tsize_t remainder_part_len = 0;\n+\tint i, j;\n+\n+\tname_len = pfx->len + a_mid->len + b_mid->len + sfx->len + strlen(ARROW)\n+\t\t+ (use_curly_braces ? 2 : 0);\n+\t\n+\tif (name_len <= name_width) {\n+\t\t/* Everthing fits in name_width */\n+\t\treturn;\n+\t}\n+\t\n+\tif (use_curly_braces) {\n+\t\tif (strlen(ELLIPSIS) + (name_len - pfx->len) <= name_width) {\n+\t\t\t/*\n+\t\t\t Just omitting left of '{' is enough\n+\t\t\t Ex: ...aaa{foofoofoo => bar}file\n+\t\t\t */\n+\t\t\tstrbuf_splice(pfx, name_len - pfx->len, name_width - (name_len - pfx->len), ELLIPSIS, strlen(ELLIPSIS));\n+\t\t\treturn;\n+\t\t} else {\n+\t\t\tif (pfx->len > strlen(ELLIPSIS)) {\n+\t\t\t\t/*\n+\t\t\t\t Just omitting left of '{' is not enough\n+\t\t\t\t name will be \"...{SOMETHING}SOMETHING\"\n+\t\t\t\t */\n+\t\t\t\tstrbuf_reset(pfx);\n+\t\t\t\tstrbuf_addstr(pfx, ELLIPSIS);\n+\t\t\t}\n+\t\t}\n+\t}\n \n-\tstrbuf_grow(&name, pfx_length + a_midlen + b_midlen + sfx_length + 7);\n-\tif (pfx_length + sfx_length) {\n-\t\tstrbuf_add(&name, a, pfx_length);\n+\t/* available length for a_mid, b_mid and sfx */\n+\tlen = name_width - strlen(ARROW) - (use_curly_braces ? 2 : 0);\n+\t\n+\t/* a_mid, b_mid, sfx will be have the same max, including ellipsis(\"...\"). */\n+\tpart_lengths[0] = (int)a_mid->len;\n+\tpart_lengths[1] = (int)b_mid->len;\n+\tpart_lengths[2] = (int)sfx->len;\n+\t\n+\t/* bubble sort of part_lengths, descending order */\n+\tfor (i=0; i<3; i++) {\n+\t\tfor (j=i+1; j<3; j++) {\n+\t\t\tif (part_lengths[j] > part_lengths[i]) {\n+\t\t\t\tswap(part_lengths[i], part_lengths[j]);\n+\t\t\t}\n+\t\t}\n+\t}\n+\t\n+\tif (part_lengths[1] + part_lengths[1] + part_lengths[2] <= len) {\n+\t\t/*\n+\t\t * \"{...foofoo => barbar}file\"\n+\t\t * There is only one omitting part.\n+\t\t */\n+\t\tmax_part_len = len - part_lengths[1] - part_lengths[2];\n+\t} else if (part_lengths[2] + part_lengths[2] + part_lengths[2] <= len) {\n+\t\t/*\n+\t\t * \"{...foofoo => ...barbar}file\"\n+\t\t * There are 2 omitting part.\n+\t\t */\n+\t\tmax_part_len = (len - part_lengths[2]) / 2;\n+\t\tremainder_part_len = (len - part_lengths[2]) - max_part_len * 2;\n+\t} else {\n+\t\t/*\n+\t\t * \"{...ofoo => ...rbar}...file\"\n+\t\t * There are 3 omitting part.\n+\t\t */\n+\t\tmax_part_len = len / 3;\n+\t\tremainder_part_len = len - (max_part_len) * 3;\n+\t}\n+\t\n+\tif (max_part_len < strlen(ELLIPSIS))\n+\t\tmax_part_len = strlen(ELLIPSIS);\n+\t\n+\tif (sfx->len > max_part_len)\n+\t\tstrbuf_splice(sfx, 0, sfx->len - max_part_len + strlen(ELLIPSIS), ELLIPSIS, strlen(ELLIPSIS));\n+\tif (remainder_part_len == 2)\n+\t\tmax_part_len++;\n+\tif (a_mid->len > max_part_len)\n+\t\tstrbuf_splice(a_mid, 0, a_mid->len - max_part_len + strlen(ELLIPSIS), ELLIPSIS, strlen(ELLIPSIS));\n+\tif (remainder_part_len == 1)\n+\t\tmax_part_len++;\n+\tif (b_mid->len > max_part_len)\n+\t\tstrbuf_splice(b_mid, 0, b_mid->len - max_part_len + strlen(ELLIPSIS), ELLIPSIS, strlen(ELLIPSIS));\n+}\n+\n+static char *pprint_rename(const char *a, const char *b, int name_width)\n+{\n+\tstruct strbuf pfx = STRBUF_INIT, a_mid = STRBUF_INIT, b_mid = STRBUF_INIT, sfx = STRBUF_INIT;\n+\tstruct strbuf name = STRBUF_INIT;\n+\t\n+\tpprint_rename_find_common_prefix_suffix(a, b, &pfx, &a_mid, &b_mid, &sfx);\n+\tpprint_rename_omit(&pfx, &a_mid, &b_mid, &sfx, name_width);\n+\t\n+\tstrbuf_grow(&name, pfx.len + a_mid.len + b_mid.len + sfx.len + 7);\n+\tif (pfx.len + sfx.len) {\n+\t\tstrbuf_addbuf(&name, &pfx);\n \t\tstrbuf_addch(&name, '{');\n \t}\n-\tstrbuf_add(&name, a + pfx_length, a_midlen);\n+\tstrbuf_addbuf(&name, &a_mid);\n \tstrbuf_addstr(&name, \" => \");\n-\tstrbuf_add(&name, b + pfx_length, b_midlen);\n-\tif (pfx_length + sfx_length) {\n+\tstrbuf_addbuf(&name, &b_mid);\n+\tif (pfx.len + sfx.len) {\n \t\tstrbuf_addch(&name, '}');\n-\t\tstrbuf_add(&name, a + len_a - sfx_length, sfx_length);\n+\t\tstrbuf_addbuf(&name, &sfx);\n \t}\n \treturn strbuf_detach(&name, NULL);\n }\n@@ -1418,23 +1551,31 @@ static void show_graph(FILE *file, char ch, int cnt, const char *set, const char\n \tfprintf(file, \"%s\", reset);\n }\n \n-static void fill_print_name(struct diffstat_file *file)\n+static void fill_print_name(struct diffstat_file *file, int name_width)\n {\n \tchar *pname;\n \n-\tif (file->print_name)\n-\t\treturn;\n-\n \tif (!file->is_renamed) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tif (file->print_name)\n+\t\t\treturn;\n \t\tif (quote_c_style(file->name, &buf, NULL, 0)) {\n \t\t\tpname = strbuf_detach(&buf, NULL);\n \t\t} else {\n \t\t\tpname = file->name;\n \t\t\tstrbuf_release(&buf);\n \t\t}\n+\t\tif (strlen(pname) > name_width) {\n+\t\t\tstruct strbuf buf2 = STRBUF_INIT;\n+\t\t\tstrbuf_addstr(&buf2, \"...\");\n+\t\t\tstrbuf_addstr(&buf2, pname + strlen(pname) - name_width - 3);\n+\t\t}\n \t} else {\n-\t\tpname = pprint_rename(file->from_name, file->name);\n+\t\tif (file->print_name) {\n+\t\t\tfree(file->print_name);\n+\t\t\tfile->print_name = NULL;\n+\t\t}\n+\t\tpname = pprint_rename(file->from_name, file->name, name_width);\n \t}\n \tfile->print_name = pname;\n }\n@@ -1517,7 +1658,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t\tcount++; /* not shown == room for one more */\n \t\t\tcontinue;\n \t\t}\n-\t\tfill_print_name(file);\n+\t\tfill_print_name(file, INT_MAX);\n \t\tlen = strlen(file->print_name);\n \t\tif (max_len < len)\n \t\t\tmax_len = len;\n@@ -1629,7 +1770,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tfor (i = 0; i < count; i++) {\n \t\tconst char *prefix = \"\";\n \t\tstruct diffstat_file *file = data->files[i];\n-\t\tchar *name = file->print_name;\n+\t\tchar *name;\n \t\tuintmax_t added = file->added;\n \t\tuintmax_t deleted = file->deleted;\n \t\tint name_len;\n@@ -1637,6 +1778,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tif (!file->is_interesting && (added + deleted == 0))\n \t\t\tcontinue;\n \n+\t\tfill_print_name(file, name_width);\n+\t\tname = file->print_name;\n \t\t/*\n \t\t * \"scale\" the filename\n \t\t */\n@@ -1772,7 +1915,7 @@ static void show_numstat(struct diffstat_t *data, struct diff_options *options)\n \t\t\t\t\"%\"PRIuMAX\"\\t%\"PRIuMAX\"\\t\",\n \t\t\t\tfile->added, file->deleted);\n \t\tif (options->line_termination) {\n-\t\t\tfill_print_name(file);\n+\t\t\tfill_print_name(file, INT_MAX);\n \t\t\tif (!file->is_renamed)\n \t\t\t\twrite_name_quoted(file->name, options->file,\n \t\t\t\t\t\t options->line_termination);\n@@ -4258,7 +4401,7 @@ static void show_mode_change(FILE *file, struct diff_filepair *p, int show_name,\n static void show_rename_copy(FILE *file, const char *renamecopy, struct diff_filepair *p,\n \t\t\tconst char *line_prefix)\n {\n-\tchar *names = pprint_rename(p->one->path, p->two->path);\n+\tchar *names = pprint_rename(p->one->path, p->two->path, INT_MAX);\n \n \tfprintf(file, \" %s %s (%d%%)\\n\", renamecopy, names, similarity_index(p));\n \tfree(names);\ndiff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh\nindex 2f327b7..03d6371 100755\n--- a/t/t4001-diff-rename.sh\n+++ b/t/t4001-diff-rename.sh\n@@ -156,4 +156,16 @@ test_expect_success 'rename pretty print common prefix and suffix overlap' '\n \ttest_i18ngrep \" d/f/{ => f}/e \" output\n '\n \n+test_expect_success 'rename of very long path shows =>' '\n+\tmkdir long_dirname_that_does_not_fit_in_a_single_line &&\n+\tmkdir another_extremely_long_path_but_not_the_same_as_the_first &&\n+\tcp path1 long_dirname*/ &&\n+\tgit add long_dirname*/path1 &&\n+\ttest_commit add_long_pathname &&\n+\tgit mv long_dirname*/path1 another_extremely_*/ &&\n+\ttest_commit move_long_pathname &&\n+\tgit diff -M --stat HEAD^ HEAD >output &&\n+\ttest_i18ngrep \"=>.*path1\" output\n+'\n+\n test_done\n-- \n1.8.4.475.g867697c\n"},{"id":"229001","messageId":"FC0E6C0F-0FF4-4D10-AC2B-B4DD9AC017A2@gmail.com","threadId":"35104","inReplyTo":"CAMP44s0mkcocVCekrgYnnwcS9op2KBu69JQ2kDrmT48a3HBp6A@mail.gmail.com","subject":"Re: [PATCH v4] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-15T10:24:49Z","receivedAt":"2013-10-15T10:24:49Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"Hello Felipe\n\nThank you for pointing out the style issue again.\nI just fixed it and posted as [PATCH v5].\n\nThanks!\n\n---\nTsuneo Yoshioka (吉岡 恒夫)\nyoshiokatsuneo@gmail.com\n\n\n\n\nOn Oct 15, 2013, at 1:07 PM, Felipe Contreras wrote:\n\n> On Tue, Oct 15, 2013 at 4:45 AM, Yoshioka Tsuneo\n> wrote:\n>> \n>> \"git diff -M --stat\" can detect rename and show renamed file name like\n>> \"foofoofoo => barbarbar\". But if destination filename is long, the line\n>> is shortened like \"...barbarbar\" so there is no way to know whether the\n>> file is renamed or existed in the source commit.\n>> Make sure there is always an arrow, like \"...foo => ...bar\".\n>> The output can contains curly braces('{','}') for grouping.\n>> So, in general, the outpu format is \"{ => }\"\n>> To keep arrow(\"=>\"), try to omit as long as possible at first\n>> because later part or changing part will be the more important part.\n>> If it is not enough, shorten , , and trying to\n>> have the maximum length the same because those will be equaly important.\n> \n> This has similar style issues as v1.\n> \n>> -static char *pprint_rename(const char *a, const char *b)\n>> +static void pprint_rename_find_common_prefix_suffix(const char *a, const char *b, struct strbuf *pfx, struct strbuf *a_mid, struct strbuf *b_mid, struct strbuf *sfx)\n>> {\n>> const char *old = a;\n>> const char *new = b;\n>> - struct strbuf name = STRBUF_INIT;\n>> int pfx_length, sfx_length;\n>> int pfx_adjust_for_slash;\n>> int len_a = strlen(a);\n>> @@ -1272,10 +1271,9 @@ static char *pprint_rename(const char *a, const char *b)\n>> int qlen_b = quote_c_style(b, NULL, NULL, 0);\n>> \n>> if (qlen_a || qlen_b) {\n>> - quote_c_style(a, &name, NULL, 0);\n>> - strbuf_addstr(&name, \" => \");\n>> - quote_c_style(b, &name, NULL, 0);\n>> - return strbuf_detach(&name, NULL);\n>> + quote_c_style(a, a_mid, NULL, 0);\n>> + quote_c_style(b, b_mid, NULL, 0);\n>> + return;\n>> }\n>> \n>> /* Find common prefix */\n>> @@ -1321,18 +1319,149 @@ static char *pprint_rename(const char *a, const char *b)\n>> a_midlen = 0;\n>> if (b_midlen < 0)\n>> b_midlen = 0;\n>> +\n>> + strbuf_add(pfx, a, pfx_length);\n>> + strbuf_add(a_mid, a + pfx_length, a_midlen);\n>> + strbuf_add(b_mid, b + pfx_length, b_midlen);\n>> + strbuf_add(sfx, a + len_a - sfx_length, sfx_length);\n>> +}\n>> \n>> - strbuf_grow(&name, pfx_length + a_midlen + b_midlen + sfx_length + 7);\n>> - if (pfx_length + sfx_length) {\n>> - strbuf_add(&name, a, pfx_length);\n>> +/*\n>> + * Omit each parts to fix in name_width.\n>> + * Formatted string is \"{ => }\".\n>> + * At first, omit as long as possible.\n>> + * If it is not enough, omit , , by tring to set the length of\n>> + * those 3 parts(including \"...\") to the same.\n>> + * Ex:\n>> + * \"foofoofoo => barbarbar\"\n>> + * will be like\n>> + * \"...foo => ...bar\".\n>> + * \"long_parent{foofoofoo => barbarbar}longfilename\"\n>> + * will be like\n>> + * \"...parent{...foofoo => ...barbar}...lename\"\n>> + */\n>> +static void pprint_rename_omit(struct strbuf *pfx, struct strbuf *a_mid, struct strbuf *b_mid, struct strbuf *sfx, int name_width)\n> \n> Seems like this line needs to be broken.\n> \n>> +{\n>> +\n>> +#define ARROW \" => \"\n>> +#define ELLIPSIS \"...\"\n>> +#define swap(a,b) myswap((a),(b),sizeof(a))\n> \n> I'm not entirely sure, but I think this should be:\n> \n> #define swap(a, b) myswap((a), (b), sizeof(a))\n> \n>> +\n>> +#define myswap(a, b, size) do { \\\n>> +unsigned char mytmp[size]; \\\n>> +memcpy(mytmp, &a, size); \\\n>> +memcpy(&a, &b, size); \\\n>> +memcpy(&b, mytmp, size); \\\n>> +} while (0)\n>> +\n>> + int use_curly_braces = (pfx->len > 0) || (sfx->len > 0);\n>> + size_t name_len;\n>> + size_t len;\n>> + size_t part_lengths[4];\n>> + size_t max_part_len = 0;\n>> + size_t remainder_part_len = 0;\n>> + int i, j;\n>> +\n>> + name_len = pfx->len + a_mid->len + b_mid->len + sfx->len + strlen(ARROW) + (use_curly_braces?2:0);\n>> +\n>> + if (name_len <= name_width){\n> \n> if () {\n> \n>> + /* Everthing fits in name_width */\n>> + return;\n>> + }\n>> +\n>> + if(use_curly_braces){\n> \n> Ditto.\n> \n>> + if(strlen(ELLIPSIS) + (name_len - pfx->len) <= name_width){\n> \n> Ditto.\n> \n>> + /*\n>> + Just omitting left of '{' is enough\n>> + Ex: ...aaa{foofoofoo => bar}file\n>> + */\n>> + strbuf_splice(pfx, name_len - pfx->len, name_width - (name_len - pfx->len), ELLIPSIS, strlen(ELLIPSIS));\n>> + return;\n>> + }else{\n> \n> } else {\n> \n>> + if (pfx->len > strlen(ELLIPSIS)) {\n>> + /*\n>> + Just omitting left of '{' is not enough\n>> + name will be \"...{SOMETHING}SOMETHING\"\n>> + */\n>> + strbuf_reset(pfx);\n>> + strbuf_addstr(pfx, ELLIPSIS);\n>> + }\n>> + }\n>> + }\n>> +\n>> + /* available length for a_mid, b_mid and sfx */\n>> + len = name_width - strlen(ARROW) - (use_curly_braces?2:0);\n> \n> use_curly_braces ? 2 : 0\n> \n>> +\n>> + /* a_mid, b_mid, sfx will be have the same max, including ellipsis(\"...\"). */\n>> + part_lengths[0] = (int)a_mid->len;\n>> + part_lengths[1] = (int)b_mid->len;\n>> + part_lengths[2] = (int)sfx->len;\n>> +\n>> + /* bubble sort of part_lengths, descending order */\n>> + for(i=0;i<3;i++){\n> \n> for (i = 0; i < 3; i++) {\n> \n>> + for(j= i+1; j<3; j++){\n> \n> Ditto.\n> \n>> + if(part_lengths[j] > part_lengths[i]){\n> \n> if ()\n> foo;\n> \n> (it's a single line, no need for braces, In fact all the fors could\n> get rid of them, but not really required.)\n> \n>> + swap(part_lengths[i], part_lengths[j]);\n>> + }\n>> + }\n>> + }\n>> +\n>> + if (part_lengths[1] + part_lengths[1] + part_lengths[2] <= len) {\n>> + /*\n>> + * \"{...foofoo => barbar}file\"\n>> + * There is only one omitting part.\n>> + */\n>> + max_part_len = len - part_lengths[1] - part_lengths[2];\n>> + } else if (part_lengths[2] + part_lengths[2] + part_lengths[2] <= len){\n> \n> } else if () {\n> \n>> + /*\n>> + * \"{...foofoo => ...barbar}file\"\n>> + * There are 2 omitting part.\n>> + */\n>> + max_part_len = (len - part_lengths[2])/2;\n> \n> (len - part_lengths[2]) / 2\n> \n>> + remainder_part_len = (len - part_lengths[2]) - max_part_len * 2;\n>> + } else {\n>> + /*\n>> + * \"{...ofoo => ...rbar}...file\"\n>> + * There are 3 omitting part.\n>> + */\n>> + max_part_len = len / 3;\n>> + remainder_part_len = len - (max_part_len) * 3;\n>> + }\n>> +\n>> + if (max_part_len < strlen(ELLIPSIS))\n>> + max_part_len = strlen(ELLIPSIS);\n>> +\n>> + if (sfx->len > max_part_len)\n>> + strbuf_splice(sfx, 0, sfx->len - max_part_len + strlen(ELLIPSIS), ELLIPSIS, strlen(ELLIPSIS));\n>> + if (remainder_part_len==2)\n> \n> remainder_part_len == 2\n> \n>> + max_part_len++;\n>> + if (a_mid->len > max_part_len)\n>> + strbuf_splice(a_mid, 0, a_mid->len - max_part_len + strlen(ELLIPSIS), ELLIPSIS, strlen(ELLIPSIS));\n>> + if (remainder_part_len==1)\n> \n> Ditto.\n> \n>> + max_part_len++;\n>> + if (b_mid->len > max_part_len)\n>> + strbuf_splice(b_mid, 0, b_mid->len - max_part_len + strlen(ELLIPSIS), ELLIPSIS, strlen(ELLIPSIS));\n>> +}\n>> +\n>> +static char *pprint_rename(const char *a, const char *b, int name_width)\n>> +{\n>> + struct strbuf pfx = STRBUF_INIT, a_mid = STRBUF_INIT, b_mid = STRBUF_INIT, sfx = STRBUF_INIT;\n>> + struct strbuf name = STRBUF_INIT;\n>> +\n>> + pprint_rename_find_common_prefix_suffix(a, b, &pfx, &a_mid, &b_mid, &sfx);\n>> + pprint_rename_omit(&pfx, &a_mid, &b_mid, &sfx, name_width);\n>> +\n>> + strbuf_grow(&name, pfx.len + a_mid.len + b_mid.len + sfx.len + 7);\n> \n>> + if (pfx.len + sfx.len) {\n>> + strbuf_addbuf(&name, &pfx);\n>> strbuf_addch(&name, '{');\n>> }\n>> - strbuf_add(&name, a + pfx_length, a_midlen);\n>> + strbuf_addbuf(&name, &a_mid);\n>> strbuf_addstr(&name, \" => \");\n>> - strbuf_add(&name, b + pfx_length, b_midlen);\n>> - if (pfx_length + sfx_length) {\n>> + strbuf_addbuf(&name, &b_mid);\n>> + if (pfx.len + sfx.len) {\n>> strbuf_addch(&name, '}');\n>> - strbuf_add(&name, a + len_a - sfx_length, sfx_length);\n>> + strbuf_addbuf(&name, &sfx);\n>> }\n>> return strbuf_detach(&name, NULL);\n>> }\n>> @@ -1418,23 +1547,31 @@ static void show_graph(FILE *file, char ch, int cnt, const char *set, const char\n>> fprintf(file, \"%s\", reset);\n>> }\n>> \n>> -static void fill_print_name(struct diffstat_file *file)\n>> +static void fill_print_name(struct diffstat_file *file, int name_width)\n>> {\n>> char *pname;\n>> \n>> - if (file->print_name)\n>> - return;\n>> -\n>> if (!file->is_renamed) {\n>> struct strbuf buf = STRBUF_INIT;\n>> + if (file->print_name)\n>> + return;\n>> if (quote_c_style(file->name, &buf, NULL, 0)) {\n>> pname = strbuf_detach(&buf, NULL);\n>> } else {\n>> pname = file->name;\n>> strbuf_release(&buf);\n>> }\n>> + if(strlen(pname) > name_width){\n> \n> if () {\n> \n>> + struct strbuf buf2 = STRBUF_INIT;\n>> + strbuf_addstr(&buf2, \"...\");\n>> + strbuf_addstr(&buf2, pname + strlen(pname) - name_width - 3);\n>> + }\n>> } else {\n>> - pname = pprint_rename(file->from_name, file->name);\n>> + if (file->print_name){\n> \n> Ditto\n> \n>> + free(file->print_name);\n>> + file->print_name = NULL;\n>> + }\n>> + pname = pprint_rename(file->from_name, file->name, name_width);\n>> }\n>> file->print_name = pname;\n>> }\n>> @@ -1517,7 +1654,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n>> count++; /* not shown == room for one more */\n>> continue;\n>> }\n>> - fill_print_name(file);\n>> + fill_print_name(file, INT_MAX);\n>> len = strlen(file->print_name);\n>> if (max_len < len)\n>> max_len = len;\n>> @@ -1629,7 +1766,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n>> for (i = 0; i < count; i++) {\n>> const char *prefix = \"\";\n>> struct diffstat_file *file = data->files[i];\n>> - char *name = file->print_name;\n>> + char *name;\n>> uintmax_t added = file->added;\n>> uintmax_t deleted = file->deleted;\n>> int name_len;\n>> @@ -1637,6 +1774,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n>> if (!file->is_interesting && (added + deleted == 0))\n>> continue;\n>> \n>> + fill_print_name(file, name_width);\n>> + name = file->print_name;\n>> /*\n>> * \"scale\" the filename\n>> */\n>> @@ -1772,7 +1911,7 @@ static void show_numstat(struct diffstat_t *data, struct diff_options *options)\n>> \"%\"PRIuMAX\"\\t%\"PRIuMAX\"\\t\",\n>> file->added, file->deleted);\n>> if (options->line_termination) {\n>> - fill_print_name(file);\n>> + fill_print_name(file, INT_MAX);\n>> if (!file->is_renamed)\n>> write_name_quoted(file->name, options->file,\n>> options->line_termination);\n>> @@ -4258,7 +4397,7 @@ static void show_mode_change(FILE *file, struct diff_filepair *p, int show_name,\n>> static void show_rename_copy(FILE *file, const char *renamecopy, struct diff_filepair *p,\n>> const char *line_prefix)\n>> {\n>> - char *names = pprint_rename(p->one->path, p->two->path);\n>> + char *names = pprint_rename(p->one->path, p->two->path, INT_MAX);\n>> \n>> fprintf(file, \" %s %s (%d%%)\\n\", renamecopy, names, similarity_index(p));\n>> free(names);\n>> diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh\n>> index 2f327b7..03d6371 100755\n>> --- a/t/t4001-diff-rename.sh\n>> +++ b/t/t4001-diff-rename.sh\n>> @@ -156,4 +156,16 @@ test_expect_success 'rename pretty print common prefix and suffix overlap' '\n>> test_i18ngrep \" d/f/{ => f}/e \" output\n>> '\n>> \n>> +test_expect_success 'rename of very long path shows =>' '\n>> + mkdir long_dirname_that_does_not_fit_in_a_single_line &&\n>> + mkdir another_extremely_long_path_but_not_the_same_as_the_first &&\n>> + cp path1 long_dirname*/ &&\n>> + git add long_dirname*/path1 &&\n>> + test_commit add_long_pathname &&\n>> + git mv long_dirname*/path1 another_extremely_*/ &&\n>> + test_commit move_long_pathname &&\n>> + git diff -M --stat HEAD^ HEAD >output &&\n>> + test_i18ngrep \"=>.*path1\" output\n>> +'\n>> +\n>> test_done\n> \n> -- \n> Felipe Contreras\n"},{"id":"229037","messageId":"xmqqbo2qb0wk.fsf@gitster.dls.corp.google.com","threadId":"35104","inReplyTo":"79A13931-694C-4DDC-BEDF-71A0DBA0ECA1@gmail.com","subject":"Re: [PATCH v5] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-15T22:54:35Z","receivedAt":"2013-10-15T22:54:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yoshioka Tsuneo writes:\n\n> \"git diff -M --stat\" can detect rename and show renamed file name like\n> \"foofoofoo => barbarbar\". But if destination filename is long, the line\n> is shortened like \"...barbarbar\" so there is no way to know whether the\n> file is renamed or existed in the source commit.\n\nIs \"destination\" filename more special than the source filename?\nPerhaps \"s/if destination filename is/if filenames are/\"?\n\n\tNote: I do not want you to reroll using the suggested\n\twording without explanation; it may be possible that I am\n\tmissing something obvious and do not understand why you\n\tsingled out destination, in which case I'd rather see it\n\texplained better in the log message than the potentially\n\tsuboptimal suggestion I made in the review without\n\tunderstanding the issue. Of course, it is possible that you\n\twant to do the same when source is overlong, in which case\n\tyou can just say \"Yeah, you're right; will reroll\".\n\n The above applies to all the other comments in this message.\n\nAlso \"s/source commit/original/\". You may not be comparing two\ncommits after all.\n\n> Make sure there is always an arrow, like \"...foo => ...bar\".\n> The output can contains curly braces('{','}') for grouping.\n\ns/contains/contain/;\n\n> So, in general, the outpu format is \"{ => }\"\n\ns/outpu/&t/;\n\n> To keep arrow(\"=>\"), try to omit as long as possible at first\n> because later part or changing part will be the more important part.\n> If it is not enough, shorten , , and trying to\n> have the maximum length the same because those will be equaly important.\n\nA sound reasoning.\n\n> Signed-off-by: Tsuneo Yoshioka \n> Test-added-by: Thomas Rast \n> ---\n> diff.c | 187 +++++++++++++++++++++++++++++++++++++++++++------\n> t/t4001-diff-rename.sh | 12 ++++\n> 2 files changed, 177 insertions(+), 22 deletions(-)\n>\n> diff --git a/diff.c b/diff.c\n> index a04a34d..cf50807 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1258,11 +1258,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n> \t}\n> }\n> \n> -static char *pprint_rename(const char *a, const char *b)\n> +static void pprint_rename_find_common_prefix_suffix(const char *a, const char *b\n> +\t\t\t\t\t\t\t\t\t\t\t\t\t, struct strbuf *pfx, struct strbuf *a_mid\n> +\t\t\t\t\t\t\t\t\t\t\t\t\t, struct strbuf *b_mid, struct strbuf *sfx)\n\nWhat kind of line splitting is this?\n\nI think the real issue is that the function name is overly long, but\naside from that,\n\n - comma comes at the end of the line, not at the beginning of the\n next line;\n\n - the second and subsequent lines are indented, but not more than\n the usual line width (align with the first letter inside the\n opening parenthesis of the first line);\n\n - a_mid and b_mid are more \"alike\" than pfx and a_mid.\n\nso I would expect to see it more like:\n\nstatic void abbrev_rename(const char *a, const char *b,\n\t\t\t struct strbuf *pfx,\n\t\t\t struct strbuf *a_mid, struct strbuf *b_mid,\n\t\t\t struct strbuf *sfx)\n\nNote that the suggested name does not say \"pprint\", because in your\nversion of this file, the code around here is no longer doing any\nprinting. The caller does so after using this function to decide\nhow to abbreviate renames, so naming the helper function after what\nit does (e.g. abbreviate renames) is more appropriate.\n\n> {\n> \tconst char *old = a;\n> \tconst char *new = b;\n> -\tstruct strbuf name = STRBUF_INIT;\n> \tint pfx_length, sfx_length;\n> \tint pfx_adjust_for_slash;\n> \tint len_a = strlen(a);\n> @@ -1272,10 +1273,9 @@ static char *pprint_rename(const char *a, const char *b)\n> \tint qlen_b = quote_c_style(b, NULL, NULL, 0);\n> \n> \tif (qlen_a || qlen_b) {\n> -\t\tquote_c_style(a, &name, NULL, 0);\n> -\t\tstrbuf_addstr(&name, \" => \");\n> -\t\tquote_c_style(b, &name, NULL, 0);\n> -\t\treturn strbuf_detach(&name, NULL);\n> +\t\tquote_c_style(a, a_mid, NULL, 0);\n> +\t\tquote_c_style(b, b_mid, NULL, 0);\n> +\t\treturn;\n> \t}\n> \n> \t/* Find common prefix */\n> @@ -1321,18 +1321,151 @@ static char *pprint_rename(const char *a, const char *b)\n> \t\ta_midlen = 0;\n> \tif (b_midlen < 0)\n> \t\tb_midlen = 0;\n> +\t\n\nTrailing whitespace (there are many others you added to this file; I\nwon't bother to point out all of them).\n\n> +\tstrbuf_add(pfx, a, pfx_length);\n> +\tstrbuf_add(a_mid, a + pfx_length, a_midlen);\n> +\tstrbuf_add(b_mid, b + pfx_length, b_midlen);\n> +\tstrbuf_add(sfx, a + len_a - sfx_length, sfx_length);\n> +}\n> +\n> +/*\n> + * Omit each parts to fix in name_width.\n> + * Formatted string is \"{ => }\".\n> + * At first, omit as long as possible.\n> + * If it is not enough, omit , , by tring to set the length of\n> + * those 3 parts(including \"...\") to the same.\n> + * Ex:\n> + * \"foofoofoo => barbarbar\"\n> + * will be like\n> + * \"...foo => ...bar\".\n> + * \"long_parent{foofoofoo => barbarbar}longfilename\"\n> + * will be like\n> + * \"...parent{...foofoo => ...barbar}...lename\"\n> + */\n> +static void pprint_rename_omit(struct strbuf *pfx, struct strbuf *a_mid, struct strbuf *b_mid\n> +\t\t\t\t\t\t\t , struct strbuf *sfx, int name_width)\n\nBad line splitting.\n\n> +{\n> +\n> +#define ARROW \" => \"\n> +#define ELLIPSIS \"...\"\n\nUgly and leaks these symbols after the function is done using them\nto the remainder of this file. Write them like this instead, perhaps?\n\n\tstatic const char arrow[] = \" => \";\n static const char dots[] = \"...\";\n\n> +#define swap(a, b) myswap((a), (b), sizeof(a))\n> +\t\n> +#define myswap(a, b, size) do {\t\t\\\n> +unsigned char mytmp[size];\t\\\n> +memcpy(mytmp, &a, size);\t\t\\\n> +memcpy(&a, &b, size);\t\t\\\n> +memcpy(&b, mytmp, size);\t\t\\\n> +} while (0)\n\nThese are totally unneeded, I suspect (see below).\n\n> +\n> +\tint use_curly_braces = (pfx->len > 0) || (sfx->len > 0);\n> +\tsize_t name_len;\n> +\tsize_t len;\n> +\tsize_t part_lengths[4];\n\nDo not name an array in plural, i.e. elements[], unless there is a\ncompelling reason to do so. By using singular, e.g. element[], the\nthird element can be spelled as element[3], which is more logical\nthan having to call it elements[3].\n\n\tSide note. a notable exception is an array that is used as a\n\thash-table and frequently passed around as an argument; you\n\tare usually not interested in iterating over it in ascending\n\torder, and being able to call such a collection of things\n\t\"things\" in plural, e.g. struct object objects[], is more\n\timportant.\n\n> +\tsize_t max_part_len = 0;\n> +\tsize_t remainder_part_len = 0;\n> +\tint i, j;\n> +\n> +\tname_len = pfx->len + a_mid->len + b_mid->len + sfx->len + strlen(ARROW)\n> +\t\t+ (use_curly_braces ? 2 : 0);\n> +\t\n> +\tif (name_len <= name_width) {\n> +\t\t/* Everthing fits in name_width */\n> +\t\treturn;\n> +\t}\n> +\t\n> +\tif (use_curly_braces) {\n> +\t\tif (strlen(ELLIPSIS) + (name_len - pfx->len) <= name_width) {\n> +\t\t\t/*\n> +\t\t\t Just omitting left of '{' is enough\n> +\t\t\t Ex: ...aaa{foofoofoo => bar}file\n> +\t\t\t */\n\n\t/*\n * We format our multi-line\n * comments like\n * this.\n */\n\n> +\t\t\tstrbuf_splice(pfx, name_len - pfx->len, name_width - (name_len - pfx->len), ELLIPSIS, strlen(ELLIPSIS));\n\nOverlong line.\n\nIs the math for the second and third arguments correct? If you are\nmaking \"abcdefghij\" into \"...hij\", you would splice at position 0\nfor length up to 'g', so it felt strange to see any arithmetic as\nthe second argument, but I didn't look at this code very closely.\n\n> +\t\t\treturn;\n> +\t\t} else {\n> +\t\t\tif (pfx->len > strlen(ELLIPSIS)) {\n> +\t\t\t\t/*\n> +\t\t\t\t Just omitting left of '{' is not enough\n> +\t\t\t\t name will be \"...{SOMETHING}SOMETHING\"\n> +\t\t\t\t */\n> +\t\t\t\tstrbuf_reset(pfx);\n> +\t\t\t\tstrbuf_addstr(pfx, ELLIPSIS);\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> \n> -\tstrbuf_grow(&name, pfx_length + a_midlen + b_midlen + sfx_length + 7);\n> -\tif (pfx_length + sfx_length) {\n> -\t\tstrbuf_add(&name, a, pfx_length);\n> +\t/* available length for a_mid, b_mid and sfx */\n> +\tlen = name_width - strlen(ARROW) - (use_curly_braces ? 2 : 0);\n> +\t\n> +\t/* a_mid, b_mid, sfx will be have the same max, including ellipsis(\"...\"). */\n> +\tpart_lengths[0] = (int)a_mid->len;\n> +\tpart_lengths[1] = (int)b_mid->len;\n> +\tpart_lengths[2] = (int)sfx->len;\n\nWhat are these casts about? strbuf.len is of size_t which is\nalready the correct type for part_length[].\n\n> +\t\n> +\t/* bubble sort of part_lengths, descending order */\n\nDo not bubble sort. Unless there is a compelling reason not to\n(liek you are in a performance critical section and want to use a\ncustom sort algorithm), just let the platform-supplied qsort(3) do\nthe job by writing a small comparison function.\n\n> +\tfor (i=0; i<3; i++) {\n> +\t\tfor (j=i+1; j<3; j++) {\n> +\t\t\tif (part_lengths[j] > part_lengths[i]) {\n> +\t\t\t\tswap(part_lengths[i], part_lengths[j]);\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\t\n> +\tif (part_lengths[1] + part_lengths[1] + part_lengths[2] <= len) {\n> +\t\t/*\n> +\t\t * \"{...foofoo => barbar}file\"\n> +\t\t * There is only one omitting part.\n\ns/omitting/omitted/;\n\n> +\t\t */\n> +\t\tmax_part_len = len - part_lengths[1] - part_lengths[2];\n> +\t} else if (part_lengths[2] + part_lengths[2] + part_lengths[2] <= len) {\n> +\t\t/*\n> +\t\t * \"{...foofoo => ...barbar}file\"\n> +\t\t * There are 2 omitting part.\n\ns/omitting part/omitted parts/;\n\n> +\t\t */\n> +\t\tmax_part_len = (len - part_lengths[2]) / 2;\n> +\t\tremainder_part_len = (len - part_lengths[2]) - max_part_len * 2;\n> +\t} else {\n> +\t\t/*\n> +\t\t * \"{...ofoo => ...rbar}...file\"\n> +\t\t * There are 3 omitting part.\n\nLikewise.\n"},{"id":"229040","messageId":"87wqlexhth.fsf@gmail.com","threadId":"35104","inReplyTo":"xmqqbo2qb0wk.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Keshav Kini","fromEmail":"keshav.kini@gmail.com","sentAt":"2013-10-15T22:58:18Z","receivedAt":"2013-10-15T22:58:18Z","isPatch":true,"sender":{"key":"keshav.kini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/691290?v=4"},"body":"Junio C Hamano writes:\n> Yoshioka Tsuneo writes:\n>\n>> \"git diff -M --stat\" can detect rename and show renamed file name like\n>> \"foofoofoo => barbarbar\". But if destination filename is long, the line\n>> is shortened like \"...barbarbar\" so there is no way to know whether the\n>> file is renamed or existed in the source commit.\n>\n> Is \"destination\" filename more special than the source filename?\n> Perhaps \"s/if destination filename is/if filenames are/\"?\n>\n> \tNote: I do not want you to reroll using the suggested\n> \twording without explanation; it may be possible that I am\n> \tmissing something obvious and do not understand why you\n> \tsingled out destination, in which case I'd rather see it\n> \texplained better in the log message than the potentially\n> \tsuboptimal suggestion I made in the review without\n> \tunderstanding the issue. Of course, it is possible that you\n> \twant to do the same when source is overlong, in which case\n> \tyou can just say \"Yeah, you're right; will reroll\".\n>\n> The above applies to all the other comments in this message.\n>\n> Also \"s/source commit/original/\". You may not be comparing two\n> commits after all.\n>\n>> Make sure there is always an arrow, like \"...foo => ...bar\".\n>> The output can contains curly braces('{','}') for grouping.\n>\n> s/contains/contain/;\n>\n>> So, in general, the outpu format is \"{ => }\"\n>\n> s/outpu/&t/;\n>\n>> To keep arrow(\"=>\"), try to omit as long as possible at first\n>> because later part or changing part will be the more important part.\n>> If it is not enough, shorten , , and trying to\n>> have the maximum length the same because those will be equaly important.\n>\n> A sound reasoning.\n\nAlso s/equaly/equally/;\n\n-Keshav\n"},{"id":"229066","messageId":"89A4E8C6-C233-49E2-8141-837ABDBBC976@gmail.com","threadId":"35104","inReplyTo":"79A13931-694C-4DDC-BEDF-71A0DBA0ECA1@gmail.com","subject":"[PATCH v6] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-16T09:53:44Z","receivedAt":"2013-10-16T09:53:44Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"\n\"git diff -M --stat\" can detect rename and show renamed file name like\n\"foofoofoo => barbarbar\".\nBefore this commit, this output is shortened always by omitting left most\npart like \"...foo => barbarbar\". So, if the destination filename is too long,\nsource filename putting left or arrow can be totally omitted like\n\"...barbarbar\", without including any of \"foofoofoo =>\".\nIn such a case where arrow symbol is omitted, there is no way to know\nwhether the file is renamed or existed in the original.\nMake sure there is always an arrow, like \"...foo => ...bar\".\nThe output can contain curly braces('{','}') for grouping.\nSo, in general, the output format is \"{ => }\"\nTo keep arrow(\"=>\"), try to omit as long as possible at first\nbecause later part or changing part will be the more important part.\nIf it is not enough, shorten , , and trying to\nhave the maximum length the same because those will be equally important.\n\nSigned-off-by: Tsuneo Yoshioka \nTest-added-by: Thomas Rast \n---\n diff.c | 180 +++++++++++++++++++++++++++++++++++++++++++------\n t/t4001-diff-rename.sh | 12 ++++\n 2 files changed, 170 insertions(+), 22 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a04a34d..afe6a36 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1258,11 +1258,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t}\n }\n \n-static char *pprint_rename(const char *a, const char *b)\n+static void find_common_prefix_suffix(const char *a, const char *b,\n+\t\t\t\tstruct strbuf *pfx,\n+\t\t\t\tstruct strbuf *a_mid, struct strbuf *b_mid,\n+\t\t\t\tstruct strbuf *sfx)\n {\n \tconst char *old = a;\n \tconst char *new = b;\n-\tstruct strbuf name = STRBUF_INIT;\n \tint pfx_length, sfx_length;\n \tint pfx_adjust_for_slash;\n \tint len_a = strlen(a);\n@@ -1272,10 +1274,9 @@ static char *pprint_rename(const char *a, const char *b)\n \tint qlen_b = quote_c_style(b, NULL, NULL, 0);\n \n \tif (qlen_a || qlen_b) {\n-\t\tquote_c_style(a, &name, NULL, 0);\n-\t\tstrbuf_addstr(&name, \" => \");\n-\t\tquote_c_style(b, &name, NULL, 0);\n-\t\treturn strbuf_detach(&name, NULL);\n+\t\tquote_c_style(a, a_mid, NULL, 0);\n+\t\tquote_c_style(b, b_mid, NULL, 0);\n+\t\treturn;\n \t}\n \n \t/* Find common prefix */\n@@ -1322,17 +1323,142 @@ static char *pprint_rename(const char *a, const char *b)\n \tif (b_midlen < 0)\n \t\tb_midlen = 0;\n \n-\tstrbuf_grow(&name, pfx_length + a_midlen + b_midlen + sfx_length + 7);\n-\tif (pfx_length + sfx_length) {\n-\t\tstrbuf_add(&name, a, pfx_length);\n+\tstrbuf_add(pfx, a, pfx_length);\n+\tstrbuf_add(a_mid, a + pfx_length, a_midlen);\n+\tstrbuf_add(b_mid, b + pfx_length, b_midlen);\n+\tstrbuf_add(sfx, a + len_a - sfx_length, sfx_length);\n+}\n+\n+static int compare_size_t_descending_order(const void *left, const void *right)\n+{\n+\tsize_t left_val = *(size_t *)left;\n+\tsize_t right_val = *(size_t *)right;\n+\treturn (int)(right_val - left_val);\n+}\n+\n+/*\n+ * Omit each parts to fix in name_width.\n+ * Formatted string is \"{ => }\".\n+ * At first, omit as long as possible.\n+ * If it is not enough, omit , , by tring to set the length of\n+ * those 3 parts(including \"...\") to the same.\n+ * Ex:\n+ * \"foofoofoo => barbarbar\"\n+ * will be like\n+ * \"...foo => ...bar\".\n+ * \"long_parent{foofoofoo => barbarbar}longfilename\"\n+ * will be like\n+ * \"...parent{...foofoo => ...barbar}...lename\"\n+ */\n+static void rename_omit(struct strbuf *pfx,\n+\t\t\t\tstruct strbuf *a_mid, struct strbuf *b_mid,\n+\t\t\t\tstruct strbuf *sfx,\n+\t\t\t\tint name_width)\n+{\n+\tstatic const char arrow[] = \" => \";\n+\tstatic const char dots[] = \"...\";\n+\tint use_curly_braces = (pfx->len > 0) || (sfx->len > 0);\n+\tsize_t name_len;\n+\tsize_t len;\n+\tsize_t part_length[3];\n+\tsize_t max_part_len = 0;\n+\tsize_t remainder_part_len = 0;\n+\n+\tname_len = pfx->len + a_mid->len + b_mid->len + sfx->len + strlen(arrow)\n+\t\t+ (use_curly_braces ? 2 : 0);\n+\n+\tif (name_len <= name_width) {\n+\t\t/* Everthing fits in name_width */\n+\t\treturn;\n+\t}\n+\n+\tif (use_curly_braces) {\n+\t\tif (strlen(dots) + (name_len - pfx->len) <= name_width) {\n+\t\t\t/*\n+\t\t\t * Just omitting left of '{' is enough\n+\t\t\t * Ex: ...aaa{foofoofoo => bar}file\n+\t\t\t */\n+\t\t\tstrbuf_splice(pfx, 0, name_len - name_width + strlen(dots), dots, strlen(dots));\n+\t\t\treturn;\n+\t\t} else {\n+\t\t\tif (pfx->len > strlen(dots)) {\n+\t\t\t\t/*\n+\t\t\t\t * Just omitting left of '{' is not enough\n+\t\t\t\t * name will be \"...{SOMETHING}SOMETHING\"\n+\t\t\t\t */\n+\t\t\t\tstrbuf_reset(pfx);\n+\t\t\t\tstrbuf_addstr(pfx, dots);\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t/* available length for a_mid, b_mid and sfx */\n+\tlen = name_width - strlen(arrow) - (use_curly_braces ? 2 : 0);\n+\n+\t/* a_mid, b_mid, sfx will be have the same max, including ellipsis(\"...\"). */\n+\tpart_length[0] = a_mid->len;\n+\tpart_length[1] = b_mid->len;\n+\tpart_length[2] = sfx->len;\n+\n+\tqsort(part_length, sizeof(part_length)/sizeof(part_length[0]), sizeof(part_length[0])\n+\t\t , compare_size_t_descending_order);\n+\n+\tif (part_length[1] + part_length[1] + part_length[2] <= len) {\n+\t\t/*\n+\t\t * \"{...foofoo => barbar}file\"\n+\t\t * There is only one omitted part.\n+\t\t */\n+\t\tmax_part_len = len - part_length[1] - part_length[2];\n+\t} else if (part_length[2] + part_length[2] + part_length[2] <= len) {\n+\t\t/*\n+\t\t * \"{...foofoo => ...barbar}file\"\n+\t\t * There are 2 omitted parts.\n+\t\t */\n+\t\tmax_part_len = (len - part_length[2]) / 2;\n+\t\tremainder_part_len = (len - part_length[2]) - max_part_len * 2;\n+\t} else {\n+\t\t/*\n+\t\t * \"{...ofoo => ...rbar}...file\"\n+\t\t * There are 3 omitted parts.\n+\t\t */\n+\t\tmax_part_len = len / 3;\n+\t\tremainder_part_len = len - (max_part_len) * 3;\n+\t}\n+\n+\tif (max_part_len < strlen(dots))\n+\t\tmax_part_len = strlen(dots);\n+\n+\tif (sfx->len > max_part_len)\n+\t\tstrbuf_splice(sfx, 0, sfx->len - max_part_len + strlen(dots), dots, strlen(dots));\n+\tif (remainder_part_len == 2)\n+\t\tmax_part_len++;\n+\tif (a_mid->len > max_part_len)\n+\t\tstrbuf_splice(a_mid, 0, a_mid->len - max_part_len + strlen(dots), dots, strlen(dots));\n+\tif (remainder_part_len == 1)\n+\t\tmax_part_len++;\n+\tif (b_mid->len > max_part_len)\n+\t\tstrbuf_splice(b_mid, 0, b_mid->len - max_part_len + strlen(dots), dots, strlen(dots));\n+}\n+\n+static char *pprint_rename(const char *a, const char *b, int name_width)\n+{\n+\tstruct strbuf pfx = STRBUF_INIT, a_mid = STRBUF_INIT, b_mid = STRBUF_INIT, sfx = STRBUF_INIT;\n+\tstruct strbuf name = STRBUF_INIT;\n+\n+\tfind_common_prefix_suffix(a, b, &pfx, &a_mid, &b_mid, &sfx);\n+\trename_omit(&pfx, &a_mid, &b_mid, &sfx, name_width);\n+\n+\tstrbuf_grow(&name, pfx.len + a_mid.len + b_mid.len + sfx.len + 7);\n+\tif (pfx.len + sfx.len) {\n+\t\tstrbuf_addbuf(&name, &pfx);\n \t\tstrbuf_addch(&name, '{');\n \t}\n-\tstrbuf_add(&name, a + pfx_length, a_midlen);\n+\tstrbuf_addbuf(&name, &a_mid);\n \tstrbuf_addstr(&name, \" => \");\n-\tstrbuf_add(&name, b + pfx_length, b_midlen);\n-\tif (pfx_length + sfx_length) {\n+\tstrbuf_addbuf(&name, &b_mid);\n+\tif (pfx.len + sfx.len) {\n \t\tstrbuf_addch(&name, '}');\n-\t\tstrbuf_add(&name, a + len_a - sfx_length, sfx_length);\n+\t\tstrbuf_addbuf(&name, &sfx);\n \t}\n \treturn strbuf_detach(&name, NULL);\n }\n@@ -1418,23 +1544,31 @@ static void show_graph(FILE *file, char ch, int cnt, const char *set, const char\n \tfprintf(file, \"%s\", reset);\n }\n \n-static void fill_print_name(struct diffstat_file *file)\n+static void fill_print_name(struct diffstat_file *file, int name_width)\n {\n \tchar *pname;\n \n-\tif (file->print_name)\n-\t\treturn;\n-\n \tif (!file->is_renamed) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tif (file->print_name)\n+\t\t\treturn;\n \t\tif (quote_c_style(file->name, &buf, NULL, 0)) {\n \t\t\tpname = strbuf_detach(&buf, NULL);\n \t\t} else {\n \t\t\tpname = file->name;\n \t\t\tstrbuf_release(&buf);\n \t\t}\n+\t\tif (strlen(pname) > name_width) {\n+\t\t\tstruct strbuf buf2 = STRBUF_INIT;\n+\t\t\tstrbuf_addstr(&buf2, \"...\");\n+\t\t\tstrbuf_addstr(&buf2, pname + strlen(pname) - name_width - 3);\n+\t\t}\n \t} else {\n-\t\tpname = pprint_rename(file->from_name, file->name);\n+\t\tif (file->print_name) {\n+\t\t\tfree(file->print_name);\n+\t\t\tfile->print_name = NULL;\n+\t\t}\n+\t\tpname = pprint_rename(file->from_name, file->name, name_width);\n \t}\n \tfile->print_name = pname;\n }\n@@ -1517,7 +1651,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t\tcount++; /* not shown == room for one more */\n \t\t\tcontinue;\n \t\t}\n-\t\tfill_print_name(file);\n+\t\tfill_print_name(file, INT_MAX);\n \t\tlen = strlen(file->print_name);\n \t\tif (max_len < len)\n \t\t\tmax_len = len;\n@@ -1629,7 +1763,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tfor (i = 0; i < count; i++) {\n \t\tconst char *prefix = \"\";\n \t\tstruct diffstat_file *file = data->files[i];\n-\t\tchar *name = file->print_name;\n+\t\tchar *name;\n \t\tuintmax_t added = file->added;\n \t\tuintmax_t deleted = file->deleted;\n \t\tint name_len;\n@@ -1637,6 +1771,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tif (!file->is_interesting && (added + deleted == 0))\n \t\t\tcontinue;\n \n+\t\tfill_print_name(file, name_width);\n+\t\tname = file->print_name;\n \t\t/*\n \t\t * \"scale\" the filename\n \t\t */\n@@ -1772,7 +1908,7 @@ static void show_numstat(struct diffstat_t *data, struct diff_options *options)\n \t\t\t\t\"%\"PRIuMAX\"\\t%\"PRIuMAX\"\\t\",\n \t\t\t\tfile->added, file->deleted);\n \t\tif (options->line_termination) {\n-\t\t\tfill_print_name(file);\n+\t\t\tfill_print_name(file, INT_MAX);\n \t\t\tif (!file->is_renamed)\n \t\t\t\twrite_name_quoted(file->name, options->file,\n \t\t\t\t\t\t options->line_termination);\n@@ -4258,7 +4394,7 @@ static void show_mode_change(FILE *file, struct diff_filepair *p, int show_name,\n static void show_rename_copy(FILE *file, const char *renamecopy, struct diff_filepair *p,\n \t\t\tconst char *line_prefix)\n {\n-\tchar *names = pprint_rename(p->one->path, p->two->path);\n+\tchar *names = pprint_rename(p->one->path, p->two->path, INT_MAX);\n \n \tfprintf(file, \" %s %s (%d%%)\\n\", renamecopy, names, similarity_index(p));\n \tfree(names);\ndiff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh\nindex 2f327b7..03d6371 100755\n--- a/t/t4001-diff-rename.sh\n+++ b/t/t4001-diff-rename.sh\n@@ -156,4 +156,16 @@ test_expect_success 'rename pretty print common prefix and suffix overlap' '\n \ttest_i18ngrep \" d/f/{ => f}/e \" output\n '\n \n+test_expect_success 'rename of very long path shows =>' '\n+\tmkdir long_dirname_that_does_not_fit_in_a_single_line &&\n+\tmkdir another_extremely_long_path_but_not_the_same_as_the_first &&\n+\tcp path1 long_dirname*/ &&\n+\tgit add long_dirname*/path1 &&\n+\ttest_commit add_long_pathname &&\n+\tgit mv long_dirname*/path1 another_extremely_*/ &&\n+\ttest_commit move_long_pathname &&\n+\tgit diff -M --stat HEAD^ HEAD >output &&\n+\ttest_i18ngrep \"=>.*path1\" output\n+'\n+\n test_done\n-- \n1.8.4.475.g867697c\n"},{"id":"229067","messageId":"027C65BD-1110-4EBA-B854-16F15F85952A@gmail.com","threadId":"35104","inReplyTo":"xmqqbo2qb0wk.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-16T09:53:53Z","receivedAt":"2013-10-16T09:53:53Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"Hello Junio, Keshav\n\nThank you very much for your detailed reviewing.\nI just tried to update the patch and posted as \"[PATCH v6]\".\n\n---\nTsuneo Yoshioka (吉岡 恒夫)\nyoshiokatsuneo@gmail.com\n\n\n\n\nOn Oct 16, 2013, at 1:54 AM, Junio C Hamano wrote:\n\n> Yoshioka Tsuneo writes:\n> \n>> \"git diff -M --stat\" can detect rename and show renamed file name like\n>> \"foofoofoo => barbarbar\". But if destination filename is long, the line\n>> is shortened like \"...barbarbar\" so there is no way to know whether the\n>> file is renamed or existed in the source commit.\n> \n> Is \"destination\" filename more special than the source filename?\n> Perhaps \"s/if destination filename is/if filenames are/\"?\n> \n> \tNote: I do not want you to reroll using the suggested\n> \twording without explanation; it may be possible that I am\n> \tmissing something obvious and do not understand why you\n> \tsingled out destination, in which case I'd rather see it\n> \texplained better in the log message than the potentially\n> \tsuboptimal suggestion I made in the review without\n> \tunderstanding the issue. Of course, it is possible that you\n> \twant to do the same when source is overlong, in which case\n> \tyou can just say \"Yeah, you're right; will reroll\".\n> \n> The above applies to all the other comments in this message.\n> \n> Also \"s/source commit/original/\". You may not be comparing two\n> commits after all.\n> \n>> Make sure there is always an arrow, like \"...foo => ...bar\".\n>> The output can contains curly braces('{','}') for grouping.\n> \n> s/contains/contain/;\n> \n>> So, in general, the outpu format is \"{ => }\"\n> \n> s/outpu/&t/;\n> \n>> To keep arrow(\"=>\"), try to omit as long as possible at first\n>> because later part or changing part will be the more important part.\n>> If it is not enough, shorten , , and trying to\n>> have the maximum length the same because those will be equaly important.\n> \n> A sound reasoning.\n> \n>> Signed-off-by: Tsuneo Yoshioka \n>> Test-added-by: Thomas Rast \n>> ---\n>> diff.c | 187 +++++++++++++++++++++++++++++++++++++++++++------\n>> t/t4001-diff-rename.sh | 12 ++++\n>> 2 files changed, 177 insertions(+), 22 deletions(-)\n>> \n>> diff --git a/diff.c b/diff.c\n>> index a04a34d..cf50807 100644\n>> --- a/diff.c\n>> +++ b/diff.c\n>> @@ -1258,11 +1258,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n>> \t}\n>> }\n>> \n>> -static char *pprint_rename(const char *a, const char *b)\n>> +static void pprint_rename_find_common_prefix_suffix(const char *a, const char *b\n>> +\t\t\t\t\t\t\t\t\t\t\t\t\t, struct strbuf *pfx, struct strbuf *a_mid\n>> +\t\t\t\t\t\t\t\t\t\t\t\t\t, struct strbuf *b_mid, struct strbuf *sfx)\n> \n> What kind of line splitting is this?\n> \n> I think the real issue is that the function name is overly long, but\n> aside from that,\n> \n> - comma comes at the end of the line, not at the beginning of the\n> next line;\n> \n> - the second and subsequent lines are indented, but not more than\n> the usual line width (align with the first letter inside the\n> opening parenthesis of the first line);\n> \n> - a_mid and b_mid are more \"alike\" than pfx and a_mid.\n> \n> so I would expect to see it more like:\n> \n> static void abbrev_rename(const char *a, const char *b,\n> \t\t\t struct strbuf *pfx,\n> \t\t\t struct strbuf *a_mid, struct strbuf *b_mid,\n> \t\t\t struct strbuf *sfx)\n> \n> Note that the suggested name does not say \"pprint\", because in your\n> version of this file, the code around here is no longer doing any\n> printing. The caller does so after using this function to decide\n> how to abbreviate renames, so naming the helper function after what\n> it does (e.g. abbreviate renames) is more appropriate.\n> \n>> {\n>> \tconst char *old = a;\n>> \tconst char *new = b;\n>> -\tstruct strbuf name = STRBUF_INIT;\n>> \tint pfx_length, sfx_length;\n>> \tint pfx_adjust_for_slash;\n>> \tint len_a = strlen(a);\n>> @@ -1272,10 +1273,9 @@ static char *pprint_rename(const char *a, const char *b)\n>> \tint qlen_b = quote_c_style(b, NULL, NULL, 0);\n>> \n>> \tif (qlen_a || qlen_b) {\n>> -\t\tquote_c_style(a, &name, NULL, 0);\n>> -\t\tstrbuf_addstr(&name, \" => \");\n>> -\t\tquote_c_style(b, &name, NULL, 0);\n>> -\t\treturn strbuf_detach(&name, NULL);\n>> +\t\tquote_c_style(a, a_mid, NULL, 0);\n>> +\t\tquote_c_style(b, b_mid, NULL, 0);\n>> +\t\treturn;\n>> \t}\n>> \n>> \t/* Find common prefix */\n>> @@ -1321,18 +1321,151 @@ static char *pprint_rename(const char *a, const char *b)\n>> \t\ta_midlen = 0;\n>> \tif (b_midlen < 0)\n>> \t\tb_midlen = 0;\n>> +\t\n> \n> Trailing whitespace (there are many others you added to this file; I\n> won't bother to point out all of them).\n> \n>> +\tstrbuf_add(pfx, a, pfx_length);\n>> +\tstrbuf_add(a_mid, a + pfx_length, a_midlen);\n>> +\tstrbuf_add(b_mid, b + pfx_length, b_midlen);\n>> +\tstrbuf_add(sfx, a + len_a - sfx_length, sfx_length);\n>> +}\n>> +\n>> +/*\n>> + * Omit each parts to fix in name_width.\n>> + * Formatted string is \"{ => }\".\n>> + * At first, omit as long as possible.\n>> + * If it is not enough, omit , , by tring to set the length of\n>> + * those 3 parts(including \"...\") to the same.\n>> + * Ex:\n>> + * \"foofoofoo => barbarbar\"\n>> + * will be like\n>> + * \"...foo => ...bar\".\n>> + * \"long_parent{foofoofoo => barbarbar}longfilename\"\n>> + * will be like\n>> + * \"...parent{...foofoo => ...barbar}...lename\"\n>> + */\n>> +static void pprint_rename_omit(struct strbuf *pfx, struct strbuf *a_mid, struct strbuf *b_mid\n>> +\t\t\t\t\t\t\t , struct strbuf *sfx, int name_width)\n> \n> Bad line splitting.\n> \n>> +{\n>> +\n>> +#define ARROW \" => \"\n>> +#define ELLIPSIS \"...\"\n> \n> Ugly and leaks these symbols after the function is done using them\n> to the remainder of this file. Write them like this instead, perhaps?\n> \n> \tstatic const char arrow[] = \" => \";\n> static const char dots[] = \"...\";\n> \n>> +#define swap(a, b) myswap((a), (b), sizeof(a))\n>> +\t\n>> +#define myswap(a, b, size) do {\t\t\\\n>> +unsigned char mytmp[size];\t\\\n>> +memcpy(mytmp, &a, size);\t\t\\\n>> +memcpy(&a, &b, size);\t\t\\\n>> +memcpy(&b, mytmp, size);\t\t\\\n>> +} while (0)\n> \n> These are totally unneeded, I suspect (see below).\n> \n>> +\n>> +\tint use_curly_braces = (pfx->len > 0) || (sfx->len > 0);\n>> +\tsize_t name_len;\n>> +\tsize_t len;\n>> +\tsize_t part_lengths[4];\n> \n> Do not name an array in plural, i.e. elements[], unless there is a\n> compelling reason to do so. By using singular, e.g. element[], the\n> third element can be spelled as element[3], which is more logical\n> than having to call it elements[3].\n> \n> \tSide note. a notable exception is an array that is used as a\n> \thash-table and frequently passed around as an argument; you\n> \tare usually not interested in iterating over it in ascending\n> \torder, and being able to call such a collection of things\n> \t\"things\" in plural, e.g. struct object objects[], is more\n> \timportant.\n> \n>> +\tsize_t max_part_len = 0;\n>> +\tsize_t remainder_part_len = 0;\n>> +\tint i, j;\n>> +\n>> +\tname_len = pfx->len + a_mid->len + b_mid->len + sfx->len + strlen(ARROW)\n>> +\t\t+ (use_curly_braces ? 2 : 0);\n>> +\t\n>> +\tif (name_len <= name_width) {\n>> +\t\t/* Everthing fits in name_width */\n>> +\t\treturn;\n>> +\t}\n>> +\t\n>> +\tif (use_curly_braces) {\n>> +\t\tif (strlen(ELLIPSIS) + (name_len - pfx->len) <= name_width) {\n>> +\t\t\t/*\n>> +\t\t\t Just omitting left of '{' is enough\n>> +\t\t\t Ex: ...aaa{foofoofoo => bar}file\n>> +\t\t\t */\n> \n> \t/*\n> * We format our multi-line\n> * comments like\n> * this.\n> */\n> \n>> +\t\t\tstrbuf_splice(pfx, name_len - pfx->len, name_width - (name_len - pfx->len), ELLIPSIS, strlen(ELLIPSIS));\n> \n> Overlong line.\n> \n> Is the math for the second and third arguments correct? If you are\n> making \"abcdefghij\" into \"...hij\", you would splice at position 0\n> for length up to 'g', so it felt strange to see any arithmetic as\n> the second argument, but I didn't look at this code very closely.\n> \n>> +\t\t\treturn;\n>> +\t\t} else {\n>> +\t\t\tif (pfx->len > strlen(ELLIPSIS)) {\n>> +\t\t\t\t/*\n>> +\t\t\t\t Just omitting left of '{' is not enough\n>> +\t\t\t\t name will be \"...{SOMETHING}SOMETHING\"\n>> +\t\t\t\t */\n>> +\t\t\t\tstrbuf_reset(pfx);\n>> +\t\t\t\tstrbuf_addstr(pfx, ELLIPSIS);\n>> +\t\t\t}\n>> +\t\t}\n>> +\t}\n>> \n>> -\tstrbuf_grow(&name, pfx_length + a_midlen + b_midlen + sfx_length + 7);\n>> -\tif (pfx_length + sfx_length) {\n>> -\t\tstrbuf_add(&name, a, pfx_length);\n>> +\t/* available length for a_mid, b_mid and sfx */\n>> +\tlen = name_width - strlen(ARROW) - (use_curly_braces ? 2 : 0);\n>> +\t\n>> +\t/* a_mid, b_mid, sfx will be have the same max, including ellipsis(\"...\"). */\n>> +\tpart_lengths[0] = (int)a_mid->len;\n>> +\tpart_lengths[1] = (int)b_mid->len;\n>> +\tpart_lengths[2] = (int)sfx->len;\n> \n> What are these casts about? strbuf.len is of size_t which is\n> already the correct type for part_length[].\n> \n>> +\t\n>> +\t/* bubble sort of part_lengths, descending order */\n> \n> Do not bubble sort. Unless there is a compelling reason not to\n> (liek you are in a performance critical section and want to use a\n> custom sort algorithm), just let the platform-supplied qsort(3) do\n> the job by writing a small comparison function.\n> \n>> +\tfor (i=0; i<3; i++) {\n>> +\t\tfor (j=i+1; j<3; j++) {\n>> +\t\t\tif (part_lengths[j] > part_lengths[i]) {\n>> +\t\t\t\tswap(part_lengths[i], part_lengths[j]);\n>> +\t\t\t}\n>> +\t\t}\n>> +\t}\n>> +\t\n>> +\tif (part_lengths[1] + part_lengths[1] + part_lengths[2] <= len) {\n>> +\t\t/*\n>> +\t\t * \"{...foofoo => barbar}file\"\n>> +\t\t * There is only one omitting part.\n> \n> s/omitting/omitted/;\n> \n>> +\t\t */\n>> +\t\tmax_part_len = len - part_lengths[1] - part_lengths[2];\n>> +\t} else if (part_lengths[2] + part_lengths[2] + part_lengths[2] <= len) {\n>> +\t\t/*\n>> +\t\t * \"{...foofoo => ...barbar}file\"\n>> +\t\t * There are 2 omitting part.\n> \n> s/omitting part/omitted parts/;\n> \n>> +\t\t */\n>> +\t\tmax_part_len = (len - part_lengths[2]) / 2;\n>> +\t\tremainder_part_len = (len - part_lengths[2]) - max_part_len * 2;\n>> +\t} else {\n>> +\t\t/*\n>> +\t\t * \"{...ofoo => ...rbar}...file\"\n>> +\t\t * There are 3 omitting part.\n> \n> Likewise.\n> \n"},{"id":"229128","messageId":"xmqqmwm71ysp.fsf@gitster.dls.corp.google.com","threadId":"35104","inReplyTo":"89A4E8C6-C233-49E2-8141-837ABDBBC976@gmail.com","subject":"Re: [PATCH v6] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-17T19:29:26Z","receivedAt":"2013-10-17T19:29:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yoshioka Tsuneo writes:\n\n> \"git diff -M --stat\" can detect rename and show renamed file name like\n> \"foofoofoo => barbarbar\".\n> Before this commit, this output is shortened always by omitting left most\n> part like \"...foo => barbarbar\". So, if the destination filename is too long,\n> source filename putting left or arrow can be totally omitted like\n> \"...barbarbar\", without including any of \"foofoofoo =>\".\n> In such a case where arrow symbol is omitted, there is no way to know\n> whether the file is renamed or existed in the original.\n> Make sure there is always an arrow, like \"...foo => ...bar\".\n> The output can contain curly braces('{','}') for grouping.\n> So, in general, the output format is \"{ => }\"\n> To keep arrow(\"=>\"), try to omit as long as possible at first\n> because later part or changing part will be the more important part.\n> If it is not enough, shorten , , and trying to\n> have the maximum length the same because those will be equally important.\n\nI somehow find this solid wall of text extremely hard to\nread. Adding a blank line as a paragraph break may make it easier to\nread, perhaps.\n\nAlso it is customary in our history to omit the full-stop from the\npatch title on the Subject: line.\n\n> +\tname_len = pfx->len + a_mid->len + b_mid->len + sfx->len + strlen(arrow)\n> +\t\t+ (use_curly_braces ? 2 : 0);\n> +\n> +\tif (name_len <= name_width) {\n> +\t\t/* Everthing fits in name_width */\n> +\t\treturn;\n> +\t}\n\nLogic up to this point seems good; drop {} around a single statement\n\"return;\", i.e.\n\n\tif (name_len <= name_width)\n \treturn; /* everything fits */\n\n> +\t\t} else {\n> +\t\t\tif (pfx->len > strlen(dots)) {\n> +\t\t\t\t/*\n> +\t\t\t\t * Just omitting left of '{' is not enough\n> +\t\t\t\t * name will be \"...{SOMETHING}SOMETHING\"\n> +\t\t\t\t */\n> +\t\t\t\tstrbuf_reset(pfx);\n> +\t\t\t\tstrbuf_addstr(pfx, dots);\n> +\t\t\t}\n\n(mental note) ... otherwise, i.e. with a short common prefix, the\nfinal result will be \"ab{SOMETHING}SOMETHING\", which is also fine\nfor the purpose of the remainder of this function.\n\n> +\t\t}\n> +\t}\n> +\n> +\t/* available length for a_mid, b_mid and sfx */\n> +\tlen = name_width - strlen(arrow) - (use_curly_braces ? 2 : 0);\n> +\n> +\t/* a_mid, b_mid, sfx will be have the same max, including ellipsis(\"...\"). */\n> +\tpart_length[0] = a_mid->len;\n> +\tpart_length[1] = b_mid->len;\n> +\tpart_length[2] = sfx->len;\n> +\n> +\tqsort(part_length, sizeof(part_length)/sizeof(part_length[0]), sizeof(part_length[0])\n> +\t\t , compare_size_t_descending_order);\n\nIn our code, comma does not come at the beginning of continued\nline.\n\n> +\tif (part_length[1] + part_length[1] + part_length[2] <= len) {\n> +\t\t/*\n> +\t\t * \"{...foofoo => barbar}file\"\n> +\t\t * There is only one omitted part.\n> +\t\t */\n> +\t\tmax_part_len = len - part_length[1] - part_length[2];\n\nIt would be clearer to explicitly set remainder to zero here, and\nomit the initialization of the variable. That would make what the\nthree parts of if/elseif/else do more consistent.\n\n> +\t} else if (part_length[2] + part_length[2] + part_length[2] <= len) {\n> +\t\t/*\n> +\t\t * \"{...foofoo => ...barbar}file\"\n> +\t\t * There are 2 omitted parts.\n> +\t\t */\n> +\t\tmax_part_len = (len - part_length[2]) / 2;\n> +\t\tremainder_part_len = (len - part_length[2]) - max_part_len * 2;\n> +\t} else {\n> +\t\t/*\n> +\t\t * \"{...ofoo => ...rbar}...file\"\n> +\t\t * There are 3 omitted parts.\n> +\t\t */\n> +\t\tmax_part_len = len / 3;\n> +\t\tremainder_part_len = len - (max_part_len) * 3;\n> +\t}\n\nI am not sure if distributing the burden of truncation equally to\nthree parts so that the resulting pieces are of similar lengths is\nreally a good idea. Between these two\n\n\t{...SourceDirectory => ...nationDirectory}...ileThatWasMoved \n\t{...ceDirectory => ...ionDirectory}nameOfTheFileThatWasMoved\n\nthat attempt to show that the file nameOfTheFileThatWasMoved was\nmoved from the longSourceDirectory to the DestinationDirectory, the\nlatter is much more informative, I would think.\n\n> diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh\n> index 2f327b7..03d6371 100755\n> --- a/t/t4001-diff-rename.sh\n> +++ b/t/t4001-diff-rename.sh\n> @@ -156,4 +156,16 @@ test_expect_success 'rename pretty print common prefix and suffix overlap' '\n> \ttest_i18ngrep \" d/f/{ => f}/e \" output\n> '\n> \n> +test_expect_success 'rename of very long path shows =>' '\n> +\tmkdir long_dirname_that_does_not_fit_in_a_single_line &&\n> +\tmkdir another_extremely_long_path_but_not_the_same_as_the_first &&\n> +\tcp path1 long_dirname*/ &&\n> +\tgit add long_dirname*/path1 &&\n> +\ttest_commit add_long_pathname &&\n> +\tgit mv long_dirname*/path1 another_extremely_*/ &&\n> +\ttest_commit move_long_pathname &&\n> +\tgit diff -M --stat HEAD^ HEAD >output &&\n> +\ttest_i18ngrep \"=>.*path1\" output\n\nDoes this have to be i18ngrep? I had a feeling that we would not\nwant this part of the output localized, in which case \"grep\" may be\nmore appropriate.\n\n> +'\n> +\n> test_done\n"},{"id":"229133","messageId":"xmqqfvrzznbp.fsf@gitster.dls.corp.google.com","threadId":"35104","inReplyTo":"89A4E8C6-C233-49E2-8141-837ABDBBC976@gmail.com","subject":"Re: [PATCH v6] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-17T19:53:14Z","receivedAt":"2013-10-17T19:53:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yoshioka Tsuneo writes:\n\n> Before this commit, this output is shortened always by omitting left most\n> part like \"...foo => barbarbar\". So, if the destination filename is too long,\n> source filename putting left or arrow can be totally omitted like\n> \"...barbarbar\", without including any of \"foofoofoo =>\".\n\nThis is an explanation much easier to understand than the one in the\nprevious iteration. Thanks.\n"},{"id":"229147","messageId":"FB9897CC-EDC7-4EBB-8DAB-140CEB5F93B3@gmail.com","threadId":"35104","inReplyTo":"89A4E8C6-C233-49E2-8141-837ABDBBC976@gmail.com","subject":"[PATCH v7] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-17T22:08:24Z","receivedAt":"2013-10-17T22:08:24Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"\n\"git diff -M --stat\" can detect rename and show renamed file name like\n\"foofoofoo => barbarbar\".\n\nBefore this commit, this output is shortened always by omitting left most\npart like \"...foo => barbarbar\". So, if the destination filename is too long,\nsource filename putting left or arrow can be totally omitted like\n\"...barbarbar\", without including any of \"foofoofoo =>\".\nIn such a case where arrow symbol is omitted, there is no way to know\nwhether the file is renamed or existed in the original.\n\nMake sure there is always an arrow, like \"...foo => ...bar\".\n\nThe output can contain curly braces('{','}') for grouping.\nSo, in general, the output format is \"{ => }\"\n\nTo keep arrow(\"=>\"), try to omit as long as possible at first\nbecause later part or changing part will be the more important part.\nIf it is not enough, shorten , , and trying to\nhave the same maximum length, but as long as filename part of \nis kept.\n\nSigned-off-by: Tsuneo Yoshioka \nTest-added-by: Thomas Rast \n---\n diff.c | 184 +++++++++++++++++++++++++++++++++++++++++++------\n t/t4001-diff-rename.sh | 12 ++++\n 2 files changed, 174 insertions(+), 22 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a04a34d..cdf59c0 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1258,11 +1258,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t}\n }\n \n-static char *pprint_rename(const char *a, const char *b)\n+static void find_common_prefix_suffix(const char *a, const char *b,\n+\t\t\t\tstruct strbuf *pfx,\n+\t\t\t\tstruct strbuf *a_mid, struct strbuf *b_mid,\n+\t\t\t\tstruct strbuf *sfx)\n {\n \tconst char *old = a;\n \tconst char *new = b;\n-\tstruct strbuf name = STRBUF_INIT;\n \tint pfx_length, sfx_length;\n \tint pfx_adjust_for_slash;\n \tint len_a = strlen(a);\n@@ -1272,10 +1274,9 @@ static char *pprint_rename(const char *a, const char *b)\n \tint qlen_b = quote_c_style(b, NULL, NULL, 0);\n \n \tif (qlen_a || qlen_b) {\n-\t\tquote_c_style(a, &name, NULL, 0);\n-\t\tstrbuf_addstr(&name, \" => \");\n-\t\tquote_c_style(b, &name, NULL, 0);\n-\t\treturn strbuf_detach(&name, NULL);\n+\t\tquote_c_style(a, a_mid, NULL, 0);\n+\t\tquote_c_style(b, b_mid, NULL, 0);\n+\t\treturn;\n \t}\n \n \t/* Find common prefix */\n@@ -1322,17 +1323,146 @@ static char *pprint_rename(const char *a, const char *b)\n \tif (b_midlen < 0)\n \t\tb_midlen = 0;\n \n-\tstrbuf_grow(&name, pfx_length + a_midlen + b_midlen + sfx_length + 7);\n-\tif (pfx_length + sfx_length) {\n-\t\tstrbuf_add(&name, a, pfx_length);\n+\tstrbuf_add(pfx, a, pfx_length);\n+\tstrbuf_add(a_mid, a + pfx_length, a_midlen);\n+\tstrbuf_add(b_mid, b + pfx_length, b_midlen);\n+\tstrbuf_add(sfx, a + len_a - sfx_length, sfx_length);\n+}\n+\n+/*\n+ * Omit each parts to fix in name_width.\n+ * Formatted string is \"{ => }\".\n+ * At first, omit as long as possible.\n+ * If it is not enough, omit , , by tring to set the length of\n+ * those 3 parts(including \"...\") to the same, but keeping filename part of .\n+ * Ex:\n+ * \"foofoofoo => barbarbar\"\n+ * will be like\n+ * \"...foo => ...bar\".\n+ * \"long_parent{foofoofoo => barbarbar}path/filename\"\n+ * will be like\n+ * \"...parent{...foofoo => ...barbar}.../filename\"\n+ */\n+static void rename_omit(struct strbuf *pfx,\n+\t\t\t\tstruct strbuf *a_mid, struct strbuf *b_mid,\n+\t\t\t\tstruct strbuf *sfx,\n+\t\t\t\tint name_width)\n+{\n+\tstatic const char arrow[] = \" => \";\n+\tstatic const char dots[] = \"...\";\n+\tint use_curly_braces = (pfx->len > 0) || (sfx->len > 0);\n+\tsize_t name_len;\n+\tsize_t max_part_len = 0;\n+\tsize_t remainder_part_len = 0;\n+\tsize_t left, right;\n+\tsize_t sfx_minlen;\n+\tchar *sfx_last_slash;\n+\tsize_t max_sfx_len;\n+\n+\tname_len = pfx->len + a_mid->len + b_mid->len + sfx->len + strlen(arrow)\n+\t\t+ (use_curly_braces ? 2 : 0);\n+\n+\tif (name_len <= name_width)\n+\t\treturn; /* everything fits in name_width */\n+\n+\tif (use_curly_braces) {\n+\t\tif (strlen(dots) + (name_len - pfx->len) <= name_width) {\n+\t\t\t/*\n+\t\t\t * Just omitting left of '{' is enough\n+\t\t\t * Ex: ...aaa{foofoofoo => bar}file\n+\t\t\t */\n+\t\t\tstrbuf_splice(pfx, 0, name_len - name_width + strlen(dots), dots, strlen(dots));\n+\t\t\treturn;\n+\t\t} else {\n+\t\t\tif (pfx->len > strlen(dots)) {\n+\t\t\t\t/*\n+\t\t\t\t * Just omitting left of '{' is not enough\n+\t\t\t\t * name will be \"...{SOMETHING}SOMETHING\"\n+\t\t\t\t */\n+\t\t\t\tstrbuf_reset(pfx);\n+\t\t\t\tstrbuf_addstr(pfx, dots);\n+\t\t\t}else{\n+\t\t\t\t/*\n+\t\t\t\t * If is shorter than dots(\"...\"),\n+\t\t\t\t * there is no sense to replace to dots\n+\t\t\t\t * but name will be just like \"a{SOMETHING}SOMETHING\".\n+\t\t\t\t */\n+\t\t\t\t;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tleft = 0;\n+\tright = name_width + 1;\n+\n+#define MIN(X, Y) ((X < Y) ? (X) : (Y))\n+#define MAX(X, Y) ((X > Y) ? (X) : (Y))\n+\n+\t/* Try to keep filename part(later than last '/') of */\n+\tsfx_last_slash = strrchr(sfx->buf, '/');\n+\tif(sfx_last_slash){\n+\t\tsfx_minlen = MIN(sfx->len - (sfx_last_slash - sfx->buf) + strlen(dots), sfx->len);\n+\t}else{\n+\t\tsfx_minlen = sfx->len;\n+\t}\n+\t/* In case other than is omitted like: \"...{... => ...}\" */\n+\tsfx_minlen = MIN(sfx_minlen,\n+\t\t\tname_width\n+\t\t\t- MIN(strlen(dots), pfx->len)\n+\t\t\t- MIN(strlen(dots), a_mid->len)\n+\t\t\t- MIN(strlen(dots), b_mid->len)\n+\t\t\t- strlen(arrow)\n+\t\t\t- (use_curly_braces ? 2 : 0) );\n+\n+\t/* binary search to find max_part_len(maximum length of omitted parts) */\n+\twhile(left + 1 < right){\n+\t\tsize_t mid = (left + right) / 2;\n+\n+\t\t/* length of \"{ => }\" */\n+\t\tsize_t l = pfx->len + MIN(mid, a_mid->len) + MIN(mid, b_mid->len) + MAX(MIN(mid, sfx->len), sfx_minlen) + strlen(arrow) + (use_curly_braces ? 2 : 0);\n+\t\tif(l <= name_width){\n+\t\t\tleft = mid;\n+\t\t\tremainder_part_len = name_width - l;\n+\t\t}else{\n+\t\t\tright = mid;\n+\t\t}\n+\t}\n+\tmax_part_len = left;\n+\n+\tif (max_part_len < strlen(dots))\n+\t\tmax_part_len = strlen(dots);\n+\tmax_sfx_len = MAX(max_part_len, sfx_minlen);\n+\tif (sfx->len > max_sfx_len)\n+\t\tstrbuf_splice(sfx, 0, sfx->len - max_sfx_len + strlen(dots), dots, strlen(dots));\n+\tif (remainder_part_len == 2)\n+\t\tmax_part_len++;\n+\tif (a_mid->len > max_part_len)\n+\t\tstrbuf_splice(a_mid, 0, a_mid->len - max_part_len + strlen(dots), dots, strlen(dots));\n+\tif (remainder_part_len == 1)\n+\t\tmax_part_len++;\n+\tif (b_mid->len > max_part_len)\n+\t\tstrbuf_splice(b_mid, 0, b_mid->len - max_part_len + strlen(dots), dots, strlen(dots));\n+}\n+\n+static char *pprint_rename(const char *a, const char *b, int name_width)\n+{\n+\tstruct strbuf pfx = STRBUF_INIT, a_mid = STRBUF_INIT, b_mid = STRBUF_INIT, sfx = STRBUF_INIT;\n+\tstruct strbuf name = STRBUF_INIT;\n+\n+\tfind_common_prefix_suffix(a, b, &pfx, &a_mid, &b_mid, &sfx);\n+\trename_omit(&pfx, &a_mid, &b_mid, &sfx, name_width);\n+\n+\tstrbuf_grow(&name, pfx.len + a_mid.len + b_mid.len + sfx.len + 7);\n+\tif (pfx.len + sfx.len) {\n+\t\tstrbuf_addbuf(&name, &pfx);\n \t\tstrbuf_addch(&name, '{');\n \t}\n-\tstrbuf_add(&name, a + pfx_length, a_midlen);\n+\tstrbuf_addbuf(&name, &a_mid);\n \tstrbuf_addstr(&name, \" => \");\n-\tstrbuf_add(&name, b + pfx_length, b_midlen);\n-\tif (pfx_length + sfx_length) {\n+\tstrbuf_addbuf(&name, &b_mid);\n+\tif (pfx.len + sfx.len) {\n \t\tstrbuf_addch(&name, '}');\n-\t\tstrbuf_add(&name, a + len_a - sfx_length, sfx_length);\n+\t\tstrbuf_addbuf(&name, &sfx);\n \t}\n \treturn strbuf_detach(&name, NULL);\n }\n@@ -1418,23 +1548,31 @@ static void show_graph(FILE *file, char ch, int cnt, const char *set, const char\n \tfprintf(file, \"%s\", reset);\n }\n \n-static void fill_print_name(struct diffstat_file *file)\n+static void fill_print_name(struct diffstat_file *file, int name_width)\n {\n \tchar *pname;\n \n-\tif (file->print_name)\n-\t\treturn;\n-\n \tif (!file->is_renamed) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tif (file->print_name)\n+\t\t\treturn;\n \t\tif (quote_c_style(file->name, &buf, NULL, 0)) {\n \t\t\tpname = strbuf_detach(&buf, NULL);\n \t\t} else {\n \t\t\tpname = file->name;\n \t\t\tstrbuf_release(&buf);\n \t\t}\n+\t\tif (strlen(pname) > name_width) {\n+\t\t\tstruct strbuf buf2 = STRBUF_INIT;\n+\t\t\tstrbuf_addstr(&buf2, \"...\");\n+\t\t\tstrbuf_addstr(&buf2, pname + strlen(pname) - name_width - 3);\n+\t\t}\n \t} else {\n-\t\tpname = pprint_rename(file->from_name, file->name);\n+\t\tif (file->print_name) {\n+\t\t\tfree(file->print_name);\n+\t\t\tfile->print_name = NULL;\n+\t\t}\n+\t\tpname = pprint_rename(file->from_name, file->name, name_width);\n \t}\n \tfile->print_name = pname;\n }\n@@ -1517,7 +1655,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t\tcount++; /* not shown == room for one more */\n \t\t\tcontinue;\n \t\t}\n-\t\tfill_print_name(file);\n+\t\tfill_print_name(file, INT_MAX);\n \t\tlen = strlen(file->print_name);\n \t\tif (max_len < len)\n \t\t\tmax_len = len;\n@@ -1629,7 +1767,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tfor (i = 0; i < count; i++) {\n \t\tconst char *prefix = \"\";\n \t\tstruct diffstat_file *file = data->files[i];\n-\t\tchar *name = file->print_name;\n+\t\tchar *name;\n \t\tuintmax_t added = file->added;\n \t\tuintmax_t deleted = file->deleted;\n \t\tint name_len;\n@@ -1637,6 +1775,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tif (!file->is_interesting && (added + deleted == 0))\n \t\t\tcontinue;\n \n+\t\tfill_print_name(file, name_width);\n+\t\tname = file->print_name;\n \t\t/*\n \t\t * \"scale\" the filename\n \t\t */\n@@ -1772,7 +1912,7 @@ static void show_numstat(struct diffstat_t *data, struct diff_options *options)\n \t\t\t\t\"%\"PRIuMAX\"\\t%\"PRIuMAX\"\\t\",\n \t\t\t\tfile->added, file->deleted);\n \t\tif (options->line_termination) {\n-\t\t\tfill_print_name(file);\n+\t\t\tfill_print_name(file, INT_MAX);\n \t\t\tif (!file->is_renamed)\n \t\t\t\twrite_name_quoted(file->name, options->file,\n \t\t\t\t\t\t options->line_termination);\n@@ -4258,7 +4398,7 @@ static void show_mode_change(FILE *file, struct diff_filepair *p, int show_name,\n static void show_rename_copy(FILE *file, const char *renamecopy, struct diff_filepair *p,\n \t\t\tconst char *line_prefix)\n {\n-\tchar *names = pprint_rename(p->one->path, p->two->path);\n+\tchar *names = pprint_rename(p->one->path, p->two->path, INT_MAX);\n \n \tfprintf(file, \" %s %s (%d%%)\\n\", renamecopy, names, similarity_index(p));\n \tfree(names);\ndiff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh\nindex 2f327b7..f79b526 100755\n--- a/t/t4001-diff-rename.sh\n+++ b/t/t4001-diff-rename.sh\n@@ -156,4 +156,16 @@ test_expect_success 'rename pretty print common prefix and suffix overlap' '\n \ttest_i18ngrep \" d/f/{ => f}/e \" output\n '\n \n+test_expect_success 'rename of very long path shows =>' '\n+\tmkdir long_dirname_that_does_not_fit_in_a_single_line &&\n+\tmkdir another_extremely_long_path_but_not_the_same_as_the_first &&\n+\tcp path1 long_dirname*/ &&\n+\tgit add long_dirname*/path1 &&\n+\ttest_commit add_long_pathname &&\n+\tgit mv long_dirname*/path1 another_extremely_*/ &&\n+\ttest_commit move_long_pathname &&\n+\tgit diff -M --stat HEAD^ HEAD >output &&\n+\tgrep \"=>.*path1\" output\n+'\n+\n test_done\n-- \n1.8.4.475.g867697c\n"},{"id":"229148","messageId":"B690713F-6FF1-46A7-85A7-C92303BBAF0E@gmail.com","threadId":"35104","inReplyTo":"xmqqmwm71ysp.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v6] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-17T22:08:33Z","receivedAt":"2013-10-17T22:08:33Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"Hello Junio\n\nThank you very much for the reviewing.\nI try to fix the issues, and posted the updated patch as \"[PATCH v7]\".\n\n> I am not sure if distributing the burden of truncation equally to\n> three parts so that the resulting pieces are of similar lengths is\n> really a good idea. Between these two\n> \n> \t{...SourceDirectory => ...nationDirectory}...ileThatWasMoved \n> \t{...ceDirectory => ...ionDirectory}nameOfTheFileThatWasMoved\n> \n> that attempt to show that the file nameOfTheFileThatWasMoved was\n> moved from the longSourceDirectory to the DestinationDirectory, the\n> latter is much more informative, I would think.\nIn the \"[PATCH v7]\", I changed to keep filename part of suffix to handle\nabove case, but not always keep directory part because I feel totally\nkeeping all part of long suffix including directory name may cause output like:\n …{… => …}…ongPath1/LongPath2/nameOfTheFileThatWasMoved \nAnd, above may be worse than:\n ...{...ceDirectory => …ionDirectory}.../nameOfTheFileThatWasMoved\nI think.\n\nThank you !\n\n---\nTsuneo Yoshioka (吉岡 恒夫)\nyoshiokatsuneo@gmail.com\n\n\n\n\nOn Oct 17, 2013, at 10:29 PM, Junio C Hamano wrote:\n\n> Yoshioka Tsuneo writes:\n> \n>> \"git diff -M --stat\" can detect rename and show renamed file name like\n>> \"foofoofoo => barbarbar\".\n>> Before this commit, this output is shortened always by omitting left most\n>> part like \"...foo => barbarbar\". So, if the destination filename is too long,\n>> source filename putting left or arrow can be totally omitted like\n>> \"...barbarbar\", without including any of \"foofoofoo =>\".\n>> In such a case where arrow symbol is omitted, there is no way to know\n>> whether the file is renamed or existed in the original.\n>> Make sure there is always an arrow, like \"...foo => ...bar\".\n>> The output can contain curly braces('{','}') for grouping.\n>> So, in general, the output format is \"{ => }\"\n>> To keep arrow(\"=>\"), try to omit as long as possible at first\n>> because later part or changing part will be the more important part.\n>> If it is not enough, shorten , , and trying to\n>> have the maximum length the same because those will be equally important.\n> \n> I somehow find this solid wall of text extremely hard to\n> read. Adding a blank line as a paragraph break may make it easier to\n> read, perhaps.\n> \n> Also it is customary in our history to omit the full-stop from the\n> patch title on the Subject: line.\n> \n>> +\tname_len = pfx->len + a_mid->len + b_mid->len + sfx->len + strlen(arrow)\n>> +\t\t+ (use_curly_braces ? 2 : 0);\n>> +\n>> +\tif (name_len <= name_width) {\n>> +\t\t/* Everthing fits in name_width */\n>> +\t\treturn;\n>> +\t}\n> \n> Logic up to this point seems good; drop {} around a single statement\n> \"return;\", i.e.\n> \n> \tif (name_len <= name_width)\n> \treturn; /* everything fits */\n> \n>> +\t\t} else {\n>> +\t\t\tif (pfx->len > strlen(dots)) {\n>> +\t\t\t\t/*\n>> +\t\t\t\t * Just omitting left of '{' is not enough\n>> +\t\t\t\t * name will be \"...{SOMETHING}SOMETHING\"\n>> +\t\t\t\t */\n>> +\t\t\t\tstrbuf_reset(pfx);\n>> +\t\t\t\tstrbuf_addstr(pfx, dots);\n>> +\t\t\t}\n> \n> (mental note) ... otherwise, i.e. with a short common prefix, the\n> final result will be \"ab{SOMETHING}SOMETHING\", which is also fine\n> for the purpose of the remainder of this function.\n> \n>> +\t\t}\n>> +\t}\n>> +\n>> +\t/* available length for a_mid, b_mid and sfx */\n>> +\tlen = name_width - strlen(arrow) - (use_curly_braces ? 2 : 0);\n>> +\n>> +\t/* a_mid, b_mid, sfx will be have the same max, including ellipsis(\"...\"). */\n>> +\tpart_length[0] = a_mid->len;\n>> +\tpart_length[1] = b_mid->len;\n>> +\tpart_length[2] = sfx->len;\n>> +\n>> +\tqsort(part_length, sizeof(part_length)/sizeof(part_length[0]), sizeof(part_length[0])\n>> +\t\t , compare_size_t_descending_order);\n> \n> In our code, comma does not come at the beginning of continued\n> line.\n> \n>> +\tif (part_length[1] + part_length[1] + part_length[2] <= len) {\n>> +\t\t/*\n>> +\t\t * \"{...foofoo => barbar}file\"\n>> +\t\t * There is only one omitted part.\n>> +\t\t */\n>> +\t\tmax_part_len = len - part_length[1] - part_length[2];\n> \n> It would be clearer to explicitly set remainder to zero here, and\n> omit the initialization of the variable. That would make what the\n> three parts of if/elseif/else do more consistent.\n> \n>> +\t} else if (part_length[2] + part_length[2] + part_length[2] <= len) {\n>> +\t\t/*\n>> +\t\t * \"{...foofoo => ...barbar}file\"\n>> +\t\t * There are 2 omitted parts.\n>> +\t\t */\n>> +\t\tmax_part_len = (len - part_length[2]) / 2;\n>> +\t\tremainder_part_len = (len - part_length[2]) - max_part_len * 2;\n>> +\t} else {\n>> +\t\t/*\n>> +\t\t * \"{...ofoo => ...rbar}...file\"\n>> +\t\t * There are 3 omitted parts.\n>> +\t\t */\n>> +\t\tmax_part_len = len / 3;\n>> +\t\tremainder_part_len = len - (max_part_len) * 3;\n>> +\t}\n> \n> I am not sure if distributing the burden of truncation equally to\n> three parts so that the resulting pieces are of similar lengths is\n> really a good idea. Between these two\n> \n> \t{...SourceDirectory => ...nationDirectory}...ileThatWasMoved \n> \t{...ceDirectory => ...ionDirectory}nameOfTheFileThatWasMoved\n> \n> that attempt to show that the file nameOfTheFileThatWasMoved was\n> moved from the longSourceDirectory to the DestinationDirectory, the\n> latter is much more informative, I would think.\n> \n>> diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh\n>> index 2f327b7..03d6371 100755\n>> --- a/t/t4001-diff-rename.sh\n>> +++ b/t/t4001-diff-rename.sh\n>> @@ -156,4 +156,16 @@ test_expect_success 'rename pretty print common prefix and suffix overlap' '\n>> \ttest_i18ngrep \" d/f/{ => f}/e \" output\n>> '\n>> \n>> +test_expect_success 'rename of very long path shows =>' '\n>> +\tmkdir long_dirname_that_does_not_fit_in_a_single_line &&\n>> +\tmkdir another_extremely_long_path_but_not_the_same_as_the_first &&\n>> +\tcp path1 long_dirname*/ &&\n>> +\tgit add long_dirname*/path1 &&\n>> +\ttest_commit add_long_pathname &&\n>> +\tgit mv long_dirname*/path1 another_extremely_*/ &&\n>> +\ttest_commit move_long_pathname &&\n>> +\tgit diff -M --stat HEAD^ HEAD >output &&\n>> +\ttest_i18ngrep \"=>.*path1\" output\n> \n> Does this have to be i18ngrep? I had a feeling that we would not\n> want this part of the output localized, in which case \"grep\" may be\n> more appropriate.\n> \n>> +'\n>> +\n>> test_done\n"},{"id":"229152","messageId":"xmqqzjq7wmj7.fsf@gitster.dls.corp.google.com","threadId":"35104","inReplyTo":"B690713F-6FF1-46A7-85A7-C92303BBAF0E@gmail.com","subject":"Re: [PATCH v6] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-17T22:38:36Z","receivedAt":"2013-10-17T22:38:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yoshioka Tsuneo writes:\n\n> In the \"[PATCH v7]\", I changed to keep filename part of suffix to handle\n> above case, but not always keep directory part because I feel totally\n> keeping all part of long suffix including directory name may cause output like:\n> …{… => …}…ongPath1/LongPath2/nameOfTheFileThatWasMoved \n> And, above may be worse than:\n> ...{...ceDirectory => …ionDirectory}.../nameOfTheFileThatWasMoved\n> I think.\n\nI am not sure if I agree.\n\nLosing LongPath2 part may be more significant data loss than losing\na single bit that says the change is a rename, as the latter may not\nquite tell us what these two directories were anyway.\n"},{"id":"229160","messageId":"C876399C-9A78-4917-B0CF-D6519C7162F6@gmail.com","threadId":"35104","inReplyTo":"FB9897CC-EDC7-4EBB-8DAB-140CEB5F93B3@gmail.com","subject":"[PATCH v8] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-18T09:35:02Z","receivedAt":"2013-10-18T09:35:02Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"\n\n\"git diff -M --stat\" can detect rename and show renamed file name like\n\"foofoofoo => barbarbar\".\n\nBefore this commit, this output is shortened always by omitting left most\npart like \"...foo => barbarbar\". So, if the destination filename is too long,\nsource filename putting left or arrow can be totally omitted like\n\"...barbarbar\", without including any of \"foofoofoo =>\".\nIn such a case where arrow symbol is omitted, there is no way to know\nwhether the file is renamed or existed in the original.\n\nMake sure there is always an arrow, like \"...foo => ...bar\".\n\nThe output can contain curly braces('{','}') for grouping.\nSo, in general, the output format is \"{ => }\"\n\nTo keep arrow(\"=>\"), try to omit as long as possible at first\nbecause later part or changing part will be the more important part.\nIf it is not enough, shorten , trying to have the same\nmaximum length.\nIf it is not enough yet, omit .\n\nSigned-off-by: Tsuneo Yoshioka \nTest-added-by: Thomas Rast \n---\n diff.c | 176 ++++++++++++++++++++++++++++++++++++++++++-------\n t/t4001-diff-rename.sh | 12 ++++\n 2 files changed, 166 insertions(+), 22 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a04a34d..69c3e17 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1258,11 +1258,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t}\n }\n \n-static char *pprint_rename(const char *a, const char *b)\n+static void find_common_prefix_suffix(const char *a, const char *b,\n+\t\t\t\tstruct strbuf *pfx,\n+\t\t\t\tstruct strbuf *a_mid, struct strbuf *b_mid,\n+\t\t\t\tstruct strbuf *sfx)\n {\n \tconst char *old = a;\n \tconst char *new = b;\n-\tstruct strbuf name = STRBUF_INIT;\n \tint pfx_length, sfx_length;\n \tint pfx_adjust_for_slash;\n \tint len_a = strlen(a);\n@@ -1272,10 +1274,9 @@ static char *pprint_rename(const char *a, const char *b)\n \tint qlen_b = quote_c_style(b, NULL, NULL, 0);\n \n \tif (qlen_a || qlen_b) {\n-\t\tquote_c_style(a, &name, NULL, 0);\n-\t\tstrbuf_addstr(&name, \" => \");\n-\t\tquote_c_style(b, &name, NULL, 0);\n-\t\treturn strbuf_detach(&name, NULL);\n+\t\tquote_c_style(a, a_mid, NULL, 0);\n+\t\tquote_c_style(b, b_mid, NULL, 0);\n+\t\treturn;\n \t}\n \n \t/* Find common prefix */\n@@ -1322,17 +1323,138 @@ static char *pprint_rename(const char *a, const char *b)\n \tif (b_midlen < 0)\n \t\tb_midlen = 0;\n \n-\tstrbuf_grow(&name, pfx_length + a_midlen + b_midlen + sfx_length + 7);\n-\tif (pfx_length + sfx_length) {\n-\t\tstrbuf_add(&name, a, pfx_length);\n+\tstrbuf_add(pfx, a, pfx_length);\n+\tstrbuf_add(a_mid, a + pfx_length, a_midlen);\n+\tstrbuf_add(b_mid, b + pfx_length, b_midlen);\n+\tstrbuf_add(sfx, a + len_a - sfx_length, sfx_length);\n+}\n+\n+/*\n+ * Omit each parts to fix in name_width.\n+ * Formatted string is \"{ => }\".\n+ * At first, omit as long as possible.\n+ * If it is not enough, omit , trying to set the length of\n+ * those 2 parts(including \"...\") to the same.\n+ * If it is not enough yet, omit .\n+ * Ex:\n+ * \"foofoofoo => barbarbar\"\n+ * will be like\n+ * \"...foo => ...bar\".\n+ * \"long_parent{foofoofoo => barbarbar}path/filename\"\n+ * will be like\n+ * \"...parent{...foofoo => ...barbar}path/filename\"\n+ */\n+static void rename_omit(struct strbuf *pfx,\n+\t\t\t\tstruct strbuf *a_mid, struct strbuf *b_mid,\n+\t\t\t\tstruct strbuf *sfx,\n+\t\t\t\tint name_width)\n+{\n+\tstatic const char arrow[] = \" => \";\n+\tstatic const char dots[] = \"...\";\n+\tint use_curly_braces = (pfx->len > 0) || (sfx->len > 0);\n+\tsize_t name_len;\n+\tsize_t max_part_len = 0;\n+\tsize_t remainder_part_len = 0;\n+\tsize_t left, right;\n+\tsize_t max_sfx_len;\n+\tsize_t sfx_len;\n+\n+\tname_len = pfx->len + a_mid->len + b_mid->len + sfx->len + strlen(arrow)\n+\t\t+ (use_curly_braces ? 2 : 0);\n+\n+\tif (name_len <= name_width)\n+\t\treturn; /* everything fits in name_width */\n+\n+\tif (use_curly_braces) {\n+\t\tif (strlen(dots) + (name_len - pfx->len) <= name_width) {\n+\t\t\t/*\n+\t\t\t * Just omitting left of '{' is enough\n+\t\t\t * Ex: ...aaa{foofoofoo => bar}file\n+\t\t\t */\n+\t\t\tstrbuf_splice(pfx, 0, name_len - name_width + strlen(dots), dots, strlen(dots));\n+\t\t\treturn;\n+\t\t} else {\n+\t\t\tif (pfx->len > strlen(dots)) {\n+\t\t\t\t/*\n+\t\t\t\t * Just omitting left of '{' is not enough\n+\t\t\t\t * name will be \"...{SOMETHING}SOMETHING\"\n+\t\t\t\t */\n+\t\t\t\tstrbuf_reset(pfx);\n+\t\t\t\tstrbuf_addstr(pfx, dots);\n+\t\t\t}else{\n+\t\t\t\t/*\n+\t\t\t\t * If is shorter than dots(\"...\"),\n+\t\t\t\t * there is no sense to replace to dots\n+\t\t\t\t * but name will be just like \"a{SOMETHING}SOMETHING\".\n+\t\t\t\t */\n+\t\t\t\t;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tleft = 0;\n+\tright = name_width + 1;\n+\n+#define MIN(X, Y) ((X < Y) ? (X) : (Y))\n+#define MAX(X, Y) ((X > Y) ? (X) : (Y))\n+\n+\t/* In case other than is omitted like: \"...{... => ...}\" */\n+\tmax_sfx_len = name_width\n+\t\t- MIN(strlen(dots), pfx->len)\n+\t\t- MIN(strlen(dots), a_mid->len)\n+\t\t- MIN(strlen(dots), b_mid->len)\n+\t\t- strlen(arrow)\n+\t\t- (use_curly_braces ? 2 : 0);\n+\tsfx_len = MIN(sfx->len, max_sfx_len);\n+\n+\t/* binary search to find max_part_len(maximum length of omitted parts) */\n+\twhile(left + 1 < right){\n+\t\tsize_t mid = (left + right) / 2;\n+\n+\t\t/* length of \"{ => }\" */\n+\t\tsize_t l = pfx->len + MIN(mid, a_mid->len) + MIN(mid, b_mid->len) + sfx_len + strlen(arrow) + (use_curly_braces ? 2 : 0);\n+\t\tif(l <= name_width){\n+\t\t\tleft = mid;\n+\t\t\tremainder_part_len = name_width - l;\n+\t\t}else{\n+\t\t\tright = mid;\n+\t\t}\n+\t}\n+\tmax_part_len = left;\n+\n+\tif (max_part_len < strlen(dots))\n+\t\tmax_part_len = strlen(dots);\n+\tif (sfx->len > sfx_len)\n+\t\tstrbuf_splice(sfx, 0, sfx->len - sfx_len + strlen(dots), dots, strlen(dots));\n+\tif (remainder_part_len == 2)\n+\t\tmax_part_len++;\n+\tif (a_mid->len > max_part_len)\n+\t\tstrbuf_splice(a_mid, 0, a_mid->len - max_part_len + strlen(dots), dots, strlen(dots));\n+\tif (remainder_part_len == 1)\n+\t\tmax_part_len++;\n+\tif (b_mid->len > max_part_len)\n+\t\tstrbuf_splice(b_mid, 0, b_mid->len - max_part_len + strlen(dots), dots, strlen(dots));\n+}\n+\n+static char *pprint_rename(const char *a, const char *b, int name_width)\n+{\n+\tstruct strbuf pfx = STRBUF_INIT, a_mid = STRBUF_INIT, b_mid = STRBUF_INIT, sfx = STRBUF_INIT;\n+\tstruct strbuf name = STRBUF_INIT;\n+\n+\tfind_common_prefix_suffix(a, b, &pfx, &a_mid, &b_mid, &sfx);\n+\trename_omit(&pfx, &a_mid, &b_mid, &sfx, name_width);\n+\n+\tstrbuf_grow(&name, pfx.len + a_mid.len + b_mid.len + sfx.len + 7);\n+\tif (pfx.len + sfx.len) {\n+\t\tstrbuf_addbuf(&name, &pfx);\n \t\tstrbuf_addch(&name, '{');\n \t}\n-\tstrbuf_add(&name, a + pfx_length, a_midlen);\n+\tstrbuf_addbuf(&name, &a_mid);\n \tstrbuf_addstr(&name, \" => \");\n-\tstrbuf_add(&name, b + pfx_length, b_midlen);\n-\tif (pfx_length + sfx_length) {\n+\tstrbuf_addbuf(&name, &b_mid);\n+\tif (pfx.len + sfx.len) {\n \t\tstrbuf_addch(&name, '}');\n-\t\tstrbuf_add(&name, a + len_a - sfx_length, sfx_length);\n+\t\tstrbuf_addbuf(&name, &sfx);\n \t}\n \treturn strbuf_detach(&name, NULL);\n }\n@@ -1418,23 +1540,31 @@ static void show_graph(FILE *file, char ch, int cnt, const char *set, const char\n \tfprintf(file, \"%s\", reset);\n }\n \n-static void fill_print_name(struct diffstat_file *file)\n+static void fill_print_name(struct diffstat_file *file, int name_width)\n {\n \tchar *pname;\n \n-\tif (file->print_name)\n-\t\treturn;\n-\n \tif (!file->is_renamed) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tif (file->print_name)\n+\t\t\treturn;\n \t\tif (quote_c_style(file->name, &buf, NULL, 0)) {\n \t\t\tpname = strbuf_detach(&buf, NULL);\n \t\t} else {\n \t\t\tpname = file->name;\n \t\t\tstrbuf_release(&buf);\n \t\t}\n+\t\tif (strlen(pname) > name_width) {\n+\t\t\tstruct strbuf buf2 = STRBUF_INIT;\n+\t\t\tstrbuf_addstr(&buf2, \"...\");\n+\t\t\tstrbuf_addstr(&buf2, pname + strlen(pname) - name_width - 3);\n+\t\t}\n \t} else {\n-\t\tpname = pprint_rename(file->from_name, file->name);\n+\t\tif (file->print_name) {\n+\t\t\tfree(file->print_name);\n+\t\t\tfile->print_name = NULL;\n+\t\t}\n+\t\tpname = pprint_rename(file->from_name, file->name, name_width);\n \t}\n \tfile->print_name = pname;\n }\n@@ -1517,7 +1647,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t\tcount++; /* not shown == room for one more */\n \t\t\tcontinue;\n \t\t}\n-\t\tfill_print_name(file);\n+\t\tfill_print_name(file, INT_MAX);\n \t\tlen = strlen(file->print_name);\n \t\tif (max_len < len)\n \t\t\tmax_len = len;\n@@ -1629,7 +1759,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \tfor (i = 0; i < count; i++) {\n \t\tconst char *prefix = \"\";\n \t\tstruct diffstat_file *file = data->files[i];\n-\t\tchar *name = file->print_name;\n+\t\tchar *name;\n \t\tuintmax_t added = file->added;\n \t\tuintmax_t deleted = file->deleted;\n \t\tint name_len;\n@@ -1637,6 +1767,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\tif (!file->is_interesting && (added + deleted == 0))\n \t\t\tcontinue;\n \n+\t\tfill_print_name(file, name_width);\n+\t\tname = file->print_name;\n \t\t/*\n \t\t * \"scale\" the filename\n \t\t */\n@@ -1772,7 +1904,7 @@ static void show_numstat(struct diffstat_t *data, struct diff_options *options)\n \t\t\t\t\"%\"PRIuMAX\"\\t%\"PRIuMAX\"\\t\",\n \t\t\t\tfile->added, file->deleted);\n \t\tif (options->line_termination) {\n-\t\t\tfill_print_name(file);\n+\t\t\tfill_print_name(file, INT_MAX);\n \t\t\tif (!file->is_renamed)\n \t\t\t\twrite_name_quoted(file->name, options->file,\n \t\t\t\t\t\t options->line_termination);\n@@ -4258,7 +4390,7 @@ static void show_mode_change(FILE *file, struct diff_filepair *p, int show_name,\n static void show_rename_copy(FILE *file, const char *renamecopy, struct diff_filepair *p,\n \t\t\tconst char *line_prefix)\n {\n-\tchar *names = pprint_rename(p->one->path, p->two->path);\n+\tchar *names = pprint_rename(p->one->path, p->two->path, INT_MAX);\n \n \tfprintf(file, \" %s %s (%d%%)\\n\", renamecopy, names, similarity_index(p));\n \tfree(names);\ndiff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh\nindex 2f327b7..f79b526 100755\n--- a/t/t4001-diff-rename.sh\n+++ b/t/t4001-diff-rename.sh\n@@ -156,4 +156,16 @@ test_expect_success 'rename pretty print common prefix and suffix overlap' '\n \ttest_i18ngrep \" d/f/{ => f}/e \" output\n '\n \n+test_expect_success 'rename of very long path shows =>' '\n+\tmkdir long_dirname_that_does_not_fit_in_a_single_line &&\n+\tmkdir another_extremely_long_path_but_not_the_same_as_the_first &&\n+\tcp path1 long_dirname*/ &&\n+\tgit add long_dirname*/path1 &&\n+\ttest_commit add_long_pathname &&\n+\tgit mv long_dirname*/path1 another_extremely_*/ &&\n+\ttest_commit move_long_pathname &&\n+\tgit diff -M --stat HEAD^ HEAD >output &&\n+\tgrep \"=>.*path1\" output\n+'\n+\n test_done\n-- \n1.8.4.475.g867697c\n"},{"id":"229161","messageId":"21F30E1F-3497-41F2-81C4-F4193C58FE11@gmail.com","threadId":"35104","inReplyTo":"xmqqzjq7wmj7.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v6] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-18T09:35:08Z","receivedAt":"2013-10-18T09:35:08Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"Hello Junio\n\n>> In the \"[PATCH v7]\", I changed to keep filename part of suffix to handle\n>> above case, but not always keep directory part because I feel totally\n>> keeping all part of long suffix including directory name may cause output like:\n>> …{… => …}…ongPath1/LongPath2/nameOfTheFileThatWasMoved \n>> And, above may be worse than:\n>> ...{...ceDirectory => …ionDirectory}.../nameOfTheFileThatWasMoved\n>> I think.\n> \n> I am not sure if I agree.\n> \n> Losing LongPath2 part may be more significant data loss than losing\n> a single bit that says the change is a rename, as the latter may not\n> quite tell us what these two directories were anyway.\nI'm not sure which is the better in general.\nBut anyway, I don't have strong opinion about this.\nSo, I just changed to keep the all of the part(lator than '}').\nI just sent the updated patch as \"[PATCH v8]\".\n\nThanks !\n\n---\nTsuneo Yoshioka (吉岡 恒夫)\nyoshiokatsuneo@gmail.com\n\n\n\n\nOn Oct 18, 2013, at 1:38 AM, Junio C Hamano wrote:\n\n> Yoshioka Tsuneo writes:\n> \n>> In the \"[PATCH v7]\", I changed to keep filename part of suffix to handle\n>> above case, but not always keep directory part because I feel totally\n>> keeping all part of long suffix including directory name may cause output like:\n>> …{… => …}…ongPath1/LongPath2/nameOfTheFileThatWasMoved \n>> And, above may be worse than:\n>> ...{...ceDirectory => …ionDirectory}.../nameOfTheFileThatWasMoved\n>> I think.\n> \n> I am not sure if I agree.\n> \n> Losing LongPath2 part may be more significant data loss than losing\n> a single bit that says the change is a rename, as the latter may not\n> quite tell us what these two directories were anyway.\n"},{"id":"229206","messageId":"87mwm5vkue.fsf@linux-k42r.v.cablecom.net","threadId":"35104","inReplyTo":"C876399C-9A78-4917-B0CF-D6519C7162F6@gmail.com","subject":"Re: [PATCH v8] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2013-10-19T06:24:57Z","receivedAt":"2013-10-19T06:24:57Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Yoshioka Tsuneo writes:\n\n> \"git diff -M --stat\" can detect rename and show renamed file name like\n> \"foofoofoo => barbarbar\".\n>\n> Before this commit, this output is shortened always by omitting left most\n> part like \"...foo => barbarbar\". So, if the destination filename is too long,\n> source filename putting left or arrow can be totally omitted like\n> \"...barbarbar\", without including any of \"foofoofoo =>\".\n> In such a case where arrow symbol is omitted, there is no way to know\n> whether the file is renamed or existed in the original.\n>\n> Make sure there is always an arrow, like \"...foo => ...bar\".\n>\n> The output can contain curly braces('{','}') for grouping.\n> So, in general, the output format is \"{ => }\"\n>\n> To keep arrow(\"=>\"), try to omit as long as possible at first\n> because later part or changing part will be the more important part.\n> If it is not enough, shorten , trying to have the same\n> maximum length.\n> If it is not enough yet, omit .\n>\n> Signed-off-by: Tsuneo Yoshioka \n> Test-added-by: Thomas Rast \n> ---\n\nCan you briefly describe what you changed in v7 and v8, both compared to\nearlier versions and between v7 and v8?\n\nIt would be very nice if you could always include such a \"patch\nchangelog\" after the \"---\" above. git-am will ignore the text between\n\"---\" and the diff, so you can write comments for the reviewers there\nwithout creating noise in the commit message.\n\nAlso, please keep reviewers in the Cc list for future discussion/patches\nso that they will see them.\n\n-- \nThomas Rast\ntr@thomasrast.ch\n"},{"id":"229229","messageId":"BB9AEFCE-0E64-4EAA-8DEA-9A8125B8C553@gmail.com","threadId":"35104","inReplyTo":"87mwm5vkue.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH v8] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-20T01:49:14Z","receivedAt":"2013-10-20T01:49:14Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"Hello Thomas\n\n> Can you briefly describe what you changed in v7 and v8, both compared to\n> earlier versions and between v7 and v8?\nOn v7, 's basename part is tried to kept. On v7, whole part is tried to kept.\nFor example, in case below:\n parent_path{sourceDirectory => DestinationDirectory}path1/path2//longlongFilename.txt\n On v7, this can be like:\n …{...ceDirectory => …onDirectory}.../longlongFilename.txt\nOn v8, it will be like:\n …{...irectory => …irectory}path1/path2/longlongFilename.txt\n\n\nThis change is based on the review from Junio below.\n(I myself is not sure what is the better way.)\n================================\nOn Oct 17, 2013, at 10:29 PM, Junio C Hamano wrote:\n> I am not sure if distributing the burden of truncation equally to\n> three parts so that the resulting pieces are of similar lengths is\n> really a good idea. Between these two\n> \n> \t{...SourceDirectory => ...nationDirectory}...ileThatWasMoved \n> \t{...ceDirectory => ...ionDirectory}nameOfTheFileThatWasMoved\n> \n> that attempt to show that the file nameOfTheFileThatWasMoved was\n> moved from the longSourceDirectory to the DestinationDirectory, the\n> latter is much more informative, I would think.\n\n\nOn Oct 18, 2013, at 1:38 AM, Junio C Hamano wrote:\n> Yoshioka Tsuneo writes:\n> \n>> In the \"[PATCH v7]\", I changed to keep filename part of suffix to handle\n>> above case, but not always keep directory part because I feel totally\n>> keeping all part of long suffix including directory name may cause output like:\n>> …{… => …}…ongPath1/LongPath2/nameOfTheFileThatWasMoved \n>> And, above may be worse than:\n>> ...{...ceDirectory => …ionDirectory}.../nameOfTheFileThatWasMoved\n>> I think.\n> \n> I am not sure if I agree.\n> \n> Losing LongPath2 part may be more significant data loss than losing\n> a single bit that says the change is a rename, as the latter may not\n> quite tell us what these two directories were anyway.\n================================\n\nAlso, I guess Junio might be suspicious to the idea to keep arrow(\"=>\") itself, maybe ?\n=================================\n(From What's cooking in git.git (Oct 2013, #04; Fri, 18))\n- diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible\n\nAttempts to give more weight on the fact that a filepair represents\na rename than showing substring of the actual path when diffstat\nlines are not wide enough.\n\nI am not sure if that is solving a right problem, though.\n=================================\n\nThanks!\n\n---\nTsuneo Yoshioka (吉岡 恒夫)\nyoshiokatsuneo@gmail.com\n\n\n\n\nOn Oct 19, 2013, at 9:24 AM, Thomas Rast wrote:\n\n> Yoshioka Tsuneo writes:\n> \n>> \"git diff -M --stat\" can detect rename and show renamed file name like\n>> \"foofoofoo => barbarbar\".\n>> \n>> Before this commit, this output is shortened always by omitting left most\n>> part like \"...foo => barbarbar\". So, if the destination filename is too long,\n>> source filename putting left or arrow can be totally omitted like\n>> \"...barbarbar\", without including any of \"foofoofoo =>\".\n>> In such a case where arrow symbol is omitted, there is no way to know\n>> whether the file is renamed or existed in the original.\n>> \n>> Make sure there is always an arrow, like \"...foo => ...bar\".\n>> \n>> The output can contain curly braces('{','}') for grouping.\n>> So, in general, the output format is \"{ => }\"\n>> \n>> To keep arrow(\"=>\"), try to omit as long as possible at first\n>> because later part or changing part will be the more important part.\n>> If it is not enough, shorten , trying to have the same\n>> maximum length.\n>> If it is not enough yet, omit .\n>> \n>> Signed-off-by: Tsuneo Yoshioka \n>> Test-added-by: Thomas Rast \n>> ---\n> \n> Can you briefly describe what you changed in v7 and v8, both compared to\n> earlier versions and between v7 and v8?\n> \n> It would be very nice if you could always include such a \"patch\n> changelog\" after the \"---\" above. git-am will ignore the text between\n> \"---\" and the diff, so you can write comments for the reviewers there\n> without creating noise in the commit message.\n> \n> Also, please keep reviewers in the Cc list for future discussion/patches\n> so that they will see them.\n> \n> -- \n> Thomas Rast\n> tr@thomasrast.ch\n"},{"id":"229296","messageId":"xmqqob6htbx9.fsf@gitster.dls.corp.google.com","threadId":"35104","inReplyTo":"BB9AEFCE-0E64-4EAA-8DEA-9A8125B8C553@gmail.com","subject":"Re: [PATCH v8] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-22T18:09:38Z","receivedAt":"2013-10-22T18:09:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yoshioka Tsuneo writes:\n\n> Also, I guess Junio might be suspicious to the idea to keep arrow(\"=>\") itself, maybe ?\n\nI think there is no single \"right\" solution to this issue, and it\nhas to boils down to the taste.\n\nWhen you are viewing \"diff --stat -M\" output in wide-enough medium,\nyou are seeing three pieces of information: what the source path\nwas, what the destination path will be, and what amount of change is\nmade with the change. When the output width is too narrow to show\nthese paths, with the current code, you see truncated destination\npath, possibly without the source path, but this patch will show the\nsource and the destination paths, both of which are truncated even\nmore severely, because it always has to spend display columns for an\nextra \"...\" (to show truncation of the source side), \" => \" (to show\nthat it is a rename), and <\"{\",\"}\"> pair (again to show that it is a\nrename). If the destination does not fit, the output before this\npatch would have thrown these away as part of left-truncation, to\nshow the destination path as maximally as possible. We do not have\neven half the width of the current \"truncated to be destination\nonly\" output for each path.\n\nI am afraid that in the cases where the patch makes a difference,\nwhat happens would be that you can no longer tell what source or\ndestination paths really are, because the leading directory part\ngets truncated too much, and if we didn't have this patch, at least\nyou can tell what destination path is affected. We would trade the\nguessability of at least one path (the destination) with just a\nsingle bit of information (an unidentifiable path got renamed to\nanother unidentifiable path).\n\nI am not yet convinced that it is a good trade-off. Especially\ngiven the diffstat output is not about files but more about\ncontents, between an output in the extreme case the version after\nthe patch needs to produce\n\n\t{... => ...}/controller/Makefile | 7 +++++++\n\nthat tells us \"7 lines were updated in the procedure to build some\nunknown controller by copying or renaming from the build procedure\nof some other unknown controller\", and the output the current code\nwould give to the same rename\n\n\t.}/fooGadget/controller/Makefile | 7 +++++++\n \nthat tells us \"7 lines were updated in the build procedure for the\nfoo Gadget\", I think the latter contains more useful information,\neven though it does lose one bit of information (\"there was a rename\ninvolved in producing this final path\") compared to the version with\nthe patch.\n\nSo you are correct to say that I am still skeptical.\n\nIn any case, the output from \"diff --stat -M\" should match the\noutput from \"apply --stat -M\", I think.\n"},{"id":"229303","messageId":"2CB6100D-747E-4F65-8F73-7BA381AC4BD4@gmail.com","threadId":"35104","inReplyTo":"xmqqob6htbx9.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v8] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible","fromName":"Yoshioka Tsuneo","fromEmail":"yoshiokatsuneo@gmail.com","sentAt":"2013-10-22T20:14:46Z","receivedAt":"2013-10-22T20:14:46Z","isPatch":true,"sender":{"key":"yoshiokatsuneo@gmail.com","avatar":"https://gravatar.com/avatar/1a1dddcd048d4e847e125557e79d2cad1cbe84a9775bb0298a24a6bf687aa040?d=mp&s=160"},"body":"Hello Junio\n\nThank you for your comment.\n\n> but this patch will show the\n> source and the destination paths, both of which are truncated even\n> more severely, because it always has to spend display columns for an\n> extra \"...\" (to show truncation of the source side), \" => \" (to show\n> that it is a rename), and <\"{\",\"}\"> pair (again to show that it is a\n> rename). \nTo be more accurate, renaming output dose not always contains \"{\" or \"}\"\nif there is no common part in source and destination paths, although\nprobably there are enough large possibility to include \"{\" or \"}\".\nAnd, in the original patch, \"{\" or \"}\" is not kept, but changed to be kept\nbased Thomas Rast's feedback below.\n(So, there was no possibility to have \"{… => …}\" in the original patch.)\n\nOn Oct 13, 2013, at 11:29 PM, Thomas Rast wrote:\n> Note that in the test, the generated line looks like this:\n> \n> {..._does_not_fit_in_a_single_line => .../path1 | 0\n> \n> I don't want to go all bikesheddey, but I think it's somewhat\n> unfortunate that the elided parts do not correspond to each other. In\n> particular, I think the closing brace should not be omitted. Perhaps\n> something like this would be ideal (making it up on the spot, don't\n> count characters):\n> \n> {...a_single_line => ..._as_the_first}/path1 | 0\n\n\n\nAnd, it might be a bit nicer for me if the patch can be rejected(or ignored as other patches)\nfrom the beginning if the concept does not fit anyway.\n# Though I know we can know more after seeing the implementation, anyway :-)\n# And, my original explanation about the patch might be not enough.\n\nThanks !\n\n---\nTsuneo Yoshioka (吉岡 恒夫)\nyoshiokatsuneo@gmail.com\n\n\n\n\nOn Oct 22, 2013, at 9:09 PM, Junio C Hamano wrote:\n\n> Yoshioka Tsuneo writes:\n> \n>> Also, I guess Junio might be suspicious to the idea to keep arrow(\"=>\") itself, maybe ?\n> \n> I think there is no single \"right\" solution to this issue, and it\n> has to boils down to the taste.\n> \n> When you are viewing \"diff --stat -M\" output in wide-enough medium,\n> you are seeing three pieces of information: what the source path\n> was, what the destination path will be, and what amount of change is\n> made with the change. When the output width is too narrow to show\n> these paths, with the current code, you see truncated destination\n> path, possibly without the source path, but this patch will show the\n> source and the destination paths, both of which are truncated even\n> more severely, because it always has to spend display columns for an\n> extra \"...\" (to show truncation of the source side), \" => \" (to show\n> that it is a rename), and <\"{\",\"}\"> pair (again to show that it is a\n> rename). If the destination does not fit, the output before this\n> patch would have thrown these away as part of left-truncation, to\n> show the destination path as maximally as possible. We do not have\n> even half the width of the current \"truncated to be destination\n> only\" output for each path.\n> \n> I am afraid that in the cases where the patch makes a difference,\n> what happens would be that you can no longer tell what source or\n> destination paths really are, because the leading directory part\n> gets truncated too much, and if we didn't have this patch, at least\n> you can tell what destination path is affected. We would trade the\n> guessability of at least one path (the destination) with just a\n> single bit of information (an unidentifiable path got renamed to\n> another unidentifiable path).\n> \n> I am not yet convinced that it is a good trade-off. Especially\n> given the diffstat output is not about files but more about\n> contents, between an output in the extreme case the version after\n> the patch needs to produce\n> \n> \t{... => ...}/controller/Makefile | 7 +++++++\n> \n> that tells us \"7 lines were updated in the procedure to build some\n> unknown controller by copying or renaming from the build procedure\n> of some other unknown controller\", and the output the current code\n> would give to the same rename\n> \n> \t.}/fooGadget/controller/Makefile | 7 +++++++\n> \n> that tells us \"7 lines were updated in the build procedure for the\n> foo Gadget\", I think the latter contains more useful information,\n> even though it does lose one bit of information (\"there was a rename\n> involved in producing this final path\") compared to the version with\n> the patch.\n> \n> So you are correct to say that I am still skeptical.\n> \n> In any case, the output from \"diff --stat -M\" should match the\n> output from \"apply --stat -M\", I think.\n"},{"id":"229304","messageId":"xmqqtxg9rr1h.fsf@gitster.dls.corp.google.com","threadId":"35104","inReplyTo":"2CB6100D-747E-4F65-8F73-7BA381AC4BD4@gmail.com","subject":"Re: [PATCH v8] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-22T20:26:01Z","receivedAt":"2013-10-22T20:26:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yoshioka Tsuneo writes:\n\n> And, it might be a bit nicer for me if the patch can be\n> rejected(or ignored as other patches) from the beginning if the\n> concept does not fit anyway.\n\nYes, but...\n\n> # Though I know we can know more after seeing the implementation, anyway :-)\n\n... you are very correct about this.\n\nNote that I am not rejecting the topic yet. I am just saying that I\nam not yet convinced the patch improves the situation where an\noptimal solution (i.e. no information loss at all) cannot exist\nbecause we do not have enough output columns to work with.\n\nThanks.\n\n>> ...\n>> So you are correct to say that I am still skeptical.\n>> \n>> In any case, the output from \"diff --stat -M\" should match the\n>> output from \"apply --stat -M\", I think.\n"}]}