[PATCH 0/2] branch: update error messages

[PATCH 0/2] branch: update error messages

[Rubén Justo]

A message like "branch 'frog' checked out at ..." may be confusing to
the user if the branch 'frog' is not the currently checked out branch,
which may occur while rebasing, bisecting or other operations.

Let's reword those messages as it has been done in 4970bedef2 (branch:
update the message to refuse touching a branch in-use, 2023-07-21).

Rubén Justo (2):
  branch: error message deleting a branch in use
  branch: error message checking out a branch in use

 branch.c                |  2 +-
 builtin/branch.c        |  2 +-
 t/t2400-worktree-add.sh |  3 ++-
 t/t3200-branch.sh       | 14 +++++++++++++-
 t/t3400-rebase.sh       |  2 +-
 5 files changed, 18 insertions(+), 5 deletions(-)

-- 
2.42.0.rc0.2.gbb987841e8

[PATCH 1/2] branch: error message deleting a branch in use

[Rubén Justo]

Let's update the error message we show when the user tries to delete a
branch which is being used in another worktree, following the guideline
reasoned in 4970bedef2 (branch: update the message to refuse touching a
branch in-use, 2023-07-21).

Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
 builtin/branch.c  |  2 +-
 t/t3200-branch.sh | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 08da650516..2ec190b14a 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -261,7 +261,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 			const char *path;
 			if ((path = branch_checked_out(name))) {
 				error(_("Cannot delete branch '%s' "
-					"checked out at '%s'"),
+					"used by worktree at '%s'"),
 				      bname.buf, path);
 				ret = 1;
 				continue;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index daf1666df7..080e4f24a6 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -942,7 +942,19 @@ test_expect_success 'test deleting branch without config' '
 test_expect_success 'deleting currently checked out branch fails' '
 	git worktree add -b my7 my7 &&
 	test_must_fail git -C my7 branch -d my7 &&
-	test_must_fail git branch -d my7 &&
+	test_must_fail git branch -d my7 2>actual &&
+	grep "^error: Cannot delete branch .my7. used by worktree at " actual &&
+	rm -r my7 &&
+	git worktree prune
+'
+
+test_expect_success 'deleting in-use branch fails' '
+	git worktree add my7 &&
+	test_commit -C my7 bt7 &&
+	git -C my7 bisect start HEAD HEAD~2 &&
+	test_must_fail git -C my7 branch -d my7 &&
+	test_must_fail git branch -d my7 2>actual &&
+	grep "^error: Cannot delete branch .my7. used by worktree at " actual &&
 	rm -r my7 &&
 	git worktree prune
 '
-- 
2.42.0.rc0.2.gbb987841e8

[PATCH 2/2] branch: error message checking out a branch in use

[Rubén Justo]

Let's update the error message we show when the user tries to check out
a branch which is being used in another worktree, following the
guideline reasoned in 4970bedef2 (branch: update the message to refuse
touching a branch in-use, 2023-07-21).

Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
 branch.c                | 2 +-
 t/t2400-worktree-add.sh | 3 ++-
 t/t3400-rebase.sh       | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/branch.c b/branch.c
index 3e4684f79f..98c199f7b7 100644
--- a/branch.c
+++ b/branch.c
@@ -838,7 +838,7 @@ void die_if_checked_out(const char *branch, int ignore_current_worktree)
 
 		if (is_shared_symref(worktrees[i], "HEAD", branch)) {
 			skip_prefix(branch, "refs/heads/", &branch);
-			die(_("'%s' is already checked out at '%s'"),
+			die(_("'%s' already used by worktree at '%s'"),
 				branch, worktrees[i]->path);
 		}
 	}
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 051363acbb..89447a4a0c 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -121,7 +121,8 @@ test_expect_success '"add" worktree creating new branch' '
 test_expect_success 'die the same branch is already checked out' '
 	(
 		cd here &&
-		test_must_fail git checkout newmain
+		test_must_fail git checkout newmain 2> actual &&
+		grep "already used by worktree at" actual
 	)
 '
 
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 3ce918fdb8..d3df19a51f 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -421,7 +421,7 @@ test_expect_success 'refuse to switch to branch checked out elsewhere' '
 	git checkout main &&
 	git worktree add wt &&
 	test_must_fail git -C wt rebase main main 2>err &&
-	test_i18ngrep "already checked out" err
+	test_i18ngrep "already used by worktree at" err
 '
 
 test_expect_success MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' '
-- 
2.42.0.rc0.2.gbb987841e8

Re: [PATCH 2/2] branch: error message checking out a branch in use

[Taylor Blau]

On Mon, Aug 07, 2023 at 10:42:40PM +0200, Rubén Justo wrote:
> Let's update the error message we show when the user tries to check out
> a branch which is being used in another worktree, following the
> guideline reasoned in 4970bedef2 (branch: update the message to refuse
> touching a branch in-use, 2023-07-21).
>
> Signed-off-by: Rubén Justo <rjusto@gmail.com>
> ---
>  branch.c                | 2 +-
>  t/t2400-worktree-add.sh | 3 ++-
>  t/t3400-rebase.sh       | 2 +-
>  3 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/branch.c b/branch.c
> index 3e4684f79f..98c199f7b7 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -838,7 +838,7 @@ void die_if_checked_out(const char *branch, int ignore_current_worktree)
>
>  		if (is_shared_symref(worktrees[i], "HEAD", branch)) {
>  			skip_prefix(branch, "refs/heads/", &branch);
> -			die(_("'%s' is already checked out at '%s'"),
> +			die(_("'%s' already used by worktree at '%s'"),

I wonder if "'%s' is already used by ..." (with the additional "is"
between the branch name and "already") would be clearer / more natural.

Thanks,
Taylor

Re: [PATCH 2/2] branch: error message checking out a branch in use

[Rubén Justo]

On 7/8/23 23:34, Taylor Blau wrote:
> On Mon, Aug 07, 2023 at 10:42:40PM +0200, Rubén Justo wrote:
>> Let's update the error message we show when the user tries to check out
>> a branch which is being used in another worktree, following the
>> guideline reasoned in 4970bedef2 (branch: update the message to refuse
>> touching a branch in-use, 2023-07-21).
>>
>> Signed-off-by: Rubén Justo <rjusto@gmail.com>
>> ---
>>  branch.c                | 2 +-
>>  t/t2400-worktree-add.sh | 3 ++-
>>  t/t3400-rebase.sh       | 2 +-
>>  3 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/branch.c b/branch.c
>> index 3e4684f79f..98c199f7b7 100644
>> --- a/branch.c
>> +++ b/branch.c
>> @@ -838,7 +838,7 @@ void die_if_checked_out(const char *branch, int ignore_current_worktree)
>>
>>  		if (is_shared_symref(worktrees[i], "HEAD", branch)) {
>>  			skip_prefix(branch, "refs/heads/", &branch);
>> -			die(_("'%s' is already checked out at '%s'"),
>> +			die(_("'%s' already used by worktree at '%s'"),
> 
> I wonder if "'%s' is already used by ..." (with the additional "is"
> between the branch name and "already") would be clearer / more natural.

I have no problem with that change.  I think I was trying to keep the message
sort ... the tests does not need to be adjusted.

> 
> Thanks,
> Taylor
> 

Thanks for reading carefully.

Re: [PATCH 0/2] branch: update error messages

[Junio C Hamano]

Rubén Justo <rjusto@gmail.com> writes:

> A message like "branch 'frog' checked out at ..." may be confusing to
> the user if the branch 'frog' is not the currently checked out branch,
> which may occur while rebasing, bisecting or other operations.

Makes sense.  Will queue.


>
> Let's reword those messages as it has been done in 4970bedef2 (branch:
> update the message to refuse touching a branch in-use, 2023-07-21).
>
> Rubén Justo (2):
>   branch: error message deleting a branch in use
>   branch: error message checking out a branch in use
>
>  branch.c                |  2 +-
>  builtin/branch.c        |  2 +-
>  t/t2400-worktree-add.sh |  3 ++-
>  t/t3200-branch.sh       | 14 +++++++++++++-
>  t/t3400-rebase.sh       |  2 +-
>  5 files changed, 18 insertions(+), 5 deletions(-)