[PATCH] branch: let '--edit-description' default to rebased branch during rebase

[PATCH] branch: let '--edit-description' default to rebased branch during rebase

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Defaulting to editing the description of the rebased branch without an
explicit branchname argument would be useful.  Even the git bash prompt
shows the name of the rebased branch, and then

  ~/src/git (mybranch|REBASE-i 1/2)$ git branch --edit-description
  fatal: Cannot give description to detached HEAD

looks quite unhelpful.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
Changed in v2:
 - add a test
 - fix commit message & summary
 - simplify code

builtin/branch.c  | 24 +++++++++++++++++++-----
 t/t3200-branch.sh | 19 +++++++++++++++++++
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index d8297f80ff..ee82dc828e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -745,15 +745,27 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		string_list_clear(&output, 0);
 		return 0;
 	} else if (edit_description) {
-		const char *branch_name;
+		char *branch_name = NULL;
 		struct strbuf branch_ref = STRBUF_INIT;
 
 		if (!argc) {
-			if (filter.detached)
-				die(_("Cannot give description to detached HEAD"));
-			branch_name = head;
+			if (filter.detached) {
+				struct wt_status_state state;
+
+				memset(&state, 0, sizeof(state));
+
+				if (wt_status_check_rebase(NULL, &state)) {
+					branch_name = state.branch;
+				}
+
+				if (!branch_name)
+					die(_("Cannot give description to detached HEAD"));
+
+				free(state.onto);
+			} else
+				branch_name = xstrdup(head);
 		} else if (argc == 1)
-			branch_name = argv[0];
+			branch_name = xstrdup(argv[0]);
 		else
 			die(_("cannot edit description of more than one branch"));
 
@@ -772,6 +784,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 		if (edit_branch_description(branch_name))
 			return 1;
+
+		free(branch_name);
 	} else if (copy) {
 		if (!argc)
 			die(_("branch name required"));
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 411a70b0ce..a20ebedea0 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1260,6 +1260,25 @@ test_expect_success 'use --edit-description' '
 	test_cmp expect EDITOR_OUTPUT
 '
 
+test_expect_success 'use --edit-description during rebase' '
+	write_script editor <<-\EOF &&
+		echo "Rebase contents" >"$1"
+	EOF
+	(
+		set_fake_editor &&
+		FAKE_LINES="break 1" git rebase -i HEAD^ &&
+		EDITOR=./editor git branch --edit-description &&
+		git rebase --continue
+	) &&
+	write_script editor <<-\EOF &&
+		git stripspace -s <"$1" >"EDITOR_OUTPUT"
+	EOF
+	EDITOR=./editor git branch --edit-description &&
+	echo "Rebase contents" >expect &&
+	test_cmp expect EDITOR_OUTPUT
+'
+test_done
+
 test_expect_success 'detect typo in branch name when using --edit-description' '
 	write_script editor <<-\EOF &&
 		echo "New contents" >"$1"

base-commit: 7a6a90c6ec48fc78c83d7090d6c1b95d8f3739c0
-- 
2.25.0.rc2.2.g5aece98438

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[Eric Sunshine]

On Sat, Jan 11, 2020 at 7:36 AM <marcandre.lureau@redhat.com> wrote:
> Defaulting to editing the description of the rebased branch without an
> explicit branchname argument would be useful.  Even the git bash prompt
> shows the name of the rebased branch, and then
>
>   ~/src/git (mybranch|REBASE-i 1/2)$ git branch --edit-description
>   fatal: Cannot give description to detached HEAD
>
> looks quite unhelpful.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> diff --git a/builtin/branch.c b/builtin/branch.c
> @@ -745,15 +745,27 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>                 if (!argc) {
> -                       if (filter.detached)
> -                               die(_("Cannot give description to detached HEAD"));
> -                       branch_name = head;
> +                       if (filter.detached) {
> +                               struct wt_status_state state;
> +
> +                               memset(&state, 0, sizeof(state));
> +
> +                               if (wt_status_check_rebase(NULL, &state)) {
> +                                       branch_name = state.branch;
> +                               }

Style: drop unneeded braces.

> +
> +                               if (!branch_name)
> +                                       die(_("Cannot give description to detached HEAD"));
> +
> +                               free(state.onto);

Also, no need for all the blank lines which eat up valuable vertical
screen real-estate without making the code clearer.

> +                       } else
> +                               branch_name = xstrdup(head);

It would be easier to see what happens in the common case (when not
rebasing) if you invert the condition to `if (!filter.detached)` and
turn this one-line 'else' branch into the 'if' branch.

> @@ -772,6 +784,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>                 if (edit_branch_description(branch_name))
>                         return 1;
> +
> +               free(branch_name);

That `return 1` just above this free() is leaking 'branch_name', isn't it?

> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> @@ -1260,6 +1260,25 @@ test_expect_success 'use --edit-description' '
> +test_expect_success 'use --edit-description during rebase' '
> +       write_script editor <<-\EOF &&
> +               echo "Rebase contents" >"$1"
> +       EOF
> +       (
> +               set_fake_editor &&
> +               FAKE_LINES="break 1" git rebase -i HEAD^ &&
> +               EDITOR=./editor git branch --edit-description &&
> +               git rebase --continue
> +       ) &&
> +       write_script editor <<-\EOF &&
> +               git stripspace -s <"$1" >"EDITOR_OUTPUT"
> +       EOF
> +       EDITOR=./editor git branch --edit-description &&
> +       echo "Rebase contents" >expect &&
> +       test_cmp expect EDITOR_OUTPUT
> +'
> +test_done

Strange place for a test_done() invocation considering that existing
tests follow the new one added by this patch.

>  test_expect_success 'detect typo in branch name when using --edit-description' '
>         write_script editor <<-\EOF &&
>                 echo "New contents" >"$1"

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[Marc-André Lureau]

Hi

On Sat, Jan 11, 2020 at 5:28 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Sat, Jan 11, 2020 at 7:36 AM <marcandre.lureau@redhat.com> wrote:
> > Defaulting to editing the description of the rebased branch without an
> > explicit branchname argument would be useful.  Even the git bash prompt
> > shows the name of the rebased branch, and then
> >
> >   ~/src/git (mybranch|REBASE-i 1/2)$ git branch --edit-description
> >   fatal: Cannot give description to detached HEAD
> >
> > looks quite unhelpful.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > diff --git a/builtin/branch.c b/builtin/branch.c
> > @@ -745,15 +745,27 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> >                 if (!argc) {
> > -                       if (filter.detached)
> > -                               die(_("Cannot give description to detached HEAD"));
> > -                       branch_name = head;
> > +                       if (filter.detached) {
> > +                               struct wt_status_state state;
> > +
> > +                               memset(&state, 0, sizeof(state));
> > +
> > +                               if (wt_status_check_rebase(NULL, &state)) {
> > +                                       branch_name = state.branch;
> > +                               }
>
> Style: drop unneeded braces.

ok

>
> > +
> > +                               if (!branch_name)
> > +                                       die(_("Cannot give description to detached HEAD"));
> > +
> > +                               free(state.onto);
>
> Also, no need for all the blank lines which eat up valuable vertical
> screen real-estate without making the code clearer.

ok

>
> > +                       } else
> > +                               branch_name = xstrdup(head);
>
> It would be easier to see what happens in the common case (when not
> rebasing) if you invert the condition to `if (!filter.detached)` and
> turn this one-line 'else' branch into the 'if' branch.

indeed

>
> > @@ -772,6 +784,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> >                 if (edit_branch_description(branch_name))
> >                         return 1;
> > +
> > +               free(branch_name);
>
> That `return 1` just above this free() is leaking 'branch_name', isn't it?

right, let's fix that too

>
> > diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> > @@ -1260,6 +1260,25 @@ test_expect_success 'use --edit-description' '
> > +test_expect_success 'use --edit-description during rebase' '
> > +       write_script editor <<-\EOF &&
> > +               echo "Rebase contents" >"$1"
> > +       EOF
> > +       (
> > +               set_fake_editor &&
> > +               FAKE_LINES="break 1" git rebase -i HEAD^ &&
> > +               EDITOR=./editor git branch --edit-description &&
> > +               git rebase --continue
> > +       ) &&
> > +       write_script editor <<-\EOF &&
> > +               git stripspace -s <"$1" >"EDITOR_OUTPUT"
> > +       EOF
> > +       EDITOR=./editor git branch --edit-description &&
> > +       echo "Rebase contents" >expect &&
> > +       test_cmp expect EDITOR_OUTPUT
> > +'
> > +test_done
>
> Strange place for a test_done() invocation considering that existing
> tests follow the new one added by this patch.

doh, sorry
thanks for the review!

>
> >  test_expect_success 'detect typo in branch name when using --edit-description' '
> >         write_script editor <<-\EOF &&
> >                 echo "New contents" >"$1"



-- 
Marc-André Lureau

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[Eric Sunshine]

On Sat, Jan 11, 2020 at 9:55 AM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
> On Sat, Jan 11, 2020 at 5:28 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > On Sat, Jan 11, 2020 at 7:36 AM <marcandre.lureau@redhat.com> wrote:
> > > +                               if (wt_status_check_rebase(NULL, &state)) {
> > > +                                       branch_name = state.branch;
> > > +                               }

Taking a deeper look at the code, I'm wondering it would make more
sense to call wt_status_get_state(), which handles 'rebase' and
'bisect'. Is there a reason that you limited this check to only
'rebase'?

> > >                 if (edit_branch_description(branch_name))
> > >                         return 1;
> > > +
> > > +               free(branch_name);
> >
> > That `return 1` just above this free() is leaking 'branch_name', isn't it?
>
> right, let's fix that too

Looking at the code itself (rather than consulting only the patch), I
see that there are a couple more early returns leaking 'branch_name',
so they need to be handled, as well.

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[Marc-André Lureau]

Hi Eric

On Sun, Jan 12, 2020 at 5:27 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Sat, Jan 11, 2020 at 9:55 AM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> > On Sat, Jan 11, 2020 at 5:28 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > > On Sat, Jan 11, 2020 at 7:36 AM <marcandre.lureau@redhat.com> wrote:
> > > > +                               if (wt_status_check_rebase(NULL, &state)) {
> > > > +                                       branch_name = state.branch;
> > > > +                               }
>
> Taking a deeper look at the code, I'm wondering it would make more
> sense to call wt_status_get_state(), which handles 'rebase' and
> 'bisect'. Is there a reason that you limited this check to only
> 'rebase'?

No reason, I just didn't try it yet. Done, thanks

>
> > > >                 if (edit_branch_description(branch_name))
> > > >                         return 1;
> > > > +
> > > > +               free(branch_name);
> > >
> > > That `return 1` just above this free() is leaking 'branch_name', isn't it?
> >
> > right, let's fix that too
>
> Looking at the code itself (rather than consulting only the patch), I
> see that there are a couple more early returns leaking 'branch_name',
> so they need to be handled, as well.

I think I covered them now, sending v4.

thanks

-- 
Marc-André Lureau

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[SZEDER Gábor]

On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:
> On Sat, Jan 11, 2020 at 9:55 AM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> > On Sat, Jan 11, 2020 at 5:28 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > > On Sat, Jan 11, 2020 at 7:36 AM <marcandre.lureau@redhat.com> wrote:
> > > > +                               if (wt_status_check_rebase(NULL, &state)) {
> > > > +                                       branch_name = state.branch;
> > > > +                               }
> 
> Taking a deeper look at the code, I'm wondering it would make more
> sense to call wt_status_get_state(), which handles 'rebase' and
> 'bisect'. Is there a reason that you limited this check to only
> 'rebase'?

While I do think that defaulting to edit the description of the
rebased branch makes sense, I'm not sure how that would work with
bisect.

What branch name does wt_status_get_state() return while bisecting?
The branch where I started from?  Because that's what 'git status'
shows:

  ~/src/git (mybranch)$ git bisect start v2.21.0 v2.20.0
  Bisecting: 334 revisions left to test after this (roughly 8 steps)
  [b99a579f8e434a7757f90895945b5711b3f159d5] Merge branch 'sb/more-repo-in-api'
  ~/src/git ((b99a579f8e...)|BISECTING)$ git status 
  HEAD detached at b99a579f8e
  You are currently bisecting, started from branch 'mybranch'.
    (use "git bisect reset" to get back to the original branch)
  
  nothing to commit, working tree clean

But am I really on that branch?  Does it really makes sense to edit
the description of 'mybranch' by default while bisecting through an
old revision range?  I do not think so.

> > > >                 if (edit_branch_description(branch_name))
> > > >                         return 1;
> > > > +
> > > > +               free(branch_name);
> > >
> > > That `return 1` just above this free() is leaking 'branch_name', isn't it?
> >
> > right, let's fix that too
> 
> Looking at the code itself (rather than consulting only the patch), I
> see that there are a couple more early returns leaking 'branch_name',
> so they need to be handled, as well.

'git branch --edit-description' is a one-shot operation: it allows to
edit only one branch description per invocation, and then the process
exits right away, whether the operation was successful or some error
occurred.  I'm not sure free()ing 'branch_name' is worth the effort
(and even if it does, I think it should be a separate preparatory
patch).

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[Eric Sunshine]

On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:
> > Taking a deeper look at the code, I'm wondering it would make more
> > sense to call wt_status_get_state(), which handles 'rebase' and
> > 'bisect'. Is there a reason that you limited this check to only
> > 'rebase'?
>
> What branch name does wt_status_get_state() return while bisecting?
> The branch where I started from?  Because that's what 'git status'
> shows:
> But am I really on that branch?  Does it really makes sense to edit
> the description of 'mybranch' by default while bisecting through an
> old revision range?  I do not think so.

It's not clear what downside you are pointing out; i.e. why would it
be a bad thing to be able to set the branch description even while
bisecting -- especially since `git status` affirms that it knows the
branch?

> > Looking at the code itself (rather than consulting only the patch), I
> > see that there are a couple more early returns leaking 'branch_name',
> > so they need to be handled, as well.
>
> 'git branch --edit-description' is a one-shot operation: it allows to
> edit only one branch description per invocation, and then the process
> exits right away, whether the operation was successful or some error
> occurred.

It is one-shot, but the existing `--edit-description` code already
cleans up after itself by releasing resources it allocated (as do
other one-shot parts of cmd_branch()), so it would be odd and
inconsistent for this new code to not clean up after itself, as well
(or, more accurately, to only clean up after itself in some branches
but not others).

> I'm not sure free()ing 'branch_name' is worth the effort
> (and even if it does, I think it should be a separate preparatory
> patch).

A separate preparatory patch doesn't make sense in this case since
'branch_name' becomes "freeable" with this patch itself (prior to
that, it was `const char *`).

Anyhow, a different approach was later proposed[1] which eliminates
some of the ugliness.

[1]: https://lore.kernel.org/git/20200112101735.GA19676@flurp.local/

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[SZEDER Gábor]

On Sun, Jan 12, 2020 at 08:59:04PM -0500, Eric Sunshine wrote:
> On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:
> > > Taking a deeper look at the code, I'm wondering it would make more
> > > sense to call wt_status_get_state(), which handles 'rebase' and
> > > 'bisect'. Is there a reason that you limited this check to only
> > > 'rebase'?
> >
> > What branch name does wt_status_get_state() return while bisecting?
> > The branch where I started from?  Because that's what 'git status'
> > shows:
> > But am I really on that branch?  Does it really makes sense to edit
> > the description of 'mybranch' by default while bisecting through an
> > old revision range?  I do not think so.
> 
> It's not clear what downside you are pointing out; i.e. why would it
> be a bad thing to be able to set the branch description even while
> bisecting -- especially since `git status` affirms that it knows the
> branch?

No, during a bisect operation 'git status' knows the branch where I
_was_ when I started bisecting, and where a 'git bisect reset' will
eventually bring me back when I'm finished, and that has no relation
whatsoever to the revision range that I'm bisecting.

Consider this case:

  $ git checkout --orphan unrelated-history
  Switched to a new branch 'unrelated-history'
  $ git commit -m "test"
  [unrelated-history (root-commit) 639b9d1047] test
  <...>
  $ git bisect start v2.25.0 v2.24.0
  Bisecting: 361 revisions left to test after this (roughly 9 steps)
  [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1
  $ git status 
  HEAD detached at 7034cd094b
  You are currently bisecting, started from branch 'unrelated-history'.
    (use "git bisect reset" to get back to the original branch)
  
  nothing to commit, working tree clean

I can't possible be on branch 'unrelated-history' during that
bisection.


OTOH, while during a rebase we are technically on a detached HEAD as
well, that rebase operation is all about constructing the new history
of the rebased branch, and once finished that branch will be updated
to point to the tip of the new history, thus it will include all the
commits created while on the detached HEAD.  Therefore, it makes sense
conceptually to treat it as if we were on the rebased branch.  That's
why it makes sense to display the name of the rebased branch in the
Bash prompt, and that's why I think it makes sense to default to edit
the description of the rebased branch without explicitly naming it.

With bisect that just doesn't make sense.

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[Marc-André Lureau]

Hi

On Fri, Jan 24, 2020 at 11:41 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
>
> On Sun, Jan 12, 2020 at 08:59:04PM -0500, Eric Sunshine wrote:
> > On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:
> > > > Taking a deeper look at the code, I'm wondering it would make more
> > > > sense to call wt_status_get_state(), which handles 'rebase' and
> > > > 'bisect'. Is there a reason that you limited this check to only
> > > > 'rebase'?
> > >
> > > What branch name does wt_status_get_state() return while bisecting?
> > > The branch where I started from?  Because that's what 'git status'
> > > shows:
> > > But am I really on that branch?  Does it really makes sense to edit
> > > the description of 'mybranch' by default while bisecting through an
> > > old revision range?  I do not think so.
> >
> > It's not clear what downside you are pointing out; i.e. why would it
> > be a bad thing to be able to set the branch description even while
> > bisecting -- especially since `git status` affirms that it knows the
> > branch?
>
> No, during a bisect operation 'git status' knows the branch where I
> _was_ when I started bisecting, and where a 'git bisect reset' will
> eventually bring me back when I'm finished, and that has no relation
> whatsoever to the revision range that I'm bisecting.
>
> Consider this case:
>
>   $ git checkout --orphan unrelated-history
>   Switched to a new branch 'unrelated-history'
>   $ git commit -m "test"
>   [unrelated-history (root-commit) 639b9d1047] test
>   <...>
>   $ git bisect start v2.25.0 v2.24.0
>   Bisecting: 361 revisions left to test after this (roughly 9 steps)
>   [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1
>   $ git status
>   HEAD detached at 7034cd094b
>   You are currently bisecting, started from branch 'unrelated-history'.
>     (use "git bisect reset" to get back to the original branch)
>
>   nothing to commit, working tree clean
>
> I can't possible be on branch 'unrelated-history' during that
> bisection.
>
>
> OTOH, while during a rebase we are technically on a detached HEAD as
> well, that rebase operation is all about constructing the new history
> of the rebased branch, and once finished that branch will be updated
> to point to the tip of the new history, thus it will include all the
> commits created while on the detached HEAD.  Therefore, it makes sense
> conceptually to treat it as if we were on the rebased branch.  That's
> why it makes sense to display the name of the rebased branch in the
> Bash prompt, and that's why I think it makes sense to default to edit
> the description of the rebased branch without explicitly naming it.
>
> With bisect that just doesn't make sense.

If the range you are bisecting belongs or lead to the current branch,
that still makes sense. And it's probably most of the time. So, I am
not sure your objection is valid enough here.



-- 
Marc-André Lureau

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[SZEDER Gábor]

On Thu, Jan 30, 2020 at 10:37:38PM +0100, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Jan 24, 2020 at 11:41 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> >
> > On Sun, Jan 12, 2020 at 08:59:04PM -0500, Eric Sunshine wrote:
> > > On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > > > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:
> > > > > Taking a deeper look at the code, I'm wondering it would make more
> > > > > sense to call wt_status_get_state(), which handles 'rebase' and
> > > > > 'bisect'. Is there a reason that you limited this check to only
> > > > > 'rebase'?
> > > >
> > > > What branch name does wt_status_get_state() return while bisecting?
> > > > The branch where I started from?  Because that's what 'git status'
> > > > shows:
> > > > But am I really on that branch?  Does it really makes sense to edit
> > > > the description of 'mybranch' by default while bisecting through an
> > > > old revision range?  I do not think so.
> > >
> > > It's not clear what downside you are pointing out; i.e. why would it
> > > be a bad thing to be able to set the branch description even while
> > > bisecting -- especially since `git status` affirms that it knows the
> > > branch?
> >
> > No, during a bisect operation 'git status' knows the branch where I
> > _was_ when I started bisecting, and where a 'git bisect reset' will
> > eventually bring me back when I'm finished, and that has no relation
> > whatsoever to the revision range that I'm bisecting.
> >
> > Consider this case:
> >
> >   $ git checkout --orphan unrelated-history
> >   Switched to a new branch 'unrelated-history'
> >   $ git commit -m "test"
> >   [unrelated-history (root-commit) 639b9d1047] test
> >   <...>
> >   $ git bisect start v2.25.0 v2.24.0
> >   Bisecting: 361 revisions left to test after this (roughly 9 steps)
> >   [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1
> >   $ git status
> >   HEAD detached at 7034cd094b
> >   You are currently bisecting, started from branch 'unrelated-history'.
> >     (use "git bisect reset" to get back to the original branch)
> >
> >   nothing to commit, working tree clean
> >
> > I can't possible be on branch 'unrelated-history' during that
> > bisection.
> >
> >
> > OTOH, while during a rebase we are technically on a detached HEAD as
> > well, that rebase operation is all about constructing the new history
> > of the rebased branch, and once finished that branch will be updated
> > to point to the tip of the new history, thus it will include all the
> > commits created while on the detached HEAD.  Therefore, it makes sense
> > conceptually to treat it as if we were on the rebased branch.  That's
> > why it makes sense to display the name of the rebased branch in the
> > Bash prompt, and that's why I think it makes sense to default to edit
> > the description of the rebased branch without explicitly naming it.
> >
> > With bisect that just doesn't make sense.
> 
> If the range you are bisecting belongs or lead to the current branch,
> that still makes sense. And it's probably most of the time. So, I am
> not sure your objection is valid enough here.

I'm not sure what you mean with "belongs or lead to" a branch.

Do you mean that the range is reachable from the branch that just so
happened to be checked out when the bisection was started?  Well, I
have over 30 branches from where v2.25.0 is reachable, and all of them
are obviously bad candidates for editing their descriptions by default
while bisecting a totally unrelated issue.

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[Marc-André Lureau]

Hi

On Fri, Jan 31, 2020 at 4:52 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
>
> On Thu, Jan 30, 2020 at 10:37:38PM +0100, Marc-André Lureau wrote:
> > Hi
> >
> > On Fri, Jan 24, 2020 at 11:41 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > >
> > > On Sun, Jan 12, 2020 at 08:59:04PM -0500, Eric Sunshine wrote:
> > > > On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > > > > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:
> > > > > > Taking a deeper look at the code, I'm wondering it would make more
> > > > > > sense to call wt_status_get_state(), which handles 'rebase' and
> > > > > > 'bisect'. Is there a reason that you limited this check to only
> > > > > > 'rebase'?
> > > > >
> > > > > What branch name does wt_status_get_state() return while bisecting?
> > > > > The branch where I started from?  Because that's what 'git status'
> > > > > shows:
> > > > > But am I really on that branch?  Does it really makes sense to edit
> > > > > the description of 'mybranch' by default while bisecting through an
> > > > > old revision range?  I do not think so.
> > > >
> > > > It's not clear what downside you are pointing out; i.e. why would it
> > > > be a bad thing to be able to set the branch description even while
> > > > bisecting -- especially since `git status` affirms that it knows the
> > > > branch?
> > >
> > > No, during a bisect operation 'git status' knows the branch where I
> > > _was_ when I started bisecting, and where a 'git bisect reset' will
> > > eventually bring me back when I'm finished, and that has no relation
> > > whatsoever to the revision range that I'm bisecting.
> > >
> > > Consider this case:
> > >
> > >   $ git checkout --orphan unrelated-history
> > >   Switched to a new branch 'unrelated-history'
> > >   $ git commit -m "test"
> > >   [unrelated-history (root-commit) 639b9d1047] test
> > >   <...>
> > >   $ git bisect start v2.25.0 v2.24.0
> > >   Bisecting: 361 revisions left to test after this (roughly 9 steps)
> > >   [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1
> > >   $ git status
> > >   HEAD detached at 7034cd094b
> > >   You are currently bisecting, started from branch 'unrelated-history'.
> > >     (use "git bisect reset" to get back to the original branch)
> > >
> > >   nothing to commit, working tree clean
> > >
> > > I can't possible be on branch 'unrelated-history' during that
> > > bisection.
> > >
> > >
> > > OTOH, while during a rebase we are technically on a detached HEAD as
> > > well, that rebase operation is all about constructing the new history
> > > of the rebased branch, and once finished that branch will be updated
> > > to point to the tip of the new history, thus it will include all the
> > > commits created while on the detached HEAD.  Therefore, it makes sense
> > > conceptually to treat it as if we were on the rebased branch.  That's
> > > why it makes sense to display the name of the rebased branch in the
> > > Bash prompt, and that's why I think it makes sense to default to edit
> > > the description of the rebased branch without explicitly naming it.
> > >
> > > With bisect that just doesn't make sense.
> >
> > If the range you are bisecting belongs or lead to the current branch,
> > that still makes sense. And it's probably most of the time. So, I am
> > not sure your objection is valid enough here.
>
> I'm not sure what you mean with "belongs or lead to" a branch.
>
> Do you mean that the range is reachable from the branch that just so
> happened to be checked out when the bisection was started?  Well, I
> have over 30 branches from where v2.25.0 is reachable, and all of them
> are obviously bad candidates for editing their descriptions by default
> while bisecting a totally unrelated issue.
>


If we take that simple example:

* (my-branch)
*
* bisect bad
*
* (HEAD)
* bisect good
*

It makes a lot of sense to me to edit my-branch description by
default, even if the range good-bad happen to exist in other branches.

-- 
Marc-André Lureau

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[SZEDER Gábor]

On Fri, Jan 31, 2020 at 04:59:15PM +0100, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Jan 31, 2020 at 4:52 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> >
> > On Thu, Jan 30, 2020 at 10:37:38PM +0100, Marc-André Lureau wrote:
> > > Hi
> > >
> > > On Fri, Jan 24, 2020 at 11:41 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > > >
> > > > On Sun, Jan 12, 2020 at 08:59:04PM -0500, Eric Sunshine wrote:
> > > > > On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > > > > > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:
> > > > > > > Taking a deeper look at the code, I'm wondering it would make more
> > > > > > > sense to call wt_status_get_state(), which handles 'rebase' and
> > > > > > > 'bisect'. Is there a reason that you limited this check to only
> > > > > > > 'rebase'?
> > > > > >
> > > > > > What branch name does wt_status_get_state() return while bisecting?
> > > > > > The branch where I started from?  Because that's what 'git status'
> > > > > > shows:
> > > > > > But am I really on that branch?  Does it really makes sense to edit
> > > > > > the description of 'mybranch' by default while bisecting through an
> > > > > > old revision range?  I do not think so.
> > > > >
> > > > > It's not clear what downside you are pointing out; i.e. why would it
> > > > > be a bad thing to be able to set the branch description even while
> > > > > bisecting -- especially since `git status` affirms that it knows the
> > > > > branch?
> > > >
> > > > No, during a bisect operation 'git status' knows the branch where I
> > > > _was_ when I started bisecting, and where a 'git bisect reset' will
> > > > eventually bring me back when I'm finished, and that has no relation
> > > > whatsoever to the revision range that I'm bisecting.
> > > >
> > > > Consider this case:
> > > >
> > > >   $ git checkout --orphan unrelated-history
> > > >   Switched to a new branch 'unrelated-history'
> > > >   $ git commit -m "test"
> > > >   [unrelated-history (root-commit) 639b9d1047] test
> > > >   <...>
> > > >   $ git bisect start v2.25.0 v2.24.0
> > > >   Bisecting: 361 revisions left to test after this (roughly 9 steps)
> > > >   [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1
> > > >   $ git status
> > > >   HEAD detached at 7034cd094b
> > > >   You are currently bisecting, started from branch 'unrelated-history'.
> > > >     (use "git bisect reset" to get back to the original branch)
> > > >
> > > >   nothing to commit, working tree clean
> > > >
> > > > I can't possible be on branch 'unrelated-history' during that
> > > > bisection.
> > > >
> > > >
> > > > OTOH, while during a rebase we are technically on a detached HEAD as
> > > > well, that rebase operation is all about constructing the new history
> > > > of the rebased branch, and once finished that branch will be updated
> > > > to point to the tip of the new history, thus it will include all the
> > > > commits created while on the detached HEAD.  Therefore, it makes sense
> > > > conceptually to treat it as if we were on the rebased branch.  That's
> > > > why it makes sense to display the name of the rebased branch in the
> > > > Bash prompt, and that's why I think it makes sense to default to edit
> > > > the description of the rebased branch without explicitly naming it.
> > > >
> > > > With bisect that just doesn't make sense.
> > >
> > > If the range you are bisecting belongs or lead to the current branch,
> > > that still makes sense. And it's probably most of the time. So, I am
> > > not sure your objection is valid enough here.
> >
> > I'm not sure what you mean with "belongs or lead to" a branch.
> >
> > Do you mean that the range is reachable from the branch that just so
> > happened to be checked out when the bisection was started?  Well, I
> > have over 30 branches from where v2.25.0 is reachable, and all of them
> > are obviously bad candidates for editing their descriptions by default
> > while bisecting a totally unrelated issue.
> >
> 
> 
> If we take that simple example:
> 
> * (my-branch)
> *
> * bisect bad
> *
> * (HEAD)
> * bisect good
> *
> 
> It makes a lot of sense to me to edit my-branch description by
> default, even if the range good-bad happen to exist in other branches.

I still don't understand why it would make sense.

Furthermore, how do you think you could avoid choosing an obviously
bad branch to default to?

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[Marc-André Lureau]

Hi

On Fri, Jan 31, 2020 at 5:16 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
>
> On Fri, Jan 31, 2020 at 04:59:15PM +0100, Marc-André Lureau wrote:
> > Hi
> >
> > On Fri, Jan 31, 2020 at 4:52 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > >
> > > On Thu, Jan 30, 2020 at 10:37:38PM +0100, Marc-André Lureau wrote:
> > > > Hi
> > > >
> > > > On Fri, Jan 24, 2020 at 11:41 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > > > >
> > > > > On Sun, Jan 12, 2020 at 08:59:04PM -0500, Eric Sunshine wrote:
> > > > > > On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > > > > > > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:
> > > > > > > > Taking a deeper look at the code, I'm wondering it would make more
> > > > > > > > sense to call wt_status_get_state(), which handles 'rebase' and
> > > > > > > > 'bisect'. Is there a reason that you limited this check to only
> > > > > > > > 'rebase'?
> > > > > > >
> > > > > > > What branch name does wt_status_get_state() return while bisecting?
> > > > > > > The branch where I started from?  Because that's what 'git status'
> > > > > > > shows:
> > > > > > > But am I really on that branch?  Does it really makes sense to edit
> > > > > > > the description of 'mybranch' by default while bisecting through an
> > > > > > > old revision range?  I do not think so.
> > > > > >
> > > > > > It's not clear what downside you are pointing out; i.e. why would it
> > > > > > be a bad thing to be able to set the branch description even while
> > > > > > bisecting -- especially since `git status` affirms that it knows the
> > > > > > branch?
> > > > >
> > > > > No, during a bisect operation 'git status' knows the branch where I
> > > > > _was_ when I started bisecting, and where a 'git bisect reset' will
> > > > > eventually bring me back when I'm finished, and that has no relation
> > > > > whatsoever to the revision range that I'm bisecting.
> > > > >
> > > > > Consider this case:
> > > > >
> > > > >   $ git checkout --orphan unrelated-history
> > > > >   Switched to a new branch 'unrelated-history'
> > > > >   $ git commit -m "test"
> > > > >   [unrelated-history (root-commit) 639b9d1047] test
> > > > >   <...>
> > > > >   $ git bisect start v2.25.0 v2.24.0
> > > > >   Bisecting: 361 revisions left to test after this (roughly 9 steps)
> > > > >   [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1
> > > > >   $ git status
> > > > >   HEAD detached at 7034cd094b
> > > > >   You are currently bisecting, started from branch 'unrelated-history'.
> > > > >     (use "git bisect reset" to get back to the original branch)
> > > > >
> > > > >   nothing to commit, working tree clean
> > > > >
> > > > > I can't possible be on branch 'unrelated-history' during that
> > > > > bisection.
> > > > >
> > > > >
> > > > > OTOH, while during a rebase we are technically on a detached HEAD as
> > > > > well, that rebase operation is all about constructing the new history
> > > > > of the rebased branch, and once finished that branch will be updated
> > > > > to point to the tip of the new history, thus it will include all the
> > > > > commits created while on the detached HEAD.  Therefore, it makes sense
> > > > > conceptually to treat it as if we were on the rebased branch.  That's
> > > > > why it makes sense to display the name of the rebased branch in the
> > > > > Bash prompt, and that's why I think it makes sense to default to edit
> > > > > the description of the rebased branch without explicitly naming it.
> > > > >
> > > > > With bisect that just doesn't make sense.
> > > >
> > > > If the range you are bisecting belongs or lead to the current branch,
> > > > that still makes sense. And it's probably most of the time. So, I am
> > > > not sure your objection is valid enough here.
> > >
> > > I'm not sure what you mean with "belongs or lead to" a branch.
> > >
> > > Do you mean that the range is reachable from the branch that just so
> > > happened to be checked out when the bisection was started?  Well, I
> > > have over 30 branches from where v2.25.0 is reachable, and all of them
> > > are obviously bad candidates for editing their descriptions by default
> > > while bisecting a totally unrelated issue.
> > >
> >
> >
> > If we take that simple example:
> >
> > * (my-branch)
> > *
> > * bisect bad
> > *
> > * (HEAD)
> > * bisect good
> > *
> >
> > It makes a lot of sense to me to edit my-branch description by
> > default, even if the range good-bad happen to exist in other branches.
>
> I still don't understand why it would make sense.
>
> Furthermore, how do you think you could avoid choosing an obviously
> bad branch to default to?

It uses the same branch that git status displays. In your example:

You are currently bisecting, started from branch 'unrelated-history'.

So it's not completely off to pick that branch by default for
--edit-description.

But again, I think you are focusing on a rather rare case, please
consider the most common case.

-- 
Marc-André Lureau

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[SZEDER Gábor]

On Thu, Feb 06, 2020 at 11:26:56PM +0100, Marc-André Lureau wrote:
> > > > > > > > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:
> > > > > > > > > Taking a deeper look at the code, I'm wondering it would make more
> > > > > > > > > sense to call wt_status_get_state(), which handles 'rebase' and
> > > > > > > > > 'bisect'. Is there a reason that you limited this check to only
> > > > > > > > > 'rebase'?
> > > > > > > >
> > > > > > > > What branch name does wt_status_get_state() return while bisecting?
> > > > > > > > The branch where I started from?  Because that's what 'git status'
> > > > > > > > shows:
> > > > > > > > But am I really on that branch?  Does it really makes sense to edit
> > > > > > > > the description of 'mybranch' by default while bisecting through an
> > > > > > > > old revision range?  I do not think so.
> > > > > > >
> > > > > > > It's not clear what downside you are pointing out; i.e. why would it
> > > > > > > be a bad thing to be able to set the branch description even while
> > > > > > > bisecting -- especially since `git status` affirms that it knows the
> > > > > > > branch?
> > > > > >
> > > > > > No, during a bisect operation 'git status' knows the branch where I
> > > > > > _was_ when I started bisecting, and where a 'git bisect reset' will
> > > > > > eventually bring me back when I'm finished, and that has no relation
> > > > > > whatsoever to the revision range that I'm bisecting.
> > > > > >
> > > > > > Consider this case:
> > > > > >
> > > > > >   $ git checkout --orphan unrelated-history
> > > > > >   Switched to a new branch 'unrelated-history'
> > > > > >   $ git commit -m "test"
> > > > > >   [unrelated-history (root-commit) 639b9d1047] test
> > > > > >   <...>
> > > > > >   $ git bisect start v2.25.0 v2.24.0
> > > > > >   Bisecting: 361 revisions left to test after this (roughly 9 steps)
> > > > > >   [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1
> > > > > >   $ git status
> > > > > >   HEAD detached at 7034cd094b
> > > > > >   You are currently bisecting, started from branch 'unrelated-history'.
> > > > > >     (use "git bisect reset" to get back to the original branch)
> > > > > >
> > > > > >   nothing to commit, working tree clean
> > > > > >
> > > > > > I can't possible be on branch 'unrelated-history' during that
> > > > > > bisection.
> > > > > >
> > > > > >
> > > > > > OTOH, while during a rebase we are technically on a detached HEAD as
> > > > > > well, that rebase operation is all about constructing the new history
> > > > > > of the rebased branch, and once finished that branch will be updated
> > > > > > to point to the tip of the new history, thus it will include all the
> > > > > > commits created while on the detached HEAD.  Therefore, it makes sense
> > > > > > conceptually to treat it as if we were on the rebased branch.  That's
> > > > > > why it makes sense to display the name of the rebased branch in the
> > > > > > Bash prompt, and that's why I think it makes sense to default to edit
> > > > > > the description of the rebased branch without explicitly naming it.
> > > > > >
> > > > > > With bisect that just doesn't make sense.
> > > > >
> > > > > If the range you are bisecting belongs or lead to the current branch,
> > > > > that still makes sense. And it's probably most of the time. So, I am
> > > > > not sure your objection is valid enough here.
> > > >
> > > > I'm not sure what you mean with "belongs or lead to" a branch.
> > > >
> > > > Do you mean that the range is reachable from the branch that just so
> > > > happened to be checked out when the bisection was started?  Well, I
> > > > have over 30 branches from where v2.25.0 is reachable, and all of them
> > > > are obviously bad candidates for editing their descriptions by default
> > > > while bisecting a totally unrelated issue.
> > > >
> > >
> > >
> > > If we take that simple example:
> > >
> > > * (my-branch)
> > > *
> > > * bisect bad
> > > *
> > > * (HEAD)
> > > * bisect good
> > > *
> > >
> > > It makes a lot of sense to me to edit my-branch description by
> > > default, even if the range good-bad happen to exist in other branches.
> >
> > I still don't understand why it would make sense.
> >
> > Furthermore, how do you think you could avoid choosing an obviously
> > bad branch to default to?
> 
> It uses the same branch that git status displays. In your example:
> 
> You are currently bisecting, started from branch 'unrelated-history'.

Yes, that's the problem: it shows "started from branch", not "On
branch".  Conceptually a huge difference.

> So it's not completely off to pick that branch by default for
> --edit-description.
> 
> But again, I think you are focusing on a rather rare case, please
> consider the most common case.

I do focus on the most common case: I'm on branch 'foo' built on top
of current master, when a bugreport comes in, and I start bisecting on
the range v2.12.0..v2.16.0.  It can not possibly be considered that
during the bisect I'm on the branch 'foo' during the bisection, they
are totally unrelated.

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[Marc-André Lureau]

Hi

On Fri, Feb 7, 2020 at 11:02 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
>
> On Thu, Feb 06, 2020 at 11:26:56PM +0100, Marc-André Lureau wrote:
> > > > > > > > > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:
> > > > > > > > > > Taking a deeper look at the code, I'm wondering it would make more
> > > > > > > > > > sense to call wt_status_get_state(), which handles 'rebase' and
> > > > > > > > > > 'bisect'. Is there a reason that you limited this check to only
> > > > > > > > > > 'rebase'?
> > > > > > > > >
> > > > > > > > > What branch name does wt_status_get_state() return while bisecting?
> > > > > > > > > The branch where I started from?  Because that's what 'git status'
> > > > > > > > > shows:
> > > > > > > > > But am I really on that branch?  Does it really makes sense to edit
> > > > > > > > > the description of 'mybranch' by default while bisecting through an
> > > > > > > > > old revision range?  I do not think so.
> > > > > > > >
> > > > > > > > It's not clear what downside you are pointing out; i.e. why would it
> > > > > > > > be a bad thing to be able to set the branch description even while
> > > > > > > > bisecting -- especially since `git status` affirms that it knows the
> > > > > > > > branch?
> > > > > > >
> > > > > > > No, during a bisect operation 'git status' knows the branch where I
> > > > > > > _was_ when I started bisecting, and where a 'git bisect reset' will
> > > > > > > eventually bring me back when I'm finished, and that has no relation
> > > > > > > whatsoever to the revision range that I'm bisecting.
> > > > > > >
> > > > > > > Consider this case:
> > > > > > >
> > > > > > >   $ git checkout --orphan unrelated-history
> > > > > > >   Switched to a new branch 'unrelated-history'
> > > > > > >   $ git commit -m "test"
> > > > > > >   [unrelated-history (root-commit) 639b9d1047] test
> > > > > > >   <...>
> > > > > > >   $ git bisect start v2.25.0 v2.24.0
> > > > > > >   Bisecting: 361 revisions left to test after this (roughly 9 steps)
> > > > > > >   [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1
> > > > > > >   $ git status
> > > > > > >   HEAD detached at 7034cd094b
> > > > > > >   You are currently bisecting, started from branch 'unrelated-history'.
> > > > > > >     (use "git bisect reset" to get back to the original branch)
> > > > > > >
> > > > > > >   nothing to commit, working tree clean
> > > > > > >
> > > > > > > I can't possible be on branch 'unrelated-history' during that
> > > > > > > bisection.
> > > > > > >
> > > > > > >
> > > > > > > OTOH, while during a rebase we are technically on a detached HEAD as
> > > > > > > well, that rebase operation is all about constructing the new history
> > > > > > > of the rebased branch, and once finished that branch will be updated
> > > > > > > to point to the tip of the new history, thus it will include all the
> > > > > > > commits created while on the detached HEAD.  Therefore, it makes sense
> > > > > > > conceptually to treat it as if we were on the rebased branch.  That's
> > > > > > > why it makes sense to display the name of the rebased branch in the
> > > > > > > Bash prompt, and that's why I think it makes sense to default to edit
> > > > > > > the description of the rebased branch without explicitly naming it.
> > > > > > >
> > > > > > > With bisect that just doesn't make sense.
> > > > > >
> > > > > > If the range you are bisecting belongs or lead to the current branch,
> > > > > > that still makes sense. And it's probably most of the time. So, I am
> > > > > > not sure your objection is valid enough here.
> > > > >
> > > > > I'm not sure what you mean with "belongs or lead to" a branch.
> > > > >
> > > > > Do you mean that the range is reachable from the branch that just so
> > > > > happened to be checked out when the bisection was started?  Well, I
> > > > > have over 30 branches from where v2.25.0 is reachable, and all of them
> > > > > are obviously bad candidates for editing their descriptions by default
> > > > > while bisecting a totally unrelated issue.
> > > > >
> > > >
> > > >
> > > > If we take that simple example:
> > > >
> > > > * (my-branch)
> > > > *
> > > > * bisect bad
> > > > *
> > > > * (HEAD)
> > > > * bisect good
> > > > *
> > > >
> > > > It makes a lot of sense to me to edit my-branch description by
> > > > default, even if the range good-bad happen to exist in other branches.
> > >
> > > I still don't understand why it would make sense.
> > >
> > > Furthermore, how do you think you could avoid choosing an obviously
> > > bad branch to default to?
> >
> > It uses the same branch that git status displays. In your example:
> >
> > You are currently bisecting, started from branch 'unrelated-history'.
>
> Yes, that's the problem: it shows "started from branch", not "On
> branch".  Conceptually a huge difference.
>
> > So it's not completely off to pick that branch by default for
> > --edit-description.
> >
> > But again, I think you are focusing on a rather rare case, please
> > consider the most common case.
>
> I do focus on the most common case: I'm on branch 'foo' built on top
> of current master, when a bugreport comes in, and I start bisecting on
> the range v2.12.0..v2.16.0.  It can not possibly be considered that
> during the bisect I'm on the branch 'foo' during the bisection, they
> are totally unrelated.


And usually that bisection is ancestry of master, rarely unrelated.

Also, when doing --edit-description there are comments like:

# Please edit the description for the branch
#   unrelated-history

What else do you suggest?


-- 
Marc-André Lureau

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[Junio C Hamano]

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Also, when doing --edit-description there are comments like:
>
> # Please edit the description for the branch
> #   unrelated-history
>
> What else do you suggest?

How about teaching "git branch --edit-description [HEAD]" notice
when/if HEAD is detached and always error out, no matter what
operation is in progress?

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[Marc-André Lureau]

Hi

On Fri, Feb 7, 2020 at 7:57 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Marc-André Lureau <marcandre.lureau@gmail.com> writes:
>
> > Also, when doing --edit-description there are comments like:
> >
> > # Please edit the description for the branch
> > #   unrelated-history
> >
> > What else do you suggest?
>
> How about teaching "git branch --edit-description [HEAD]" notice
> when/if HEAD is detached and always error out, no matter what
> operation is in progress?

Then --edit-description during won't default to the branch you started
from the bisect. So we are back to my original proposal only, having
branch default during rebase. Eric, do you mind?


-- 
Marc-André Lureau

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[Junio C Hamano]

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Fri, Feb 7, 2020 at 7:57 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Marc-André Lureau <marcandre.lureau@gmail.com> writes:
>>
>> > Also, when doing --edit-description there are comments like:
>> >
>> > # Please edit the description for the branch
>> > #   unrelated-history
>> >
>> > What else do you suggest?
>>
>> How about teaching "git branch --edit-description [HEAD]" notice
>> when/if HEAD is detached and always error out, no matter what
>> operation is in progress?
>
> Then --edit-description during won't default to the branch you started
> from the bisect. So we are back to my original proposal only, having
> branch default during rebase. Eric, do you mind?

What I meant by "no matter what is in progress" is not to special
case "during rebase", either.

Sorry for the confusion.

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[Eric Sunshine]

On Fri, Feb 7, 2020 at 2:12 PM Junio C Hamano <gitster@pobox.com> wrote:
> Marc-André Lureau <marcandre.lureau@gmail.com> writes:
> > Then --edit-description during won't default to the branch you started
> > from the bisect. So we are back to my original proposal only, having
> > branch default during rebase. Eric, do you mind?

I don't feel strongly one way or the other.

> > > How about teaching "git branch --edit-description [HEAD]" notice
> > > when/if HEAD is detached and always error out, no matter what
> > > operation is in progress?
>
> What I meant by "no matter what is in progress" is not to special
> case "during rebase", either.

That would defeat the original purpose[1] of this submission, I think.
As I understand it, the idea all along was to make this operation work
during a rebase.

[1]: https://lore.kernel.org/git/20200110071929.119000-1-marcandre.lureau@redhat.com/

Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

>> > > How about teaching "git branch --edit-description [HEAD]" notice
>> > > when/if HEAD is detached and always error out, no matter what
>> > > operation is in progress?
>>
>> What I meant by "no matter what is in progress" is not to special
>> case "during rebase", either.
>
> That would defeat the original purpose[1] of this submission, I think.
> As I understand it, the idea all along was to make this operation work
> during a rebase.

I know.  But I do not think it is a good thing to begin with.

While you are rebasing the branch X and get control back before
rebase finishes, you are *not* on branch X.  You are *preparing* a
new version of the history leading to the tip of branch X, in the
hope that once you are done, you would make that new version of the
history the history of branch X.  Until that happens, you are not on
branch X.

If you were on branch X, then "git checkout -m another-branch"
followed by some other operations, and then finally coming back with
"git checkout -m X" would work.  But it would not, because you are
not on branch X.

After all, you may well say "git rebase --abort" before you are
done.  Would "edit description" you do in the middle be reverted
if you did so?

It is bad for the user to blur the distinction between "detached and
not on X but preparing to update X" and "working on X to advance X",
and I think the original patch that started the thread takes us in
that direction.

Thanks.