Re: [PATCH] branch: report kind of checkout when rejecting delete

René Scharfe <[email protected]> writes:

>>> +				int kind = branch_checkout_kind(name);
>> 
>> Not "enum branch_checkout_kind" but "int"?
>
> Yes, it doesn't matter for the switch and is easier to print.

I do not understand the "print" part.  I was probably in the last
group of people who was forced to switch from CPP macros to enum
and their argument was always "'print kind' in GDB gives symbolic
output".  As "enum" is an glorified "int", wouldn't 

	int i_kind;
	enum branch_checkout_kind e_kind;
	BUG(_("we did not expect %d %d"), e_kind, i_kind);

do just what we expect?

>>> +				switch (kind) {
>>> +				case BRANCH_CHECKOUT_KIND_CHECKOUT:
>>> +					error(_("cannot delete branch '%s' "
>>> +						"used by worktree at '%s'"),
>>> +					      bname.buf, path);
>>> +					break;
>> 
>> We may want to be more explicit and say "cannot delete
>> branch 'frotz' checked out in worktree at '/tmp/nitfol'"
>> instead.  Unless this is a catch-all entry for states that
>> are neither 'rebase', 'bisect', nor 'rebase-merges' but are
>> somehow otherwise in use, that is.
>> 
>>> +				case BRANCH_CHECKOUT_KIND_UPDATE_REF:
>>> +					error(_("cannot delete branch '%s' "
>>> +						"used by worktree at '%s' "
>>> +						"for update-ref"),
>>> +					      bname.buf, path);
>>> +					break;
>> 
>> I was quite lost when searching for cases where this 'update-ref'
>> state might be encountered, and I still lack confidence.  Can
>> we make the diagnostic message a bit friendlier to our users?
>> 
>> For instance, something like: 'You are rebasing a history with
>> merges in that other worktree, and the tip of this branch will
>> be updated when that process completes, so you cannot delete
>> it from here.'  (Naturally, I may have misidentified the exact
>> nature of the error, but this illustrates the level of detail and
>> user-facing clarity I hope to see.)
>
> That's quite long.  Would it make sense to throw that update-ref
> case into the rebase bin, i.e. only distinguish between checkout,
> bisect and rebase?

Shortening a quite long expression down to digestable pieces is left
as an exercise for those with this particular itch to scratch ;-).
I do not personally mind if it ends up indistinguishable from other
"rebase" case (or unified the "kind" enum into one), but others may
have ideas to shorten the message to fit in the pattern we see
above.

Thanks.