threads / patch / 51639

patchworktree remove: clarify error message on dirty worktree

Subject: [PATCH] worktree remove: clarify error message on dirty worktree

## tl;dr

3 messages between Aug 13, 2019 and Aug 13, 2019. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

SZEDER Gábor· Aug 13, 2019, 18:02 UTC · lore

To avoid data loss, 'git worktree remove' refuses to delete a worktree if it's dirty or contains untracked files. However, the error message only mentions that the worktree "is dirty", even if the worktree in question is in fact clean, but contains untracked files:

  $ git worktree add test-worktree
  Preparing worktree (new branch 'test-worktree')
  HEAD is now at aa53e60 Initial
  $ >test-worktree/untracked-file
  $ git worktree remove test-worktree/
  fatal: 'test-worktree/' is dirty, use --force to delete it
  $ git -C test-worktree/ diff
  $ git -C test-worktree/ diff --cached
  $ # Huh?  Where are those dirty files?!

Clarify this error message to say that the worktree "contains modified or untracked files".

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---

I spent more time searching for those dirty files that I would like to admit...

 builtin/worktree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to builtin/worktree.c +1 −1

diff --git a/builtin/worktree.c b/builtin/worktree.c
index a5bb02b207..7f094f8170 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -880,7 +880,7 @@ static void check_clean_worktree(struct worktree *wt,
 			  original_path);
 	ret = xread(cp.out, buf, sizeof(buf));
 	if (ret)
-		die(_("'%s' is dirty, use --force to delete it"),
+		die(_("'%s' contains modified or untracked files, use --force to delete it"),
 		    original_path);
 	close(cp.out);
 	ret = finish_command(&cp);

-- 
2.23.0.rc2.350.gf4fdc32db7

Eric Sunshine· Aug 13, 2019, 18:52 UTC · re: SZEDER Gábor · lore

Re: [PATCH] worktree remove: clarify error message on dirty worktree

On Tue, Aug 13, 2019 at 2:04 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:

Show 17 quoted lines

> To avoid data loss, 'git worktree remove' refuses to delete a worktree
> if it's dirty or contains untracked files.  However, the error message
> only mentions that the worktree "is dirty", even if the worktree in
> question is in fact clean, but contains untracked files:
> [...]
> Clarify this error message to say that the worktree "contains modified
> or untracked files".
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -880,7 +880,7 @@ static void check_clean_worktree(struct worktree *wt,
>         ret = xread(cp.out, buf, sizeof(buf));
>         if (ret)
> -               die(_("'%s' is dirty, use --force to delete it"),
> +               die(_("'%s' contains modified or untracked files, use --force to delete it"),
>                     original_path);

Makes sense. This is a different type of "dirtiness" than, say, "git rebase --interactive" which cares about unstaged changes but generally doesn't mind untracked files. So, it deserves an error message which mentions untracked files explicitly.

We could actually parse the output of "git status --porcelain" (which is invoked just above this spot) and provide a more specific error message ("...contains modified files" or "...contains untracked files") but that's probably not worth the effort.

Anyhow, for what it's worth:
Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

Junio C Hamano· Aug 13, 2019, 20:10 UTC · re: Eric Sunshine · lore

Re: [PATCH] worktree remove: clarify error message on dirty worktree

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 31 quoted lines

> On Tue, Aug 13, 2019 at 2:04 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
>> To avoid data loss, 'git worktree remove' refuses to delete a worktree
>> if it's dirty or contains untracked files.  However, the error message
>> only mentions that the worktree "is dirty", even if the worktree in
>> question is in fact clean, but contains untracked files:
>> [...]
>> Clarify this error message to say that the worktree "contains modified
>> or untracked files".
>>
>> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
>> ---
>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> @@ -880,7 +880,7 @@ static void check_clean_worktree(struct worktree *wt,
>>         ret = xread(cp.out, buf, sizeof(buf));
>>         if (ret)
>> -               die(_("'%s' is dirty, use --force to delete it"),
>> +               die(_("'%s' contains modified or untracked files, use --force to delete it"),
>>                     original_path);
>
> Makes sense. This is a different type of "dirtiness" than, say, "git
> rebase --interactive" which cares about unstaged changes but generally
> doesn't mind untracked files. So, it deserves an error message which
> mentions untracked files explicitly.
>
> We could actually parse the output of "git status --porcelain" (which
> is invoked just above this spot) and provide a more specific error
> message ("...contains modified files" or "...contains untracked
> files") but that's probably not worth the effort.
>
> Anyhow, for what it's worth:
> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

Thanks, both.