Re: [RFC PATCH] builtin/worktree: enhance worktree removal

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Nov 21, 2017 at 10:55 PM, Junio C Hamano <gitster@pobox.com> wrote:
> OK, so the proposed log message was a bit confusing for those who
> are *not* the person who wrote it (who knew why existing behaviour
> was inadequate and did not describe how "worktree remove" would fail
> under such a scenario to illustrate it, incorrectly assuming that
> everybody who reads the proposed log message already *knows* how it
> would fail).

Correct. The log message is confusing, enough so that my knee-jerk
reaction was that the patch was trying to re-invent 'git worktree
prune'. That seemed odd, so I spent extra time trying to figure out
what it really meant, which led to my rather lengthy response asking
if I was understanding the situation correctly.

>         "git worktree remove" removes both the named worktree
>         directory and the administrative information for it after
>         checking that there is no local modifications that would be
>         lost (which is a handy safety measure).  It however refuses
>         to work if the worktree directory is _already_ removed.

The "refusal" is by accident, so perhaps phrase it like this:

    However, due to an oversight, it aborts with an error if the
    worktree directory is _already_ removed.

or something.

>         The user could use "git worktree prune" after seeing the
>         error and realizing the situation, but at that point, there
>         is nothing gained by leaving only the administrative data
>         behind.  Teach "git worktree remove" to go ahead and remove
>         the trace of the worktree in such a case.
>
> or soemthing like that?

Yes, I quite like it; it conveys the information necessary to
understand the issue.

Kaartic: Regarding the actual patch, rather than silencing
validate_worktree() (which seems an unfortunate thing to do), isn't it
possible simply to do a quick test to see if the worktree directory
exists before calling validate_worktree()? If it doesn't exist, then
just skip down to the part of the code which does the 'prune'
operation.