Re: [PATCH v2 3/4] Make die_if_checked_out() prune missing checkouts of unlocked worktrees.

[Junio C Hamano <gitster@pobox.com>]

Peter Jones <pjones@redhat.com> writes:

[jc: won't repeat comments on the title]

> @@ -360,6 +360,12 @@ void die_if_checked_out(const char *branch, int ignore_current_worktree)
>  	wt = find_shared_symref("HEAD", branch);
>  	if (!wt || (ignore_current_worktree && wt->is_current))
>  		return;

die-if-checked-out is called from callers that expect to be stopped
before they do any harm, so it feels dirty to make a side effect
like this.

If the user tries to check out a branch that used to be checked out
in an already removed worktree, doesn't that indicate that an
earlier worktree removal was done incorrectly, which is something
worth reporting to the user and give the user a chance to think and
choose what corrective action(s) need to be taken?

For that, instead of automatically losing information like this
patch does, it may make more sense to fail the checkout and stop at
giving diagnosis (e.g. "our record shows that the branch is checked
out in that worktree, but you seem to have lost it.  if you forgot
to prune it, then here is the command you can give to do so.")
without actually touching the filesystem.

Thanks.


> +
> +	if (prune_worktree_if_missing(wt) >= 0) {
> +		delete_worktrees_dir_if_empty();
> +		return;
> +	}
> +
>  	skip_prefix(branch, "refs/heads/", &branch);
>  	die(_("'%s' is already checked out at '%s'"),
>  	    branch, wt->path);