{"thread":{"id":"51639","subject":"[PATCH] worktree remove: clarify error message on dirty worktree","startedAt":"2019-08-13T18:02:51Z","lastAt":"2019-08-13T20:11:07Z","messageCount":3,"participants":["SZEDER Gábor","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"380401","messageId":"20190813180244.28641-1-szeder.dev@gmail.com","threadId":"51639","inReplyTo":null,"subject":"[PATCH] worktree remove: clarify error message on dirty worktree","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-08-13T18:02:44Z","receivedAt":"2019-08-13T18:02:51Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"To avoid data loss, 'git worktree remove' refuses to delete a worktree\nif it's dirty or contains untracked files. However, the error message\nonly mentions that the worktree \"is dirty\", even if the worktree in\nquestion is in fact clean, but contains untracked files:\n\n $ git worktree add test-worktree\n Preparing worktree (new branch 'test-worktree')\n HEAD is now at aa53e60 Initial\n $ >test-worktree/untracked-file\n $ git worktree remove test-worktree/\n fatal: 'test-worktree/' is dirty, use --force to delete it\n $ git -C test-worktree/ diff\n $ git -C test-worktree/ diff --cached\n $ # Huh? Where are those dirty files?!\n\nClarify this error message to say that the worktree \"contains modified\nor untracked files\".\n\nSigned-off-by: SZEDER Gábor \n---\n\nI spent more time searching for those dirty files that I would like to\nadmit...\n\n builtin/worktree.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex a5bb02b207..7f094f8170 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -880,7 +880,7 @@ static void check_clean_worktree(struct worktree *wt,\n \t\t\t original_path);\n \tret = xread(cp.out, buf, sizeof(buf));\n \tif (ret)\n-\t\tdie(_(\"'%s' is dirty, use --force to delete it\"),\n+\t\tdie(_(\"'%s' contains modified or untracked files, use --force to delete it\"),\n \t\t original_path);\n \tclose(cp.out);\n \tret = finish_command(&cp);\n-- \n2.23.0.rc2.350.gf4fdc32db7\n\n"},{"id":"380411","messageId":"CAPig+cSDxBFhnJSmH+WzNZBjY4O0OczazZ7EOqn1P=zgL+ec4g@mail.gmail.com","threadId":"51639","inReplyTo":"20190813180244.28641-1-szeder.dev@gmail.com","subject":"Re: [PATCH] worktree remove: clarify error message on dirty worktree","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-08-13T18:52:39Z","receivedAt":"2019-08-13T18:52:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Aug 13, 2019 at 2:04 PM SZEDER Gábor wrote:\n> To avoid data loss, 'git worktree remove' refuses to delete a worktree\n> if it's dirty or contains untracked files. However, the error message\n> only mentions that the worktree \"is dirty\", even if the worktree in\n> question is in fact clean, but contains untracked files:\n> [...]\n> Clarify this error message to say that the worktree \"contains modified\n> or untracked files\".\n>\n> Signed-off-by: SZEDER Gábor \n> ---\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> @@ -880,7 +880,7 @@ static void check_clean_worktree(struct worktree *wt,\n> ret = xread(cp.out, buf, sizeof(buf));\n> if (ret)\n> - die(_(\"'%s' is dirty, use --force to delete it\"),\n> + die(_(\"'%s' contains modified or untracked files, use --force to delete it\"),\n> original_path);\n\nMakes sense. This is a different type of \"dirtiness\" than, say, \"git\nrebase --interactive\" which cares about unstaged changes but generally\ndoesn't mind untracked files. So, it deserves an error message which\nmentions untracked files explicitly.\n\nWe could actually parse the output of \"git status --porcelain\" (which\nis invoked just above this spot) and provide a more specific error\nmessage (\"...contains modified files\" or \"...contains untracked\nfiles\") but that's probably not worth the effort.\n\nAnyhow, for what it's worth:\nReviewed-by: Eric Sunshine \n"},{"id":"380414","messageId":"xmqqv9v07rn1.fsf@gitster-ct.c.googlers.com","threadId":"51639","inReplyTo":"CAPig+cSDxBFhnJSmH+WzNZBjY4O0OczazZ7EOqn1P=zgL+ec4g@mail.gmail.com","subject":"Re: [PATCH] worktree remove: clarify error message on dirty worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-13T20:10:58Z","receivedAt":"2019-08-13T20:11:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Tue, Aug 13, 2019 at 2:04 PM SZEDER Gábor wrote:\n>> To avoid data loss, 'git worktree remove' refuses to delete a worktree\n>> if it's dirty or contains untracked files. However, the error message\n>> only mentions that the worktree \"is dirty\", even if the worktree in\n>> question is in fact clean, but contains untracked files:\n>> [...]\n>> Clarify this error message to say that the worktree \"contains modified\n>> or untracked files\".\n>>\n>> Signed-off-by: SZEDER Gábor \n>> ---\n>> diff --git a/builtin/worktree.c b/builtin/worktree.c\n>> @@ -880,7 +880,7 @@ static void check_clean_worktree(struct worktree *wt,\n>> ret = xread(cp.out, buf, sizeof(buf));\n>> if (ret)\n>> - die(_(\"'%s' is dirty, use --force to delete it\"),\n>> + die(_(\"'%s' contains modified or untracked files, use --force to delete it\"),\n>> original_path);\n>\n> Makes sense. This is a different type of \"dirtiness\" than, say, \"git\n> rebase --interactive\" which cares about unstaged changes but generally\n> doesn't mind untracked files. So, it deserves an error message which\n> mentions untracked files explicitly.\n>\n> We could actually parse the output of \"git status --porcelain\" (which\n> is invoked just above this spot) and provide a more specific error\n> message (\"...contains modified files\" or \"...contains untracked\n> files\") but that's probably not worth the effort.\n>\n> Anyhow, for what it's worth:\n> Reviewed-by: Eric Sunshine \n\nThanks, both.\n"}]}