Re: [PATCH v2 1/1] branch: advise the user to checkout a different branch before deleting

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 7, 2020, 16:34 UTC
Message-ID: <xmqqh8176xab.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAPig+cQ0qY8KDZrQ8khuz34DqPimorN7JHHn0Ms=KpvJYtxJoA@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

[jc: skipped all the good suggestions I agree with]

Show 19 quoted lines

>> +                                       }
>> +                                       else {
>> +                                               advise(_("The branch you are trying to delete is checked "
>> +                                                       "out on another worktree, run the following command "
>> +                                                       "to checkout a different branch then try again:\n"
>> +                                                       "git -C %s switch <branch>"), wt->path);
>
> I like the use of -C here because it makes the command self-contained,
> however, I also worry because wt->path is an absolute path, thus
> likely to be quite lengthy, which means that the important part of the
> example command (the "switch <branch>") can get pushed quite far away,
> thus is more easily overlooked by the reader. I wonder if it would
> make more sense to show the 'cd' command explicitly, although doing so
> ties the example to a particular shell, which may be a downside.
>
>     cd %s
>     git switch <different-branch>
>     cd -
>     git branch -%c <this-branch>

Note that wt->path may have special characters that would need to be protected from the user's shell (worse, the quoting convention may be different depending on which shell is in use). That is one of the reasons why I would suggest to stay away from giving an advice that pretends to be cut-and-paste-able without being so. In this case, <different-branch> and <this-branch> must be filled by the user anyway, and the only thing worth cutting-and-pasting is the path to the other worktree, not the "git -C" or "cd" that users should be able to come up with.

	"The branch is checked out on another worktree at\n"
	"path '%s'\n"
	"and cannot be deleted.  Go there, check out some other\n"
	"branch and try again."

or something like that, perhaps?

> (It is rather verbose and ugly, though.)

I tend to agree. It also feels to me that it is giving too much hand-holding, but after all advise() may turning out to be about giving that.