{"thread":{"id":"47486","subject":"[BUG] File move with `add -N` shows as rename to same name","startedAt":"2017-12-23T02:51:05Z","lastAt":"2018-01-10T09:26:59Z","messageCount":37,"participants":["Alex Vandiver","Duy Nguyen","Nguyễn Thái Ngọc Duy","Igor Djordjevic","Torsten Bögershausen","Junio C Hamano","Jeff Hostetler"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"335216","messageId":"alpine.DEB.2.10.1712221829430.29746@alexmv-linux","threadId":"47486","inReplyTo":null,"subject":"[BUG] File move with `add -N` shows as rename to same name","fromName":"Alex Vandiver","fromEmail":"alexmv@dropbox.com","sentAt":"2017-12-23T02:42:21Z","receivedAt":"2017-12-23T02:51:05Z","isPatch":false,"sender":{"key":"alexmv@dropbox.com","avatar":null},"body":"I just stumbled across the following oddity:\n\n mv tracked-file new-path\n git add -N new-path\n git status\n\n..shows:\n\n On branch master\n Changes not staged for commit:\n (use \"git add ...\" to update what will be committed)\n (use \"git checkout -- ...\" to discard changes in working directory)\n\n renamed: tracked-file -> tracked-file\n\n no changes added to commit (use \"git add\" and/or \"git commit -a\")\n\n\nBisect points at 425a28e0a (\"diff-lib: allow ita entries treated as\n\"not yet exist in index\"\", 2016-10-24), but I don't have enough\ncontext to suggest the right fix.\n\nFailing test is included below.\n - Alex\n\n--------------------8<--------------------\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex 1bdf38e80..97b6c0f05 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -150,5 +150,19 @@ test_expect_success 'commit: ita entries ignored in empty commit check' '\n \t)\n '\n \n+test_expect_success 'rename detection finds the right names' '\n+\tgit init rename-detection &&\n+\t(\n+\t\tcd rename-detection &&\n+\t\techo contents > original-file\n+\t\tgit add original-file\n+\t\tgit commit -m first-commit\n+\t\tmv original-file new-file\n+\t\tgit add -N new-file\n+\t\tgit status --porcelain > actual\n+\t\tgrep new-file actual\n+\t)\n+'\n+\n test_done\n \n--------------------8<--------------------\n"},{"id":"335278","messageId":"CACsJy8A=jZ9LAuM50GVjNT5gtdiYYMyMuPBSrJFO4LmKVQsETQ@mail.gmail.com","threadId":"47486","inReplyTo":"alpine.DEB.2.10.1712221829430.29746@alexmv-linux","subject":"Re: [BUG] File move with `add -N` shows as rename to same name","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-25T09:00:26Z","receivedAt":"2017-12-25T09:01:02Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Dec 23, 2017 at 9:42 AM, Alex Vandiver wrote:\n> I just stumbled across the following oddity:\n\nThanks. I'm looking into it.\n-- \nDuy\n"},{"id":"335282","messageId":"20171225103718.24443-1-pclouds@gmail.com","threadId":"47486","inReplyTo":"alpine.DEB.2.10.1712221829430.29746@alexmv-linux","subject":"[PATCH] status: handle worktree renames","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-25T10:37:18Z","receivedAt":"2017-12-25T10:37:30Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Before 425a28e0a4 (diff-lib: allow ita entries treated as \"not yet exist\nin index\" - 2016-10-24) there are never \"new files\" in the index, which\nessentially disables rename detection because we only detect renames\nwhen a new file appears in a diff pair.\n\nAfter that commit, an i-t-a entry can appear as a new file in \"git\ndiff-files\". But the diff callback function in wt-status.c does not\nhandle this case and produces incorrect status output.\n\nHandle this rename case. While at there make sure unhandled diff status\ncode is reported to catch cases like this easier in the future.\n\nThe reader may notice that this patch adds a new xstrdup() but not a\nfree(). Yes we leak memory (the same for head_path). But wt_status so\nfar has been short lived, this leak should not matter in practice.\n\nNoticed-by: Alex Vandiver \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n t/t2203-add-intent.sh | 15 +++++++++++++++\n wt-status.c | 24 +++++++++++++++++++-----\n wt-status.h | 1 +\n 3 files changed, 35 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex 1bdf38e80d..41a8874e60 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -150,5 +150,20 @@ test_expect_success 'commit: ita entries ignored in empty commit check' '\n \t)\n '\n \n+test_expect_success 'rename detection finds the right names' '\n+\tgit init rename-detection &&\n+\t(\n+\t\tcd rename-detection &&\n+\t\techo contents > original-file &&\n+\t\tgit add original-file &&\n+\t\tgit commit -m first-commit &&\n+\t\tmv original-file new-file &&\n+\t\tgit add -N new-file &&\n+\t\tgit status --porcelain | grep -v actual >actual &&\n+\t\techo \" R original-file -> new-file\" >expected &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n test_done\n \ndiff --git a/wt-status.c b/wt-status.c\nindex ef26f07446..f0b5b3d46a 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -376,6 +376,8 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n \t\t\tstrbuf_addch(&extra, ')');\n \t\t}\n \t\tstatus = d->worktree_status;\n+\t\tif (d->worktree_path)\n+\t\t\tone_name = d->worktree_path;\n \t\tbreak;\n \tdefault:\n \t\tdie(\"BUG: unhandled change_type %d in wt_longstatus_print_change_data\",\n@@ -432,7 +434,7 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\tstruct wt_status_change_data *d;\n \n \t\tp = q->queue[i];\n-\t\tit = string_list_insert(&s->change, p->one->path);\n+\t\tit = string_list_insert(&s->change, p->two->path);\n \t\td = it->util;\n \t\tif (!d) {\n \t\t\td = xcalloc(1, sizeof(*d));\n@@ -459,6 +461,12 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\t\t/* mode_worktree is zero for a delete. */\n \t\t\tbreak;\n \n+\t\tcase DIFF_STATUS_COPIED:\n+\t\tcase DIFF_STATUS_RENAMED:\n+\t\t\td->worktree_path = xstrdup(p->one->path);\n+\t\t\td->score = p->score * 100 / MAX_SCORE;\n+\t\t\t/* fallthru */\n+\n \t\tcase DIFF_STATUS_MODIFIED:\n \t\tcase DIFF_STATUS_TYPE_CHANGED:\n \t\tcase DIFF_STATUS_UNMERGED:\n@@ -467,8 +475,8 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\t\toidcpy(&d->oid_index, &p->one->oid);\n \t\t\tbreak;\n \n-\t\tcase DIFF_STATUS_UNKNOWN:\n-\t\t\tdie(\"BUG: worktree status unknown???\");\n+\t\tdefault:\n+\t\t\tdie(\"BUG: unhandled worktree status '%c'\", p->status);\n \t\t\tbreak;\n \t\t}\n \n@@ -548,6 +556,10 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,\n \t\t\t * values in these fields.\n \t\t\t */\n \t\t\tbreak;\n+\n+\t\tdefault:\n+\t\t\tdie(\"BUG: unhandled worktree status '%c'\", p->status);\n+\t\t\tbreak;\n \t\t}\n \t}\n }\n@@ -1724,8 +1736,10 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \t} else {\n \t\tstruct strbuf onebuf = STRBUF_INIT;\n \t\tconst char *one;\n-\t\tif (d->head_path) {\n-\t\t\tone = quote_path(d->head_path, s->prefix, &onebuf);\n+\n+\t\tone = d->head_path ? d->head_path : d->worktree_path;\n+\t\tif (one) {\n+\t\t\tone = quote_path(one, s->prefix, &onebuf);\n \t\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n \t\t\t\tputchar('\"');\n \t\t\t\tstrbuf_addch(&onebuf, '\"');\ndiff --git a/wt-status.h b/wt-status.h\nindex fe27b465e2..572a720123 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -48,6 +48,7 @@ struct wt_status_change_data {\n \tint mode_head, mode_index, mode_worktree;\n \tstruct object_id oid_head, oid_index;\n \tchar *head_path;\n+\tchar *worktree_path;\n \tunsigned dirty_submodule : 2;\n \tunsigned new_submodule_commits : 1;\n };\n-- \n2.15.0.320.g0453912d77\n\n"},{"id":"335298","messageId":"b3e90960-d743-3299-ba43-150849b591d2@gmail.com","threadId":"47486","inReplyTo":"20171225103718.24443-1-pclouds@gmail.com","subject":"Re: [PATCH] status: handle worktree renames","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2017-12-25T18:26:27Z","receivedAt":"2017-12-25T18:26:46Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"Hi Duy,\n\nOn 25/12/2017 11:37, Nguyễn Thái Ngọc Duy wrote:\n> Before 425a28e0a4 (diff-lib: allow ita entries treated as \"not yet exist\n> in index\" - 2016-10-24) there are never \"new files\" in the index, which\n> essentially disables rename detection because we only detect renames\n> when a new file appears in a diff pair.\n> \n> After that commit, an i-t-a entry can appear as a new file in \"git\n> diff-files\". But the diff callback function in wt-status.c does not\n> handle this case and produces incorrect status output.\n> \n> Handle this rename case. While at there make sure unhandled diff status\n> code is reported to catch cases like this easier in the future.\n> \n> The reader may notice that this patch adds a new xstrdup() but not a\n> free(). Yes we leak memory (the same for head_path). But wt_status so\n> far has been short lived, this leak should not matter in practice.\n> \n> Noticed-by: Alex Vandiver \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> t/t2203-add-intent.sh | 15 +++++++++++++++\n> wt-status.c | 24 +++++++++++++++++++-----\n> wt-status.h | 1 +\n> 3 files changed, 35 insertions(+), 5 deletions(-)\n> \n> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\n> index 1bdf38e80d..41a8874e60 100755\n> --- a/t/t2203-add-intent.sh\n> +++ b/t/t2203-add-intent.sh\n> @@ -150,5 +150,20 @@ test_expect_success 'commit: ita entries ignored in empty commit check' '\n> \t)\n> '\n> \n> +test_expect_success 'rename detection finds the right names' '\n> +\tgit init rename-detection &&\n> +\t(\n> +\t\tcd rename-detection &&\n> +\t\techo contents > original-file &&\n> +\t\tgit add original-file &&\n> +\t\tgit commit -m first-commit &&\n> +\t\tmv original-file new-file &&\n> +\t\tgit add -N new-file &&\n> +\t\tgit status --porcelain | grep -v actual >actual &&\n> +\t\techo \" R original-file -> new-file\" >expected &&\n> +\t\ttest_cmp expected actual\n> +\t)\n> +'\n> +\n> test_done\n> \n> diff --git a/wt-status.c b/wt-status.c\n> index ef26f07446..f0b5b3d46a 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -376,6 +376,8 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n> \t\t\tstrbuf_addch(&extra, ')');\n> \t\t}\n> \t\tstatus = d->worktree_status;\n> +\t\tif (d->worktree_path)\n> +\t\t\tone_name = d->worktree_path;\n> \t\tbreak;\n> \tdefault:\n> \t\tdie(\"BUG: unhandled change_type %d in wt_longstatus_print_change_data\",\n> @@ -432,7 +434,7 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n> \t\tstruct wt_status_change_data *d;\n> \n> \t\tp = q->queue[i];\n> -\t\tit = string_list_insert(&s->change, p->one->path);\n> +\t\tit = string_list_insert(&s->change, p->two->path);\n> \t\td = it->util;\n> \t\tif (!d) {\n> \t\t\td = xcalloc(1, sizeof(*d));\n> @@ -459,6 +461,12 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n> \t\t\t/* mode_worktree is zero for a delete. */\n> \t\t\tbreak;\n> \n> +\t\tcase DIFF_STATUS_COPIED:\n> +\t\tcase DIFF_STATUS_RENAMED:\n> +\t\t\td->worktree_path = xstrdup(p->one->path);\n> +\t\t\td->score = p->score * 100 / MAX_SCORE;\n> +\t\t\t/* fallthru */\n> +\n> \t\tcase DIFF_STATUS_MODIFIED:\n> \t\tcase DIFF_STATUS_TYPE_CHANGED:\n> \t\tcase DIFF_STATUS_UNMERGED:\n> @@ -467,8 +475,8 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n> \t\t\toidcpy(&d->oid_index, &p->one->oid);\n> \t\t\tbreak;\n> \n> -\t\tcase DIFF_STATUS_UNKNOWN:\n> -\t\t\tdie(\"BUG: worktree status unknown???\");\n> +\t\tdefault:\n> +\t\t\tdie(\"BUG: unhandled worktree status '%c'\", p->status);\n> \t\t\tbreak;\n> \t\t}\n> \n> @@ -548,6 +556,10 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,\n> \t\t\t * values in these fields.\n> \t\t\t */\n> \t\t\tbreak;\n> +\n> +\t\tdefault:\n> +\t\t\tdie(\"BUG: unhandled worktree status '%c'\", p->status);\n> +\t\t\tbreak;\n> \t\t}\n> \t}\n> }\n> @@ -1724,8 +1736,10 @@ static void wt_shortstatus_status(struct string_list_item *it,\n> \t} else {\n> \t\tstruct strbuf onebuf = STRBUF_INIT;\n> \t\tconst char *one;\n> -\t\tif (d->head_path) {\n> -\t\t\tone = quote_path(d->head_path, s->prefix, &onebuf);\n> +\n> +\t\tone = d->head_path ? d->head_path : d->worktree_path;\n> +\t\tif (one) {\n> +\t\t\tone = quote_path(one, s->prefix, &onebuf);\n> \t\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n> \t\t\t\tputchar('\"');\n> \t\t\t\tstrbuf_addch(&onebuf, '\"');\n> diff --git a/wt-status.h b/wt-status.h\n> index fe27b465e2..572a720123 100644\n> --- a/wt-status.h\n> +++ b/wt-status.h\n> @@ -48,6 +48,7 @@ struct wt_status_change_data {\n> \tint mode_head, mode_index, mode_worktree;\n> \tstruct object_id oid_head, oid_index;\n> \tchar *head_path;\n> +\tchar *worktree_path;\n> \tunsigned dirty_submodule : 2;\n> \tunsigned new_submodule_commits : 1;\n> };\n> \n\nThanks, I`ve tested it and the reported case seems to work correctly, \nindeed.\n\nBut I`ve noticed that \"--porcelain=v2\" output might still be buggy - \nthis is what having both files staged shows:\n\n $ git status --porcelain=v2\n 2 R. N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 12f00e90b6ef79117ce6e650416b8cf517099b78 R100 new-file\toriginal-file\n\n..., where having old/deleted file unstaged, and new/created file \nstaged with `git add -N` shows this:\n\n $ git status --porcelain=v2\n 1 .R N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 12f00e90b6ef79117ce6e650416b8cf517099b78 new-file\n\nSo even though unstaged value is correctly recognized as \"R\" (renamed), \nfirst number is \"1\" (instead of \"2\" to signal rename/copy), and both \nrename score and original file name are missing.\n\nNot sure if this is a bug, but it seems so, as `git status` \"Porcelain \nFormat Version 2\"[1] says the last path is \"pathname in the commit at \nHEAD\" (in case of copy/rename), which is missing here.\n\nRegards, Buga\n\n[1] https://git-scm.com/docs/git-status#_porcelain_format_version_2\n"},{"id":"335300","messageId":"86b4d2af-18ce-36eb-4823-105757a196fc@gmail.com","threadId":"47486","inReplyTo":"b3e90960-d743-3299-ba43-150849b591d2@gmail.com","subject":"Re: [PATCH] status: handle worktree renames","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2017-12-25T19:45:15Z","receivedAt":"2017-12-25T19:45:34Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"On 25/12/2017 19:26, Igor Djordjevic wrote:\n> \n> But I`ve noticed that \"--porcelain=v2\" output might still be buggy - \n> this is what having both files staged shows:\n> \n> $ git status --porcelain=v2\n> 2 R. N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 12f00e90b6ef79117ce6e650416b8cf517099b78 R100 new-file\toriginal-file\n> \n> ..., where having old/deleted file unstaged, and new/created file \n> staged with `git add -N` shows this:\n> \n> $ git status --porcelain=v2\n> 1 .R N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 12f00e90b6ef79117ce6e650416b8cf517099b78 new-file\n> \n> So even though unstaged value is correctly recognized as \"R\" (renamed), \n> first number is \"1\" (instead of \"2\" to signal rename/copy), and both \n> rename score and original file name are missing.\n\nAs an exercise, might be something like this as a fixup on top of \nyour patch could work.\n\nI`ve tried to follow your lead on what you did yourself, but please \nnote that, besides being relatively new to Git codebase, this is my \nfirst C code for almost 10 years (since university), so... :)\n\nI guess an additional test for this would be good, too.\n\nRegards, Buga\n\n---\n wt-status.c | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex f0b5b3d46..55c0ad249 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -2050,7 +2050,7 @@ static void wt_porcelain_v2_print_changed_entry(\n \tconst char *path_head = NULL;\n \tchar key[3];\n \tchar submodule_token[5];\n-\tchar sep_char, eol_char;\n+\tchar sep_char, eol_char, score_char;\n \n \twt_porcelain_v2_fix_up_changed(it, s);\n \twt_porcelain_v2_submodule_state(d, submodule_token);\n@@ -2059,6 +2059,8 @@ static void wt_porcelain_v2_print_changed_entry(\n \tkey[1] = d->worktree_status ? d->worktree_status : '.';\n \tkey[2] = 0;\n \n+\tpath_head = d->head_path ? d->head_path : d->worktree_path;\n+\tscore_char = d->index_status ? key[0] : key[1];\n \tif (s->null_termination) {\n \t\t/*\n \t\t * In -z mode, we DO NOT C-quote pathnames. Current path is ALWAYS first.\n@@ -2067,7 +2069,6 @@ static void wt_porcelain_v2_print_changed_entry(\n \t\tsep_char = '\\0';\n \t\teol_char = '\\0';\n \t\tpath_index = it->string;\n-\t\tpath_head = d->head_path;\n \t} else {\n \t\t/*\n \t\t * Path(s) are C-quoted if necessary. Current path is ALWAYS first.\n@@ -2078,8 +2079,8 @@ static void wt_porcelain_v2_print_changed_entry(\n \t\tsep_char = '\\t';\n \t\teol_char = '\\n';\n \t\tpath_index = quote_path(it->string, s->prefix, &buf_index);\n-\t\tif (d->head_path)\n-\t\t\tpath_head = quote_path(d->head_path, s->prefix, &buf_head);\n+\t\tif (path_head)\n+\t\t\tpath_head = quote_path(path_head, s->prefix, &buf_head);\n \t}\n \n \tif (path_head)\n@@ -2087,7 +2088,7 @@ static void wt_porcelain_v2_print_changed_entry(\n \t\t\t\tkey, submodule_token,\n \t\t\t\td->mode_head, d->mode_index, d->mode_worktree,\n \t\t\t\toid_to_hex(&d->oid_head), oid_to_hex(&d->oid_index),\n-\t\t\t\tkey[0], d->score,\n+\t\t\t\tscore_char, d->score,\n \t\t\t\tpath_index, sep_char, path_head, eol_char);\n \telse\n \t\tfprintf(s->fp, \"1 %s %s %06o %06o %06o %s %s %s%c\",\n-- \n2.15.1.windows.2\n"},{"id":"335302","messageId":"fd0eb127-39b6-94fc-86c7-46fa750ec2e3@gmail.com","threadId":"47486","inReplyTo":"86b4d2af-18ce-36eb-4823-105757a196fc@gmail.com","subject":"Re: [PATCH] status: handle worktree renames","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2017-12-25T21:49:05Z","receivedAt":"2017-12-25T21:49:21Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"On 25/12/2017 20:45, Igor Djordjevic wrote:\n> \n> I guess an additional test for this would be good, too.\n\n... aaand here it is. Again based on your test, but please double \ncheck, I`m not sure if it`s ok to compare file modes like that, \nexpecting them to be the same (hashes should be fine, I guess).\n\n---\n t/t2203-add-intent.sh | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex 41a8874e6..394b1047c 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -165,5 +165,20 @@ test_expect_success 'rename detection finds the right names' '\n \t)\n '\n \n+test_expect_success 'rename detection finds the right names (porcelain v2)' '\n+\tgit init rename-detection-v2 &&\n+\t(\n+\t\tcd rename-detection-v2 &&\n+\t\techo contents > original-file &&\n+\t\tgit add original-file &&\n+\t\tgit commit -m first-commit &&\n+\t\tmv original-file new-file &&\n+\t\tgit add -N new-file &&\n+\t\tgit status --porcelain=v2 | grep -v actual >actual &&\n+\t\techo \"2 .R N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 12f00e90b6ef79117ce6e650416b8cf517099b78 R100 new-file\toriginal-file\" >expected &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n test_done\n \n-- \n2.15.1.windows.2\n"},{"id":"335310","messageId":"20171226021150.GA10059@duynguyen.vn.dektech.internal","threadId":"47486","inReplyTo":"b3e90960-d743-3299-ba43-150849b591d2@gmail.com","subject":"Re: [PATCH] status: handle worktree renames","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-26T02:11:50Z","receivedAt":"2017-12-26T02:12:00Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Dec 25, 2017 at 07:26:27PM +0100, Igor Djordjevic wrote:\n> But I`ve noticed that \"--porcelain=v2\" output might still be buggy -\n> this is what having both files staged shows:\n>\n> $ git status --porcelain=v2\n> 2 R. N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 12f00e90b6ef79117ce6e650416b8cf517099b78 R100 new-file\toriginal-file\n>\n> ..., where having old/deleted file unstaged, and new/created file\n> staged with `git add -N` shows this:\n>\n> $ git status --porcelain=v2\n> 1 .R N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 12f00e90b6ef79117ce6e650416b8cf517099b78 new-file\n>\n> So even though unstaged value is correctly recognized as \"R\" (renamed),\n> first number is \"1\" (instead of \"2\" to signal rename/copy), and both\n> rename score and original file name are missing.\n>\n> Not sure if this is a bug, but it seems so, as `git status` \"Porcelain\n> Format Version 2\"[1] says the last path is \"pathname in the commit at\n> HEAD\" (in case of copy/rename), which is missing here.\n\nYeah v2 looks problematic. The way the document is written, it's not\nprepared to deal with a rename pair coming from comparing the index\n(with intent-to-add entries) with worktree, only from comparing with\nHEAD. So either we could ajust v2 semantics slightly like this\n\ndiff --git a/Documentation/git-status.txt b/Documentation/git-status.txt\nindex 81cab9aefb..3da10020aa 100644\n--- a/Documentation/git-status.txt\n+++ b/Documentation/git-status.txt\n@@ -309,13 +309,13 @@ Renamed or copied entries have the following format:\n \t\tof similarity between the source and target of the\n \t\tmove or copy). For example \"R100\" or \"C75\".\n The pathname. In a renamed/copied entry, this\n-\t\tis the path in the index and in the working tree.\n+\t\tis the path in the index.\n When the `-z` option is used, the 2 pathnames are separated\n \t\twith a NUL (ASCII 0x00) byte; otherwise, a tab (ASCII 0x09)\n \t\tbyte separates them.\n- The pathname in the commit at HEAD. This is only\n-\t\tpresent in a renamed/copied entry, and tells\n-\t\twhere the renamed/copied contents came from.\n+ The pathname in the commit at HEAD or in the worktree.\n+\t\tThis is only present in a renamed/copied entry, and\n+\t\ttells where the renamed/copied contents came from.\n --------------------------------------------------------\n \n Unmerged entries have the following format; the first character is\n\nThe problem is, you cannot know if it's a rename from HEAD or from\nworktree with this updated v2 (or perhaps you could because HEAD name\nshould be all zero?).\n\nOr we disable rename-from-worktree when porcelain v2 is requested (and\noptionally introduce v3 to support it). Jeff, any preference?\n--\nDuy\n"},{"id":"335311","messageId":"CACsJy8CN58ivOeAr83X86NZcBy+yrJd0SbFhej99Pjb8x8_gBA@mail.gmail.com","threadId":"47486","inReplyTo":"20171226021150.GA10059@duynguyen.vn.dektech.internal","subject":"Re: [PATCH] status: handle worktree renames","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-26T02:53:14Z","receivedAt":"2017-12-26T02:53:52Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Dec 26, 2017 at 9:11 AM, Duy Nguyen wrote:\n> On Mon, Dec 25, 2017 at 07:26:27PM +0100, Igor Djordjevic wrote:\n>> But I`ve noticed that \"--porcelain=v2\" output might still be buggy -\n>> this is what having both files staged shows:\n>>\n>> $ git status --porcelain=v2\n>> 2 R. N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 12f00e90b6ef79117ce6e650416b8cf517099b78 R100 new-file original-file\n>>\n>> ..., where having old/deleted file unstaged, and new/created file\n>> staged with `git add -N` shows this:\n>>\n>> $ git status --porcelain=v2\n>> 1 .R N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 12f00e90b6ef79117ce6e650416b8cf517099b78 new-file\n>>\n>> So even though unstaged value is correctly recognized as \"R\" (renamed),\n>> first number is \"1\" (instead of \"2\" to signal rename/copy), and both\n>> rename score and original file name are missing.\n>>\n>> Not sure if this is a bug, but it seems so, as `git status` \"Porcelain\n>> Format Version 2\"[1] says the last path is \"pathname in the commit at\n>> HEAD\" (in case of copy/rename), which is missing here.\n>\n> Yeah v2 looks problematic. The way the document is written, it's not\n> prepared to deal with a rename pair coming from comparing the index\n> (with intent-to-add entries) with worktree, only from comparing with\n> HEAD. So either we could ajust v2 semantics slightly like this\n>\n> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt\n> index 81cab9aefb..3da10020aa 100644\n> --- a/Documentation/git-status.txt\n> +++ b/Documentation/git-status.txt\n> @@ -309,13 +309,13 @@ Renamed or copied entries have the following format:\n> of similarity between the source and target of the\n> move or copy). For example \"R100\" or \"C75\".\n> The pathname. In a renamed/copied entry, this\n> - is the path in the index and in the working tree.\n> + is the path in the index.\n> When the `-z` option is used, the 2 pathnames are separated\n> with a NUL (ASCII 0x00) byte; otherwise, a tab (ASCII 0x09)\n> byte separates them.\n> - The pathname in the commit at HEAD. This is only\n> - present in a renamed/copied entry, and tells\n> - where the renamed/copied contents came from.\n> + The pathname in the commit at HEAD or in the worktree.\n> + This is only present in a renamed/copied entry, and\n> + tells where the renamed/copied contents came from.\n> --------------------------------------------------------\n>\n> Unmerged entries have the following format; the first character is\n>\n> The problem is, you cannot know if it's a rename from HEAD or from\n> worktree with this updated v2 (or perhaps you could because HEAD name\n> should be all zero?).\n\nI'm wrong about this. the \"\" code for HEAD rename would be \"R.\"\nwhile worktree rename is \".R\" so I think we're good.\n-- \nDuy\n"},{"id":"335314","messageId":"20171226091012.24315-1-pclouds@gmail.com","threadId":"47486","inReplyTo":"20171225103718.24443-1-pclouds@gmail.com","subject":"[PATCH v2 0/7] Renames in git-status \"changed not staged\" section","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-26T09:10:05Z","receivedAt":"2017-12-26T09:10:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The changes in 425a28e0a4 allow new files to show up in\n\"git diff-files\" (aka \"changed but not staged\") which is a problem\nbecause status code does not handle renaming in this case.\n\nThe main change to fix this is 6/7. The interesting corner case is in\n7/7 where I decided to go with a middle ground, disabling only double\nrenames. We have two other options to go:\n\n - unconditionally disable rename in wt_status_collect_changes_worktree\n which turns this 7-patch series into one patch\n - support double renames too but this may change porcelain output\n (or we have to add new output format). This could be done in a\n follow series if we want to.\n\nNguyễn Thái Ngọc Duy (7):\n t2203: test status output with porcelain v2 format\n Use DIFF_DETECT_RENAME for detect_rename assignments\n wt-status.c: coding style fix\n wt-status.c: rename wt_status_change_data::score\n wt-status.c: catch unhandled diff status codes\n wt-status.c: handle worktree renames\n wt-status.c: avoid double renames in short/porcelain format\n\n builtin/commit.c | 2 +-\n diff.c | 2 +-\n t/t2203-add-intent.sh | 73 ++++++++++++++++++++++++++\n wt-status.c | 141 ++++++++++++++++++++++++++++++++++++++++----------\n wt-status.h | 4 +-\n 5 files changed, 191 insertions(+), 31 deletions(-)\n\n-- \n2.15.0.320.g0453912d77\n\n"},{"id":"335315","messageId":"20171226091012.24315-2-pclouds@gmail.com","threadId":"47486","inReplyTo":"20171226091012.24315-1-pclouds@gmail.com","subject":"[PATCH v2 1/7] t2203: test status output with porcelain v2 format","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-26T09:10:06Z","receivedAt":"2017-12-26T09:10:38Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n t/t2203-add-intent.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex 1bdf38e80d..878e73fe98 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -25,6 +25,18 @@ test_expect_success 'git status' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git status with porcelain v2' '\n+\tgit status --porcelain=v2 | grep -v \"^?\" >actual &&\n+\tnam1=d00491fd7e5bb6fa28c517a0bb32b8b506539d4d &&\n+\tnam2=ce013625030ba8dba906f756967f9e9ca394464a &&\n+\tcat >expect <<-EOF &&\n+\t1 DA N... 100644 000000 100644 $nam1 $_z40 1.t\n+\t1 A. N... 000000 100644 100644 $_z40 $nam2 elif\n+\t1 .A N... 000000 000000 100644 $_z40 $_z40 file\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'check result of \"add -N\"' '\n \tgit ls-files -s file >actual &&\n \tempty=$(git hash-object --stdin \n---\n builtin/commit.c | 2 +-\n diff.c | 2 +-\n wt-status.c | 4 ++--\n 3 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8a87701414..1f11e3992d 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1507,7 +1507,7 @@ static void print_summary(const char *prefix, const struct object_id *oid,\n \trev.show_root_diff = 1;\n \tget_commit_format(format.buf, &rev);\n \trev.always_show_header = 0;\n-\trev.diffopt.detect_rename = 1;\n+\trev.diffopt.detect_rename = DIFF_DETECT_RENAME;\n \trev.diffopt.break_opt = 0;\n \tdiff_setup_done(&rev.diffopt);\n \ndiff --git a/diff.c b/diff.c\nindex 3fb445a54d..51fe31c7aa 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -246,7 +246,7 @@ static int parse_ws_error_highlight(const char *arg)\n */\n void init_diff_ui_defaults(void)\n {\n-\tdiff_detect_rename_default = 1;\n+\tdiff_detect_rename_default = DIFF_DETECT_RENAME;\n }\n \n int git_diff_heuristic_config(const char *var, const char *value, void *cb)\ndiff --git a/wt-status.c b/wt-status.c\nindex ef26f07446..59338adb8b 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -602,7 +602,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = wt_status_collect_updated_cb;\n \trev.diffopt.format_callback_data = s;\n-\trev.diffopt.detect_rename = 1;\n+\trev.diffopt.detect_rename = DIFF_DETECT_RENAME;\n \trev.diffopt.rename_limit = 200;\n \trev.diffopt.break_opt = 0;\n \tcopy_pathspec(&rev.prune_data, &s->pathspec);\n@@ -962,7 +962,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n \tsetup_revisions(0, NULL, &rev, &opt);\n \n \trev.diffopt.output_format |= DIFF_FORMAT_PATCH;\n-\trev.diffopt.detect_rename = 1;\n+\trev.diffopt.detect_rename = DIFF_DETECT_RENAME;\n \trev.diffopt.file = s->fp;\n \trev.diffopt.close_file = 0;\n \t/*\n-- \n2.15.0.320.g0453912d77\n\n"},{"id":"335317","messageId":"20171226091012.24315-4-pclouds@gmail.com","threadId":"47486","inReplyTo":"20171226091012.24315-1-pclouds@gmail.com","subject":"[PATCH v2 3/7] wt-status.c: coding style fix","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-26T09:10:08Z","receivedAt":"2017-12-26T09:10:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n wt-status.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 59338adb8b..db06fc7c85 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -406,7 +406,8 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n \tstrbuf_release(&twobuf);\n }\n \n-static char short_submodule_status(struct wt_status_change_data *d) {\n+static char short_submodule_status(struct wt_status_change_data *d)\n+{\n \tif (d->new_submodule_commits)\n \t\treturn 'M';\n \tif (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)\n-- \n2.15.0.320.g0453912d77\n\n"},{"id":"335318","messageId":"20171226091012.24315-5-pclouds@gmail.com","threadId":"47486","inReplyTo":"20171226091012.24315-1-pclouds@gmail.com","subject":"[PATCH v2 4/7] wt-status.c: rename wt_status_change_data::score","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-26T09:10:09Z","receivedAt":"2017-12-26T09:10:52Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"We are about to adding support for \"diff-files\" rename, which has its\nown rename score in addition to the \"diff-index\" one. Rename score to\nhead_score to indicate this score comes from diff-index. The new score\nwill be named worktree_score.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n wt-status.c | 4 ++--\n wt-status.h | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex db06fc7c85..0f089c5789 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -532,7 +532,7 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,\n \t\tcase DIFF_STATUS_COPIED:\n \t\tcase DIFF_STATUS_RENAMED:\n \t\t\td->head_path = xstrdup(p->one->path);\n-\t\t\td->score = p->score * 100 / MAX_SCORE;\n+\t\t\td->head_score = p->score * 100 / MAX_SCORE;\n \t\t\t/* fallthru */\n \t\tcase DIFF_STATUS_MODIFIED:\n \t\tcase DIFF_STATUS_TYPE_CHANGED:\n@@ -2074,7 +2074,7 @@ static void wt_porcelain_v2_print_changed_entry(\n \t\t\t\tkey, submodule_token,\n \t\t\t\td->mode_head, d->mode_index, d->mode_worktree,\n \t\t\t\toid_to_hex(&d->oid_head), oid_to_hex(&d->oid_index),\n-\t\t\t\tkey[0], d->score,\n+\t\t\t\tkey[0], d->head_score,\n \t\t\t\tpath_index, sep_char, path_head, eol_char);\n \telse\n \t\tfprintf(s->fp, \"1 %s %s %06o %06o %06o %s %s %s%c\",\ndiff --git a/wt-status.h b/wt-status.h\nindex fe27b465e2..f9330982ac 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -44,7 +44,7 @@ struct wt_status_change_data {\n \tint worktree_status;\n \tint index_status;\n \tint stagemask;\n-\tint score;\n+\tint head_score;\n \tint mode_head, mode_index, mode_worktree;\n \tstruct object_id oid_head, oid_index;\n \tchar *head_path;\n-- \n2.15.0.320.g0453912d77\n\n"},{"id":"335319","messageId":"20171226091012.24315-6-pclouds@gmail.com","threadId":"47486","inReplyTo":"20171226091012.24315-1-pclouds@gmail.com","subject":"[PATCH v2 5/7] wt-status.c: catch unhandled diff status codes","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-26T09:10:10Z","receivedAt":"2017-12-26T09:10:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n wt-status.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 0f089c5789..c124d7589c 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -468,8 +468,8 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\t\toidcpy(&d->oid_index, &p->one->oid);\n \t\t\tbreak;\n \n-\t\tcase DIFF_STATUS_UNKNOWN:\n-\t\t\tdie(\"BUG: worktree status unknown???\");\n+\t\tdefault:\n+\t\t\tdie(\"BUG: unhandled diff-files status '%c'\", p->status);\n \t\t\tbreak;\n \t\t}\n \n@@ -549,6 +549,10 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,\n \t\t\t * values in these fields.\n \t\t\t */\n \t\t\tbreak;\n+\n+\t\tdefault:\n+\t\t\tdie(\"BUG: unhandled diff-index status '%c'\", p->status);\n+\t\t\tbreak;\n \t\t}\n \t}\n }\n-- \n2.15.0.320.g0453912d77\n\n"},{"id":"335320","messageId":"20171226091012.24315-7-pclouds@gmail.com","threadId":"47486","inReplyTo":"20171226091012.24315-1-pclouds@gmail.com","subject":"[PATCH v2 6/7] wt-status.c: handle worktree renames","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-26T09:10:11Z","receivedAt":"2017-12-26T09:11:03Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Before 425a28e0a4 (diff-lib: allow ita entries treated as \"not yet exist\nin index\" - 2016-10-24) there are never \"new files\" in the index, which\nessentially disables rename detection because we only detect renames\nwhen a new file appears in a diff pair.\n\nAfter that commit, an i-t-a entry can appear as a new file in \"git\ndiff-files\". But the diff callback function in wt-status.c does not\nhandle this case and produces incorrect status output.\n\nPS. The reader may notice that this patch adds a new xstrdup() but not\na free(). Yes we leak memory (the same for head_path). But wt_status\nso far has been short lived, this leak should not matter in\npractice.\n\nNoticed-by: Alex Vandiver \nHelped-by: Igor Djordjevic \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n t/t2203-add-intent.sh | 28 ++++++++++++++++++++\n wt-status.c | 72 +++++++++++++++++++++++++++++++++++++++------------\n wt-status.h | 4 +--\n 3 files changed, 85 insertions(+), 19 deletions(-)\n\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex 878e73fe98..e5bfda1853 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -162,5 +162,33 @@ test_expect_success 'commit: ita entries ignored in empty commit check' '\n \t)\n '\n \n+test_expect_success 'rename detection finds the right names' '\n+\tgit init rename-detection &&\n+\t(\n+\t\tcd rename-detection &&\n+\t\techo contents > first &&\n+\t\tgit add first &&\n+\t\tgit commit -m first &&\n+\t\tmv first third &&\n+\t\tgit add -N third &&\n+\n+\t\tgit status | grep -v \"^?\" >actual.1 &&\n+\t\ttest_i18ngrep \"renamed: *first -> third\" actual.1 &&\n+\n+\t\tgit status --porcelain | grep -v \"^?\" >actual.2 &&\n+\t\tcat >expected.2 <<-\\EOF &&\n+\t\t R first -> third\n+\t\tEOF\n+\t\ttest_cmp expected.2 actual.2 &&\n+\n+\t\toid=12f00e90b6ef79117ce6e650416b8cf517099b78 &&\n+\t\tgit status --porcelain=v2 | grep -v \"^?\" >actual.3 &&\n+\t\tcat >expected.3 <<-EOF &&\n+\t\t2 .R N... 100644 100644 100644 $oid $oid R100 first\tthird\n+\t\tEOF\n+\t\ttest_cmp expected.3 actual.3\n+\t)\n+'\n+\n test_done\n \ndiff --git a/wt-status.c b/wt-status.c\nindex c124d7589c..d5bdf4c2e9 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -376,6 +376,8 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n \t\t\tstrbuf_addch(&extra, ')');\n \t\t}\n \t\tstatus = d->worktree_status;\n+\t\tif (d->worktree_path)\n+\t\t\ttwo_name = d->worktree_path;\n \t\tbreak;\n \tdefault:\n \t\tdie(\"BUG: unhandled change_type %d in wt_longstatus_print_change_data\",\n@@ -460,6 +462,12 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\t\t/* mode_worktree is zero for a delete. */\n \t\t\tbreak;\n \n+\t\tcase DIFF_STATUS_COPIED:\n+\t\tcase DIFF_STATUS_RENAMED:\n+\t\t\td->worktree_path = xstrdup(p->two->path);\n+\t\t\td->worktree_score = p->score * 100 / MAX_SCORE;\n+\t\t\t/* fallthru */\n+\n \t\tcase DIFF_STATUS_MODIFIED:\n \t\tcase DIFF_STATUS_TYPE_CHANGED:\n \t\tcase DIFF_STATUS_UNMERGED:\n@@ -1712,6 +1720,7 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \t\t\t struct wt_status *s)\n {\n \tstruct wt_status_change_data *d = it->util;\n+\tconst char *from, *to;\n \n \tif (d->index_status)\n \t\tcolor_fprintf(s->fp, color(WT_STATUS_UPDATED, s), \"%c\", d->index_status);\n@@ -1722,15 +1731,30 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \telse\n \t\tputchar(' ');\n \tputchar(' ');\n+\n+\tif (d->head_path && d->worktree_path)\n+\t\tdie(\"BUG: to be addressed in the next patch\");\n+\n+\tif (d->head_path) {\n+\t\tfrom = d->head_path;\n+\t\tto = it->string;\n+\t} else if (d->worktree_path) {\n+\t\tfrom = it->string;\n+\t\tto = d->worktree_path;\n+\t} else {\n+\t\tfrom = it->string;\n+\t\tto = NULL;\n+\t}\n \tif (s->null_termination) {\n-\t\tfprintf(stdout, \"%s%c\", it->string, 0);\n-\t\tif (d->head_path)\n-\t\t\tfprintf(stdout, \"%s%c\", d->head_path, 0);\n+\t\tfprintf(stdout, \"%s%c\", from, 0);\n+\t\tif (to)\n+\t\t\tfprintf(stdout, \"%s%c\", to, 0);\n \t} else {\n \t\tstruct strbuf onebuf = STRBUF_INIT;\n \t\tconst char *one;\n-\t\tif (d->head_path) {\n-\t\t\tone = quote_path(d->head_path, s->prefix, &onebuf);\n+\n+\t\tif (to) {\n+\t\t\tone = quote_path(from, s->prefix, &onebuf);\n \t\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n \t\t\t\tputchar('\"');\n \t\t\t\tstrbuf_addch(&onebuf, '\"');\n@@ -1738,8 +1762,9 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \t\t\t}\n \t\t\tprintf(\"%s -> \", one);\n \t\t\tstrbuf_release(&onebuf);\n-\t\t}\n-\t\tone = quote_path(it->string, s->prefix, &onebuf);\n+\t\t\tone = quote_path(to, s->prefix, &onebuf);\n+\t\t} else\n+\t\t\tone = quote_path(from, s->prefix, &onebuf);\n \t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n \t\t\tputchar('\"');\n \t\t\tstrbuf_addch(&onebuf, '\"');\n@@ -2036,12 +2061,13 @@ static void wt_porcelain_v2_print_changed_entry(\n {\n \tstruct wt_status_change_data *d = it->util;\n \tstruct strbuf buf_index = STRBUF_INIT;\n-\tstruct strbuf buf_head = STRBUF_INIT;\n+\tstruct strbuf buf_other = STRBUF_INIT;\n \tconst char *path_index = NULL;\n-\tconst char *path_head = NULL;\n-\tchar key[3];\n+\tconst char *path_other = NULL;\n+\tchar key[3], status_other;\n \tchar submodule_token[5];\n \tchar sep_char, eol_char;\n+\tint score;\n \n \twt_porcelain_v2_fix_up_changed(it, s);\n \twt_porcelain_v2_submodule_state(d, submodule_token);\n@@ -2050,6 +2076,19 @@ static void wt_porcelain_v2_print_changed_entry(\n \tkey[1] = d->worktree_status ? d->worktree_status : '.';\n \tkey[2] = 0;\n \n+\tif (d->head_path && d->worktree_path)\n+\t\tdie(\"BUG: to be addressed in the next patch\");\n+\n+\tif (d->head_path) {\n+\t\tpath_other = d->head_path;\n+\t\tstatus_other = d->index_status;\n+\t\tscore = d->head_score;\n+\t} else if (d->worktree_path) {\n+\t\tpath_other = d->worktree_path;\n+\t\tstatus_other = d->worktree_status;\n+\t\tscore = d->worktree_score;\n+\t}\n+\n \tif (s->null_termination) {\n \t\t/*\n \t\t * In -z mode, we DO NOT C-quote pathnames. Current path is ALWAYS first.\n@@ -2058,7 +2097,6 @@ static void wt_porcelain_v2_print_changed_entry(\n \t\tsep_char = '\\0';\n \t\teol_char = '\\0';\n \t\tpath_index = it->string;\n-\t\tpath_head = d->head_path;\n \t} else {\n \t\t/*\n \t\t * Path(s) are C-quoted if necessary. Current path is ALWAYS first.\n@@ -2069,17 +2107,17 @@ static void wt_porcelain_v2_print_changed_entry(\n \t\tsep_char = '\\t';\n \t\teol_char = '\\n';\n \t\tpath_index = quote_path(it->string, s->prefix, &buf_index);\n-\t\tif (d->head_path)\n-\t\t\tpath_head = quote_path(d->head_path, s->prefix, &buf_head);\n+\t\tif (path_other)\n+\t\t\tpath_other = quote_path(path_other, s->prefix, &buf_other);\n \t}\n \n-\tif (path_head)\n+\tif (path_other)\n \t\tfprintf(s->fp, \"2 %s %s %06o %06o %06o %s %s %c%d %s%c%s%c\",\n \t\t\t\tkey, submodule_token,\n \t\t\t\td->mode_head, d->mode_index, d->mode_worktree,\n \t\t\t\toid_to_hex(&d->oid_head), oid_to_hex(&d->oid_index),\n-\t\t\t\tkey[0], d->head_score,\n-\t\t\t\tpath_index, sep_char, path_head, eol_char);\n+\t\t\t\tstatus_other, score,\n+\t\t\t\tpath_index, sep_char, path_other, eol_char);\n \telse\n \t\tfprintf(s->fp, \"1 %s %s %06o %06o %06o %s %s %s%c\",\n \t\t\t\tkey, submodule_token,\n@@ -2088,7 +2126,7 @@ static void wt_porcelain_v2_print_changed_entry(\n \t\t\t\tpath_index, eol_char);\n \n \tstrbuf_release(&buf_index);\n-\tstrbuf_release(&buf_head);\n+\tstrbuf_release(&buf_other);\n }\n \n /*\ndiff --git a/wt-status.h b/wt-status.h\nindex f9330982ac..332ff545aa 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -44,10 +44,10 @@ struct wt_status_change_data {\n \tint worktree_status;\n \tint index_status;\n \tint stagemask;\n-\tint head_score;\n+\tint head_score, worktree_score;\n \tint mode_head, mode_index, mode_worktree;\n \tstruct object_id oid_head, oid_index;\n-\tchar *head_path;\n+\tchar *head_path, *worktree_path;\n \tunsigned dirty_submodule : 2;\n \tunsigned new_submodule_commits : 1;\n };\n-- \n2.15.0.320.g0453912d77\n\n"},{"id":"335321","messageId":"20171226091012.24315-8-pclouds@gmail.com","threadId":"47486","inReplyTo":"20171226091012.24315-1-pclouds@gmail.com","subject":"[PATCH v2 7/7] wt-status.c: avoid double renames in short/porcelain format","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-26T09:10:12Z","receivedAt":"2017-12-26T09:11:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The presence of worktree rename leads to an interesting situation,\nwhat if the same index entry is renamed twice, compared to HEAD and to\nworktree? We can have that with this setup\n\n echo first > first && git add first && git commit -m first\n git mv first second # rename reported in \"diff --cached\"\n mv second third # rename reported in \"diff-files\"\n\nFor the long format this is fine because we print two \"->\" rename\nlines, one in the \"updated\" section, one in \"changed\" one.\n\nFor other output formats, it gets tricky because they combine both\ndiffs in one line but can only display one rename per line. The result\n\"XY\" column of short format, for example, would be \"RR\" in that case.\n\nThis case either needs some extension in short/porcelain format\nto show something crazy like\n\n RR first -> second -> third\n\nor we could show renames as two lines instead of one, for example\nsomething like this for short form:\n\n R first -> second\n R second -> third\n\nBut for now it's safer and simpler to just break the \"second -> third\"\nrename pair and show\n\n RD first -> second\n A third\n\nlike we have been showing until now.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n t/t2203-add-intent.sh | 33 +++++++++++++++++++++++++++++\n wt-status.c | 58 ++++++++++++++++++++++++++++++++++++++++++++-------\n 2 files changed, 84 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex e5bfda1853..79aca93810 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -190,5 +190,38 @@ test_expect_success 'rename detection finds the right names' '\n \t)\n '\n \n+test_expect_success 'double rename detection in status' '\n+\tgit init rename-detection-2 &&\n+\t(\n+\t\tcd rename-detection-2 &&\n+\t\techo contents > first &&\n+\t\tgit add first &&\n+\t\tgit commit -m first &&\n+\t\tgit mv first second &&\n+\t\tmv second third &&\n+\t\tgit add -N third &&\n+\n+\t\tgit status | grep -v \"^?\" >actual.1 &&\n+\t\ttest_i18ngrep \"renamed: *first -> second\" actual.1 &&\n+\t\ttest_i18ngrep \"renamed: *second -> third\" actual.1 &&\n+\n+\n+\t\tgit status --porcelain | grep -v \"^?\" >actual.2 &&\n+\t\tcat >expected.2 <<-\\EOF &&\n+\t\tRD first -> second\n+\t\t A third\n+\t\tEOF\n+\t\ttest_cmp expected.2 actual.2 &&\n+\n+\t\toid=12f00e90b6ef79117ce6e650416b8cf517099b78 &&\n+\t\tgit status --porcelain=v2 | grep -v \"^?\" >actual.3 &&\n+\t\tcat >expected.3 <<-EOF &&\n+\t\t2 RD N... 100644 100644 000000 $oid $oid R100 second\tfirst\n+\t\t1 .A N... 000000 000000 100644 $_z40 $_z40 third\n+\t\tEOF\n+\t\ttest_cmp expected.3 actual.3\n+\t)\n+'\n+\n test_done\n \ndiff --git a/wt-status.c b/wt-status.c\nindex d5bdf4c2e9..e62853f748 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -419,6 +419,47 @@ static char short_submodule_status(struct wt_status_change_data *d)\n \treturn d->worktree_status;\n }\n \n+static struct string_list_item * break_double_rename(\n+\t\tstruct wt_status *s, struct string_list_item *it,\n+\t\tint *status, struct diff_filepair *p)\n+{\n+\tstruct wt_status_change_data *d;\n+\tstruct string_list_item *new_it;\n+\n+\td = it->util;\n+\t/*\n+\t * _collect_index_changes() must have been called or\n+\t * d->head_path does not contain a real value.\n+\t */\n+\tif (!d || !d->head_path)\n+\t\treturn it;\n+\n+\tswitch (s->status_format) {\n+\tcase STATUS_FORMAT_SHORT:\n+\tcase STATUS_FORMAT_PORCELAIN:\n+\tcase STATUS_FORMAT_PORCELAIN_V2:\n+\t\tbreak;\n+\tcase STATUS_FORMAT_LONG:\n+\tcase STATUS_FORMAT_NONE:\n+\t\t/* this output can handle double renames ok */\n+\t\treturn it;\n+\tdefault:\n+\t\tdie(\"BUG: finalize_deferred_config() should have been called\");\n+\t}\n+\n+\tswitch (*status) {\n+\tcase DIFF_STATUS_RENAMED:\n+\t\td->worktree_status = DIFF_STATUS_DELETED;\n+\t\t/* fallthru */\n+\tcase DIFF_STATUS_COPIED:\n+\t\t*status = DIFF_STATUS_ADDED;\n+\t\tnew_it = string_list_insert(&s->change, p->two->path);\n+\t\treturn new_it;\n+\t}\n+\n+\treturn it;\n+}\n+\n static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\t\t\t\t struct diff_options *options,\n \t\t\t\t\t void *data)\n@@ -433,16 +474,19 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\tstruct diff_filepair *p;\n \t\tstruct string_list_item *it;\n \t\tstruct wt_status_change_data *d;\n+\t\tint status;\n \n \t\tp = q->queue[i];\n+\t\tstatus = p->status;\n \t\tit = string_list_insert(&s->change, p->one->path);\n+\t\tit = break_double_rename(s, it, &status, p);\n \t\td = it->util;\n \t\tif (!d) {\n \t\t\td = xcalloc(1, sizeof(*d));\n \t\t\tit->util = d;\n \t\t}\n \t\tif (!d->worktree_status)\n-\t\t\td->worktree_status = p->status;\n+\t\t\td->worktree_status = status;\n \t\tif (S_ISGITLINK(p->two->mode)) {\n \t\t\td->dirty_submodule = p->two->dirty_submodule;\n \t\t\td->new_submodule_commits = !!oidcmp(&p->one->oid,\n@@ -451,7 +495,7 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\t\t\td->worktree_status = short_submodule_status(d);\n \t\t}\n \n-\t\tswitch (p->status) {\n+\t\tswitch (status) {\n \t\tcase DIFF_STATUS_ADDED:\n \t\t\td->mode_worktree = p->two->mode;\n \t\t\tbreak;\n@@ -477,7 +521,7 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\t\tbreak;\n \n \t\tdefault:\n-\t\t\tdie(\"BUG: unhandled diff-files status '%c'\", p->status);\n+\t\t\tdie(\"BUG: unhandled diff-files status '%c'\", status);\n \t\t\tbreak;\n \t\t}\n \n@@ -710,12 +754,12 @@ static void wt_status_collect_untracked(struct wt_status *s)\n \n void wt_status_collect(struct wt_status *s)\n {\n-\twt_status_collect_changes_worktree(s);\n-\n \tif (s->is_initial)\n \t\twt_status_collect_changes_initial(s);\n \telse\n+\t\t/* must be called before _collect_changes_worktree() */\n \t\twt_status_collect_changes_index(s);\n+\twt_status_collect_changes_worktree(s);\n \twt_status_collect_untracked(s);\n }\n \n@@ -1733,7 +1777,7 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \tputchar(' ');\n \n \tif (d->head_path && d->worktree_path)\n-\t\tdie(\"BUG: to be addressed in the next patch\");\n+\t\tdie(\"BUG: break_double_rename() fails to break this pair\");\n \n \tif (d->head_path) {\n \t\tfrom = d->head_path;\n@@ -2077,7 +2121,7 @@ static void wt_porcelain_v2_print_changed_entry(\n \tkey[2] = 0;\n \n \tif (d->head_path && d->worktree_path)\n-\t\tdie(\"BUG: to be addressed in the next patch\");\n+\t\tdie(\"BUG: break_double_rename() fails to break this pair\");\n \n \tif (d->head_path) {\n \t\tpath_other = d->head_path;\n-- \n2.15.0.320.g0453912d77\n\n"},{"id":"335324","messageId":"f451553f-cef5-f3c4-ccdc-5ac300a62e8c@web.de","threadId":"47486","inReplyTo":"20171225103718.24443-1-pclouds@gmail.com","subject":"Re: [PATCH] status: handle worktree renames","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2017-12-26T18:04:10Z","receivedAt":"2017-12-26T18:04:27Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2017-12-25 11:37, Nguyễn Thái Ngọc Duy wrote:\n[]\n> wt-status.c | 24 +++++++++++++++++++-----\n> wt-status.h | 1 +\n> 3 files changed, 35 insertions(+), 5 deletions(-)\n> \n> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\n> index 1bdf38e80d..41a8874e60 100755\n> --- a/t/t2203-add-intent.sh\n> +++ b/t/t2203-add-intent.sh\n> @@ -150,5 +150,20 @@ test_expect_success 'commit: ita entries ignored in empty commit check' '\n> \t)\n> '\n> \n> +test_expect_success 'rename detection finds the right names' '\n> +\tgit init rename-detection &&\n> +\t(\n> +\t\tcd rename-detection &&\n> +\t\techo contents > original-file &&\nMicro-nit, please no \" \" after \">\":\necho contents >original-file\n\n"},{"id":"335326","messageId":"d0f2055c-e0c4-32af-6371-53d0e9a5fbc5@gmail.com","threadId":"47486","inReplyTo":"20171226091012.24315-7-pclouds@gmail.com","subject":"Re: [PATCH v2 6/7] wt-status.c: handle worktree renames","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2017-12-26T18:14:40Z","receivedAt":"2017-12-26T18:14:58Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"Hi Duy,\n\nOn 26/12/2017 10:10, Nguyễn Thái Ngọc Duy wrote:\n> Before 425a28e0a4 (diff-lib: allow ita entries treated as \"not yet exist\n> in index\" - 2016-10-24) there are never \"new files\" in the index, which\n> essentially disables rename detection because we only detect renames\n> when a new file appears in a diff pair.\n> \n> After that commit, an i-t-a entry can appear as a new file in \"git\n> diff-files\". But the diff callback function in wt-status.c does not\n> handle this case and produces incorrect status output.\n> \n> PS. The reader may notice that this patch adds a new xstrdup() but not\n> a free(). Yes we leak memory (the same for head_path). But wt_status\n> so far has been short lived, this leak should not matter in\n> practice.\n> \n> Noticed-by: Alex Vandiver \n> Helped-by: Igor Djordjevic \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> t/t2203-add-intent.sh | 28 ++++++++++++++++++++\n> wt-status.c | 72 +++++++++++++++++++++++++++++++++++++++------------\n> wt-status.h | 4 +--\n> 3 files changed, 85 insertions(+), 19 deletions(-)\n> \n> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\n> index 878e73fe98..e5bfda1853 100755\n> --- a/t/t2203-add-intent.sh\n> +++ b/t/t2203-add-intent.sh\n> @@ -162,5 +162,33 @@ test_expect_success 'commit: ita entries ignored in empty commit check' '\n> \t)\n> '\n> \n> +test_expect_success 'rename detection finds the right names' '\n> +\tgit init rename-detection &&\n> +\t(\n> +\t\tcd rename-detection &&\n> +\t\techo contents > first &&\n> +\t\tgit add first &&\n> +\t\tgit commit -m first &&\n> +\t\tmv first third &&\n> +\t\tgit add -N third &&\n> +\n> +\t\tgit status | grep -v \"^?\" >actual.1 &&\n> +\t\ttest_i18ngrep \"renamed: *first -> third\" actual.1 &&\n> +\n> +\t\tgit status --porcelain | grep -v \"^?\" >actual.2 &&\n> +\t\tcat >expected.2 <<-\\EOF &&\n> +\t\t R first -> third\n> +\t\tEOF\n> +\t\ttest_cmp expected.2 actual.2 &&\n> +\n> +\t\toid=12f00e90b6ef79117ce6e650416b8cf517099b78 &&\n> +\t\tgit status --porcelain=v2 | grep -v \"^?\" >actual.3 &&\n> +\t\tcat >expected.3 <<-EOF &&\n> +\t\t2 .R N... 100644 100644 100644 $oid $oid R100 first\tthird\n> +\t\tEOF\n> +\t\ttest_cmp expected.3 actual.3\n> +\t)\n> +'\n> +\n> test_done\n\nI`m afraid \"--porcelain=v2\" test might be incorrect here, as `git \nstatus --porcelain=v2` output seems to be too, with this v2 series \napplied. Test I sent previously[1] fails, and it looks valid.\n\nThis is output I now get, with old/deleted file unstaged and \nnew/created file staged with `git add -N`:\n\n $ git status --porcelain=v2\n 2 .R N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 12f00e90b6ef79117ce6e650416b8cf517099b78 R100 original-file\tnew-file\n\nNote \"original-file\" listed first, where \"new-file\" listed second \n(last). According the \"v2\" documentation[2] (excerpt):\n\n ... \n\n The pathname. In a renamed/copied entry, this\n is the path in the index and in the working tree.\n ...\n The pathname in the commit at HEAD. This is only\n present in a renamed/copied entry, and tells\n where the renamed/copied contents came from.\n\n\nIf I`m reading this correctly, it should be vice-versa - value from \nHEAD, being \"original-file\", should come last, where value from \nworking tree (\"new-file\") should be first.\n\nIf I stage both files and try again, output is as expected \n(\"new-file\" comes first):\n\n $ git status --porcelain=v2\n 2 R. N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 12f00e90b6ef79117ce6e650416b8cf517099b78 R100 new-file\toriginal-file\n\n> diff --git a/wt-status.c b/wt-status.c\n> index c124d7589c..d5bdf4c2e9 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -376,6 +376,8 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n> \t\t\tstrbuf_addch(&extra, ')');\n> \t\t}\n> \t\tstatus = d->worktree_status;\n> +\t\tif (d->worktree_path)\n> +\t\t\ttwo_name = d->worktree_path;\n> \t\tbreak;\n> \tdefault:\n> \t\tdie(\"BUG: unhandled change_type %d in wt_longstatus_print_change_data\",\n> @@ -460,6 +462,12 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n> \t\t\t/* mode_worktree is zero for a delete. */\n> \t\t\tbreak;\n> \n> +\t\tcase DIFF_STATUS_COPIED:\n> +\t\tcase DIFF_STATUS_RENAMED:\n> +\t\t\td->worktree_path = xstrdup(p->two->path);\n \t\t\t ^^^\nThis is changed from v1 of this patch, where it was:\n\n+\t\t\td->worktree_path = xstrdup(p->one->path);\n \t\t\t ^^^\n..., and might be it introduced the issue here...? Or, if this is \ncorrect now, then might be the other part further below should then \nbe adapted accordingly, inside wt_porcelain_v2_print_changed_entry()?\n\n> +\t\t\td->worktree_score = p->score * 100 / MAX_SCORE;\n> +\t\t\t/* fallthru */\n> +\n> \t\tcase DIFF_STATUS_MODIFIED:\n> \t\tcase DIFF_STATUS_TYPE_CHANGED:\n> \t\tcase DIFF_STATUS_UNMERGED:\n> @@ -1712,6 +1720,7 @@ static void wt_shortstatus_status(struct string_list_item *it,\n> \t\t\t struct wt_status *s)\n> {\n> \tstruct wt_status_change_data *d = it->util;\n> +\tconst char *from, *to;\n> \n> \tif (d->index_status)\n> \t\tcolor_fprintf(s->fp, color(WT_STATUS_UPDATED, s), \"%c\", d->index_status);\n> @@ -1722,15 +1731,30 @@ static void wt_shortstatus_status(struct string_list_item *it,\n> \telse\n> \t\tputchar(' ');\n> \tputchar(' ');\n> +\n> +\tif (d->head_path && d->worktree_path)\n> +\t\tdie(\"BUG: to be addressed in the next patch\");\n> +\n> +\tif (d->head_path) {\n> +\t\tfrom = d->head_path;\n> +\t\tto = it->string;\n> +\t} else if (d->worktree_path) {\n> +\t\tfrom = it->string;\n> +\t\tto = d->worktree_path;\n> +\t} else {\n> +\t\tfrom = it->string;\n> +\t\tto = NULL;\n> +\t}\n> \tif (s->null_termination) {\n> -\t\tfprintf(stdout, \"%s%c\", it->string, 0);\n> -\t\tif (d->head_path)\n> -\t\t\tfprintf(stdout, \"%s%c\", d->head_path, 0);\n> +\t\tfprintf(stdout, \"%s%c\", from, 0);\n> +\t\tif (to)\n> +\t\t\tfprintf(stdout, \"%s%c\", to, 0);\n> \t} else {\n> \t\tstruct strbuf onebuf = STRBUF_INIT;\n> \t\tconst char *one;\n> -\t\tif (d->head_path) {\n> -\t\t\tone = quote_path(d->head_path, s->prefix, &onebuf);\n> +\n> +\t\tif (to) {\n> +\t\t\tone = quote_path(from, s->prefix, &onebuf);\n> \t\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n> \t\t\t\tputchar('\"');\n> \t\t\t\tstrbuf_addch(&onebuf, '\"');\n> @@ -1738,8 +1762,9 @@ static void wt_shortstatus_status(struct string_list_item *it,\n> \t\t\t}\n> \t\t\tprintf(\"%s -> \", one);\n> \t\t\tstrbuf_release(&onebuf);\n> -\t\t}\n> -\t\tone = quote_path(it->string, s->prefix, &onebuf);\n> +\t\t\tone = quote_path(to, s->prefix, &onebuf);\n> +\t\t} else\n> +\t\t\tone = quote_path(from, s->prefix, &onebuf);\n> \t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n> \t\t\tputchar('\"');\n> \t\t\tstrbuf_addch(&onebuf, '\"');\n> @@ -2036,12 +2061,13 @@ static void wt_porcelain_v2_print_changed_entry(\n> {\n> \tstruct wt_status_change_data *d = it->util;\n> \tstruct strbuf buf_index = STRBUF_INIT;\n> -\tstruct strbuf buf_head = STRBUF_INIT;\n> +\tstruct strbuf buf_other = STRBUF_INIT;\n> \tconst char *path_index = NULL;\n> -\tconst char *path_head = NULL;\n> -\tchar key[3];\n> +\tconst char *path_other = NULL;\n> +\tchar key[3], status_other;\n> \tchar submodule_token[5];\n> \tchar sep_char, eol_char;\n> +\tint score;\n> \n> \twt_porcelain_v2_fix_up_changed(it, s);\n> \twt_porcelain_v2_submodule_state(d, submodule_token);\n> @@ -2050,6 +2076,19 @@ static void wt_porcelain_v2_print_changed_entry(\n> \tkey[1] = d->worktree_status ? d->worktree_status : '.';\n> \tkey[2] = 0;\n> \n> +\tif (d->head_path && d->worktree_path)\n> +\t\tdie(\"BUG: to be addressed in the next patch\");\n> +\n> +\tif (d->head_path) {\n> +\t\tpath_other = d->head_path;\n> +\t\tstatus_other = d->index_status;\n> +\t\tscore = d->head_score;\n> +\t} else if (d->worktree_path) {\n> +\t\tpath_other = d->worktree_path;\n> +\t\tstatus_other = d->worktree_status;\n> +\t\tscore = d->worktree_score;\n> +\t}\n> +\n\n\"path_other\" calculation seems incorrect here...? As that one is \nlater used as \"\", coming last, it should be value from HEAD \nexclusively (in case of rename/copy, otherwise empty).\n\nHere, in case HEAD is missing, \"path_other\" is set to worktree path - \nwhich should be only (possibly) found inside \"\", being the \nfirst listed of the two paths (currently represented by \"path_index\" \nfurther below).\n\nThat said, might be even better to have previously existing \"path_head\"\nvariable restored, and \"path_index\" replaced with this new \"path_other\" \ninstead, as that`s what documentation seems to talk about.\n\n> \tif (s->null_termination) {\n> \t\t/*\n> \t\t * In -z mode, we DO NOT C-quote pathnames. Current path is ALWAYS first.\n> @@ -2058,7 +2097,6 @@ static void wt_porcelain_v2_print_changed_entry(\n> \t\tsep_char = '\\0';\n> \t\teol_char = '\\0';\n> \t\tpath_index = it->string;\n> -\t\tpath_head = d->head_path;\n> \t} else {\n> \t\t/*\n> \t\t * Path(s) are C-quoted if necessary. Current path is ALWAYS first.\n> @@ -2069,17 +2107,17 @@ static void wt_porcelain_v2_print_changed_entry(\n> \t\tsep_char = '\\t';\n> \t\teol_char = '\\n';\n> \t\tpath_index = quote_path(it->string, s->prefix, &buf_index);\n> -\t\tif (d->head_path)\n> -\t\t\tpath_head = quote_path(d->head_path, s->prefix, &buf_head);\n> +\t\tif (path_other)\n> +\t\t\tpath_other = quote_path(path_other, s->prefix, &buf_other);\n> \t}\n> \n> -\tif (path_head)\n> +\tif (path_other)\n> \t\tfprintf(s->fp, \"2 %s %s %06o %06o %06o %s %s %c%d %s%c%s%c\",\n> \t\t\t\tkey, submodule_token,\n> \t\t\t\td->mode_head, d->mode_index, d->mode_worktree,\n> \t\t\t\toid_to_hex(&d->oid_head), oid_to_hex(&d->oid_index),\n> -\t\t\t\tkey[0], d->head_score,\n> -\t\t\t\tpath_index, sep_char, path_head, eol_char);\n> +\t\t\t\tstatus_other, score,\n> +\t\t\t\tpath_index, sep_char, path_other, eol_char);\n \t\t\t\t^^^^^^^^^^ ^^^^^^^^^^\nThis seems mixed up a bit - first value (now \"path_index\") should be \n\"index and/or working tree\" (so \"path_other\", I guess), and second \nvalue (now \"path_other\") should be \"path_head\" (exclusively).\n\n> \telse\n> \t\tfprintf(s->fp, \"1 %s %s %06o %06o %06o %s %s %s%c\",\n> \t\t\t\tkey, submodule_token,\n> @@ -2088,7 +2126,7 @@ static void wt_porcelain_v2_print_changed_entry(\n> \t\t\t\tpath_index, eol_char);\n> \n> \tstrbuf_release(&buf_index);\n> -\tstrbuf_release(&buf_head);\n> +\tstrbuf_release(&buf_other);\n> }\n> \n> /*\n> diff --git a/wt-status.h b/wt-status.h\n> index f9330982ac..332ff545aa 100644\n> --- a/wt-status.h\n> +++ b/wt-status.h\n> @@ -44,10 +44,10 @@ struct wt_status_change_data {\n> \tint worktree_status;\n> \tint index_status;\n> \tint stagemask;\n> -\tint head_score;\n> +\tint head_score, worktree_score;\n> \tint mode_head, mode_index, mode_worktree;\n> \tstruct object_id oid_head, oid_index;\n> -\tchar *head_path;\n> +\tchar *head_path, *worktree_path;\n> \tunsigned dirty_submodule : 2;\n> \tunsigned new_submodule_commits : 1;\n> };\n> \n\nFunny thing is that, overall, changes this patch introduces regarding \n\"--porcelain=v2\" are (functionally) pretty much what I did in that \nexercise patch I sent earlier[3], but while that one worked (on top \nof your v1), this one seems not to...\n\nCould it be because of that \"p->one->path\" vs \"p->two->path\" \ndifference introduced inside wt_status_collect_changed_cb() in patch \nseries v2, where wt_porcelain_v2_print_changed_entry() (using the \naffected value) stayed the same as in v1?\n\nRegards, Buga\n\n[1] https://public-inbox.org/git/20171226091012.24315-2-pclouds@gmail.com/T/#m866cf8b7edfd40172972771079c50bd5ff5bd535\n[2] https://git-scm.com/docs/git-status#_porcelain_format_version_2\n[3] https://public-inbox.org/git/20171226091012.24315-2-pclouds@gmail.com/T/#m095c33d69994c6ecb4f1adbf80dd48eab66750d8\n"},{"id":"335341","messageId":"cd764b8d-8a85-964f-eaf2-3e6fb784a2ff@gmail.com","threadId":"47486","inReplyTo":"20171226091012.24315-8-pclouds@gmail.com","subject":"Re: [PATCH v2 7/7] wt-status.c: avoid double renames in short/porcelain format","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2017-12-26T22:14:04Z","receivedAt":"2017-12-26T22:14:21Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"Hi Duy,\n\nOn 26/12/2017 10:10, Nguyễn Thái Ngọc Duy wrote:\n> \n> The presence of worktree rename leads to an interesting situation,\n> what if the same index entry is renamed twice, compared to HEAD and to\n> worktree? We can have that with this setup\n> \n> echo first > first && git add first && git commit -m first\n> git mv first second # rename reported in \"diff --cached\"\n> mv second third # rename reported in \"diff-files\"\n> \n> For the long format this is fine because we print two \"->\" rename\n> lines, one in the \"updated\" section, one in \"changed\" one.\n> \n> For other output formats, it gets tricky because they combine both\n> diffs in one line but can only display one rename per line. The result\n> \"XY\" column of short format, for example, would be \"RR\" in that case.\n> \n> This case either needs some extension in short/porcelain format\n> to show something crazy like\n> \n> RR first -> second -> third\n> \n> or we could show renames as two lines instead of one, for example\n> something like this for short form:\n> \n> R first -> second\n> R second -> third\n> \n> But for now it's safer and simpler to just break the \"second -> third\"\n> rename pair and show\n> \n> RD first -> second\n> A third\n> \n> like we have been showing until now.\n> \n\nI lost you a bit here, partially because of what seems to be an \nincomplete setup script, partially because of this last sentence, as \nGit v2.15.1 doesn`t seem to be showing this, so not sure about \"like \nwe have been showing until now\" part...?\n\nHere, with your setup script, with plain Git v2.15.1, we have:\n\n $ git status\n On branch master\n Changes to be committed:\n (use \"git reset HEAD ...\" to unstage)\n \n renamed: first -> second\n \n Changes not staged for commit:\n (use \"git add/rm ...\" to update what will be committed)\n (use \"git checkout -- ...\" to discard changes in working directory)\n \n deleted: second\n \n Untracked files:\n (use \"git add ...\" to include in what will be committed)\n \n third\n\nMight be an additional `git add -N -- third` is needed here, to show \nwhat (I assume) you wanted...? If so:\n\n $ git add -N third\n(1) $ git status\n On branch master\n Changes to be committed:\n (use \"git reset HEAD ...\" to unstage)\n \n renamed: first -> second\n \n Changes not staged for commit:\n (use \"git add ...\" to update what will be committed)\n (use \"git checkout -- ...\" to discard changes in working directory)\n \n renamed: second -> second\n ^^^^^^\nNow we can see two renames I believe you were talking about...? (Note \noriginal bug showing above, which started this thread.) Now, still \nusing v2.15.1, let`s see porcelain statuses:\n\n(2) $ git status --porcelain\n RR first -> second\n \n(3) $ git status --porcelain=v2\n 2 RR N... 100644 100644 000000 9c59e24b8393179a5d712de4f990178df5734d99 9c59e24b8393179a5d712de4f990178df5734d99 R100 second first\n\nHere, they both report renames in _both_ index and working tree (RR), \nbut they show \"index\" renamed path only (\"second\", in comparison to \noriginal value in HEAD, \"first\").\n\nI`m inclined to say this doesn`t align with what `git status` shows, \ndisrespecting `add -N` (or respecting it only partially, through that \nsecond R, but not showing the actual working tree rename, \"third\").\n\nWithout influencing porcelain format, and to fully respect `add -N`, \nI believe showing two renames (index and working tree) as two lines \nwould be the correct approach - and that`s what default `git status` \ndoes, too.\n\nNow, let`s examine this patch series v2 outputs:\n\n(1) $ git status\n On branch master\n Changes to be committed:\n (use \"git reset HEAD ...\" to unstage)\n \n \trenamed: first -> second\n \n Changes not staged for commit:\n (use \"git add ...\" to update what will be committed)\n (use \"git checkout -- ...\" to discard changes in working directory)\n \n \trenamed: second -> third\n \n(2) $ git status --porcelain\n RD first -> second\n A third\n\n(3) $ git status --porcelain=v2\n 2 RD N... 100644 100644 000000 9c59e24b8393179a5d712de4f990178df5734d99 9c59e24b8393179a5d712de4f990178df5734d99 R100 second\tfirst\n 1 .A N... 000000 000000 100644 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 third\n\nHere, porcelain statuses make situation a bit better, as now at least \n`add -N` is respected, showing new \"tracked\" path appearing in the \nworking tree.\n\nBut, we now lost any idea about the rename that happened there as \nwell - which Git v2.15.1 porcelain was partially showing (through \nRR), and which `git status` still reports correctly - and which we \nstill differ from.\n \nI don`t think this looks like what we have been showing until now \n(unless I misunderstood which exact \"now\" are we talking about), so I \ndon`t see that as a valid argument to support this case.\n\nSo, while we still changed output of what we were showing so far to \ntwo-line output, it seems there`s no real gain, as it looks like we \nreplaced one partial output (recognize rename, omit path) for the \nother (recognize path, omit rename).\n\nFinally, let`s see your initial patch v1[1], with my exercise \npatch[2] on top:\n\n(1) $ git status\n On branch master\n Changes to be committed:\n (use \"git reset HEAD ...\" to unstage)\n \n \trenamed: first -> second\n \n Changes not staged for commit:\n (use \"git add ...\" to update what will be committed)\n (use \"git checkout -- ...\" to discard changes in working directory)\n \n \trenamed: second -> third\n\n(2) $ git status --porcelain\n R first -> second\n R second -> third\n\n(3) $ git status --porcelain=v2\n 2 R. N... 100644 100644 100644 9c59e24b8393179a5d712de4f990178df5734d99 9c59e24b8393179a5d712de4f990178df5734d99 R100 second\tfirst\n 2 .R N... 100644 100644 100644 9c59e24b8393179a5d712de4f990178df5734d99 9c59e24b8393179a5d712de4f990178df5734d99 R100 third\tsecond\n\nHere, both \"--porcelain\" outputs (2) and (3) seem to much better \nreplicate what default `git status` is showing, too - namely separate \nrenames in comparison to HEAD for both \"index\" (2) and \"working tree\" (3).\n\nAnd if you don`t like two lines here in comparison to one (incomplete) \nline from Git v2.15.1, I would remark that patch series v2 prints two \nlines as well (so different from v2.15.1 in a same way), but with \nwhat looks like inferior output in comparison to v1 shown above, where \nboth renames are correctly recognized and reported - and finally \nfully compatible with default `git status` output, too.\n\nAnd if we really think about it, what v1 shows is what actually \nhappened - and more important, it`s possible to recreate hypothetical \n\"first -> second -> third\" change from there. With v2 output, that is \nimpossible, that information is lost as second line doesn`t relate to \nthe first one in any way.\n\nNow, unless I`m totally missing something here, the only thing left \nis that you mentioned v2 approach being \"safer and simpler\" than v1, \nsomething I`m not really competent to comment on, but just wanted to \nprovide a second opinion, maybe helping to change your mind in favor \nof v1 outputs, which seem to be _the_ correct ones...? :)\n\nIf not that much more complicated/unsafe, of course.\n\nThanks, Buga\n\n[1] https://public-inbox.org/git/20171226091012.24315-8-pclouds@gmail.com/T/#mf60e88fd351f7ff6a076279794c8343a79835f67\n[2] https://public-inbox.org/git/20171226091012.24315-8-pclouds@gmail.com/T/#m095c33d69994c6ecb4f1adbf80dd48eab66750d8\n"},{"id":"335343","messageId":"CACsJy8B7P-ObwG8yDH8A6RXccUgk-HowJfa4t+yPJ0ovSoUazg@mail.gmail.com","threadId":"47486","inReplyTo":"cd764b8d-8a85-964f-eaf2-3e6fb784a2ff@gmail.com","subject":"Re: [PATCH v2 7/7] wt-status.c: avoid double renames in short/porcelain format","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-27T00:49:58Z","receivedAt":"2017-12-27T00:50:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Dec 27, 2017 at 5:14 AM, Igor Djordjevic\n wrote:\n> Hi Duy,\n>\n> On 26/12/2017 10:10, Nguyễn Thái Ngọc Duy wrote:\n>>\n>> The presence of worktree rename leads to an interesting situation,\n>> what if the same index entry is renamed twice, compared to HEAD and to\n>> worktree? We can have that with this setup\n>>\n>> echo first > first && git add first && git commit -m first\n>> git mv first second # rename reported in \"diff --cached\"\n>> mv second third # rename reported in \"diff-files\"\n>>\n>> For the long format this is fine because we print two \"->\" rename\n>> lines, one in the \"updated\" section, one in \"changed\" one.\n>>\n>> For other output formats, it gets tricky because they combine both\n>> diffs in one line but can only display one rename per line. The result\n>> \"XY\" column of short format, for example, would be \"RR\" in that case.\n>>\n>> This case either needs some extension in short/porcelain format\n>> to show something crazy like\n>>\n>> RR first -> second -> third\n>>\n>> or we could show renames as two lines instead of one, for example\n>> something like this for short form:\n>>\n>> R first -> second\n>> R second -> third\n>>\n>> But for now it's safer and simpler to just break the \"second -> third\"\n>> rename pair and show\n>>\n>> RD first -> second\n>> A third\n>>\n>> like we have been showing until now.\n>>\n>\n> I lost you a bit here, partially because of what seems to be an\n> incomplete setup script, partially because of this last sentence, as\n> Git v2.15.1 doesn`t seem to be showing this, so not sure about \"like\n> we have been showing until now\" part...?\n\nYeah I missed a \"git add -N third\" in the setup. And \"until now\" was a\npoor choice of words. It should have been \"before 425a28e0a4\", where\n\"new files\" could not show up, which prevented rename detection in the\n\"Changed bot not staged for commit\" section in the first place.\n\nThough it's not _exactly_ like before. If you replace\n\"ita_invisible_in_index = 1\" with \"ita_invisible_in_index = 0\" in\nwt-status.c, you effectively roll back 425a28e0a4 and \"git status\n--short\" would show\n\n RD first -> second\n AM third\n\nThe second line is different and is what 425a28e0a4 tries to fix.\n\n> Now, still using v2.15.1, let`s see porcelain statuses:\n>\n> (2) $ git status --porcelain\n> RR first -> second\n>\n> (3) $ git status --porcelain=v2\n> 2 RR N... 100644 100644 000000 9c59e24b8393179a5d712de4f990178df5734d99 9c59e24b8393179a5d712de4f990178df5734d99 R100 second first\n>\n> Here, they both report renames in _both_ index and working tree (RR),\n> but they show \"index\" renamed path only (\"second\", in comparison to\n> original value in HEAD, \"first\").\n>\n> I`m inclined to say this doesn`t align with what `git status` shows,\n> disrespecting `add -N` (or respecting it only partially, through that\n> second R, but not showing the actual working tree rename, \"third\").\n>\n> Without influencing porcelain format, and to fully respect `add -N`,\n> I believe showing two renames (index and working tree) as two lines\n> would be the correct approach - and that`s what default `git status`\n> does, too.\n\nI agree. What worries me though, is the path in index seems to be be\nthe unique \"key\" of each line. Porcelain v2 shows this clearer when\n\"second\" is always in the same column. By showing two lines with the\nsame key (i.e. \"second\"), I'm not sure if we are breaking the\nporcelain format. Perhaps this is undefined area that we can just\ntweak?\n\n> Now, let`s examine this patch series v2 outputs:\n>\n> (1) $ git status\n> On branch master\n> Changes to be committed:\n> (use \"git reset HEAD ...\" to unstage)\n>\n> renamed: first -> second\n>\n> Changes not staged for commit:\n> (use \"git add ...\" to update what will be committed)\n> (use \"git checkout -- ...\" to discard changes in working directory)\n>\n> renamed: second -> third\n>\n> (2) $ git status --porcelain\n> RD first -> second\n> A third\n>\n> (3) $ git status --porcelain=v2\n> 2 RD N... 100644 100644 000000 9c59e24b8393179a5d712de4f990178df5734d99 9c59e24b8393179a5d712de4f990178df5734d99 R100 second first\n> 1 .A N... 000000 000000 100644 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 third\n>\n> Here, porcelain statuses make situation a bit better, as now at least\n> `add -N` is respected, showing new \"tracked\" path appearing in the\n> working tree.\n>\n> But, we now lost any idea about the rename that happened there as\n> well - which Git v2.15.1 porcelain was partially showing (through\n> RR), and which `git status` still reports correctly - and which we\n> still differ from.\n\nSorry again about \"now\". Before 425a28e0a4 rename detection would not\nkick in to find \"second -> third\" so people wouldn't know about rename\nanyway.\n\n>\n> I don`t think this looks like what we have been showing until now\n> (unless I misunderstood which exact \"now\" are we talking about), so I\n> don`t see that as a valid argument to support this case.\n>\n> So, while we still changed output of what we were showing so far to\n> two-line output, it seems there`s no real gain, as it looks like we\n> replaced one partial output (recognize rename, omit path) for the\n> other (recognize path, omit rename).\n>\n> Finally, let`s see your initial patch v1[1], with my exercise\n> patch[2] on top:\n>\n> (1) $ git status\n> On branch master\n> Changes to be committed:\n> (use \"git reset HEAD ...\" to unstage)\n>\n> renamed: first -> second\n>\n> Changes not staged for commit:\n> (use \"git add ...\" to update what will be committed)\n> (use \"git checkout -- ...\" to discard changes in working directory)\n>\n> renamed: second -> third\n>\n> (2) $ git status --porcelain\n> R first -> second\n> R second -> third\n>\n> (3) $ git status --porcelain=v2\n> 2 R. N... 100644 100644 100644 9c59e24b8393179a5d712de4f990178df5734d99 9c59e24b8393179a5d712de4f990178df5734d99 R100 second first\n> 2 .R N... 100644 100644 100644 9c59e24b8393179a5d712de4f990178df5734d99 9c59e24b8393179a5d712de4f990178df5734d99 R100 third second\n>\n> Here, both \"--porcelain\" outputs (2) and (3) seem to much better\n> replicate what default `git status` is showing, too - namely separate\n> renames in comparison to HEAD for both \"index\" (2) and \"working tree\" (3).\n\nThere is a problem with my v1 (or my misunderstanding of the code and\nthe porcelain v2 format). The first path column must always be the\npath in index according to git-status.txt\n\n The pathname. In a renamed/copied entry, this\n is the path in the index.\n\nwhich means the paths in the second line are swapped to\n\n 2 .R .... R100 secondthird\n\nThe current code ('master', before any of my changes) seems to also\nkey all entries by index path with those\n\"string_list_insert(&s->change,...\". And it fits porcelain v2\ndefinition. But it introduces this problem.\n\nPerhaps keying by index path is just not intentional (and porcelain v2\naccidentally exposes that), I don't know. If we relax this a bit like\nin my v1 (and conclude that we are not breaking any porcelain\nformats), then yes, the problem goes away. The new definition would be\n\n The pathname. In a renamed/copied entry, this\n is the target path.\n\n(and we leave the user to determine whether the target is in index or\nin worktree using code). I like this actually, \"R100 second\nthird\" looks just weird, but I'm not porcelain user and don't know how\nit's actually used.\n\n> And if you don`t like two lines here in comparison to one (incomplete)\n> line from Git v2.15.1, I would remark that patch series v2 prints two\n> lines as well (so different from v2.15.1 in a same way), but with\n> what looks like inferior output in comparison to v1 shown above, where\n> both renames are correctly recognized and reported - and finally\n> fully compatible with default `git status` output, too.\n>\n> And if we really think about it, what v1 shows is what actually\n> happened - and more important, it`s possible to recreate hypothetical\n> \"first -> second -> third\" change from there. With v2 output, that is\n> impossible, that information is lost as second line doesn`t relate to\n> the first one in any way.\n>\n> Now, unless I`m totally missing something here, the only thing left\n> is that you mentioned v2 approach being \"safer and simpler\" than v1,\n> something I`m not really competent to comment on, but just wanted to\n> provide a second opinion, maybe helping to change your mind in favor\n> of v1 outputs, which seem to be _the_ correct ones...? :)\n\nYeah it's \"safe\" to not break porcelain formats (and \"simpler\" than\nchanging them). But if it turns out I'm just paranoid, I'll happily\nrevert back to v1.\n\n> If not that much more complicated/unsafe, of course.\n>\n> Thanks, Buga\n>\n> [1] https://public-inbox.org/git/20171226091012.24315-8-pclouds@gmail.com/T/#mf60e88fd351f7ff6a076279794c8343a79835f67\n> [2] https://public-inbox.org/git/20171226091012.24315-8-pclouds@gmail.com/T/#m095c33d69994c6ecb4f1adbf80dd48eab66750d8\n-- \nDuy\n"},{"id":"335344","messageId":"CACsJy8Dn_XKA8=iLRZpj2EKYOSZqHT0jw9o_HzPH_vncGGeCCQ@mail.gmail.com","threadId":"47486","inReplyTo":"d0f2055c-e0c4-32af-6371-53d0e9a5fbc5@gmail.com","subject":"Re: [PATCH v2 6/7] wt-status.c: handle worktree renames","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-27T01:06:13Z","receivedAt":"2017-12-27T01:06:51Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Dec 27, 2017 at 1:14 AM, Igor Djordjevic\n wrote:\n> I`m afraid \"--porcelain=v2\" test might be incorrect here, as `git\n> status --porcelain=v2` output seems to be too, with this v2 series\n> applied. Test I sent previously[1] fails, and it looks valid.\n>\n> This is output I now get, with old/deleted file unstaged and\n> new/created file staged with `git add -N`:\n>\n> $ git status --porcelain=v2\n> 2 .R N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 12f00e90b6ef79117ce6e650416b8cf517099b78 R100 original-file new-file\n>\n> Note \"original-file\" listed first, where \"new-file\" listed second\n> (last). According the \"v2\" documentation[2] (excerpt):\n>\n> ... \n>\n> The pathname. In a renamed/copied entry, this\n> is the path in the index and in the working tree.\n\nGaah.. as you may see in the other mail when I quoted this\n(incorrectly). I must have modified this file at some point and\nthought it was true (my version did not have \"and in the worktree\").\n\nThe \"and\" is still problematic if you take this very seriously\n(because in this case index name and worktree name are different) but\nI think it's ok to ignore that \"and\" and switch it to \"or\".\n\n> ...\n> The pathname in the commit at HEAD. This is only\n> present in a renamed/copied entry, and tells\n> where the renamed/copied contents came from.\n>\n>\n> If I`m reading this correctly, it should be vice-versa - value from\n> HEAD, being \"original-file\", should come last, where value from\n> working tree (\"new-file\") should be first.\n\nYeah I think the \"where the renamed/copied contents came from\" clears\nup my confusion in this format. Back to v1 it is!\n-- \nDuy\n"},{"id":"335347","messageId":"20171227101839.26427-1-pclouds@gmail.com","threadId":"47486","inReplyTo":"20171226091012.24315-1-pclouds@gmail.com","subject":"[PATCH v3 0/6] Renames in git-status \"changed not staged\" section","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-27T10:18:33Z","receivedAt":"2017-12-27T10:19:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"v3 more or less goes back to v1 after my discussion with Igor about\nporcelain formats. So 7/7 is not needed anymore. 4/7 becomes 5/6. The\nmeat is still in 6/6, now with some more updates in git-status.txt and\nto address the comment from Torsten.\n\nNguyễn Thái Ngọc Duy (6):\n t2203: test status output with porcelain v2 format\n Use DIFF_DETECT_RENAME for detect_rename assignments\n wt-status.c: coding style fix\n wt-status.c: catch unhandled diff status codes\n wt-status.c: rename rename-related fields in wt_status_change_data\n wt-status.c: handle worktree renames\n\n Documentation/git-status.txt | 23 ++++++------\n builtin/commit.c | 2 +-\n diff.c | 2 +-\n t/t2203-add-intent.sh | 72 ++++++++++++++++++++++++++++++++++++++\n wt-status.c | 83 ++++++++++++++++++++++++++++----------------\n wt-status.h | 5 +--\n 6 files changed, 143 insertions(+), 44 deletions(-)\n\n-- \n2.15.0.320.g0453912d77\n\n"},{"id":"335348","messageId":"20171227101839.26427-2-pclouds@gmail.com","threadId":"47486","inReplyTo":"20171227101839.26427-1-pclouds@gmail.com","subject":"[PATCH v3 1/6] t2203: test status output with porcelain v2 format","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-27T10:18:34Z","receivedAt":"2017-12-27T10:20:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n t/t2203-add-intent.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex 1bdf38e80d..878e73fe98 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -25,6 +25,18 @@ test_expect_success 'git status' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git status with porcelain v2' '\n+\tgit status --porcelain=v2 | grep -v \"^?\" >actual &&\n+\tnam1=d00491fd7e5bb6fa28c517a0bb32b8b506539d4d &&\n+\tnam2=ce013625030ba8dba906f756967f9e9ca394464a &&\n+\tcat >expect <<-EOF &&\n+\t1 DA N... 100644 000000 100644 $nam1 $_z40 1.t\n+\t1 A. N... 000000 100644 100644 $_z40 $nam2 elif\n+\t1 .A N... 000000 000000 100644 $_z40 $_z40 file\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'check result of \"add -N\"' '\n \tgit ls-files -s file >actual &&\n \tempty=$(git hash-object --stdin \n---\n builtin/commit.c | 2 +-\n diff.c | 2 +-\n wt-status.c | 4 ++--\n 3 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 8a87701414..1f11e3992d 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1507,7 +1507,7 @@ static void print_summary(const char *prefix, const struct object_id *oid,\n \trev.show_root_diff = 1;\n \tget_commit_format(format.buf, &rev);\n \trev.always_show_header = 0;\n-\trev.diffopt.detect_rename = 1;\n+\trev.diffopt.detect_rename = DIFF_DETECT_RENAME;\n \trev.diffopt.break_opt = 0;\n \tdiff_setup_done(&rev.diffopt);\n \ndiff --git a/diff.c b/diff.c\nindex 3fb445a54d..51fe31c7aa 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -246,7 +246,7 @@ static int parse_ws_error_highlight(const char *arg)\n */\n void init_diff_ui_defaults(void)\n {\n-\tdiff_detect_rename_default = 1;\n+\tdiff_detect_rename_default = DIFF_DETECT_RENAME;\n }\n \n int git_diff_heuristic_config(const char *var, const char *value, void *cb)\ndiff --git a/wt-status.c b/wt-status.c\nindex ef26f07446..59338adb8b 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -602,7 +602,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = wt_status_collect_updated_cb;\n \trev.diffopt.format_callback_data = s;\n-\trev.diffopt.detect_rename = 1;\n+\trev.diffopt.detect_rename = DIFF_DETECT_RENAME;\n \trev.diffopt.rename_limit = 200;\n \trev.diffopt.break_opt = 0;\n \tcopy_pathspec(&rev.prune_data, &s->pathspec);\n@@ -962,7 +962,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n \tsetup_revisions(0, NULL, &rev, &opt);\n \n \trev.diffopt.output_format |= DIFF_FORMAT_PATCH;\n-\trev.diffopt.detect_rename = 1;\n+\trev.diffopt.detect_rename = DIFF_DETECT_RENAME;\n \trev.diffopt.file = s->fp;\n \trev.diffopt.close_file = 0;\n \t/*\n-- \n2.15.0.320.g0453912d77\n\n"},{"id":"335350","messageId":"20171227101839.26427-4-pclouds@gmail.com","threadId":"47486","inReplyTo":"20171227101839.26427-1-pclouds@gmail.com","subject":"[PATCH v3 3/6] wt-status.c: coding style fix","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-27T10:18:36Z","receivedAt":"2017-12-27T10:20:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n wt-status.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 59338adb8b..db06fc7c85 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -406,7 +406,8 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n \tstrbuf_release(&twobuf);\n }\n \n-static char short_submodule_status(struct wt_status_change_data *d) {\n+static char short_submodule_status(struct wt_status_change_data *d)\n+{\n \tif (d->new_submodule_commits)\n \t\treturn 'M';\n \tif (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)\n-- \n2.15.0.320.g0453912d77\n\n"},{"id":"335351","messageId":"20171227101839.26427-5-pclouds@gmail.com","threadId":"47486","inReplyTo":"20171227101839.26427-1-pclouds@gmail.com","subject":"[PATCH v3 4/6] wt-status.c: catch unhandled diff status codes","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-27T10:18:37Z","receivedAt":"2017-12-27T10:20:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n wt-status.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex db06fc7c85..05af895fe2 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -468,8 +468,8 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\t\toidcpy(&d->oid_index, &p->one->oid);\n \t\t\tbreak;\n \n-\t\tcase DIFF_STATUS_UNKNOWN:\n-\t\t\tdie(\"BUG: worktree status unknown???\");\n+\t\tdefault:\n+\t\t\tdie(\"BUG: unhandled diff-files status '%c'\", p->status);\n \t\t\tbreak;\n \t\t}\n \n@@ -549,6 +549,10 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,\n \t\t\t * values in these fields.\n \t\t\t */\n \t\t\tbreak;\n+\n+\t\tdefault:\n+\t\t\tdie(\"BUG: unhandled diff-index status '%c'\", p->status);\n+\t\t\tbreak;\n \t\t}\n \t}\n }\n-- \n2.15.0.320.g0453912d77\n\n"},{"id":"335352","messageId":"20171227101839.26427-6-pclouds@gmail.com","threadId":"47486","inReplyTo":"20171227101839.26427-1-pclouds@gmail.com","subject":"[PATCH v3 5/6] wt-status.c: rename rename-related fields in wt_status_change_data","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-27T10:18:38Z","receivedAt":"2017-12-27T10:20:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"These field \"head_path\" is used for rename display only. In the next\npatch we introduce another rename pair where the rename source is no\nlonger HEAD. Rename it to something more generic.\n\nWhile at there, rename \"score\" as well and store the rename diff code\nin a separate field instead of hardcoding key[0] (i.e. diff-index) in\nporcelain v2 code.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n wt-status.c | 50 ++++++++++++++++++++++++++------------------------\n wt-status.h | 5 +++--\n 2 files changed, 29 insertions(+), 26 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 05af895fe2..fab6951573 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -360,8 +360,8 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n \tswitch (change_type) {\n \tcase WT_STATUS_UPDATED:\n \t\tstatus = d->index_status;\n-\t\tif (d->head_path)\n-\t\t\tone_name = d->head_path;\n+\t\tif (d->rename_source)\n+\t\t\tone_name = d->rename_source;\n \t\tbreak;\n \tcase WT_STATUS_CHANGED:\n \t\tif (d->new_submodule_commits || d->dirty_submodule) {\n@@ -391,7 +391,7 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n \t\tdie(\"BUG: unhandled diff status %c\", status);\n \tlen = label_width - utf8_strwidth(what);\n \tassert(len >= 0);\n-\tif (status == DIFF_STATUS_COPIED || status == DIFF_STATUS_RENAMED)\n+\tif (one_name != two_name)\n \t\tstatus_printf_more(s, c, \"%s%.*s%s -> %s\",\n \t\t\t\t what, len, padding, one, two);\n \telse\n@@ -531,8 +531,9 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,\n \n \t\tcase DIFF_STATUS_COPIED:\n \t\tcase DIFF_STATUS_RENAMED:\n-\t\t\td->head_path = xstrdup(p->one->path);\n-\t\t\td->score = p->score * 100 / MAX_SCORE;\n+\t\t\td->rename_source = xstrdup(p->one->path);\n+\t\t\td->rename_score = p->score * 100 / MAX_SCORE;\n+\t\t\td->rename_status = p->status;\n \t\t\t/* fallthru */\n \t\tcase DIFF_STATUS_MODIFIED:\n \t\tcase DIFF_STATUS_TYPE_CHANGED:\n@@ -1724,13 +1725,14 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \tputchar(' ');\n \tif (s->null_termination) {\n \t\tfprintf(stdout, \"%s%c\", it->string, 0);\n-\t\tif (d->head_path)\n-\t\t\tfprintf(stdout, \"%s%c\", d->head_path, 0);\n+\t\tif (d->rename_source)\n+\t\t\tfprintf(stdout, \"%s%c\", d->rename_source, 0);\n \t} else {\n \t\tstruct strbuf onebuf = STRBUF_INIT;\n \t\tconst char *one;\n-\t\tif (d->head_path) {\n-\t\t\tone = quote_path(d->head_path, s->prefix, &onebuf);\n+\n+\t\tif (d->rename_source) {\n+\t\t\tone = quote_path(d->rename_source, s->prefix, &onebuf);\n \t\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n \t\t\t\tputchar('\"');\n \t\t\t\tstrbuf_addch(&onebuf, '\"');\n@@ -2035,10 +2037,10 @@ static void wt_porcelain_v2_print_changed_entry(\n \tstruct wt_status *s)\n {\n \tstruct wt_status_change_data *d = it->util;\n-\tstruct strbuf buf_index = STRBUF_INIT;\n-\tstruct strbuf buf_head = STRBUF_INIT;\n-\tconst char *path_index = NULL;\n-\tconst char *path_head = NULL;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf buf_from = STRBUF_INIT;\n+\tconst char *path = NULL;\n+\tconst char *path_from = NULL;\n \tchar key[3];\n \tchar submodule_token[5];\n \tchar sep_char, eol_char;\n@@ -2057,8 +2059,8 @@ static void wt_porcelain_v2_print_changed_entry(\n \t\t */\n \t\tsep_char = '\\0';\n \t\teol_char = '\\0';\n-\t\tpath_index = it->string;\n-\t\tpath_head = d->head_path;\n+\t\tpath = it->string;\n+\t\tpath_from = d->rename_source;\n \t} else {\n \t\t/*\n \t\t * Path(s) are C-quoted if necessary. Current path is ALWAYS first.\n@@ -2068,27 +2070,27 @@ static void wt_porcelain_v2_print_changed_entry(\n \t\t */\n \t\tsep_char = '\\t';\n \t\teol_char = '\\n';\n-\t\tpath_index = quote_path(it->string, s->prefix, &buf_index);\n-\t\tif (d->head_path)\n-\t\t\tpath_head = quote_path(d->head_path, s->prefix, &buf_head);\n+\t\tpath = quote_path(it->string, s->prefix, &buf);\n+\t\tif (d->rename_source)\n+\t\t\tpath_from = quote_path(d->rename_source, s->prefix, &buf_from);\n \t}\n \n-\tif (path_head)\n+\tif (path_from)\n \t\tfprintf(s->fp, \"2 %s %s %06o %06o %06o %s %s %c%d %s%c%s%c\",\n \t\t\t\tkey, submodule_token,\n \t\t\t\td->mode_head, d->mode_index, d->mode_worktree,\n \t\t\t\toid_to_hex(&d->oid_head), oid_to_hex(&d->oid_index),\n-\t\t\t\tkey[0], d->score,\n-\t\t\t\tpath_index, sep_char, path_head, eol_char);\n+\t\t\t\td->rename_status, d->rename_score,\n+\t\t\t\tpath, sep_char, path_from, eol_char);\n \telse\n \t\tfprintf(s->fp, \"1 %s %s %06o %06o %06o %s %s %s%c\",\n \t\t\t\tkey, submodule_token,\n \t\t\t\td->mode_head, d->mode_index, d->mode_worktree,\n \t\t\t\toid_to_hex(&d->oid_head), oid_to_hex(&d->oid_index),\n-\t\t\t\tpath_index, eol_char);\n+\t\t\t\tpath, eol_char);\n \n-\tstrbuf_release(&buf_index);\n-\tstrbuf_release(&buf_head);\n+\tstrbuf_release(&buf);\n+\tstrbuf_release(&buf_from);\n }\n \n /*\ndiff --git a/wt-status.h b/wt-status.h\nindex fe27b465e2..3f84d5c29f 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -44,10 +44,11 @@ struct wt_status_change_data {\n \tint worktree_status;\n \tint index_status;\n \tint stagemask;\n-\tint score;\n \tint mode_head, mode_index, mode_worktree;\n \tstruct object_id oid_head, oid_index;\n-\tchar *head_path;\n+\tint rename_status;\n+\tint rename_score;\n+\tchar *rename_source;\n \tunsigned dirty_submodule : 2;\n \tunsigned new_submodule_commits : 1;\n };\n-- \n2.15.0.320.g0453912d77\n\n"},{"id":"335353","messageId":"20171227101839.26427-7-pclouds@gmail.com","threadId":"47486","inReplyTo":"20171227101839.26427-1-pclouds@gmail.com","subject":"[PATCH v3 6/6] wt-status.c: handle worktree renames","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-12-27T10:18:39Z","receivedAt":"2017-12-27T10:20:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Before 425a28e0a4 (diff-lib: allow ita entries treated as \"not yet exist\nin index\" - 2016-10-24) there are never \"new files\" in the index, which\nessentially disables rename detection because we only detect renames\nwhen a new file appears in a diff pair.\n\nAfter that commit, an i-t-a entry can appear as a new file in \"git\ndiff-files\". But the diff callback function in wt-status.c does not\nhandle this case and produces incorrect status output.\n\nPS. The reader may notice that this patch adds a new xstrdup() but not\na free(). Yes we leak memory (the same for head_path). But wt_status\nso far has been short lived, this leak should not matter in\npractice.\n\nNoticed-by: Alex Vandiver \nHelped-by: Igor Djordjevic \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/git-status.txt | 23 +++++++++--------\n t/t2203-add-intent.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++\n wt-status.c | 22 +++++++++++++---\n 3 files changed, 92 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-status.txt b/Documentation/git-status.txt\nindex 81cab9aefb..72bfb87f66 100644\n--- a/Documentation/git-status.txt\n+++ b/Documentation/git-status.txt\n@@ -149,14 +149,15 @@ the status.relativePaths config option below.\n Short Format\n ~~~~~~~~~~~~\n \n-In the short-format, the status of each path is shown as\n+In the short-format, the status of each path is shown as one of these\n+forms\n \n-\tXY PATH1 -> PATH2\n+\tXY PATH\n+\tXY ORIG_PATH -> PATH\n \n-where `PATH1` is the path in the `HEAD`, and the \" `-> PATH2`\" part is\n-shown only when `PATH1` corresponds to a different path in the\n-index/worktree (i.e. the file is renamed). The `XY` is a two-letter\n-status code.\n+where `ORIG_PATH` is where the renamed/copied contents came\n+from. `ORIG_PATH` is only shown when the entry is renamed or\n+copied. The `XY` is a two-letter status code.\n \n The fields (including the `->`) are separated from each other by a\n single space. If a filename contains whitespace or other nonprintable\n@@ -192,6 +193,8 @@ in which case `XY` are `!!`.\n [MARC] index and work tree matches\n [ MARC] M work tree changed since index\n [ MARC] D deleted in work tree\n+ [ D] R renamed in work tree\n+ [ D] C copied in work tree\n -------------------------------------------------\n D D unmerged, both deleted\n A U unmerged, added by us\n@@ -309,13 +312,13 @@ Renamed or copied entries have the following format:\n \t\tof similarity between the source and target of the\n \t\tmove or copy). For example \"R100\" or \"C75\".\n The pathname. In a renamed/copied entry, this\n-\t\tis the path in the index and in the working tree.\n+\t\tis the target path.\n When the `-z` option is used, the 2 pathnames are separated\n \t\twith a NUL (ASCII 0x00) byte; otherwise, a tab (ASCII 0x09)\n \t\tbyte separates them.\n- The pathname in the commit at HEAD. This is only\n-\t\tpresent in a renamed/copied entry, and tells\n-\t\twhere the renamed/copied contents came from.\n+ The pathname in the commit at HEAD or in the index.\n+\t\tThis is only present in a renamed/copied entry, and\n+\t\ttells where the renamed/copied contents came from.\n --------------------------------------------------------\n \n Unmerged entries have the following format; the first character is\ndiff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh\nindex 878e73fe98..78236dc7d8 100755\n--- a/t/t2203-add-intent.sh\n+++ b/t/t2203-add-intent.sh\n@@ -162,5 +162,65 @@ test_expect_success 'commit: ita entries ignored in empty commit check' '\n \t)\n '\n \n+test_expect_success 'rename detection finds the right names' '\n+\tgit init rename-detection &&\n+\t(\n+\t\tcd rename-detection &&\n+\t\techo contents >first &&\n+\t\tgit add first &&\n+\t\tgit commit -m first &&\n+\t\tmv first third &&\n+\t\tgit add -N third &&\n+\n+\t\tgit status | grep -v \"^?\" >actual.1 &&\n+\t\ttest_i18ngrep \"renamed: *first -> third\" actual.1 &&\n+\n+\t\tgit status --porcelain | grep -v \"^?\" >actual.2 &&\n+\t\tcat >expected.2 <<-\\EOF &&\n+\t\t R first -> third\n+\t\tEOF\n+\t\ttest_cmp expected.2 actual.2 &&\n+\n+\t\thash=12f00e90b6ef79117ce6e650416b8cf517099b78 &&\n+\t\tgit status --porcelain=v2 | grep -v \"^?\" >actual.3 &&\n+\t\tcat >expected.3 <<-EOF &&\n+\t\t2 .R N... 100644 100644 100644 $hash $hash R100 third\tfirst\n+\t\tEOF\n+\t\ttest_cmp expected.3 actual.3\n+\t)\n+'\n+\n+test_expect_success 'double rename detection in status' '\n+\tgit init rename-detection-2 &&\n+\t(\n+\t\tcd rename-detection-2 &&\n+\t\techo contents >first &&\n+\t\tgit add first &&\n+\t\tgit commit -m first &&\n+\t\tgit mv first second &&\n+\t\tmv second third &&\n+\t\tgit add -N third &&\n+\n+\t\tgit status | grep -v \"^?\" >actual.1 &&\n+\t\ttest_i18ngrep \"renamed: *first -> second\" actual.1 &&\n+\t\ttest_i18ngrep \"renamed: *second -> third\" actual.1 &&\n+\n+\t\tgit status --porcelain | grep -v \"^?\" >actual.2 &&\n+\t\tcat >expected.2 <<-\\EOF &&\n+\t\tR first -> second\n+\t\t R second -> third\n+\t\tEOF\n+\t\ttest_cmp expected.2 actual.2 &&\n+\n+\t\thash=12f00e90b6ef79117ce6e650416b8cf517099b78 &&\n+\t\tgit status --porcelain=v2 | grep -v \"^?\" >actual.3 &&\n+\t\tcat >expected.3 <<-EOF &&\n+\t\t2 R. N... 100644 100644 100644 $hash $hash R100 second\tfirst\n+\t\t2 .R N... 100644 100644 100644 $hash $hash R100 third\tsecond\n+\t\tEOF\n+\t\ttest_cmp expected.3 actual.3\n+\t)\n+'\n+\n test_done\n \ndiff --git a/wt-status.c b/wt-status.c\nindex fab6951573..f5debcd2b4 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -360,8 +360,6 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n \tswitch (change_type) {\n \tcase WT_STATUS_UPDATED:\n \t\tstatus = d->index_status;\n-\t\tif (d->rename_source)\n-\t\t\tone_name = d->rename_source;\n \t\tbreak;\n \tcase WT_STATUS_CHANGED:\n \t\tif (d->new_submodule_commits || d->dirty_submodule) {\n@@ -382,6 +380,14 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n \t\t change_type);\n \t}\n \n+\t/*\n+\t * Only pick up the rename it's relevant. If the rename is for\n+\t * the changed section and we're printing the updated section,\n+\t * ignore it.\n+\t */\n+\tif (d->rename_status == status)\n+\t\tone_name = d->rename_source;\n+\n \tone = quote_path(one_name, s->prefix, &onebuf);\n \ttwo = quote_path(two_name, s->prefix, &twobuf);\n \n@@ -433,7 +439,7 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\tstruct wt_status_change_data *d;\n \n \t\tp = q->queue[i];\n-\t\tit = string_list_insert(&s->change, p->one->path);\n+\t\tit = string_list_insert(&s->change, p->two->path);\n \t\td = it->util;\n \t\tif (!d) {\n \t\t\td = xcalloc(1, sizeof(*d));\n@@ -460,6 +466,14 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\t\t/* mode_worktree is zero for a delete. */\n \t\t\tbreak;\n \n+\t\tcase DIFF_STATUS_COPIED:\n+\t\tcase DIFF_STATUS_RENAMED:\n+\t\t\tif (d->rename_status)\n+\t\t\t\tdie(\"BUG: multiple renames on the same target? how?\");\n+\t\t\td->rename_source = xstrdup(p->one->path);\n+\t\t\td->rename_score = p->score * 100 / MAX_SCORE;\n+\t\t\td->rename_status = p->status;\n+\t\t\t/* fallthru */\n \t\tcase DIFF_STATUS_MODIFIED:\n \t\tcase DIFF_STATUS_TYPE_CHANGED:\n \t\tcase DIFF_STATUS_UNMERGED:\n@@ -531,6 +545,8 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,\n \n \t\tcase DIFF_STATUS_COPIED:\n \t\tcase DIFF_STATUS_RENAMED:\n+\t\t\tif (d->rename_status)\n+\t\t\t\tdie(\"BUG: multiple renames on the same target? how?\");\n \t\t\td->rename_source = xstrdup(p->one->path);\n \t\t\td->rename_score = p->score * 100 / MAX_SCORE;\n \t\t\td->rename_status = p->status;\n-- \n2.15.0.320.g0453912d77\n\n"},{"id":"335370","messageId":"xmqqh8scnip5.fsf@gitster.mtv.corp.google.com","threadId":"47486","inReplyTo":"20171226021150.GA10059@duynguyen.vn.dektech.internal","subject":"Re: [PATCH] status: handle worktree renames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-27T18:12:38Z","receivedAt":"2017-12-27T18:12:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> Or we disable rename-from-worktree when porcelain v2 is requested (and\n> optionally introduce v3 to support it). Jeff, any preference?\n\nI actually think disabling rename-from-worktree consistently may be\nthe best way to go.\n\n"},{"id":"335371","messageId":"xmqqd130nigv.fsf@gitster.mtv.corp.google.com","threadId":"47486","inReplyTo":"CACsJy8CN58ivOeAr83X86NZcBy+yrJd0SbFhej99Pjb8x8_gBA@mail.gmail.com","subject":"Re: [PATCH] status: handle worktree renames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-27T18:17:36Z","receivedAt":"2017-12-27T18:17:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n>> The problem is, you cannot know if it's a rename from HEAD or from\n>> worktree with this updated v2 (or perhaps you could because HEAD name\n>> should be all zero?).\n>\n> I'm wrong about this. the \"\" code for HEAD rename would be \"R.\"\n> while worktree rename is \".R\" so I think we're good.\n\nAh, OK, then. Thanks.\n\n"},{"id":"335413","messageId":"46af07ed-31bf-f9a8-7e28-621baac93e14@gmail.com","threadId":"47486","inReplyTo":"CACsJy8B7P-ObwG8yDH8A6RXccUgk-HowJfa4t+yPJ0ovSoUazg@mail.gmail.com","subject":"Re: [PATCH v2 7/7] wt-status.c: avoid double renames in short/porcelain format","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2017-12-27T23:53:49Z","receivedAt":"2017-12-27T23:54:08Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"On 27/12/2017 01:49, Duy Nguyen wrote:\n> \n> > I lost you a bit here, partially because of what seems to be an\n> > incomplete setup script, partially because of this last sentence, as\n> > Git v2.15.1 doesn`t seem to be showing this, so not sure about \"like\n> > we have been showing until now\" part...?\n> \n> Yeah I missed a \"git add -N third\" in the setup. And \"until now\" was a\n> poor choice of words. It should have been \"before 425a28e0a4\", where\n> \"new files\" could not show up, which prevented rename detection in the\n> \"Changed bot not staged for commit\" section in the first place.\n> ...\n> Sorry again about \"now\". Before 425a28e0a4 rename detection would not\n> kick in to find \"second -> third\" so people wouldn't know about rename\n> anyway.\n\nYeah, no worries, I had I hunch that it might be what you meant, but \ngot too involved in all the rest that I forgot to bring that one up, \ntoo. Thanks for clarifying, though.\n\nRegards, Buga\n"},{"id":"335417","messageId":"fb29d5bf-daae-fa8b-b787-e536cd5f98c8@gmail.com","threadId":"47486","inReplyTo":"CACsJy8Dn_XKA8=iLRZpj2EKYOSZqHT0jw9o_HzPH_vncGGeCCQ@mail.gmail.com","subject":"Re: [PATCH v2 6/7] wt-status.c: handle worktree renames","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2017-12-28T00:50:29Z","receivedAt":"2017-12-28T00:50:48Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"On 27/12/2017 02:06, Duy Nguyen wrote:\n> \n> > ... \n> >\n> > The pathname. In a renamed/copied entry, this\n> > is the path in the index and in the working tree.\n> \n> Gaah.. as you may see in the other mail when I quoted this\n> (incorrectly). I must have modified this file at some point and\n> thought it was true (my version did not have \"and in the worktree\").\n\nAh, this explains a lot... :) I got really confused with your v2, it \nfelt as the series took a strange turn, and in a kind of a subtle way :P\n\n> The \"and\" is still problematic if you take this very seriously\n> (because in this case index name and worktree name are different) but\n> I think it's ok to ignore that \"and\" and switch it to \"or\".\n\nYes, I agree, and the change does feel like a good thing. But, I also \nnow hope this doesn`t break any expectations, because... (read below)\n\n> > The pathname in the commit at HEAD. This is only\n> > present in a renamed/copied entry, and tells\n> > where the renamed/copied contents came from.\n> >\n> > If I`m reading this correctly, it should be vice-versa - value from\n> > HEAD, being \"original-file\", should come last, where value from\n> > working tree (\"new-file\") should be first.\n\n... it totally slipped me that documentation is/was pretty strict \nabout being HEAD path (exclusively), where I was still \nexpecting it to show renamed working tree \"from\" value as \nin case of working tree (double) rename, too - where that exact \n(already renamed in index) path wasn`t to be found inside HEAD at \nall, so the working tree rename couldn`t really be shown as \"source\" \nand \"target\" rename pair, strictly following the \"porcelain v2\" \nspecification... :/\n\nI see now that your initial reply[1] was talking about this, but I \ndidn`t focus on it much as you replied to it yourself shortly \nafterwards, and later v2 of the series came up.\n\nMight be this is where you changed your offline documentation \nversion, too, as that is what the sample patch was about :)\n\n> Yeah I think the \"where the renamed/copied contents came from\" clears\n> up my confusion in this format. Back to v1 it is!\n\nI see you addressed this by loosening the restriction here a bit, too,\nmaking be \"pathname in the commit at HEAD _or in the index_\",\nin your \"[PATCH v3 6/6] wt-status.c: handle worktree renames\"[2].\n\nI repeat that this looks like the correct approach, making fully \ndescribed working tree rename detection possible in porcelain in the \nfirst place, but also aligning output of \"status\" --porcelain \nvariants with its default (--long) form.\n\nHopefully, on top of everything positive, it also doesn`t break \nanything (too much?)... :P Latest revision should now provide all the \nnecessary ingredients to resolve what happened, for the (small?) \nprice of tweaking previous expectations a bit.\n\nRegards, Buga\n\n[1] https://public-inbox.org/git/CACsJy8A=jZ9LAuM50GVjNT5gtdiYYMyMuPBSrJFO4LmKVQsETQ@mail.gmail.com/T/#mf2f5ae672ec6f4e1abecbd5fe65283e9d8fbed57\n[2] https://public-inbox.org/git/20171227101839.26427-7-pclouds@gmail.com/T/#u\n"},{"id":"335418","messageId":"66038d94-6e2e-9af0-d8bb-04cbe6f189b8@gmail.com","threadId":"47486","inReplyTo":"20171227101839.26427-1-pclouds@gmail.com","subject":"Re: [PATCH v3 0/6] Renames in git-status \"changed not staged\" section","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2017-12-28T00:59:17Z","receivedAt":"2017-12-28T00:59:35Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"Hi Duy,\n\nOn 27/12/2017 11:18, Nguyễn Thái Ngọc Duy wrote:\n> \n> v3 more or less goes back to v1 after my discussion with Igor about\n> porcelain formats. So 7/7 is not needed anymore. 4/7 becomes 5/6. The\n> meat is still in 6/6, now with some more updates in git-status.txt and\n> to address the comment from Torsten.\n\nAlbeit a tiny concern expressed in that last e-mail[1], this now \nseems fine, and a few tests I did came back as expected. Thanks!\n\nRegards, Buga\n\n[1] https://public-inbox.org/git/CACsJy8A=jZ9LAuM50GVjNT5gtdiYYMyMuPBSrJFO4LmKVQsETQ@mail.gmail.com/T/#m18b4e2cb2b7685fcc9650f3fb71b2191ef74cbe1\n"},{"id":"335420","messageId":"1e89cabf-c6ca-de43-fac0-399da9a9776d@gmail.com","threadId":"47486","inReplyTo":"fb29d5bf-daae-fa8b-b787-e536cd5f98c8@gmail.com","subject":"Re: [PATCH v2 6/7] wt-status.c: handle worktree renames","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2017-12-28T02:14:41Z","receivedAt":"2017-12-28T02:15:02Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"p.s. An extra note for the casual reader, just in case:\n\nOn 28/12/2017 01:50, Igor Djordjevic wrote:\n> \n> ... it totally slipped me that documentation is/was pretty strict \n> about being HEAD path (exclusively), where I was still \n> expecting it to show renamed working tree \"from\" value as \n> in case of working tree (double) rename, too - where that exact \n> (already renamed in index) path wasn`t to be found inside HEAD at \n> all, so the working tree rename couldn`t really be shown as \"source\" \n> and \"target\" rename pair, strictly following the \"porcelain v2\" \n> specification... :/\n> \n> ...\n> \n> I repeat that this looks like the correct approach, making fully \n> described working tree rename detection possible in porcelain in the \n> first place, but also aligning output of \"status\" --porcelain \n> variants with its default (--long) form.\n\nWherever \"working tree rename detection\" is discussed above, it`s all \nstill about renamed file `git add -N` case only (where old name, \nappearing \"deleted\" from HEAD/index, is not staged yet), not some new \nfunctionality where renamed file inside working tree is instantly / \nmagically recognized without being staged in the first place.\n"},{"id":"335634","messageId":"3a8169b4-1d7f-35dc-eb23-49afdaa3bf52@jeffhostetler.com","threadId":"47486","inReplyTo":"xmqqh8scnip5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] status: handle worktree renames","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2018-01-02T21:14:09Z","receivedAt":"2018-01-02T21:14:15Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 12/27/2017 1:12 PM, Junio C Hamano wrote:\n> Duy Nguyen writes:\n> \n>> Or we disable rename-from-worktree when porcelain v2 is requested (and\n>> optionally introduce v3 to support it). Jeff, any preference?\n\nSorry for the delay, I was on vacation last week.\n\nI like the \"R.\" and \".R\" lines in your 3rd patch series as that keeps\nporcelain V2 output consistent with the changes that you added to plain\nand porcelain V1 output. All 3 formats now report 2 types of renames.\nHaving a \"RR\" line would be more consistent with a \"MM\" line, but I\ndon't think that happens often enough to define a porcelain V3 format\nwith a 3 path row variant.\n\nI like that we can now show \"unstaged renames\" (in all 3 formats)\nas I think that is less confusing to the novice user than a\nnew-file/delete pair.\n\n\nHaving said that, I am a little concerned about us changing V1 and\nV2 output at all -- we are breaking the porcelain contract we have\nwith scripts. I like the change, so I'm not bothered about it, but\nothers may think differently.\n\n\nAlso, does this introduce any new cases for reporting conflicts?\nI haven't really thought about it too much yet, but if there was a\ndivergent rename in both branches of a merge, do we now have to handle\nshowing possibly 4 pathnames for a file? (merge-base, branch-a,\nbranch-b, worktree)\n\nJeff\n\n\n"},{"id":"335635","messageId":"ad27dd2c-a39b-03a0-a1ea-152d1205acef@jeffhostetler.com","threadId":"47486","inReplyTo":"20171227101839.26427-1-pclouds@gmail.com","subject":"Re: [PATCH v3 0/6] Renames in git-status \"changed not staged\" section","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2018-01-02T21:22:33Z","receivedAt":"2018-01-02T21:22:38Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 12/27/2017 5:18 AM, Nguyễn Thái Ngọc Duy wrote:\n> v3 more or less goes back to v1 after my discussion with Igor about\n> porcelain formats. So 7/7 is not needed anymore. 4/7 becomes 5/6. The\n> meat is still in 6/6, now with some more updates in git-status.txt and\n> to address the comment from Torsten.\n> \n> Nguyễn Thái Ngọc Duy (6):\n> t2203: test status output with porcelain v2 format\n> Use DIFF_DETECT_RENAME for detect_rename assignments\n> wt-status.c: coding style fix\n> wt-status.c: catch unhandled diff status codes\n> wt-status.c: rename rename-related fields in wt_status_change_data\n> wt-status.c: handle worktree renames\n> \n> Documentation/git-status.txt | 23 ++++++------\n> builtin/commit.c | 2 +-\n> diff.c | 2 +-\n> t/t2203-add-intent.sh | 72 ++++++++++++++++++++++++++++++++++++++\n> wt-status.c | 83 ++++++++++++++++++++++++++++----------------\n> wt-status.h | 5 +--\n> 6 files changed, 143 insertions(+), 44 deletions(-)\n> \n\n\nSigned-off-by: Jeff Hostetler \n\n"},{"id":"336350","messageId":"CACsJy8BgiDrr81FFM8-Wh2ufbPAaqVzncEp9ReB1N8hz8WtF1Q@mail.gmail.com","threadId":"47486","inReplyTo":"3a8169b4-1d7f-35dc-eb23-49afdaa3bf52@jeffhostetler.com","subject":"Re: [PATCH] status: handle worktree renames","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-01-10T09:26:13Z","receivedAt":"2018-01-10T09:26:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Jan 3, 2018 at 4:14 AM, Jeff Hostetler wrote:\n> Also, does this introduce any new cases for reporting conflicts?\n> I haven't really thought about it too much yet, but if there was a\n> divergent rename in both branches of a merge, do we now have to handle\n> showing possibly 4 pathnames for a file? (merge-base, branch-a,\n> branch-b, worktree)\n\nIt's an interesting question but unfortunately I don't have an answer\n(I read your mail earlier but waited for so long because I didn't know\nthe answer then).\n-- \nDuy\n"}]}