{"thread":{"id":"52615","subject":"[PATCH] branch: let '--edit-description' default to rebased branch during rebase","startedAt":"2020-01-11T12:35:48Z","lastAt":"2020-02-07T20:14:36Z","messageCount":20,"participants":["marcandre.lureau@redhat.com","Eric Sunshine","Marc-André Lureau","SZEDER Gábor","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"389608","messageId":"20200111123533.1613844-1-marcandre.lureau@redhat.com","threadId":"52615","inReplyTo":null,"subject":"[PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"","fromEmail":"marcandre.lureau@redhat.com","sentAt":"2020-01-11T12:35:33Z","receivedAt":"2020-01-11T12:35:48Z","isPatch":true,"sender":{"key":"marcandre.lureau@redhat.com","avatar":null},"body":"From: Marc-André Lureau \n\nDefaulting to editing the description of the rebased branch without an\nexplicit branchname argument would be useful. Even the git bash prompt\nshows the name of the rebased branch, and then\n\n ~/src/git (mybranch|REBASE-i 1/2)$ git branch --edit-description\n fatal: Cannot give description to detached HEAD\n\nlooks quite unhelpful.\n\nSigned-off-by: Marc-André Lureau \n---\nChanged in v2:\n - add a test\n - fix commit message & summary\n - simplify code\n\nbuiltin/branch.c | 24 +++++++++++++++++++-----\n t/t3200-branch.sh | 19 +++++++++++++++++++\n 2 files changed, 38 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex d8297f80ff..ee82dc828e 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -745,15 +745,27 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tstring_list_clear(&output, 0);\n \t\treturn 0;\n \t} else if (edit_description) {\n-\t\tconst char *branch_name;\n+\t\tchar *branch_name = NULL;\n \t\tstruct strbuf branch_ref = STRBUF_INIT;\n \n \t\tif (!argc) {\n-\t\t\tif (filter.detached)\n-\t\t\t\tdie(_(\"Cannot give description to detached HEAD\"));\n-\t\t\tbranch_name = head;\n+\t\t\tif (filter.detached) {\n+\t\t\t\tstruct wt_status_state state;\n+\n+\t\t\t\tmemset(&state, 0, sizeof(state));\n+\n+\t\t\t\tif (wt_status_check_rebase(NULL, &state)) {\n+\t\t\t\t\tbranch_name = state.branch;\n+\t\t\t\t}\n+\n+\t\t\t\tif (!branch_name)\n+\t\t\t\t\tdie(_(\"Cannot give description to detached HEAD\"));\n+\n+\t\t\t\tfree(state.onto);\n+\t\t\t} else\n+\t\t\t\tbranch_name = xstrdup(head);\n \t\t} else if (argc == 1)\n-\t\t\tbranch_name = argv[0];\n+\t\t\tbranch_name = xstrdup(argv[0]);\n \t\telse\n \t\t\tdie(_(\"cannot edit description of more than one branch\"));\n \n@@ -772,6 +784,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \n \t\tif (edit_branch_description(branch_name))\n \t\t\treturn 1;\n+\n+\t\tfree(branch_name);\n \t} else if (copy) {\n \t\tif (!argc)\n \t\t\tdie(_(\"branch name required\"));\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 411a70b0ce..a20ebedea0 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -1260,6 +1260,25 @@ test_expect_success 'use --edit-description' '\n \ttest_cmp expect EDITOR_OUTPUT\n '\n \n+test_expect_success 'use --edit-description during rebase' '\n+\twrite_script editor <<-\\EOF &&\n+\t\techo \"Rebase contents\" >\"$1\"\n+\tEOF\n+\t(\n+\t\tset_fake_editor &&\n+\t\tFAKE_LINES=\"break 1\" git rebase -i HEAD^ &&\n+\t\tEDITOR=./editor git branch --edit-description &&\n+\t\tgit rebase --continue\n+\t) &&\n+\twrite_script editor <<-\\EOF &&\n+\t\tgit stripspace -s <\"$1\" >\"EDITOR_OUTPUT\"\n+\tEOF\n+\tEDITOR=./editor git branch --edit-description &&\n+\techo \"Rebase contents\" >expect &&\n+\ttest_cmp expect EDITOR_OUTPUT\n+'\n+test_done\n+\n test_expect_success 'detect typo in branch name when using --edit-description' '\n \twrite_script editor <<-\\EOF &&\n \t\techo \"New contents\" >\"$1\"\n\nbase-commit: 7a6a90c6ec48fc78c83d7090d6c1b95d8f3739c0\n-- \n2.25.0.rc2.2.g5aece98438\n\n"},{"id":"389609","messageId":"CAPig+cQXkiFOz5HczPEgXuSOH_3KsCwXwVwe0qvQzLDtFgnAXw@mail.gmail.com","threadId":"52615","inReplyTo":"20200111123533.1613844-1-marcandre.lureau@redhat.com","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-01-11T13:26:53Z","receivedAt":"2020-01-11T13:27:08Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jan 11, 2020 at 7:36 AM wrote:\n> Defaulting to editing the description of the rebased branch without an\n> explicit branchname argument would be useful. Even the git bash prompt\n> shows the name of the rebased branch, and then\n>\n> ~/src/git (mybranch|REBASE-i 1/2)$ git branch --edit-description\n> fatal: Cannot give description to detached HEAD\n>\n> looks quite unhelpful.\n>\n> Signed-off-by: Marc-André Lureau \n> ---\n> diff --git a/builtin/branch.c b/builtin/branch.c\n> @@ -745,15 +745,27 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> if (!argc) {\n> - if (filter.detached)\n> - die(_(\"Cannot give description to detached HEAD\"));\n> - branch_name = head;\n> + if (filter.detached) {\n> + struct wt_status_state state;\n> +\n> + memset(&state, 0, sizeof(state));\n> +\n> + if (wt_status_check_rebase(NULL, &state)) {\n> + branch_name = state.branch;\n> + }\n\nStyle: drop unneeded braces.\n\n> +\n> + if (!branch_name)\n> + die(_(\"Cannot give description to detached HEAD\"));\n> +\n> + free(state.onto);\n\nAlso, no need for all the blank lines which eat up valuable vertical\nscreen real-estate without making the code clearer.\n\n> + } else\n> + branch_name = xstrdup(head);\n\nIt would be easier to see what happens in the common case (when not\nrebasing) if you invert the condition to `if (!filter.detached)` and\nturn this one-line 'else' branch into the 'if' branch.\n\n> @@ -772,6 +784,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> if (edit_branch_description(branch_name))\n> return 1;\n> +\n> + free(branch_name);\n\nThat `return 1` just above this free() is leaking 'branch_name', isn't it?\n\n> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> @@ -1260,6 +1260,25 @@ test_expect_success 'use --edit-description' '\n> +test_expect_success 'use --edit-description during rebase' '\n> + write_script editor <<-\\EOF &&\n> + echo \"Rebase contents\" >\"$1\"\n> + EOF\n> + (\n> + set_fake_editor &&\n> + FAKE_LINES=\"break 1\" git rebase -i HEAD^ &&\n> + EDITOR=./editor git branch --edit-description &&\n> + git rebase --continue\n> + ) &&\n> + write_script editor <<-\\EOF &&\n> + git stripspace -s <\"$1\" >\"EDITOR_OUTPUT\"\n> + EOF\n> + EDITOR=./editor git branch --edit-description &&\n> + echo \"Rebase contents\" >expect &&\n> + test_cmp expect EDITOR_OUTPUT\n> +'\n> +test_done\n\nStrange place for a test_done() invocation considering that existing\ntests follow the new one added by this patch.\n\n> test_expect_success 'detect typo in branch name when using --edit-description' '\n> write_script editor <<-\\EOF &&\n> echo \"New contents\" >\"$1\"\n"},{"id":"389612","messageId":"CAJ+F1CKW3NACgPdPbmAzYGVwR4iO3r+LCNq+g5st0gcz4X+fzA@mail.gmail.com","threadId":"52615","inReplyTo":"CAPig+cQXkiFOz5HczPEgXuSOH_3KsCwXwVwe0qvQzLDtFgnAXw@mail.gmail.com","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"Marc-André Lureau","fromEmail":"marcandre.lureau@gmail.com","sentAt":"2020-01-11T14:54:21Z","receivedAt":"2020-01-11T14:54:37Z","isPatch":true,"sender":{"key":"marcandre.lureau@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9381?v=4"},"body":"Hi\n\nOn Sat, Jan 11, 2020 at 5:28 PM Eric Sunshine wrote:\n>\n> On Sat, Jan 11, 2020 at 7:36 AM wrote:\n> > Defaulting to editing the description of the rebased branch without an\n> > explicit branchname argument would be useful. Even the git bash prompt\n> > shows the name of the rebased branch, and then\n> >\n> > ~/src/git (mybranch|REBASE-i 1/2)$ git branch --edit-description\n> > fatal: Cannot give description to detached HEAD\n> >\n> > looks quite unhelpful.\n> >\n> > Signed-off-by: Marc-André Lureau \n> > ---\n> > diff --git a/builtin/branch.c b/builtin/branch.c\n> > @@ -745,15 +745,27 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> > if (!argc) {\n> > - if (filter.detached)\n> > - die(_(\"Cannot give description to detached HEAD\"));\n> > - branch_name = head;\n> > + if (filter.detached) {\n> > + struct wt_status_state state;\n> > +\n> > + memset(&state, 0, sizeof(state));\n> > +\n> > + if (wt_status_check_rebase(NULL, &state)) {\n> > + branch_name = state.branch;\n> > + }\n>\n> Style: drop unneeded braces.\n\nok\n\n>\n> > +\n> > + if (!branch_name)\n> > + die(_(\"Cannot give description to detached HEAD\"));\n> > +\n> > + free(state.onto);\n>\n> Also, no need for all the blank lines which eat up valuable vertical\n> screen real-estate without making the code clearer.\n\nok\n\n>\n> > + } else\n> > + branch_name = xstrdup(head);\n>\n> It would be easier to see what happens in the common case (when not\n> rebasing) if you invert the condition to `if (!filter.detached)` and\n> turn this one-line 'else' branch into the 'if' branch.\n\nindeed\n\n>\n> > @@ -772,6 +784,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n> > if (edit_branch_description(branch_name))\n> > return 1;\n> > +\n> > + free(branch_name);\n>\n> That `return 1` just above this free() is leaking 'branch_name', isn't it?\n\nright, let's fix that too\n\n>\n> > diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> > @@ -1260,6 +1260,25 @@ test_expect_success 'use --edit-description' '\n> > +test_expect_success 'use --edit-description during rebase' '\n> > + write_script editor <<-\\EOF &&\n> > + echo \"Rebase contents\" >\"$1\"\n> > + EOF\n> > + (\n> > + set_fake_editor &&\n> > + FAKE_LINES=\"break 1\" git rebase -i HEAD^ &&\n> > + EDITOR=./editor git branch --edit-description &&\n> > + git rebase --continue\n> > + ) &&\n> > + write_script editor <<-\\EOF &&\n> > + git stripspace -s <\"$1\" >\"EDITOR_OUTPUT\"\n> > + EOF\n> > + EDITOR=./editor git branch --edit-description &&\n> > + echo \"Rebase contents\" >expect &&\n> > + test_cmp expect EDITOR_OUTPUT\n> > +'\n> > +test_done\n>\n> Strange place for a test_done() invocation considering that existing\n> tests follow the new one added by this patch.\n\ndoh, sorry\nthanks for the review!\n\n>\n> > test_expect_success 'detect typo in branch name when using --edit-description' '\n> > write_script editor <<-\\EOF &&\n> > echo \"New contents\" >\"$1\"\n\n\n\n-- \nMarc-André Lureau\n"},{"id":"389621","messageId":"CAPig+cRCMXjjPHc2O8fLmaSm9m-ZO3qR2BoZwG3s5dLHNbiFFQ@mail.gmail.com","threadId":"52615","inReplyTo":"CAJ+F1CKW3NACgPdPbmAzYGVwR4iO3r+LCNq+g5st0gcz4X+fzA@mail.gmail.com","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-01-12T01:27:11Z","receivedAt":"2020-01-12T01:27:27Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jan 11, 2020 at 9:55 AM Marc-André Lureau\n wrote:\n> On Sat, Jan 11, 2020 at 5:28 PM Eric Sunshine wrote:\n> > On Sat, Jan 11, 2020 at 7:36 AM wrote:\n> > > + if (wt_status_check_rebase(NULL, &state)) {\n> > > + branch_name = state.branch;\n> > > + }\n\nTaking a deeper look at the code, I'm wondering it would make more\nsense to call wt_status_get_state(), which handles 'rebase' and\n'bisect'. Is there a reason that you limited this check to only\n'rebase'?\n\n> > > if (edit_branch_description(branch_name))\n> > > return 1;\n> > > +\n> > > + free(branch_name);\n> >\n> > That `return 1` just above this free() is leaking 'branch_name', isn't it?\n>\n> right, let's fix that too\n\nLooking at the code itself (rather than consulting only the patch), I\nsee that there are a couple more early returns leaking 'branch_name',\nso they need to be handled, as well.\n"},{"id":"389625","messageId":"CAJ+F1CJP88PXP0vLtXQd82Z3RmX0uGic2NxBy3iSh0nBnRG0Vg@mail.gmail.com","threadId":"52615","inReplyTo":"CAPig+cRCMXjjPHc2O8fLmaSm9m-ZO3qR2BoZwG3s5dLHNbiFFQ@mail.gmail.com","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"Marc-André Lureau","fromEmail":"marcandre.lureau@gmail.com","sentAt":"2020-01-12T06:44:01Z","receivedAt":"2020-01-12T06:44:16Z","isPatch":true,"sender":{"key":"marcandre.lureau@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9381?v=4"},"body":"Hi Eric\n\nOn Sun, Jan 12, 2020 at 5:27 AM Eric Sunshine wrote:\n>\n> On Sat, Jan 11, 2020 at 9:55 AM Marc-André Lureau\n> wrote:\n> > On Sat, Jan 11, 2020 at 5:28 PM Eric Sunshine wrote:\n> > > On Sat, Jan 11, 2020 at 7:36 AM wrote:\n> > > > + if (wt_status_check_rebase(NULL, &state)) {\n> > > > + branch_name = state.branch;\n> > > > + }\n>\n> Taking a deeper look at the code, I'm wondering it would make more\n> sense to call wt_status_get_state(), which handles 'rebase' and\n> 'bisect'. Is there a reason that you limited this check to only\n> 'rebase'?\n\nNo reason, I just didn't try it yet. Done, thanks\n\n>\n> > > > if (edit_branch_description(branch_name))\n> > > > return 1;\n> > > > +\n> > > > + free(branch_name);\n> > >\n> > > That `return 1` just above this free() is leaking 'branch_name', isn't it?\n> >\n> > right, let's fix that too\n>\n> Looking at the code itself (rather than consulting only the patch), I\n> see that there are a couple more early returns leaking 'branch_name',\n> so they need to be handled, as well.\n\nI think I covered them now, sending v4.\n\nthanks\n\n-- \nMarc-André Lureau\n"},{"id":"389631","messageId":"20200112121402.GH32750@szeder.dev","threadId":"52615","inReplyTo":"CAPig+cRCMXjjPHc2O8fLmaSm9m-ZO3qR2BoZwG3s5dLHNbiFFQ@mail.gmail.com","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-01-12T12:14:03Z","receivedAt":"2020-01-12T12:14:09Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:\n> On Sat, Jan 11, 2020 at 9:55 AM Marc-André Lureau\n> wrote:\n> > On Sat, Jan 11, 2020 at 5:28 PM Eric Sunshine wrote:\n> > > On Sat, Jan 11, 2020 at 7:36 AM wrote:\n> > > > + if (wt_status_check_rebase(NULL, &state)) {\n> > > > + branch_name = state.branch;\n> > > > + }\n> \n> Taking a deeper look at the code, I'm wondering it would make more\n> sense to call wt_status_get_state(), which handles 'rebase' and\n> 'bisect'. Is there a reason that you limited this check to only\n> 'rebase'?\n\nWhile I do think that defaulting to edit the description of the\nrebased branch makes sense, I'm not sure how that would work with\nbisect.\n\nWhat branch name does wt_status_get_state() return while bisecting?\nThe branch where I started from? Because that's what 'git status'\nshows:\n\n ~/src/git (mybranch)$ git bisect start v2.21.0 v2.20.0\n Bisecting: 334 revisions left to test after this (roughly 8 steps)\n [b99a579f8e434a7757f90895945b5711b3f159d5] Merge branch 'sb/more-repo-in-api'\n ~/src/git ((b99a579f8e...)|BISECTING)$ git status \n HEAD detached at b99a579f8e\n You are currently bisecting, started from branch 'mybranch'.\n (use \"git bisect reset\" to get back to the original branch)\n \n nothing to commit, working tree clean\n\nBut am I really on that branch? Does it really makes sense to edit\nthe description of 'mybranch' by default while bisecting through an\nold revision range? I do not think so.\n\n> > > > if (edit_branch_description(branch_name))\n> > > > return 1;\n> > > > +\n> > > > + free(branch_name);\n> > >\n> > > That `return 1` just above this free() is leaking 'branch_name', isn't it?\n> >\n> > right, let's fix that too\n> \n> Looking at the code itself (rather than consulting only the patch), I\n> see that there are a couple more early returns leaking 'branch_name',\n> so they need to be handled, as well.\n\n'git branch --edit-description' is a one-shot operation: it allows to\nedit only one branch description per invocation, and then the process\nexits right away, whether the operation was successful or some error\noccurred. I'm not sure free()ing 'branch_name' is worth the effort\n(and even if it does, I think it should be a separate preparatory\npatch).\n"},{"id":"389648","messageId":"CAPig+cRvYzm8Cb-AWqOeANRziWyjhWXT32QJ6TsA1==8Joa4zQ@mail.gmail.com","threadId":"52615","inReplyTo":"20200112121402.GH32750@szeder.dev","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-01-13T01:59:04Z","receivedAt":"2020-01-13T01:59:19Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor wrote:\n> On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:\n> > Taking a deeper look at the code, I'm wondering it would make more\n> > sense to call wt_status_get_state(), which handles 'rebase' and\n> > 'bisect'. Is there a reason that you limited this check to only\n> > 'rebase'?\n>\n> What branch name does wt_status_get_state() return while bisecting?\n> The branch where I started from? Because that's what 'git status'\n> shows:\n> But am I really on that branch? Does it really makes sense to edit\n> the description of 'mybranch' by default while bisecting through an\n> old revision range? I do not think so.\n\nIt's not clear what downside you are pointing out; i.e. why would it\nbe a bad thing to be able to set the branch description even while\nbisecting -- especially since `git status` affirms that it knows the\nbranch?\n\n> > Looking at the code itself (rather than consulting only the patch), I\n> > see that there are a couple more early returns leaking 'branch_name',\n> > so they need to be handled, as well.\n>\n> 'git branch --edit-description' is a one-shot operation: it allows to\n> edit only one branch description per invocation, and then the process\n> exits right away, whether the operation was successful or some error\n> occurred.\n\nIt is one-shot, but the existing `--edit-description` code already\ncleans up after itself by releasing resources it allocated (as do\nother one-shot parts of cmd_branch()), so it would be odd and\ninconsistent for this new code to not clean up after itself, as well\n(or, more accurately, to only clean up after itself in some branches\nbut not others).\n\n> I'm not sure free()ing 'branch_name' is worth the effort\n> (and even if it does, I think it should be a separate preparatory\n> patch).\n\nA separate preparatory patch doesn't make sense in this case since\n'branch_name' becomes \"freeable\" with this patch itself (prior to\nthat, it was `const char *`).\n\nAnyhow, a different approach was later proposed[1] which eliminates\nsome of the ugliness.\n\n[1]: https://lore.kernel.org/git/20200112101735.GA19676@flurp.local/\n"},{"id":"390422","messageId":"20200124224113.GJ6837@szeder.dev","threadId":"52615","inReplyTo":"CAPig+cRvYzm8Cb-AWqOeANRziWyjhWXT32QJ6TsA1==8Joa4zQ@mail.gmail.com","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-01-24T22:41:13Z","receivedAt":"2020-01-24T22:41:20Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sun, Jan 12, 2020 at 08:59:04PM -0500, Eric Sunshine wrote:\n> On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor wrote:\n> > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:\n> > > Taking a deeper look at the code, I'm wondering it would make more\n> > > sense to call wt_status_get_state(), which handles 'rebase' and\n> > > 'bisect'. Is there a reason that you limited this check to only\n> > > 'rebase'?\n> >\n> > What branch name does wt_status_get_state() return while bisecting?\n> > The branch where I started from? Because that's what 'git status'\n> > shows:\n> > But am I really on that branch? Does it really makes sense to edit\n> > the description of 'mybranch' by default while bisecting through an\n> > old revision range? I do not think so.\n> \n> It's not clear what downside you are pointing out; i.e. why would it\n> be a bad thing to be able to set the branch description even while\n> bisecting -- especially since `git status` affirms that it knows the\n> branch?\n\nNo, during a bisect operation 'git status' knows the branch where I\n_was_ when I started bisecting, and where a 'git bisect reset' will\neventually bring me back when I'm finished, and that has no relation\nwhatsoever to the revision range that I'm bisecting.\n\nConsider this case:\n\n $ git checkout --orphan unrelated-history\n Switched to a new branch 'unrelated-history'\n $ git commit -m \"test\"\n [unrelated-history (root-commit) 639b9d1047] test\n <...>\n $ git bisect start v2.25.0 v2.24.0\n Bisecting: 361 revisions left to test after this (roughly 9 steps)\n [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1\n $ git status \n HEAD detached at 7034cd094b\n You are currently bisecting, started from branch 'unrelated-history'.\n (use \"git bisect reset\" to get back to the original branch)\n \n nothing to commit, working tree clean\n\nI can't possible be on branch 'unrelated-history' during that\nbisection.\n\n\nOTOH, while during a rebase we are technically on a detached HEAD as\nwell, that rebase operation is all about constructing the new history\nof the rebased branch, and once finished that branch will be updated\nto point to the tip of the new history, thus it will include all the\ncommits created while on the detached HEAD. Therefore, it makes sense\nconceptually to treat it as if we were on the rebased branch. That's\nwhy it makes sense to display the name of the rebased branch in the\nBash prompt, and that's why I think it makes sense to default to edit\nthe description of the rebased branch without explicitly naming it.\n\nWith bisect that just doesn't make sense.\n\n"},{"id":"390846","messageId":"CAJ+F1CL7RD2Rxaskk47f_UCQLP6yaM_woxTb1pag-ejqP9prBg@mail.gmail.com","threadId":"52615","inReplyTo":"20200124224113.GJ6837@szeder.dev","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"Marc-André Lureau","fromEmail":"marcandre.lureau@gmail.com","sentAt":"2020-01-30T21:37:38Z","receivedAt":"2020-01-30T21:37:54Z","isPatch":true,"sender":{"key":"marcandre.lureau@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9381?v=4"},"body":"Hi\n\nOn Fri, Jan 24, 2020 at 11:41 PM SZEDER Gábor wrote:\n>\n> On Sun, Jan 12, 2020 at 08:59:04PM -0500, Eric Sunshine wrote:\n> > On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor wrote:\n> > > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:\n> > > > Taking a deeper look at the code, I'm wondering it would make more\n> > > > sense to call wt_status_get_state(), which handles 'rebase' and\n> > > > 'bisect'. Is there a reason that you limited this check to only\n> > > > 'rebase'?\n> > >\n> > > What branch name does wt_status_get_state() return while bisecting?\n> > > The branch where I started from? Because that's what 'git status'\n> > > shows:\n> > > But am I really on that branch? Does it really makes sense to edit\n> > > the description of 'mybranch' by default while bisecting through an\n> > > old revision range? I do not think so.\n> >\n> > It's not clear what downside you are pointing out; i.e. why would it\n> > be a bad thing to be able to set the branch description even while\n> > bisecting -- especially since `git status` affirms that it knows the\n> > branch?\n>\n> No, during a bisect operation 'git status' knows the branch where I\n> _was_ when I started bisecting, and where a 'git bisect reset' will\n> eventually bring me back when I'm finished, and that has no relation\n> whatsoever to the revision range that I'm bisecting.\n>\n> Consider this case:\n>\n> $ git checkout --orphan unrelated-history\n> Switched to a new branch 'unrelated-history'\n> $ git commit -m \"test\"\n> [unrelated-history (root-commit) 639b9d1047] test\n> <...>\n> $ git bisect start v2.25.0 v2.24.0\n> Bisecting: 361 revisions left to test after this (roughly 9 steps)\n> [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1\n> $ git status\n> HEAD detached at 7034cd094b\n> You are currently bisecting, started from branch 'unrelated-history'.\n> (use \"git bisect reset\" to get back to the original branch)\n>\n> nothing to commit, working tree clean\n>\n> I can't possible be on branch 'unrelated-history' during that\n> bisection.\n>\n>\n> OTOH, while during a rebase we are technically on a detached HEAD as\n> well, that rebase operation is all about constructing the new history\n> of the rebased branch, and once finished that branch will be updated\n> to point to the tip of the new history, thus it will include all the\n> commits created while on the detached HEAD. Therefore, it makes sense\n> conceptually to treat it as if we were on the rebased branch. That's\n> why it makes sense to display the name of the rebased branch in the\n> Bash prompt, and that's why I think it makes sense to default to edit\n> the description of the rebased branch without explicitly naming it.\n>\n> With bisect that just doesn't make sense.\n\nIf the range you are bisecting belongs or lead to the current branch,\nthat still makes sense. And it's probably most of the time. So, I am\nnot sure your objection is valid enough here.\n\n\n\n-- \nMarc-André Lureau\n"},{"id":"390911","messageId":"20200131155228.GF10482@szeder.dev","threadId":"52615","inReplyTo":"CAJ+F1CL7RD2Rxaskk47f_UCQLP6yaM_woxTb1pag-ejqP9prBg@mail.gmail.com","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-01-31T15:52:28Z","receivedAt":"2020-01-31T15:52:35Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Jan 30, 2020 at 10:37:38PM +0100, Marc-André Lureau wrote:\n> Hi\n> \n> On Fri, Jan 24, 2020 at 11:41 PM SZEDER Gábor wrote:\n> >\n> > On Sun, Jan 12, 2020 at 08:59:04PM -0500, Eric Sunshine wrote:\n> > > On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor wrote:\n> > > > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:\n> > > > > Taking a deeper look at the code, I'm wondering it would make more\n> > > > > sense to call wt_status_get_state(), which handles 'rebase' and\n> > > > > 'bisect'. Is there a reason that you limited this check to only\n> > > > > 'rebase'?\n> > > >\n> > > > What branch name does wt_status_get_state() return while bisecting?\n> > > > The branch where I started from? Because that's what 'git status'\n> > > > shows:\n> > > > But am I really on that branch? Does it really makes sense to edit\n> > > > the description of 'mybranch' by default while bisecting through an\n> > > > old revision range? I do not think so.\n> > >\n> > > It's not clear what downside you are pointing out; i.e. why would it\n> > > be a bad thing to be able to set the branch description even while\n> > > bisecting -- especially since `git status` affirms that it knows the\n> > > branch?\n> >\n> > No, during a bisect operation 'git status' knows the branch where I\n> > _was_ when I started bisecting, and where a 'git bisect reset' will\n> > eventually bring me back when I'm finished, and that has no relation\n> > whatsoever to the revision range that I'm bisecting.\n> >\n> > Consider this case:\n> >\n> > $ git checkout --orphan unrelated-history\n> > Switched to a new branch 'unrelated-history'\n> > $ git commit -m \"test\"\n> > [unrelated-history (root-commit) 639b9d1047] test\n> > <...>\n> > $ git bisect start v2.25.0 v2.24.0\n> > Bisecting: 361 revisions left to test after this (roughly 9 steps)\n> > [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1\n> > $ git status\n> > HEAD detached at 7034cd094b\n> > You are currently bisecting, started from branch 'unrelated-history'.\n> > (use \"git bisect reset\" to get back to the original branch)\n> >\n> > nothing to commit, working tree clean\n> >\n> > I can't possible be on branch 'unrelated-history' during that\n> > bisection.\n> >\n> >\n> > OTOH, while during a rebase we are technically on a detached HEAD as\n> > well, that rebase operation is all about constructing the new history\n> > of the rebased branch, and once finished that branch will be updated\n> > to point to the tip of the new history, thus it will include all the\n> > commits created while on the detached HEAD. Therefore, it makes sense\n> > conceptually to treat it as if we were on the rebased branch. That's\n> > why it makes sense to display the name of the rebased branch in the\n> > Bash prompt, and that's why I think it makes sense to default to edit\n> > the description of the rebased branch without explicitly naming it.\n> >\n> > With bisect that just doesn't make sense.\n> \n> If the range you are bisecting belongs or lead to the current branch,\n> that still makes sense. And it's probably most of the time. So, I am\n> not sure your objection is valid enough here.\n\nI'm not sure what you mean with \"belongs or lead to\" a branch.\n\nDo you mean that the range is reachable from the branch that just so\nhappened to be checked out when the bisection was started? Well, I\nhave over 30 branches from where v2.25.0 is reachable, and all of them\nare obviously bad candidates for editing their descriptions by default\nwhile bisecting a totally unrelated issue.\n\n"},{"id":"390912","messageId":"CAJ+F1CLtDET6L-CGo=j0Yj0aPVSbec=57MPgaGrhr3L8dpCSSQ@mail.gmail.com","threadId":"52615","inReplyTo":"20200131155228.GF10482@szeder.dev","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"Marc-André Lureau","fromEmail":"marcandre.lureau@gmail.com","sentAt":"2020-01-31T15:59:15Z","receivedAt":"2020-01-31T15:59:32Z","isPatch":true,"sender":{"key":"marcandre.lureau@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9381?v=4"},"body":"Hi\n\nOn Fri, Jan 31, 2020 at 4:52 PM SZEDER Gábor wrote:\n>\n> On Thu, Jan 30, 2020 at 10:37:38PM +0100, Marc-André Lureau wrote:\n> > Hi\n> >\n> > On Fri, Jan 24, 2020 at 11:41 PM SZEDER Gábor wrote:\n> > >\n> > > On Sun, Jan 12, 2020 at 08:59:04PM -0500, Eric Sunshine wrote:\n> > > > On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor wrote:\n> > > > > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:\n> > > > > > Taking a deeper look at the code, I'm wondering it would make more\n> > > > > > sense to call wt_status_get_state(), which handles 'rebase' and\n> > > > > > 'bisect'. Is there a reason that you limited this check to only\n> > > > > > 'rebase'?\n> > > > >\n> > > > > What branch name does wt_status_get_state() return while bisecting?\n> > > > > The branch where I started from? Because that's what 'git status'\n> > > > > shows:\n> > > > > But am I really on that branch? Does it really makes sense to edit\n> > > > > the description of 'mybranch' by default while bisecting through an\n> > > > > old revision range? I do not think so.\n> > > >\n> > > > It's not clear what downside you are pointing out; i.e. why would it\n> > > > be a bad thing to be able to set the branch description even while\n> > > > bisecting -- especially since `git status` affirms that it knows the\n> > > > branch?\n> > >\n> > > No, during a bisect operation 'git status' knows the branch where I\n> > > _was_ when I started bisecting, and where a 'git bisect reset' will\n> > > eventually bring me back when I'm finished, and that has no relation\n> > > whatsoever to the revision range that I'm bisecting.\n> > >\n> > > Consider this case:\n> > >\n> > > $ git checkout --orphan unrelated-history\n> > > Switched to a new branch 'unrelated-history'\n> > > $ git commit -m \"test\"\n> > > [unrelated-history (root-commit) 639b9d1047] test\n> > > <...>\n> > > $ git bisect start v2.25.0 v2.24.0\n> > > Bisecting: 361 revisions left to test after this (roughly 9 steps)\n> > > [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1\n> > > $ git status\n> > > HEAD detached at 7034cd094b\n> > > You are currently bisecting, started from branch 'unrelated-history'.\n> > > (use \"git bisect reset\" to get back to the original branch)\n> > >\n> > > nothing to commit, working tree clean\n> > >\n> > > I can't possible be on branch 'unrelated-history' during that\n> > > bisection.\n> > >\n> > >\n> > > OTOH, while during a rebase we are technically on a detached HEAD as\n> > > well, that rebase operation is all about constructing the new history\n> > > of the rebased branch, and once finished that branch will be updated\n> > > to point to the tip of the new history, thus it will include all the\n> > > commits created while on the detached HEAD. Therefore, it makes sense\n> > > conceptually to treat it as if we were on the rebased branch. That's\n> > > why it makes sense to display the name of the rebased branch in the\n> > > Bash prompt, and that's why I think it makes sense to default to edit\n> > > the description of the rebased branch without explicitly naming it.\n> > >\n> > > With bisect that just doesn't make sense.\n> >\n> > If the range you are bisecting belongs or lead to the current branch,\n> > that still makes sense. And it's probably most of the time. So, I am\n> > not sure your objection is valid enough here.\n>\n> I'm not sure what you mean with \"belongs or lead to\" a branch.\n>\n> Do you mean that the range is reachable from the branch that just so\n> happened to be checked out when the bisection was started? Well, I\n> have over 30 branches from where v2.25.0 is reachable, and all of them\n> are obviously bad candidates for editing their descriptions by default\n> while bisecting a totally unrelated issue.\n>\n\n\nIf we take that simple example:\n\n* (my-branch)\n*\n* bisect bad\n*\n* (HEAD)\n* bisect good\n*\n\nIt makes a lot of sense to me to edit my-branch description by\ndefault, even if the range good-bad happen to exist in other branches.\n\n-- \nMarc-André Lureau\n"},{"id":"390914","messageId":"20200131161630.GG10482@szeder.dev","threadId":"52615","inReplyTo":"CAJ+F1CLtDET6L-CGo=j0Yj0aPVSbec=57MPgaGrhr3L8dpCSSQ@mail.gmail.com","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-01-31T16:16:30Z","receivedAt":"2020-01-31T16:16:38Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Jan 31, 2020 at 04:59:15PM +0100, Marc-André Lureau wrote:\n> Hi\n> \n> On Fri, Jan 31, 2020 at 4:52 PM SZEDER Gábor wrote:\n> >\n> > On Thu, Jan 30, 2020 at 10:37:38PM +0100, Marc-André Lureau wrote:\n> > > Hi\n> > >\n> > > On Fri, Jan 24, 2020 at 11:41 PM SZEDER Gábor wrote:\n> > > >\n> > > > On Sun, Jan 12, 2020 at 08:59:04PM -0500, Eric Sunshine wrote:\n> > > > > On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor wrote:\n> > > > > > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:\n> > > > > > > Taking a deeper look at the code, I'm wondering it would make more\n> > > > > > > sense to call wt_status_get_state(), which handles 'rebase' and\n> > > > > > > 'bisect'. Is there a reason that you limited this check to only\n> > > > > > > 'rebase'?\n> > > > > >\n> > > > > > What branch name does wt_status_get_state() return while bisecting?\n> > > > > > The branch where I started from? Because that's what 'git status'\n> > > > > > shows:\n> > > > > > But am I really on that branch? Does it really makes sense to edit\n> > > > > > the description of 'mybranch' by default while bisecting through an\n> > > > > > old revision range? I do not think so.\n> > > > >\n> > > > > It's not clear what downside you are pointing out; i.e. why would it\n> > > > > be a bad thing to be able to set the branch description even while\n> > > > > bisecting -- especially since `git status` affirms that it knows the\n> > > > > branch?\n> > > >\n> > > > No, during a bisect operation 'git status' knows the branch where I\n> > > > _was_ when I started bisecting, and where a 'git bisect reset' will\n> > > > eventually bring me back when I'm finished, and that has no relation\n> > > > whatsoever to the revision range that I'm bisecting.\n> > > >\n> > > > Consider this case:\n> > > >\n> > > > $ git checkout --orphan unrelated-history\n> > > > Switched to a new branch 'unrelated-history'\n> > > > $ git commit -m \"test\"\n> > > > [unrelated-history (root-commit) 639b9d1047] test\n> > > > <...>\n> > > > $ git bisect start v2.25.0 v2.24.0\n> > > > Bisecting: 361 revisions left to test after this (roughly 9 steps)\n> > > > [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1\n> > > > $ git status\n> > > > HEAD detached at 7034cd094b\n> > > > You are currently bisecting, started from branch 'unrelated-history'.\n> > > > (use \"git bisect reset\" to get back to the original branch)\n> > > >\n> > > > nothing to commit, working tree clean\n> > > >\n> > > > I can't possible be on branch 'unrelated-history' during that\n> > > > bisection.\n> > > >\n> > > >\n> > > > OTOH, while during a rebase we are technically on a detached HEAD as\n> > > > well, that rebase operation is all about constructing the new history\n> > > > of the rebased branch, and once finished that branch will be updated\n> > > > to point to the tip of the new history, thus it will include all the\n> > > > commits created while on the detached HEAD. Therefore, it makes sense\n> > > > conceptually to treat it as if we were on the rebased branch. That's\n> > > > why it makes sense to display the name of the rebased branch in the\n> > > > Bash prompt, and that's why I think it makes sense to default to edit\n> > > > the description of the rebased branch without explicitly naming it.\n> > > >\n> > > > With bisect that just doesn't make sense.\n> > >\n> > > If the range you are bisecting belongs or lead to the current branch,\n> > > that still makes sense. And it's probably most of the time. So, I am\n> > > not sure your objection is valid enough here.\n> >\n> > I'm not sure what you mean with \"belongs or lead to\" a branch.\n> >\n> > Do you mean that the range is reachable from the branch that just so\n> > happened to be checked out when the bisection was started? Well, I\n> > have over 30 branches from where v2.25.0 is reachable, and all of them\n> > are obviously bad candidates for editing their descriptions by default\n> > while bisecting a totally unrelated issue.\n> >\n> \n> \n> If we take that simple example:\n> \n> * (my-branch)\n> *\n> * bisect bad\n> *\n> * (HEAD)\n> * bisect good\n> *\n> \n> It makes a lot of sense to me to edit my-branch description by\n> default, even if the range good-bad happen to exist in other branches.\n\nI still don't understand why it would make sense.\n\nFurthermore, how do you think you could avoid choosing an obviously\nbad branch to default to?\n\n"},{"id":"391277","messageId":"CAJ+F1CJaszsOMeuUmk5MKXpjkX1gHNuK6xyf_mmHtnToL2Y_7A@mail.gmail.com","threadId":"52615","inReplyTo":"20200131161630.GG10482@szeder.dev","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"Marc-André Lureau","fromEmail":"marcandre.lureau@gmail.com","sentAt":"2020-02-06T22:26:56Z","receivedAt":"2020-02-06T22:27:41Z","isPatch":true,"sender":{"key":"marcandre.lureau@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9381?v=4"},"body":"Hi\n\nOn Fri, Jan 31, 2020 at 5:16 PM SZEDER Gábor wrote:\n>\n> On Fri, Jan 31, 2020 at 04:59:15PM +0100, Marc-André Lureau wrote:\n> > Hi\n> >\n> > On Fri, Jan 31, 2020 at 4:52 PM SZEDER Gábor wrote:\n> > >\n> > > On Thu, Jan 30, 2020 at 10:37:38PM +0100, Marc-André Lureau wrote:\n> > > > Hi\n> > > >\n> > > > On Fri, Jan 24, 2020 at 11:41 PM SZEDER Gábor wrote:\n> > > > >\n> > > > > On Sun, Jan 12, 2020 at 08:59:04PM -0500, Eric Sunshine wrote:\n> > > > > > On Sun, Jan 12, 2020 at 7:14 AM SZEDER Gábor wrote:\n> > > > > > > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:\n> > > > > > > > Taking a deeper look at the code, I'm wondering it would make more\n> > > > > > > > sense to call wt_status_get_state(), which handles 'rebase' and\n> > > > > > > > 'bisect'. Is there a reason that you limited this check to only\n> > > > > > > > 'rebase'?\n> > > > > > >\n> > > > > > > What branch name does wt_status_get_state() return while bisecting?\n> > > > > > > The branch where I started from? Because that's what 'git status'\n> > > > > > > shows:\n> > > > > > > But am I really on that branch? Does it really makes sense to edit\n> > > > > > > the description of 'mybranch' by default while bisecting through an\n> > > > > > > old revision range? I do not think so.\n> > > > > >\n> > > > > > It's not clear what downside you are pointing out; i.e. why would it\n> > > > > > be a bad thing to be able to set the branch description even while\n> > > > > > bisecting -- especially since `git status` affirms that it knows the\n> > > > > > branch?\n> > > > >\n> > > > > No, during a bisect operation 'git status' knows the branch where I\n> > > > > _was_ when I started bisecting, and where a 'git bisect reset' will\n> > > > > eventually bring me back when I'm finished, and that has no relation\n> > > > > whatsoever to the revision range that I'm bisecting.\n> > > > >\n> > > > > Consider this case:\n> > > > >\n> > > > > $ git checkout --orphan unrelated-history\n> > > > > Switched to a new branch 'unrelated-history'\n> > > > > $ git commit -m \"test\"\n> > > > > [unrelated-history (root-commit) 639b9d1047] test\n> > > > > <...>\n> > > > > $ git bisect start v2.25.0 v2.24.0\n> > > > > Bisecting: 361 revisions left to test after this (roughly 9 steps)\n> > > > > [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1\n> > > > > $ git status\n> > > > > HEAD detached at 7034cd094b\n> > > > > You are currently bisecting, started from branch 'unrelated-history'.\n> > > > > (use \"git bisect reset\" to get back to the original branch)\n> > > > >\n> > > > > nothing to commit, working tree clean\n> > > > >\n> > > > > I can't possible be on branch 'unrelated-history' during that\n> > > > > bisection.\n> > > > >\n> > > > >\n> > > > > OTOH, while during a rebase we are technically on a detached HEAD as\n> > > > > well, that rebase operation is all about constructing the new history\n> > > > > of the rebased branch, and once finished that branch will be updated\n> > > > > to point to the tip of the new history, thus it will include all the\n> > > > > commits created while on the detached HEAD. Therefore, it makes sense\n> > > > > conceptually to treat it as if we were on the rebased branch. That's\n> > > > > why it makes sense to display the name of the rebased branch in the\n> > > > > Bash prompt, and that's why I think it makes sense to default to edit\n> > > > > the description of the rebased branch without explicitly naming it.\n> > > > >\n> > > > > With bisect that just doesn't make sense.\n> > > >\n> > > > If the range you are bisecting belongs or lead to the current branch,\n> > > > that still makes sense. And it's probably most of the time. So, I am\n> > > > not sure your objection is valid enough here.\n> > >\n> > > I'm not sure what you mean with \"belongs or lead to\" a branch.\n> > >\n> > > Do you mean that the range is reachable from the branch that just so\n> > > happened to be checked out when the bisection was started? Well, I\n> > > have over 30 branches from where v2.25.0 is reachable, and all of them\n> > > are obviously bad candidates for editing their descriptions by default\n> > > while bisecting a totally unrelated issue.\n> > >\n> >\n> >\n> > If we take that simple example:\n> >\n> > * (my-branch)\n> > *\n> > * bisect bad\n> > *\n> > * (HEAD)\n> > * bisect good\n> > *\n> >\n> > It makes a lot of sense to me to edit my-branch description by\n> > default, even if the range good-bad happen to exist in other branches.\n>\n> I still don't understand why it would make sense.\n>\n> Furthermore, how do you think you could avoid choosing an obviously\n> bad branch to default to?\n\nIt uses the same branch that git status displays. In your example:\n\nYou are currently bisecting, started from branch 'unrelated-history'.\n\nSo it's not completely off to pick that branch by default for\n--edit-description.\n\nBut again, I think you are focusing on a rather rare case, please\nconsider the most common case.\n\n-- \nMarc-André Lureau\n"},{"id":"391315","messageId":"20200207100247.GA1111@szeder.dev","threadId":"52615","inReplyTo":"CAJ+F1CJaszsOMeuUmk5MKXpjkX1gHNuK6xyf_mmHtnToL2Y_7A@mail.gmail.com","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-02-07T10:02:47Z","receivedAt":"2020-02-07T10:02:57Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Feb 06, 2020 at 11:26:56PM +0100, Marc-André Lureau wrote:\n> > > > > > > > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:\n> > > > > > > > > Taking a deeper look at the code, I'm wondering it would make more\n> > > > > > > > > sense to call wt_status_get_state(), which handles 'rebase' and\n> > > > > > > > > 'bisect'. Is there a reason that you limited this check to only\n> > > > > > > > > 'rebase'?\n> > > > > > > >\n> > > > > > > > What branch name does wt_status_get_state() return while bisecting?\n> > > > > > > > The branch where I started from? Because that's what 'git status'\n> > > > > > > > shows:\n> > > > > > > > But am I really on that branch? Does it really makes sense to edit\n> > > > > > > > the description of 'mybranch' by default while bisecting through an\n> > > > > > > > old revision range? I do not think so.\n> > > > > > >\n> > > > > > > It's not clear what downside you are pointing out; i.e. why would it\n> > > > > > > be a bad thing to be able to set the branch description even while\n> > > > > > > bisecting -- especially since `git status` affirms that it knows the\n> > > > > > > branch?\n> > > > > >\n> > > > > > No, during a bisect operation 'git status' knows the branch where I\n> > > > > > _was_ when I started bisecting, and where a 'git bisect reset' will\n> > > > > > eventually bring me back when I'm finished, and that has no relation\n> > > > > > whatsoever to the revision range that I'm bisecting.\n> > > > > >\n> > > > > > Consider this case:\n> > > > > >\n> > > > > > $ git checkout --orphan unrelated-history\n> > > > > > Switched to a new branch 'unrelated-history'\n> > > > > > $ git commit -m \"test\"\n> > > > > > [unrelated-history (root-commit) 639b9d1047] test\n> > > > > > <...>\n> > > > > > $ git bisect start v2.25.0 v2.24.0\n> > > > > > Bisecting: 361 revisions left to test after this (roughly 9 steps)\n> > > > > > [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1\n> > > > > > $ git status\n> > > > > > HEAD detached at 7034cd094b\n> > > > > > You are currently bisecting, started from branch 'unrelated-history'.\n> > > > > > (use \"git bisect reset\" to get back to the original branch)\n> > > > > >\n> > > > > > nothing to commit, working tree clean\n> > > > > >\n> > > > > > I can't possible be on branch 'unrelated-history' during that\n> > > > > > bisection.\n> > > > > >\n> > > > > >\n> > > > > > OTOH, while during a rebase we are technically on a detached HEAD as\n> > > > > > well, that rebase operation is all about constructing the new history\n> > > > > > of the rebased branch, and once finished that branch will be updated\n> > > > > > to point to the tip of the new history, thus it will include all the\n> > > > > > commits created while on the detached HEAD. Therefore, it makes sense\n> > > > > > conceptually to treat it as if we were on the rebased branch. That's\n> > > > > > why it makes sense to display the name of the rebased branch in the\n> > > > > > Bash prompt, and that's why I think it makes sense to default to edit\n> > > > > > the description of the rebased branch without explicitly naming it.\n> > > > > >\n> > > > > > With bisect that just doesn't make sense.\n> > > > >\n> > > > > If the range you are bisecting belongs or lead to the current branch,\n> > > > > that still makes sense. And it's probably most of the time. So, I am\n> > > > > not sure your objection is valid enough here.\n> > > >\n> > > > I'm not sure what you mean with \"belongs or lead to\" a branch.\n> > > >\n> > > > Do you mean that the range is reachable from the branch that just so\n> > > > happened to be checked out when the bisection was started? Well, I\n> > > > have over 30 branches from where v2.25.0 is reachable, and all of them\n> > > > are obviously bad candidates for editing their descriptions by default\n> > > > while bisecting a totally unrelated issue.\n> > > >\n> > >\n> > >\n> > > If we take that simple example:\n> > >\n> > > * (my-branch)\n> > > *\n> > > * bisect bad\n> > > *\n> > > * (HEAD)\n> > > * bisect good\n> > > *\n> > >\n> > > It makes a lot of sense to me to edit my-branch description by\n> > > default, even if the range good-bad happen to exist in other branches.\n> >\n> > I still don't understand why it would make sense.\n> >\n> > Furthermore, how do you think you could avoid choosing an obviously\n> > bad branch to default to?\n> \n> It uses the same branch that git status displays. In your example:\n> \n> You are currently bisecting, started from branch 'unrelated-history'.\n\nYes, that's the problem: it shows \"started from branch\", not \"On\nbranch\". Conceptually a huge difference.\n\n> So it's not completely off to pick that branch by default for\n> --edit-description.\n> \n> But again, I think you are focusing on a rather rare case, please\n> consider the most common case.\n\nI do focus on the most common case: I'm on branch 'foo' built on top\nof current master, when a bugreport comes in, and I start bisecting on\nthe range v2.12.0..v2.16.0. It can not possibly be considered that\nduring the bisect I'm on the branch 'foo' during the bisection, they\nare totally unrelated.\n\n\n"},{"id":"391325","messageId":"CAJ+F1CJc4kEvxLr-wLXpvXOC8YRVf5xP1HuJh9-cYa6mGmbyXg@mail.gmail.com","threadId":"52615","inReplyTo":"20200207100247.GA1111@szeder.dev","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"Marc-André Lureau","fromEmail":"marcandre.lureau@gmail.com","sentAt":"2020-02-07T14:16:32Z","receivedAt":"2020-02-07T14:16:49Z","isPatch":true,"sender":{"key":"marcandre.lureau@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9381?v=4"},"body":"Hi\n\nOn Fri, Feb 7, 2020 at 11:02 AM SZEDER Gábor wrote:\n>\n> On Thu, Feb 06, 2020 at 11:26:56PM +0100, Marc-André Lureau wrote:\n> > > > > > > > > On Sat, Jan 11, 2020 at 08:27:11PM -0500, Eric Sunshine wrote:\n> > > > > > > > > > Taking a deeper look at the code, I'm wondering it would make more\n> > > > > > > > > > sense to call wt_status_get_state(), which handles 'rebase' and\n> > > > > > > > > > 'bisect'. Is there a reason that you limited this check to only\n> > > > > > > > > > 'rebase'?\n> > > > > > > > >\n> > > > > > > > > What branch name does wt_status_get_state() return while bisecting?\n> > > > > > > > > The branch where I started from? Because that's what 'git status'\n> > > > > > > > > shows:\n> > > > > > > > > But am I really on that branch? Does it really makes sense to edit\n> > > > > > > > > the description of 'mybranch' by default while bisecting through an\n> > > > > > > > > old revision range? I do not think so.\n> > > > > > > >\n> > > > > > > > It's not clear what downside you are pointing out; i.e. why would it\n> > > > > > > > be a bad thing to be able to set the branch description even while\n> > > > > > > > bisecting -- especially since `git status` affirms that it knows the\n> > > > > > > > branch?\n> > > > > > >\n> > > > > > > No, during a bisect operation 'git status' knows the branch where I\n> > > > > > > _was_ when I started bisecting, and where a 'git bisect reset' will\n> > > > > > > eventually bring me back when I'm finished, and that has no relation\n> > > > > > > whatsoever to the revision range that I'm bisecting.\n> > > > > > >\n> > > > > > > Consider this case:\n> > > > > > >\n> > > > > > > $ git checkout --orphan unrelated-history\n> > > > > > > Switched to a new branch 'unrelated-history'\n> > > > > > > $ git commit -m \"test\"\n> > > > > > > [unrelated-history (root-commit) 639b9d1047] test\n> > > > > > > <...>\n> > > > > > > $ git bisect start v2.25.0 v2.24.0\n> > > > > > > Bisecting: 361 revisions left to test after this (roughly 9 steps)\n> > > > > > > [7034cd094bda4edbcdff7fad1a28fcaaf9b9a040] Sync with Git 2.24.1\n> > > > > > > $ git status\n> > > > > > > HEAD detached at 7034cd094b\n> > > > > > > You are currently bisecting, started from branch 'unrelated-history'.\n> > > > > > > (use \"git bisect reset\" to get back to the original branch)\n> > > > > > >\n> > > > > > > nothing to commit, working tree clean\n> > > > > > >\n> > > > > > > I can't possible be on branch 'unrelated-history' during that\n> > > > > > > bisection.\n> > > > > > >\n> > > > > > >\n> > > > > > > OTOH, while during a rebase we are technically on a detached HEAD as\n> > > > > > > well, that rebase operation is all about constructing the new history\n> > > > > > > of the rebased branch, and once finished that branch will be updated\n> > > > > > > to point to the tip of the new history, thus it will include all the\n> > > > > > > commits created while on the detached HEAD. Therefore, it makes sense\n> > > > > > > conceptually to treat it as if we were on the rebased branch. That's\n> > > > > > > why it makes sense to display the name of the rebased branch in the\n> > > > > > > Bash prompt, and that's why I think it makes sense to default to edit\n> > > > > > > the description of the rebased branch without explicitly naming it.\n> > > > > > >\n> > > > > > > With bisect that just doesn't make sense.\n> > > > > >\n> > > > > > If the range you are bisecting belongs or lead to the current branch,\n> > > > > > that still makes sense. And it's probably most of the time. So, I am\n> > > > > > not sure your objection is valid enough here.\n> > > > >\n> > > > > I'm not sure what you mean with \"belongs or lead to\" a branch.\n> > > > >\n> > > > > Do you mean that the range is reachable from the branch that just so\n> > > > > happened to be checked out when the bisection was started? Well, I\n> > > > > have over 30 branches from where v2.25.0 is reachable, and all of them\n> > > > > are obviously bad candidates for editing their descriptions by default\n> > > > > while bisecting a totally unrelated issue.\n> > > > >\n> > > >\n> > > >\n> > > > If we take that simple example:\n> > > >\n> > > > * (my-branch)\n> > > > *\n> > > > * bisect bad\n> > > > *\n> > > > * (HEAD)\n> > > > * bisect good\n> > > > *\n> > > >\n> > > > It makes a lot of sense to me to edit my-branch description by\n> > > > default, even if the range good-bad happen to exist in other branches.\n> > >\n> > > I still don't understand why it would make sense.\n> > >\n> > > Furthermore, how do you think you could avoid choosing an obviously\n> > > bad branch to default to?\n> >\n> > It uses the same branch that git status displays. In your example:\n> >\n> > You are currently bisecting, started from branch 'unrelated-history'.\n>\n> Yes, that's the problem: it shows \"started from branch\", not \"On\n> branch\". Conceptually a huge difference.\n>\n> > So it's not completely off to pick that branch by default for\n> > --edit-description.\n> >\n> > But again, I think you are focusing on a rather rare case, please\n> > consider the most common case.\n>\n> I do focus on the most common case: I'm on branch 'foo' built on top\n> of current master, when a bugreport comes in, and I start bisecting on\n> the range v2.12.0..v2.16.0. It can not possibly be considered that\n> during the bisect I'm on the branch 'foo' during the bisection, they\n> are totally unrelated.\n\n\nAnd usually that bisection is ancestry of master, rarely unrelated.\n\nAlso, when doing --edit-description there are comments like:\n\n# Please edit the description for the branch\n# unrelated-history\n\nWhat else do you suggest?\n\n\n-- \nMarc-André Lureau\n"},{"id":"391343","messageId":"xmqq1rr6444s.fsf@gitster-ct.c.googlers.com","threadId":"52615","inReplyTo":"CAJ+F1CJc4kEvxLr-wLXpvXOC8YRVf5xP1HuJh9-cYa6mGmbyXg@mail.gmail.com","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-07T18:57:23Z","receivedAt":"2020-02-07T18:57:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marc-André Lureau writes:\n\n> Also, when doing --edit-description there are comments like:\n>\n> # Please edit the description for the branch\n> # unrelated-history\n>\n> What else do you suggest?\n\nHow about teaching \"git branch --edit-description [HEAD]\" notice\nwhen/if HEAD is detached and always error out, no matter what\noperation is in progress?\n\n"},{"id":"391344","messageId":"CAJ+F1C+qGo=6QrRw2299Apr2+-CHNWQyzWjvWbXJN5KC+T63AQ@mail.gmail.com","threadId":"52615","inReplyTo":"xmqq1rr6444s.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"Marc-André Lureau","fromEmail":"marcandre.lureau@gmail.com","sentAt":"2020-02-07T19:09:16Z","receivedAt":"2020-02-07T19:09:32Z","isPatch":true,"sender":{"key":"marcandre.lureau@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9381?v=4"},"body":"Hi\n\nOn Fri, Feb 7, 2020 at 7:57 PM Junio C Hamano wrote:\n>\n> Marc-André Lureau writes:\n>\n> > Also, when doing --edit-description there are comments like:\n> >\n> > # Please edit the description for the branch\n> > # unrelated-history\n> >\n> > What else do you suggest?\n>\n> How about teaching \"git branch --edit-description [HEAD]\" notice\n> when/if HEAD is detached and always error out, no matter what\n> operation is in progress?\n\nThen --edit-description during won't default to the branch you started\nfrom the bisect. So we are back to my original proposal only, having\nbranch default during rebase. Eric, do you mind?\n\n\n-- \nMarc-André Lureau\n"},{"id":"391345","messageId":"xmqqwo8y2ovu.fsf@gitster-ct.c.googlers.com","threadId":"52615","inReplyTo":"CAJ+F1C+qGo=6QrRw2299Apr2+-CHNWQyzWjvWbXJN5KC+T63AQ@mail.gmail.com","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-07T19:12:05Z","receivedAt":"2020-02-07T19:12:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marc-André Lureau writes:\n\n> Hi\n>\n> On Fri, Feb 7, 2020 at 7:57 PM Junio C Hamano wrote:\n>>\n>> Marc-André Lureau writes:\n>>\n>> > Also, when doing --edit-description there are comments like:\n>> >\n>> > # Please edit the description for the branch\n>> > # unrelated-history\n>> >\n>> > What else do you suggest?\n>>\n>> How about teaching \"git branch --edit-description [HEAD]\" notice\n>> when/if HEAD is detached and always error out, no matter what\n>> operation is in progress?\n>\n> Then --edit-description during won't default to the branch you started\n> from the bisect. So we are back to my original proposal only, having\n> branch default during rebase. Eric, do you mind?\n\nWhat I meant by \"no matter what is in progress\" is not to special\ncase \"during rebase\", either.\n\nSorry for the confusion.\n"},{"id":"391346","messageId":"CAPig+cTWxj+dRiYZEEbfUA7=NiEF3crTaYCVGPR2qG-VEV+Y0w@mail.gmail.com","threadId":"52615","inReplyTo":"xmqqwo8y2ovu.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-02-07T19:29:11Z","receivedAt":"2020-02-07T19:29:27Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Feb 7, 2020 at 2:12 PM Junio C Hamano wrote:\n> Marc-André Lureau writes:\n> > Then --edit-description during won't default to the branch you started\n> > from the bisect. So we are back to my original proposal only, having\n> > branch default during rebase. Eric, do you mind?\n\nI don't feel strongly one way or the other.\n\n> > > How about teaching \"git branch --edit-description [HEAD]\" notice\n> > > when/if HEAD is detached and always error out, no matter what\n> > > operation is in progress?\n>\n> What I meant by \"no matter what is in progress\" is not to special\n> case \"during rebase\", either.\n\nThat would defeat the original purpose[1] of this submission, I think.\nAs I understand it, the idea all along was to make this operation work\nduring a rebase.\n\n[1]: https://lore.kernel.org/git/20200110071929.119000-1-marcandre.lureau@redhat.com/\n"},{"id":"391347","messageId":"xmqqpneq2lzu.fsf@gitster-ct.c.googlers.com","threadId":"52615","inReplyTo":"CAPig+cTWxj+dRiYZEEbfUA7=NiEF3crTaYCVGPR2qG-VEV+Y0w@mail.gmail.com","subject":"Re: [PATCH] branch: let '--edit-description' default to rebased branch during rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-07T20:14:29Z","receivedAt":"2020-02-07T20:14:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n>> > > How about teaching \"git branch --edit-description [HEAD]\" notice\n>> > > when/if HEAD is detached and always error out, no matter what\n>> > > operation is in progress?\n>>\n>> What I meant by \"no matter what is in progress\" is not to special\n>> case \"during rebase\", either.\n>\n> That would defeat the original purpose[1] of this submission, I think.\n> As I understand it, the idea all along was to make this operation work\n> during a rebase.\n\nI know. But I do not think it is a good thing to begin with.\n\nWhile you are rebasing the branch X and get control back before\nrebase finishes, you are *not* on branch X. You are *preparing* a\nnew version of the history leading to the tip of branch X, in the\nhope that once you are done, you would make that new version of the\nhistory the history of branch X. Until that happens, you are not on\nbranch X.\n\nIf you were on branch X, then \"git checkout -m another-branch\"\nfollowed by some other operations, and then finally coming back with\n\"git checkout -m X\" would work. But it would not, because you are\nnot on branch X.\n\nAfter all, you may well say \"git rebase --abort\" before you are\ndone. Would \"edit description\" you do in the middle be reverted\nif you did so?\n\nIt is bad for the user to blur the distinction between \"detached and\nnot on X but preparing to update X\" and \"working on X to advance X\",\nand I think the original patch that started the thread takes us in\nthat direction.\n\nThanks.\n"}]}