On 07/01/2019 19:04, Junio C Hamano wrote:
nbelakovski@xxxxxxxxx writes:
From: Nickolai Belakovski <nbelakovski@xxxxxxxxx>
In order to more clearly display which branches are active, the output
of git branch is modified to mark branches checkout out in a linked
worktree with a "+" and color them in cyan (in contrast to the current
branch, which will still be denoted with a "*" and colored in green)
This is meant to simplify workflows related to worktree, particularly
due to the limitations of not being able to check out the same branch in
two worktrees and the inability to delete a branch checked out in a
worktree. When performing branch operations like checkout and delete, it
would be useful to know more readily if the branches in which the user
is interested are already checked out in a worktree.
I do not think it is warranted to paint the safety features as
"limitations".
Is this not just a case of needing to clarify that this is 'safety'
related to the _users_ mental model (or lack of) relative to the limited
information that was previously given by the branch command's list.
You are right that there is no data safety issue, but users make
mistakes when they misunderstand the situation.
A branch that is checked out in another worktree cannot be checked
out to be worked on, as that will make the checkout of the other
worktree out of sync. If you want to work on that branch, you can
either (1) go to that worktree that has a checkout of that branch
and work there, or (2) go to that worktree that has a checkout of
that branch, check out a different branch there, come back to the
worktree you want to work in and check out that branch. Knowing
where that other worktree is is the first step in either case.
And a branch that is checked out in a worktree cannot be removed, as
it is a sign that it is still being worked on for a branch to have
been checked out somewhere.
I'm not sure that all users will recognise the signs, which I think is
one reason for the value of the patch.
If you do want to remove that branch,
you need to go to that worktree that has a checkout of that branch,
check out a different branch there, and then remove it. Again,
knowing where that other worktree is is the fist thing you need to
know.
But then I am not sure if the feature being added by these patches
is a good match for that justification.
I'd agree that the justification needs clarified.
For one thing, it would be more direct and helpful way for
git checkout one-branch
git branch -d one-branch
to say "The branch `one-branch` is checked out in a worktree at
$DIRECTORY" when they refused to go ahead. And that would eliminate
the need for this new feature to help these two use cases.
In fact, these two command already behave that way, so the paragraph
I just commented on is not a good justification for this new feature
at all.
Besides, showing "That branch is checked out somewhere" would not
help user to decide "ah, if I want to work on that branch, I need to
chdir to that directory" with the patch in question, as it only
shows "It is checked out _somewhere_" without saying where.
The git worktree list command contains the relevant information, however
this is a much less frquently used command than git branch.
It is not a good justification. If the "relevant information" given
by the command is necessary one, the user can run that command. If
the situation where that "relevant information" becomes necessary is
rare, the command is run much less frequently is not a problem---it
is expected. And overloading a more frequently used command with
information that is less frequently wanted is actually not a great
design.
But leaving the older command unaware of the newer developments and the
user unwise as to its missing info is equally a poor situation.
A more relevant justification may be that even though the
information can already be found in "worktree list" output, it would
give us flexibility in presentation to allow the custom format in
for-each-ref to show it.
So, I am between moderately Meh to fairly negative on this step; Meh
in the sense that "thanks to the previous step, we _could_ do this,
it does not give incorrect information, and it makes the output more
cheerful, but it does not add that much useful and actionable piece
of information".
The patch did appear to me as being a proper update to the branch
command to include the information about the branches in the other worktrees
Philip
