[PATCH] branch: description for non-existent branch errors

[PATCH] branch: description for non-existent branch errors

[Rubén Justo]

When the repository does not yet has commits, some errors describe that
there is no branch:

    $ git init -b first

    $ git --edit-description first
    fatal: branch 'first' does not exist

    $ git --set-upstream-to=upstream
    fatal: branch 'first' does not exist

    $ git branch -c second
    error: refname refs/heads/first not found
    fatal: Branch copy failed

That "first" branch is unborn but to say it doesn't exists is confusing.

Options "-c" (copy) and "-m" (rename) show the same error when the
origin branch doesn't exists:

    $ git branch -c non-existent-branch second
    error: refname refs/heads/non-existent-branch not found
    fatal: Branch copy failed

    $ git branch -m non-existent-branch second
    error: refname refs/heads/non-existent-branch not found
    fatal: Branch rename failed

Note that "--edit-description" without an explicit argument is already
considering the _empty repository_ circumstance in its error.  Also note
that "-m" on the initial branch it is an allowed operation.

This commit makes the error descriptions for those branch operations
with unborn or non-existent branches, more informative.

This is the result of the change:

    $ git init -b first

    $ git --edit-description first
    fatal: No commit on branch 'first' yet.

    $ git --set-upstream-to=upstream
    fatal: No commit on branch 'first' yet.

    $ git -c second
    fatal: No commit on branch 'first' yet.

    $ git [-c/-m] non-existent-branch second
    fatal: No branch named 'non-existent-branch'.

---

Hi all.

I would appreciate some guidance here.

This patch is based on master but there is already in 'seen' a patch
submitted for 'branch' that produces conflicts with this.  I don't know
if it is more convenient to submit this patch based on 'seen'
considering that.

I submitted that other patch too and prefer to keep the changes separate
but maybe it's better not to and do a rollup with this on that other
patch.. I don't know how disturbing it is to submit a new patch when it
is "waiting for review".

Another approach?

Thank you.


Un saludo.

 builtin/branch.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 55cd9a6e99..5ca35064f3 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -538,6 +538,13 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
 			die(_("Invalid branch name: '%s'"), oldname);
 	}
 
+	if (copy && !ref_exists(oldref.buf)) {
+		if (!strcmp(head, oldname))
+			die(_("No commit on branch '%s' yet."), oldname);
+		else
+			die(_("No branch named '%s'."), oldname);
+	}
+
 	/*
 	 * A command like "git branch -M currentbranch currentbranch" cannot
 	 * cause the worktree to become inconsistent with HEAD, so allow it.
@@ -805,7 +812,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		if (!ref_exists(branch_ref.buf)) {
 			strbuf_release(&branch_ref);
 
-			if (!argc)
+			if (!argc || !strcmp(head, branch_name))
 				return error(_("No commit on branch '%s' yet."),
 					     branch_name);
 			else
@@ -848,8 +855,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			die(_("no such branch '%s'"), argv[0]);
 		}
 
-		if (!ref_exists(branch->refname))
+		if (!ref_exists(branch->refname)) {
+			if (!argc || !strcmp(head, branch->name))
+				die(_("No commit on branch '%s' yet."), branch->name);
 			die(_("branch '%s' does not exist"), branch->name);
+		}
 
 		dwim_and_setup_tracking(the_repository, branch->name,
 					new_upstream, BRANCH_TRACK_OVERRIDE,
-- 
2.36.1

[PATCH] branch: description for non-existent branch errors

[Rubén Justo]

When the repository does not yet has commits, some errors describe that
there is no branch:

    $ git init -b first

    $ git --edit-description first
    fatal: branch 'first' does not exist

    $ git --set-upstream-to=upstream
    fatal: branch 'first' does not exist

    $ git branch -c second
    error: refname refs/heads/first not found
    fatal: Branch copy failed

That "first" branch is unborn but to say it doesn't exists is confusing.

Options "-c" (copy) and "-m" (rename) show the same error when the
origin branch doesn't exists:

    $ git branch -c non-existent-branch second
    error: refname refs/heads/non-existent-branch not found
    fatal: Branch copy failed

    $ git branch -m non-existent-branch second
    error: refname refs/heads/non-existent-branch not found
    fatal: Branch rename failed

Note that "--edit-description" without an explicit argument is already
considering the _empty repository_ circumstance in its error.  Also note
that "-m" on the initial branch it is an allowed operation.

This commit makes the error descriptions for those branch operations
with unborn or non-existent branches, more informative.

This is the result of the change:

    $ git init -b first

    $ git --edit-description first
    fatal: No commit on branch 'first' yet.

    $ git --set-upstream-to=upstream
    fatal: No commit on branch 'first' yet.

    $ git -c second
    fatal: No commit on branch 'first' yet.

    $ git [-c/-m] non-existent-branch second
    fatal: No branch named 'non-existent-branch'.

Signed-off-by: Rubén Justo <rjusto@gmail.com>

---

Just re-sending with the Signed-off-by label. Sorry.


 builtin/branch.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 55cd9a6e99..5ca35064f3 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -538,6 +538,13 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
 			die(_("Invalid branch name: '%s'"), oldname);
 	}
 
+	if (copy && !ref_exists(oldref.buf)) {
+		if (!strcmp(head, oldname))
+			die(_("No commit on branch '%s' yet."), oldname);
+		else
+			die(_("No branch named '%s'."), oldname);
+	}
+
 	/*
 	 * A command like "git branch -M currentbranch currentbranch" cannot
 	 * cause the worktree to become inconsistent with HEAD, so allow it.
@@ -805,7 +812,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		if (!ref_exists(branch_ref.buf)) {
 			strbuf_release(&branch_ref);
 
-			if (!argc)
+			if (!argc || !strcmp(head, branch_name))
 				return error(_("No commit on branch '%s' yet."),
 					     branch_name);
 			else
@@ -848,8 +855,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			die(_("no such branch '%s'"), argv[0]);
 		}
 
-		if (!ref_exists(branch->refname))
+		if (!ref_exists(branch->refname)) {
+			if (!argc || !strcmp(head, branch->name))
+				die(_("No commit on branch '%s' yet."), branch->name);
 			die(_("branch '%s' does not exist"), branch->name);
+		}
 
 		dwim_and_setup_tracking(the_repository, branch->name,
 					new_upstream, BRANCH_TRACK_OVERRIDE,
-- 
2.36.1

Re: [PATCH] branch: description for non-existent branch errors

[Junio C Hamano]

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

> When the repository does not yet has commits, some errors describe that

"has" -> "have".

>  builtin/branch.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)

Tests to protect the new behaviour from getting broken by future
developers are missing.

> diff --git a/builtin/branch.c b/builtin/branch.c
> index 55cd9a6e99..5ca35064f3 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -538,6 +538,13 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
>  			die(_("Invalid branch name: '%s'"), oldname);
>  	}
>  
> +	if (copy && !ref_exists(oldref.buf)) {
> +		if (!strcmp(head, oldname))
> +			die(_("No commit on branch '%s' yet."), oldname);
> +		else
> +			die(_("No branch named '%s'."), oldname);
> +	}

Let's not make it worse by starting the die() message with capital
letters, even though the existing "git branch" error messages are
already mixture that they need to be cleaned up later.

Thanks.

Re: [PATCH] branch: description for non-existent branch errors

[Rubén Justo]

On 26/9/22 20:12, Junio C Hamano wrote:

Thank you for the review. I will do a reroll with your comments, but about..

>> +	if (copy && !ref_exists(oldref.buf)) {
>> +		if (!strcmp(head, oldname))
>> +			die(_("No commit on branch '%s' yet."), oldname);
>> +		else
>> +			die(_("No branch named '%s'."), oldname);
>> +	}
> 
> Let's not make it worse by starting the die() message with capital
> letters, even though the existing "git branch" error messages are
> already mixture that they need to be cleaned up later.
> 

I chose these literals for the errors because they are already translated,
appear below in that same file. I thought that would make the patch easier to
review and apply, for the translators too.

Maybe we can maintain those capitalized literals to have a simpler patch to
merge and leave the "mixtures" for a posterior patch.  I have already 
identified a leak in that command:

	static const char *head;
	...
	int cmd_branch()
	...
		head = resolve_refdup("HEAD", 0, &head_oid, NULL);

"head" is leaked but there is no easy free, because some exists are like:

		if (delete) {
			if (!argc)
				die(_("branch name required"));
			return delete_branches(argc, argv, delete > 1, filter.kind, quiet);

Without entering in this too much, maybe an atexit() approach, as in some
others commands... but maybe a more clear flow.. and that would need some
changes that can carry out the "mixtures".

Anyway, if you think there is no problem with the new literal in that file,
I will add it to the reroll with the rest of the comments in your review.

I pointed out in the first mail of this thread, there is already a patch in
'seen' that touches builtin/branch.c [1].  I would like to keep the patches
separated, but I don't know how to proceed: make the change from 'seen', keep
it from 'master'... Maybe you can give me some guidance in this.

Thank you.

Un saludo.

Re: [PATCH] branch: description for non-existent branch errors

[Junio C Hamano]

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

> I pointed out in the first mail of this thread, there is already a patch in
> 'seen' that touches builtin/branch.c [1].  I would like to keep the patches
> separated, but I don't know how to proceed: make the change from 'seen', keep
> it from 'master'... Maybe you can give me some guidance in this.

I do not see much problem in keeping them separated.  My trial merge
of the result of applying this patch on top of 'master', with the
other topic that has the "branch description for nth prior checkout"
patch does show a minor textual conflict, but the resolution does
not look too bad.

Check near the topic branch of 'seen' after I push out today's
integration result in a few hours and see if they look reasonable.

Thanks.


diff --cc builtin/branch.c
index 5ca35064f3,13d1f028da..2b3884ce61
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@@ -810,19 -807,18 +814,18 @@@ int cmd_branch(int argc, const char **a
  
  		strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
  		if (!ref_exists(branch_ref.buf)) {
- 			strbuf_release(&branch_ref);
- 
 -			if (!argc)
 +			if (!argc || !strcmp(head, branch_name))
- 				return error(_("No commit on branch '%s' yet."),
+ 				ret = error(_("No commit on branch '%s' yet."),
  					     branch_name);
  			else
- 				return error(_("No branch named '%s'."),
+ 				ret = error(_("No branch named '%s'."),
  					     branch_name);
- 		}
- 		strbuf_release(&branch_ref);
+ 		} else
+ 			ret = edit_branch_description(branch_name);
  
- 		if (edit_branch_description(branch_name))
- 			return 1;
+ 		strbuf_release(&branch_ref);
+ 		strbuf_release(&buf);
+ 		return -ret;
  	} else if (copy) {
  		if (!argc)
  			die(_("branch name required"));

Re: [PATCH] branch: description for non-existent branch errors

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Rubén Justo <rjusto@gmail.com> writes:
>
>> I pointed out in the first mail of this thread, there is already a patch in
>> 'seen' that touches builtin/branch.c [1].  I would like to keep the patches
>> separated, but I don't know how to proceed: make the change from 'seen', keep
>> it from 'master'... Maybe you can give me some guidance in this.
>
> I do not see much problem in keeping them separated.  My trial merge
> of the result of applying this patch on top of 'master', with the
> other topic that has the "branch description for nth prior checkout"
> patch does show a minor textual conflict, but the resolution does
> not look too bad.
>
> Check near the topic branch of 'seen' after I push out today's
> integration result in a few hours and see if they look reasonable.
>
> Thanks.

Ah, I forgot to mention.  As to the error messages that begin with a
capital letter, to be consistent with violating messages that are
already there in builtin/branch.c, let's keep them as they are in
your patch.  We can and should clean them up later, perhaps soon
after the patch under discussion matures, but I agree with you that
it can be left outside the scope of this patch.

But stepping back a bit, in the longer term, we might want to change
the behaviour of "git branch --edit-description", at least when no
branch is specified on the command line and we are on an unborn
branch.  It is merely the matter of setting a variable in the
configuration file, so there may not be a strong reason to forbid

    $ git init trash
    $ cd trash
    $ git branch --edit-description
    $ git commit --allow-empty -m initial

while allowing the same sequence with the last two commands reversed.

After all, renaming a branch with "branch -m" does not to require an
existing ref that points at a commit, i.e.

    $ git init -b master trash
    $ cd trash
    $ git config branch.master.description "describes master"
    $ git branch -m master main

does work fine and you end up with branch.main.description at the
end.

Re: [PATCH] branch: description for non-existent branch errors

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Rubén Justo <rjusto@gmail.com> writes:
>
>> I pointed out in the first mail of this thread, there is already a patch in
>> 'seen' that touches builtin/branch.c [1].  I would like to keep the patches
>> separated, but I don't know how to proceed: make the change from 'seen', keep
>> it from 'master'... Maybe you can give me some guidance in this.
>
> I do not see much problem in keeping them separated.  My trial merge
> of the result of applying this patch on top of 'master', with the
> other topic that has the "branch description for nth prior checkout"
> patch does show a minor textual conflict, but the resolution does
> not look too bad.
>
> Check near the topic branch of 'seen' after I push out today's
> integration result in a few hours and see if they look reasonable.
>
> Thanks.

Ah, I forgot to mention.  As to the error messages that begin with a
capital letter, to be consistent with violating messages that are
already there in builtin/branch.c, let's keep them as they are in
your patch.  We can and should clean them up later, perhaps soon
after the patch under discussion matures, but I agree with you that
it can be left outside the scope of this patch.

But stepping back a bit, in the longer term, we might want to change
the behaviour of "git branch --edit-description", at least when no
branch is specified on the command line and we are on an unborn
branch.  It is merely the matter of setting a variable in the
configuration file, so there may not be a strong reason to forbid

    $ git init trash
    $ cd trash
    $ git branch --edit-description
    $ git commit --allow-empty -m initial

while allowing the same sequence with the last two commands reversed.

After all, renaming a branch with "branch -m" does not to require an
existing ref that points at a commit, i.e.

    $ git init -b master trash
    $ cd trash
    $ git config branch.master.description "describes master"
    $ git branch -m master main

does work fine and you end up with branch.main.description at the
end.

Re: [PATCH] branch: description for non-existent branch errors

[Rubén Justo]

On 28/9/22 19:50, Junio C Hamano wrote:

> But stepping back a bit, in the longer term, we might want to change
> the behaviour of "git branch --edit-description", at least when no
> branch is specified on the command line and we are on an unborn
> branch.  It is merely the matter of setting a variable in the
> configuration file, so there may not be a strong reason to forbid
> 
>     $ git init trash
>     $ cd trash
>     $ git branch --edit-description
>     $ git commit --allow-empty -m initial
> 
> while allowing the same sequence with the last two commands reversed.
> 
> After all, renaming a branch with "branch -m" does not to require an
> existing ref that points at a commit, i.e.
> 
>     $ git init -b master trash
>     $ cd trash
>     $ git config branch.master.description "describes master"
>     $ git branch -m master main
> 
> does work fine and you end up with branch.main.description at the
> end.
 
Indeed. And "--set-upstream-to"? I haven't found any reason why we
shouldn't allow it for an unborn branch too. "--unset-upstream"
already works.

Those changes I think are worth doing along with the fix for the leak
of 'static const char *head'.

And then the error descriptions.  Not just the capitalization but
some similar but different messages too.

I'll reroll this path with the test for the current errors.

Thank you.

Un saludo.

Re: [PATCH] branch: description for non-existent branch errors

[Junio C Hamano]

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

> Those changes I think are worth doing along with the fix for the leak
> of 'static const char *head'.

Let's not grow the scope of the topic forever.  It will increase the
chance of new mistakes and throw us into endless iterations.

I think the posted patch plus tests for the new behaviour (i.e. no
longer we give a misleading error message) is a good stopping point.

Extending the behaviour to allow some of these operations that
currently fail on an unborn branch may or may not make sense, and
that should be discussed separately, possibly for each option.
After the dust from the current one settles.

Personally, I do not think a "static const char *" variable that
holds onto an allocated piece of memory smaller than 1kB is
something we should worry about.  Leak checkers should also be smart
enough not to warn about such a variable, shouldn't they?

> And then the error descriptions.  Not just the capitalization but
> some similar but different messages too.

Yup, and that is probably a separate patch after all of the above
settles.

Thanks.

[PATCH v2] branch: description for non-existent branch errors

[Rubén Justo]

When the repository does not yet have commits, some errors describe that
there is no branch:

    $ git init -b first

    $ git branch --edit-description first
    error: No branch named 'first'.

    $ git branch --set-upstream-to=upstream
    fatal: branch 'first' does not exist

    $ git branch -c second
    error: refname refs/heads/first not found
    fatal: Branch copy failed

That "first" branch is unborn but to say it doesn't exists is confusing.

Options "-c" (copy) and "-m" (rename) show the same error when the
origin branch doesn't exists:

    $ git branch -c non-existent-branch second
    error: refname refs/heads/non-existent-branch not found
    fatal: Branch copy failed

    $ git branch -m non-existent-branch second
    error: refname refs/heads/non-existent-branch not found
    fatal: Branch rename failed

Note that "--edit-description" without an explicit argument is already
considering the _empty repository_ circumstance in its error.  Also note
that "-m" on the initial branch it is an allowed operation.

This commit makes the error descriptions for those branch operations
with unborn or non-existent branches, more informative.

This is the result of the change:

    $ git init -b first

    $ git branch --edit-description first
    error: No commit on branch 'first' yet.

    $ git branch --set-upstream-to=upstream
    fatal: No commit on branch 'first' yet.

    $ git branch -c second
    fatal: No commit on branch 'first' yet.

    $ git branch [-c/-m] non-existent-branch second
    fatal: No branch named 'non-existent-branch'.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
---

Changes since V1:

 * Fixed the message, non precise descriptions.
 * Error renaming non-existent branch was not correct.
 * Tests for the new errors.


Range-diff:
1:  bba7096c4b ! 1:  096c443a29 branch: description for non-existent branch errors
    @@ Metadata
      ## Commit message ##
         branch: description for non-existent branch errors
     
    -    When the repository does not yet has commits, some errors describe that
    +    When the repository does not yet have commits, some errors describe that
         there is no branch:
     
             $ git init -b first
     
    -        $ git --edit-description first
    -        fatal: branch 'first' does not exist
    +        $ git branch --edit-description first
    +        error: No branch named 'first'.
     
    -        $ git --set-upstream-to=upstream
    +        $ git branch --set-upstream-to=upstream
             fatal: branch 'first' does not exist
     
             $ git branch -c second
    @@ Commit message
     
             $ git init -b first
     
    -        $ git --edit-description first
    -        fatal: No commit on branch 'first' yet.
    +        $ git branch --edit-description first
    +        error: No commit on branch 'first' yet.
     
    -        $ git --set-upstream-to=upstream
    +        $ git branch --set-upstream-to=upstream
             fatal: No commit on branch 'first' yet.
     
    -        $ git -c second
    +        $ git branch -c second
             fatal: No commit on branch 'first' yet.
     
    -        $ git [-c/-m] non-existent-branch second
    +        $ git branch [-c/-m] non-existent-branch second
             fatal: No branch named 'non-existent-branch'.
     
         Signed-off-by: Rubén Justo <rjusto@gmail.com>
    @@ builtin/branch.c: static void copy_or_rename_branch(const char *oldname, const c
      			die(_("Invalid branch name: '%s'"), oldname);
      	}
      
    -+	if (copy && !ref_exists(oldref.buf)) {
    -+		if (!strcmp(head, oldname))
    ++	if ((copy || strcmp(head, oldname)) && !ref_exists(oldref.buf)) {
    ++		if (copy && !strcmp(head, oldname))
     +			die(_("No commit on branch '%s' yet."), oldname);
     +		else
     +			die(_("No branch named '%s'."), oldname);
    @@ builtin/branch.c: int cmd_branch(int argc, const char **argv, const char *prefix
      
      		dwim_and_setup_tracking(the_repository, branch->name,
      					new_upstream, BRANCH_TRACK_OVERRIDE,
    +
    + ## t/t3202-show-branch.sh ##
    +@@ t/t3202-show-branch.sh: test_description='test show-branch'
    + # arbitrary reference time: 2009-08-30 19:20:00
    + GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW
    + 
    ++test_expect_success 'error descriptions on empty repository' '
    ++	current=$(git branch --show-current) &&
    ++	cat >expect <<-EOF &&
    ++	error: No commit on branch '\''$current'\'' yet.
    ++	EOF
    ++	test_must_fail git branch --edit-description 2>actual &&
    ++	test_cmp expect actual &&
    ++	test_must_fail git branch --edit-description $current 2>actual &&
    ++	test_cmp expect actual
    ++'
    ++
    ++test_expect_success 'fatal descriptions on empty repository' '
    ++	current=$(git branch --show-current) &&
    ++	cat >expect <<-EOF &&
    ++	fatal: No commit on branch '\''$current'\'' yet.
    ++	EOF
    ++	test_must_fail git branch --set-upstream-to=non-existent 2>actual &&
    ++	test_cmp expect actual &&
    ++	test_must_fail git branch -c new-branch 2>actual &&
    ++	test_cmp expect actual
    ++'
    ++
    + test_expect_success 'setup' '
    + 	test_commit initial &&
    + 	for i in $(test_seq 1 10)
    +@@ t/t3202-show-branch.sh: done <<\EOF
    + --reflog --current
    + EOF
    + 
    ++test_expect_success 'error descriptions on non-existent branch' '
    ++	cat >expect <<-EOF &&
    ++	error: No branch named '\''non-existent'\'.'
    ++	EOF
    ++	test_must_fail git branch --edit-description non-existent 2>actual &&
    ++	test_cmp expect actual
    ++'
    ++
    ++test_expect_success 'fatal descriptions on non-existent branch' '
    ++	cat >expect <<-EOF &&
    ++	fatal: branch '\''non-existent'\'' does not exist
    ++	EOF
    ++	test_must_fail git branch --set-upstream-to=non-existent non-existent 2>actual &&
    ++	test_cmp expect actual &&
    ++
    ++	cat >expect <<-EOF &&
    ++	fatal: No branch named '\''non-existent'\''.
    ++	EOF
    ++	test_must_fail git branch -c non-existent new-branch 2>actual &&
    ++	test_cmp expect actual &&
    ++	test_must_fail git branch -m non-existent new-branch 2>actual &&
    ++	test_cmp expect actual
    ++'
    ++
    + test_done

 builtin/branch.c       | 14 +++++++++++--
 t/t3202-show-branch.sh | 46 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 55cd9a6e99..499ebec99e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -538,6 +538,13 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
 			die(_("Invalid branch name: '%s'"), oldname);
 	}
 
+	if ((copy || strcmp(head, oldname)) && !ref_exists(oldref.buf)) {
+		if (copy && !strcmp(head, oldname))
+			die(_("No commit on branch '%s' yet."), oldname);
+		else
+			die(_("No branch named '%s'."), oldname);
+	}
+
 	/*
 	 * A command like "git branch -M currentbranch currentbranch" cannot
 	 * cause the worktree to become inconsistent with HEAD, so allow it.
@@ -805,7 +812,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		if (!ref_exists(branch_ref.buf)) {
 			strbuf_release(&branch_ref);
 
-			if (!argc)
+			if (!argc || !strcmp(head, branch_name))
 				return error(_("No commit on branch '%s' yet."),
 					     branch_name);
 			else
@@ -848,8 +855,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			die(_("no such branch '%s'"), argv[0]);
 		}
 
-		if (!ref_exists(branch->refname))
+		if (!ref_exists(branch->refname)) {
+			if (!argc || !strcmp(head, branch->name))
+				die(_("No commit on branch '%s' yet."), branch->name);
 			die(_("branch '%s' does not exist"), branch->name);
+		}
 
 		dwim_and_setup_tracking(the_repository, branch->name,
 					new_upstream, BRANCH_TRACK_OVERRIDE,
diff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh
index f2b9199007..ea7cfd1951 100755
--- a/t/t3202-show-branch.sh
+++ b/t/t3202-show-branch.sh
@@ -7,6 +7,28 @@ test_description='test show-branch'
 # arbitrary reference time: 2009-08-30 19:20:00
 GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW
 
+test_expect_success 'error descriptions on empty repository' '
+	current=$(git branch --show-current) &&
+	cat >expect <<-EOF &&
+	error: No commit on branch '\''$current'\'' yet.
+	EOF
+	test_must_fail git branch --edit-description 2>actual &&
+	test_cmp expect actual &&
+	test_must_fail git branch --edit-description $current 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'fatal descriptions on empty repository' '
+	current=$(git branch --show-current) &&
+	cat >expect <<-EOF &&
+	fatal: No commit on branch '\''$current'\'' yet.
+	EOF
+	test_must_fail git branch --set-upstream-to=non-existent 2>actual &&
+	test_cmp expect actual &&
+	test_must_fail git branch -c new-branch 2>actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'setup' '
 	test_commit initial &&
 	for i in $(test_seq 1 10)
@@ -175,4 +197,28 @@ done <<\EOF
 --reflog --current
 EOF
 
+test_expect_success 'error descriptions on non-existent branch' '
+	cat >expect <<-EOF &&
+	error: No branch named '\''non-existent'\'.'
+	EOF
+	test_must_fail git branch --edit-description non-existent 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'fatal descriptions on non-existent branch' '
+	cat >expect <<-EOF &&
+	fatal: branch '\''non-existent'\'' does not exist
+	EOF
+	test_must_fail git branch --set-upstream-to=non-existent non-existent 2>actual &&
+	test_cmp expect actual &&
+
+	cat >expect <<-EOF &&
+	fatal: No branch named '\''non-existent'\''.
+	EOF
+	test_must_fail git branch -c non-existent new-branch 2>actual &&
+	test_cmp expect actual &&
+	test_must_fail git branch -m non-existent new-branch 2>actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.36.1

Re: [PATCH v2] branch: description for non-existent branch errors

[Bagas Sanjaya]

On 10/1/22 05:47, Rubén Justo wrote:
> This commit makes the error descriptions for those branch operations
> with unborn or non-existent branches, more informative.
> 

s/This commit makes/Make/

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH v2] branch: description for non-existent branch errors

[Rubén Justo]

On 1/10/22 14:43, Bagas Sanjaya wrote:
> On 10/1/22 05:47, Rubén Justo wrote:
>> This commit makes the error descriptions for those branch operations
>> with unborn or non-existent branches, more informative.
>>
> 
> s/This commit makes/Make/
> 

Thanks

[PATCH v3] branch: description for non-existent branch errors

[Rubén Justo]

When the repository does not yet have commits, some errors describe that
there is no branch:

    $ git init -b first

    $ git branch --edit-description first
    error: No branch named 'first'.

    $ git branch --set-upstream-to=upstream
    fatal: branch 'first' does not exist

    $ git branch -c second
    error: refname refs/heads/first not found
    fatal: Branch copy failed

That "first" branch is unborn but to say it doesn't exists is confusing.

Options "-c" (copy) and "-m" (rename) show the same error when the
origin branch doesn't exists:

    $ git branch -c non-existent-branch second
    error: refname refs/heads/non-existent-branch not found
    fatal: Branch copy failed

    $ git branch -m non-existent-branch second
    error: refname refs/heads/non-existent-branch not found
    fatal: Branch rename failed

Note that "--edit-description" without an explicit argument is already
considering the _empty repository_ circumstance in its error.  Also note
that "-m" on the initial branch it is an allowed operation.

Make the error descriptions for those branch operations with unborn or
non-existent branches, more informative.

This is the result of the change:

    $ git init -b first

    $ git branch --edit-description first
    error: No commit on branch 'first' yet.

    $ git branch --set-upstream-to=upstream
    fatal: No commit on branch 'first' yet.

    $ git branch -c second
    fatal: No commit on branch 'first' yet.

    $ git branch [-c/-m] non-existent-branch second
    fatal: No branch named 'non-existent-branch'.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
Range-diff:
1:  d8c3242b31 ! 1:  180435ff15 branch: description for non-existent branch errors
    @@ Commit message
         considering the _empty repository_ circumstance in its error.  Also note
         that "-m" on the initial branch it is an allowed operation.
     
    -    This commit makes the error descriptions for those branch operations
    -    with unborn or non-existent branches, more informative.
    +    Make the error descriptions for those branch operations with unborn or
    +    non-existent branches, more informative.
     
         This is the result of the change:
     

 builtin/branch.c       | 14 +++++++++++--
 t/t3202-show-branch.sh | 46 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 55cd9a6e99..499ebec99e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -538,6 +538,13 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
 			die(_("Invalid branch name: '%s'"), oldname);
 	}
 
+	if ((copy || strcmp(head, oldname)) && !ref_exists(oldref.buf)) {
+		if (copy && !strcmp(head, oldname))
+			die(_("No commit on branch '%s' yet."), oldname);
+		else
+			die(_("No branch named '%s'."), oldname);
+	}
+
 	/*
 	 * A command like "git branch -M currentbranch currentbranch" cannot
 	 * cause the worktree to become inconsistent with HEAD, so allow it.
@@ -805,7 +812,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		if (!ref_exists(branch_ref.buf)) {
 			strbuf_release(&branch_ref);
 
-			if (!argc)
+			if (!argc || !strcmp(head, branch_name))
 				return error(_("No commit on branch '%s' yet."),
 					     branch_name);
 			else
@@ -848,8 +855,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			die(_("no such branch '%s'"), argv[0]);
 		}
 
-		if (!ref_exists(branch->refname))
+		if (!ref_exists(branch->refname)) {
+			if (!argc || !strcmp(head, branch->name))
+				die(_("No commit on branch '%s' yet."), branch->name);
 			die(_("branch '%s' does not exist"), branch->name);
+		}
 
 		dwim_and_setup_tracking(the_repository, branch->name,
 					new_upstream, BRANCH_TRACK_OVERRIDE,
diff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh
index f2b9199007..ea7cfd1951 100755
--- a/t/t3202-show-branch.sh
+++ b/t/t3202-show-branch.sh
@@ -7,6 +7,28 @@ test_description='test show-branch'
 # arbitrary reference time: 2009-08-30 19:20:00
 GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW
 
+test_expect_success 'error descriptions on empty repository' '
+	current=$(git branch --show-current) &&
+	cat >expect <<-EOF &&
+	error: No commit on branch '\''$current'\'' yet.
+	EOF
+	test_must_fail git branch --edit-description 2>actual &&
+	test_cmp expect actual &&
+	test_must_fail git branch --edit-description $current 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'fatal descriptions on empty repository' '
+	current=$(git branch --show-current) &&
+	cat >expect <<-EOF &&
+	fatal: No commit on branch '\''$current'\'' yet.
+	EOF
+	test_must_fail git branch --set-upstream-to=non-existent 2>actual &&
+	test_cmp expect actual &&
+	test_must_fail git branch -c new-branch 2>actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'setup' '
 	test_commit initial &&
 	for i in $(test_seq 1 10)
@@ -175,4 +197,28 @@ done <<\EOF
 --reflog --current
 EOF
 
+test_expect_success 'error descriptions on non-existent branch' '
+	cat >expect <<-EOF &&
+	error: No branch named '\''non-existent'\'.'
+	EOF
+	test_must_fail git branch --edit-description non-existent 2>actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'fatal descriptions on non-existent branch' '
+	cat >expect <<-EOF &&
+	fatal: branch '\''non-existent'\'' does not exist
+	EOF
+	test_must_fail git branch --set-upstream-to=non-existent non-existent 2>actual &&
+	test_cmp expect actual &&
+
+	cat >expect <<-EOF &&
+	fatal: No branch named '\''non-existent'\''.
+	EOF
+	test_must_fail git branch -c non-existent new-branch 2>actual &&
+	test_cmp expect actual &&
+	test_must_fail git branch -m non-existent new-branch 2>actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.32.0

Re: [PATCH v3] branch: description for non-existent branch errors

[Eric Sunshine]

On Fri, Oct 7, 2022 at 8:49 PM Rubén Justo <rjusto@gmail.com> wrote:
> [...]
> Make the error descriptions for those branch operations with unborn or
> non-existent branches, more informative.
> [...]
> Signed-off-by: Rubén Justo <rjusto@gmail.com>
> ---
> diff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh
> @@ -7,6 +7,28 @@ test_description='test show-branch'
> +test_expect_success 'error descriptions on empty repository' '
> +       current=$(git branch --show-current) &&
> +       cat >expect <<-EOF &&
> +       error: No commit on branch '\''$current'\'' yet.
> +       EOF

It's a matter of taste, but leaving and re-entering the single-quoted
context, along with escaping, can make for a difficult read. You could
instead take advantage of the SQ variable already defined by the test
scripts:

    echo "error: No commit on branch $SQ$current$SQ yet." >expect &&

Not worth a re-roll, of course.

> +test_expect_success 'fatal descriptions on empty repository' '
> +       current=$(git branch --show-current) &&
> +       cat >expect <<-EOF &&
> +       fatal: No commit on branch '\''$current'\'' yet.
> +       EOF

Ditto.

> +test_expect_success 'error descriptions on non-existent branch' '
> +       cat >expect <<-EOF &&
> +       error: No branch named '\''non-existent'\'.'
> +       EOF

Likewise.

> +test_expect_success 'fatal descriptions on non-existent branch' '
> +       cat >expect <<-EOF &&
> +       fatal: branch '\''non-existent'\'' does not exist
> +       EOF

Same.

> +       test_must_fail git branch --set-upstream-to=non-existent non-existent 2>actual &&
> +       test_cmp expect actual &&
> +
> +       cat >expect <<-EOF &&
> +       fatal: No branch named '\''non-existent'\''.
> +       EOF

Again.

Re: [PATCH v3] branch: description for non-existent branch errors

[Rubén Justo]

On 8/10/22 5:27, Eric Sunshine wrote:
> On Fri, Oct 7, 2022 at 8:49 PM Rubén Justo <rjusto@gmail.com> wrote:
>> [...]
>> Make the error descriptions for those branch operations with unborn or
>> non-existent branches, more informative.
>> [...]
>> Signed-off-by: Rubén Justo <rjusto@gmail.com>
>> ---
>> diff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh
>> @@ -7,6 +7,28 @@ test_description='test show-branch'
>> +test_expect_success 'error descriptions on empty repository' '
>> +       current=$(git branch --show-current) &&
>> +       cat >expect <<-EOF &&
>> +       error: No commit on branch '\''$current'\'' yet.
>> +       EOF
> 
> It's a matter of taste, but leaving and re-entering the single-quoted
> context, along with escaping, can make for a difficult read. You could
> instead take advantage of the SQ variable already defined by the test
> scripts:
> 
>     echo "error: No commit on branch $SQ$current$SQ yet." >expect &&
> 
> Not worth a re-roll, of course.
> 
>> +test_expect_success 'fatal descriptions on empty repository' '
>> +       current=$(git branch --show-current) &&
>> +       cat >expect <<-EOF &&
>> +       fatal: No commit on branch '\''$current'\'' yet.
>> +       EOF
> 
> Ditto.

Thanks, I didn't know about $SQ.

	'\''$current'\'' vs $SQ$current$SQ vs ${SQ}$current${SQ}

I also find ugly that escaping, but I think is harder to read and
error prone to use $SQ here.. :-/

Re: [PATCH v3] branch: description for non-existent branch errors

[Junio C Hamano]

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

> Thanks, I didn't know about $SQ.
>
> 	'\''$current'\'' vs $SQ$current$SQ vs ${SQ}$current${SQ}
>
> I also find ugly that escaping, but I think is harder to read and
> error prone to use $SQ here.. :-/

The ONLY case when $SQ shines is when the string that comes inside
the single-quote pair begins with a non-alnum.  $SQ$current$SQ is
semi-readable, but if the string begins with an alnum, then you'd be
forced to say ${SQ}current${SQ} (the first one must be inside braces
because you do not want to refer to a variable whose name is
SQcurrent, the second one wants to be inside braces for symmetry),
which is ugly.

The rhythm of '\'' is not so bad, once you get used to seeing them.
${SQ}...${SQ} is a bit too loud.