threads / patch / 60078

patch, 2 partsbranch: update error messages

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

## tl;dr

6 messages between Aug 7, 2023 and Aug 7, 2023. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Rubén Justo· Aug 7, 2023, 20:38 UTC · lore

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
Rubén Justo· Aug 7, 2023, 20:42 UTC · re: Rubén Justo · lore

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

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(-)
Show changes to 2 files +14 −2

builtin/branch.c, t/t3200-branch.sh

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
Rubén Justo· Aug 7, 2023, 20:42 UTC · re: Rubén Justo · lore

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

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(-)
Show changes to 3 files +4 −3

branch.c, t/t2400-worktree-add.sh, t/t3400-rebase.sh

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
Taylor Blau· Aug 7, 2023, 21:34 UTC · re: Rubén Justo · lore

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

On Mon, Aug 07, 2023 at 10:42:40PM +0200, Rubén Justo wrote:
Show 22 quoted lines
> 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

Rubén Justo· Aug 7, 2023, 22:49 UTC · re: Taylor Blau · lore

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

On 7/8/23 23:34, Taylor Blau wrote:
Show 26 quoted lines
> 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.
Junio C Hamano· Aug 7, 2023, 21:18 UTC · re: Rubén Justo · lore

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

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.
Show 14 quoted lines
>
> 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(-)

← back to recent threads