{"thread":{"id":"52658","subject":"[PATCH] commit: replace rebase/sequence booleans with single pick_state enum","startedAt":"2020-01-17T13:45:11Z","lastAt":"2020-01-20T21:12:05Z","messageCount":7,"participants":["Ben Curtis via GitGitGadget","Derrick Stolee","Ben Curtis","Phillip Wood","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"389981","messageId":"pull.531.git.1579268705473.gitgitgadget@gmail.com","threadId":"52658","inReplyTo":null,"subject":"[PATCH] commit: replace rebase/sequence booleans with single pick_state enum","fromName":"Ben Curtis via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-01-17T13:45:05Z","receivedAt":"2020-01-17T13:45:11Z","isPatch":true,"sender":{"key":"nospam@nowsci.com","avatar":null},"body":"From: Ben Curtis <nospam@nowsci.com>\n\nIn 116a408, the boolean `rebase_in_progress` was introduced by dscho to\nhandle instances when cherry-pick and rebase were occuring at the same time.\nThis created a situation where two independent booleans were being used\nto define the state of git at a point in time.\n\nUnder his recommendation to follow guidance from Junio, specifically\n`https://public-inbox.org/git/xmqqr234i2q0.fsf@gitster-ct.c.googlers.com/`,\nit was decided that an `enum` that defines the state of git would be a\nmore effective path forward.\n\nTasks completed:\n  - Remove multiple booleans `rebase_in_progress` and `sequencer_in_use` and\nreplaced with a single `pick_state` enum that determines if, when\ncherry-picking, we are in a rebase, multi-pick, or single-pick state\n  - Converted double `if` statement to `if/else if` prioritizing `REBASE` to\ncontinue to disallow cherry pick in rebase.\n\nSigned-off-by: Ben Curtis <nospam@nowsci.com>\n---\n    commit: replaced rebase/sequence booleans with single pick_state enum\n    \n    Addresses https://github.com/gitgitgadget/git/issues/426\n    \n    Previous discussions from @dscho and Junio led to the decision to merge\n    two independent booleans into a single enum to track the state of git \n    during a cherry-pick leading to this PR/patch.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-531%2FFmstrat%2Fjs%2Fadvise-rebase-skip-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-531/Fmstrat/js/advise-rebase-skip-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/531\n\n builtin/commit.c | 24 +++++++++++++++---------\n 1 file changed, 15 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 2beae13620..84f7e69cb1 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -125,7 +125,11 @@ static enum commit_msg_cleanup_mode cleanup_mode;\n static const char *cleanup_arg;\n \n static enum commit_whence whence;\n-static int sequencer_in_use, rebase_in_progress;\n+static enum {\n+\tSINGLE_PICK,\n+\tMULTI_PICK,\n+\tREBASE\n+} pick_state;\n static int use_editor = 1, include_status = 1;\n static int have_option_m;\n static struct strbuf message = STRBUF_INIT;\n@@ -184,10 +188,12 @@ static void determine_whence(struct wt_status *s)\n \t\twhence = FROM_MERGE;\n \telse if (file_exists(git_path_cherry_pick_head(the_repository))) {\n \t\twhence = FROM_CHERRY_PICK;\n-\t\tif (file_exists(git_path_seq_dir()))\n-\t\t\tsequencer_in_use = 1;\n \t\tif (file_exists(git_path_rebase_merge_dir()))\n-\t\t\trebase_in_progress = 1;\n+\t\t\tpick_state = REBASE;\n+\t\telse if (file_exists(git_path_seq_dir()))\n+\t\t\tpick_state = MULTI_PICK;\n+\t\telse\n+\t\t\tpick_state = SINGLE_PICK;\n \t}\n \telse\n \t\twhence = FROM_COMMIT;\n@@ -459,7 +465,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n \t\tif (whence == FROM_MERGE)\n \t\t\tdie(_(\"cannot do a partial commit during a merge.\"));\n \t\telse if (whence == FROM_CHERRY_PICK) {\n-\t\t\tif (rebase_in_progress && !sequencer_in_use)\n+\t\t\tif (pick_state == REBASE)\n \t\t\t\tdie(_(\"cannot do a partial commit during a rebase.\"));\n \t\t\tdie(_(\"cannot do a partial commit during a cherry-pick.\"));\n \t\t}\n@@ -958,9 +964,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\tfputs(_(empty_amend_advice), stderr);\n \t\telse if (whence == FROM_CHERRY_PICK) {\n \t\t\tfputs(_(empty_cherry_pick_advice), stderr);\n-\t\t\tif (sequencer_in_use)\n+\t\t\tif (pick_state == MULTI_PICK)\n \t\t\t\tfputs(_(empty_cherry_pick_advice_multi), stderr);\n-\t\t\telse if (rebase_in_progress)\n+\t\t\telse if (pick_state == REBASE)\n \t\t\t\tfputs(_(empty_rebase_advice), stderr);\n \t\t\telse\n \t\t\t\tfputs(_(empty_cherry_pick_advice_single), stderr);\n@@ -1167,7 +1173,7 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tif (whence == FROM_MERGE)\n \t\t\tdie(_(\"You are in the middle of a merge -- cannot amend.\"));\n \t\telse if (whence == FROM_CHERRY_PICK) {\n-\t\t\tif (rebase_in_progress && !sequencer_in_use)\n+\t\t\tif (pick_state == REBASE)\n \t\t\t\tdie(_(\"You are in the middle of a rebase -- cannot amend.\"));\n \t\t\tdie(_(\"You are in the middle of a cherry-pick -- cannot amend.\"));\n \t\t}\n@@ -1643,7 +1649,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tif (!reflog_msg)\n \t\t\treflog_msg = (whence != FROM_CHERRY_PICK)\n \t\t\t\t\t? \"commit\"\n-\t\t\t\t\t: rebase_in_progress && !sequencer_in_use\n+\t\t\t\t\t: pick_state == REBASE\n \t\t\t\t\t? \"commit (rebase)\"\n \t\t\t\t\t: \"commit (cherry-pick)\";\n \t\tcommit_list_insert(current_head, &parents);\n\nbase-commit: 116a408b6ffcb2496ebf10dfce1364a42e8f0b32\n-- \ngitgitgadget\n"},{"id":"389985","messageId":"551af3c3-4633-3dba-4999-1c1a63e5703a@gmail.com","threadId":"52658","inReplyTo":"pull.531.git.1579268705473.gitgitgadget@gmail.com","subject":"Re: [PATCH] commit: replace rebase/sequence booleans with single pick_state enum","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-01-17T14:29:37Z","receivedAt":"2020-01-17T14:29:41Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/17/2020 8:45 AM, Ben Curtis via GitGitGadget wrote:\n> From: Ben Curtis <nospam@nowsci.com>\n> \n> In 116a408, the boolean `rebase_in_progress` was introduced by dscho to\n\nIn 116a408 (commit: give correct advice for empty commit during a rebase,\n2019-10-22), ...\n\n> handle instances when cherry-pick and rebase were occuring at the same time.\n\ns/occuring/occurring\n\n> This created a situation where two independent booleans were being used\n> to define the state of git at a point in time.\n> \n> Under his recommendation to follow guidance from Junio, specifically\n> `https://public-inbox.org/git/xmqqr234i2q0.fsf@gitster-ct.c.googlers.com/`,\n\nUse lore.kernel.org and use \"[1]\" like a citation.\n\n[1] https://lore.kernel.org/git/xmqqr234i2q0.fsf@gitster-ct.c.googlers.com/\n\n> it was decided that an `enum` that defines the state of git would be a\n> more effective path forward.\n> \n> Tasks completed:\n\nEverything in the message is about what you did and why. It's good that\nyou prefaced the \"what\" with so much \"why\", but now you can just describe\nthe \"what\" using paragraphs. The \"Tasks completed:\" line is superfluous.\n\n>   - Remove multiple booleans `rebase_in_progress` and `sequencer_in_use` and\n> replaced with a single `pick_state` enum that determines if, when\n> cherry-picking, we are in a rebase, multi-pick, or single-pick state\n>   - Converted double `if` statement to `if/else if` prioritizing `REBASE` to\n> continue to disallow cherry pick in rebase.>\n> \n> Signed-off-by: Ben Curtis <nospam@nowsci.com>\n> ---\n>     commit: replaced rebase/sequence booleans with single pick_state enum\n>     \n>     Addresses https://github.com/gitgitgadget/git/issues/426\n>     \n>     Previous discussions from @dscho and Junio led to the decision to merge\n>     two independent booleans into a single enum to track the state of git \n>     during a cherry-pick leading to this PR/patch.\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-531%2FFmstrat%2Fjs%2Fadvise-rebase-skip-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-531/Fmstrat/js/advise-rebase-skip-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/531\n> \n>  builtin/commit.c | 24 +++++++++++++++---------\n>  1 file changed, 15 insertions(+), 9 deletions(-)\n> \n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 2beae13620..84f7e69cb1 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -125,7 +125,11 @@ static enum commit_msg_cleanup_mode cleanup_mode;\n>  static const char *cleanup_arg;\n>  \n>  static enum commit_whence whence;\n> -static int sequencer_in_use, rebase_in_progress;\n> +static enum {\n> +\tSINGLE_PICK,\n> +\tMULTI_PICK,\n> +\tREBASE\n> +} pick_state;\n>  static int use_editor = 1, include_status = 1;\n>  static int have_option_m;\n>  static struct strbuf message = STRBUF_INIT;\n> @@ -184,10 +188,12 @@ static void determine_whence(struct wt_status *s)\n>  \t\twhence = FROM_MERGE;\n>  \telse if (file_exists(git_path_cherry_pick_head(the_repository))) {\n>  \t\twhence = FROM_CHERRY_PICK;\n> -\t\tif (file_exists(git_path_seq_dir()))\n> -\t\t\tsequencer_in_use = 1;\n>  \t\tif (file_exists(git_path_rebase_merge_dir()))\n> -\t\t\trebase_in_progress = 1;\n> +\t\t\tpick_state = REBASE;\n> +\t\telse if (file_exists(git_path_seq_dir()))\n> +\t\t\tpick_state = MULTI_PICK;\n> +\t\telse\n> +\t\t\tpick_state = SINGLE_PICK;\n\nSince before the \"if\"s were not exclusive, would rebase_in_progress = 1\nalso include sequencer_in_use = 1? That would explain why you needed to\nrearrange the cases here. (Based on later checks, it seems that these\ncases are indeed independent.)\n\n> -\t\t\tif (rebase_in_progress && !sequencer_in_use)\n> +\t\t\tif (pick_state == REBASE)\n\nThis old error condition makes it appear that you _could_ be in the state\nwhere rebase_in_progress = 1 and sequencer_in_use = 0, showing that one\ndoes not imply the other.\n\n> -\t\t\tif (sequencer_in_use)\n> +\t\t\tif (pick_state == MULTI_PICK)\n>  \t\t\t\tfputs(_(empty_cherry_pick_advice_multi), stderr);\n> -\t\t\telse if (rebase_in_progress)\n> +\t\t\telse if (pick_state == REBASE)\n>  \t\t\t\tfputs(_(empty_rebase_advice), stderr);\n>  \t\t\telse\n>  \t\t\t\tfputs(_(empty_cherry_pick_advice_single), stderr);\n\nSince we are using an enum, should we rearrange these cases into a switch (pick_state)?\n\nThanks,\n-Stolee\n\n"},{"id":"389996","messageId":"fe3b20a8251b033a2392ee7f7a1110d026a4357e.camel@nowsci.com","threadId":"52658","inReplyTo":"551af3c3-4633-3dba-4999-1c1a63e5703a@gmail.com","subject":"Re: [PATCH] commit: replace rebase/sequence booleans with single pick_state enum","fromName":"Ben Curtis","fromEmail":"nospam@nowsci.com","sentAt":"2020-01-17T15:46:45Z","receivedAt":"2020-01-17T15:55:41Z","isPatch":true,"sender":{"key":"nospam@nowsci.com","avatar":null},"body":"On Fri, 2020-01-17 at 09:29 -0500, Derrick Stolee wrote:\n> On 1/17/2020 8:45 AM, Ben Curtis via GitGitGadget wrote:\n> > From: Ben Curtis <nospam@nowsci.com>\n> > \n> > In 116a408, the boolean `rebase_in_progress` was introduced by\n> > dscho to\n> \n> In 116a408 (commit: give correct advice for empty commit during a\n> rebase,\n> 2019-10-22), ...\n> \n> > handle instances when cherry-pick and rebase were occuring at the\n> > same time.\n> \n> s/occuring/occurring\n> \n> > This created a situation where two independent booleans were being\n> > used\n> > to define the state of git at a point in time.\n> > \n> > Under his recommendation to follow guidance from Junio,\n> > specifically\n> > `\n> > https://public-inbox.org/git/xmqqr234i2q0.fsf@gitster-ct.c.googlers.com/`\n> > ,\n> \n> Use lore.kernel.org and use \"[1]\" like a citation.\n> \n> [1] \n> https://lore.kernel.org/git/xmqqr234i2q0.fsf@gitster-ct.c.googlers.com/\n> \n> > it was decided that an `enum` that defines the state of git would\n> > be a\n> > more effective path forward.\n> > \n> > Tasks completed:\n> \n> Everything in the message is about what you did and why. It's good\n> that\n> you prefaced the \"what\" with so much \"why\", but now you can just\n> describe\n> the \"what\" using paragraphs. The \"Tasks completed:\" line is\n> superfluous.\n> \n> >   - Remove multiple booleans `rebase_in_progress` and\n> > `sequencer_in_use` and\n> > replaced with a single `pick_state` enum that determines if, when\n> > cherry-picking, we are in a rebase, multi-pick, or single-pick\n> > state\n> >   - Converted double `if` statement to `if/else if` prioritizing\n> > `REBASE` to\n> > continue to disallow cherry pick in rebase.>\n> > \n> > Signed-off-by: Ben Curtis <nospam@nowsci.com>\n> > ---\n> >     commit: replaced rebase/sequence booleans with single\n> > pick_state enum\n> >     \n> >     Addresses https://github.com/gitgitgadget/git/issues/426\n> >     \n> >     Previous discussions from @dscho and Junio led to the decision\n> > to merge\n> >     two independent booleans into a single enum to track the state\n> > of git \n> >     during a cherry-pick leading to this PR/patch.\n> > \n\nSure thing! I will revise the commit as described. And thanks for the\nfeedback, just diving into `git` development so this is my first time\nthrough and this is very helpful.\n\n> > Published-As: \n> > https://github.com/gitgitgadget/git/releases/tag/pr-531%2FFmstrat%2Fjs%2Fadvise-rebase-skip-v1\n> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-\n> > 531/Fmstrat/js/advise-rebase-skip-v1\n> > Pull-Request: https://github.com/gitgitgadget/git/pull/531\n> > \n> >  builtin/commit.c | 24 +++++++++++++++---------\n> >  1 file changed, 15 insertions(+), 9 deletions(-)\n> > \n> > diff --git a/builtin/commit.c b/builtin/commit.c\n> > index 2beae13620..84f7e69cb1 100644\n> > --- a/builtin/commit.c\n> > +++ b/builtin/commit.c\n> > @@ -125,7 +125,11 @@ static enum commit_msg_cleanup_mode\n> > cleanup_mode;\n> >  static const char *cleanup_arg;\n> >  \n> >  static enum commit_whence whence;\n> > -static int sequencer_in_use, rebase_in_progress;\n> > +static enum {\n> > +\tSINGLE_PICK,\n> > +\tMULTI_PICK,\n> > +\tREBASE\n> > +} pick_state;\n> >  static int use_editor = 1, include_status = 1;\n> >  static int have_option_m;\n> >  static struct strbuf message = STRBUF_INIT;\n> > @@ -184,10 +188,12 @@ static void determine_whence(struct wt_status\n> > *s)\n> >  \t\twhence = FROM_MERGE;\n> >  \telse if\n> > (file_exists(git_path_cherry_pick_head(the_repository))) {\n> >  \t\twhence = FROM_CHERRY_PICK;\n> > -\t\tif (file_exists(git_path_seq_dir()))\n> > -\t\t\tsequencer_in_use = 1;\n> >  \t\tif (file_exists(git_path_rebase_merge_dir()))\n> > -\t\t\trebase_in_progress = 1;\n> > +\t\t\tpick_state = REBASE;\n> > +\t\telse if (file_exists(git_path_seq_dir()))\n> > +\t\t\tpick_state = MULTI_PICK;\n> > +\t\telse\n> > +\t\t\tpick_state = SINGLE_PICK;\n> \n> Since before the \"if\"s were not exclusive, would rebase_in_progress =\n> 1\n> also include sequencer_in_use = 1? That would explain why you needed\n> to\n> rearrange the cases here. (Based on later checks, it seems that these\n> cases are indeed independent.)\n> \n\nWhile the above two `if` statements were not exclusive, their use in\nthe below `if` statements did appear to be (at first). The line right\nabove the if statement just below this comment is:\n\nelse if (whence == FROM_CHERRY_PICK) {\n\nSince we are always in a cherry pick state, and the new code\nprioritizes checking on a rebase first, I had thought this would work\nout. However given the below I can see how the single-pick state could\nstill crop up. I will update the commit with REBASE_SINGLE and\nREBASE_MULTI states to eliminate that without adding redundancy.\n\n> > -\t\t\tif (rebase_in_progress && !sequencer_in_use)\n> > +\t\t\tif (pick_state == REBASE)\n> \n> This old error condition makes it appear that you _could_ be in the\n> state\n> where rebase_in_progress = 1 and sequencer_in_use = 0, showing that\n> one\n> does not imply the other.\n> \n> > -\t\t\tif (sequencer_in_use)\n> > +\t\t\tif (pick_state == MULTI_PICK)\n> >  \t\t\t\tfputs(_(empty_cherry_pick_advice_multi)\n> > , stderr);\n> > -\t\t\telse if (rebase_in_progress)\n> > +\t\t\telse if (pick_state == REBASE)\n> >  \t\t\t\tfputs(_(empty_rebase_advice), stderr);\n> >  \t\t\telse\n> >  \t\t\t\tfputs(_(empty_cherry_pick_advice_single\n> > ), stderr);\n> \n> Since we are using an enum, should we rearrange these cases into a\n> switch (pick_state)?\n> \n\nYes, that would be cleaner, I will shift to a switch.\n\n> Thanks,\n> -Stolee\n> \n\nThanks!\nBen\n\n"},{"id":"390010","messageId":"17b57e7f-7f3c-abab-1da6-d2e5c9ff893d@gmail.com","threadId":"52658","inReplyTo":"pull.531.git.1579268705473.gitgitgadget@gmail.com","subject":"Re: [PATCH] commit: replace rebase/sequence booleans with single pick_state enum","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-01-17T20:01:10Z","receivedAt":"2020-01-17T20:01:17Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ben\n\nOn 17/01/2020 13:45, Ben Curtis via GitGitGadget wrote:\n> From: Ben Curtis <nospam@nowsci.com>\n> \n> In 116a408,\n\nThat commit is no longer in pu, it has been replaced by 430b75f720 \n(\"commit: give correct advice for empty commit during a rebase\", \n2019-12-06). There is now a preparatory commit 8d57f75749 (\"commit: use \nenum value for multiple cherry-picks\", 2019-12-06) which replaces the \nbooleans with an enum. I need to reroll the series \n(pw/advise-rebase-skip) that contains them, if you've got any comments \nplease let me know.\n\nBest Wishes\n\nPhillip\n\n  the boolean `rebase_in_progress` was introduced by dscho to\n> handle instances when cherry-pick and rebase were occuring at the same time.\n> This created a situation where two independent booleans were being used\n> to define the state of git at a point in time.\n> \n> Under his recommendation to follow guidance from Junio, specifically\n> `https://public-inbox.org/git/xmqqr234i2q0.fsf@gitster-ct.c.googlers.com/`,\n> it was decided that an `enum` that defines the state of git would be a\n> more effective path forward.\n> \n> Tasks completed:\n>    - Remove multiple booleans `rebase_in_progress` and `sequencer_in_use` and\n> replaced with a single `pick_state` enum that determines if, when\n> cherry-picking, we are in a rebase, multi-pick, or single-pick state\n>    - Converted double `if` statement to `if/else if` prioritizing `REBASE` to\n> continue to disallow cherry pick in rebase.\n> \n> Signed-off-by: Ben Curtis <nospam@nowsci.com>\n> ---\n>      commit: replaced rebase/sequence booleans with single pick_state enum\n>      \n>      Addresses https://github.com/gitgitgadget/git/issues/426\n>      \n>      Previous discussions from @dscho and Junio led to the decision to merge\n>      two independent booleans into a single enum to track the state of git\n>      during a cherry-pick leading to this PR/patch.\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-531%2FFmstrat%2Fjs%2Fadvise-rebase-skip-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-531/Fmstrat/js/advise-rebase-skip-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/531\n> \n>   builtin/commit.c | 24 +++++++++++++++---------\n>   1 file changed, 15 insertions(+), 9 deletions(-)\n> \n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 2beae13620..84f7e69cb1 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -125,7 +125,11 @@ static enum commit_msg_cleanup_mode cleanup_mode;\n>   static const char *cleanup_arg;\n>   \n>   static enum commit_whence whence;\n> -static int sequencer_in_use, rebase_in_progress;\n> +static enum {\n> +\tSINGLE_PICK,\n> +\tMULTI_PICK,\n> +\tREBASE\n> +} pick_state;\n>   static int use_editor = 1, include_status = 1;\n>   static int have_option_m;\n>   static struct strbuf message = STRBUF_INIT;\n> @@ -184,10 +188,12 @@ static void determine_whence(struct wt_status *s)\n>   \t\twhence = FROM_MERGE;\n>   \telse if (file_exists(git_path_cherry_pick_head(the_repository))) {\n>   \t\twhence = FROM_CHERRY_PICK;\n> -\t\tif (file_exists(git_path_seq_dir()))\n> -\t\t\tsequencer_in_use = 1;\n>   \t\tif (file_exists(git_path_rebase_merge_dir()))\n> -\t\t\trebase_in_progress = 1;\n> +\t\t\tpick_state = REBASE;\n> +\t\telse if (file_exists(git_path_seq_dir()))\n> +\t\t\tpick_state = MULTI_PICK;\n> +\t\telse\n> +\t\t\tpick_state = SINGLE_PICK;\n>   \t}\n>   \telse\n>   \t\twhence = FROM_COMMIT;\n> @@ -459,7 +465,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n>   \t\tif (whence == FROM_MERGE)\n>   \t\t\tdie(_(\"cannot do a partial commit during a merge.\"));\n>   \t\telse if (whence == FROM_CHERRY_PICK) {\n> -\t\t\tif (rebase_in_progress && !sequencer_in_use)\n> +\t\t\tif (pick_state == REBASE)\n>   \t\t\t\tdie(_(\"cannot do a partial commit during a rebase.\"));\n>   \t\t\tdie(_(\"cannot do a partial commit during a cherry-pick.\"));\n>   \t\t}\n> @@ -958,9 +964,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>   \t\t\tfputs(_(empty_amend_advice), stderr);\n>   \t\telse if (whence == FROM_CHERRY_PICK) {\n>   \t\t\tfputs(_(empty_cherry_pick_advice), stderr);\n> -\t\t\tif (sequencer_in_use)\n> +\t\t\tif (pick_state == MULTI_PICK)\n>   \t\t\t\tfputs(_(empty_cherry_pick_advice_multi), stderr);\n> -\t\t\telse if (rebase_in_progress)\n> +\t\t\telse if (pick_state == REBASE)\n>   \t\t\t\tfputs(_(empty_rebase_advice), stderr);\n>   \t\t\telse\n>   \t\t\t\tfputs(_(empty_cherry_pick_advice_single), stderr);\n> @@ -1167,7 +1173,7 @@ static int parse_and_validate_options(int argc, const char *argv[],\n>   \t\tif (whence == FROM_MERGE)\n>   \t\t\tdie(_(\"You are in the middle of a merge -- cannot amend.\"));\n>   \t\telse if (whence == FROM_CHERRY_PICK) {\n> -\t\t\tif (rebase_in_progress && !sequencer_in_use)\n> +\t\t\tif (pick_state == REBASE)\n>   \t\t\t\tdie(_(\"You are in the middle of a rebase -- cannot amend.\"));\n>   \t\t\tdie(_(\"You are in the middle of a cherry-pick -- cannot amend.\"));\n>   \t\t}\n> @@ -1643,7 +1649,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>   \t\tif (!reflog_msg)\n>   \t\t\treflog_msg = (whence != FROM_CHERRY_PICK)\n>   \t\t\t\t\t? \"commit\"\n> -\t\t\t\t\t: rebase_in_progress && !sequencer_in_use\n> +\t\t\t\t\t: pick_state == REBASE\n>   \t\t\t\t\t? \"commit (rebase)\"\n>   \t\t\t\t\t: \"commit (cherry-pick)\";\n>   \t\tcommit_list_insert(current_head, &parents);\n> \n> base-commit: 116a408b6ffcb2496ebf10dfce1364a42e8f0b32\n> \n"},{"id":"390051","messageId":"bb0e232924305697596743d3fa93d5bf3b0e8d4d.camel@nowsci.com","threadId":"52658","inReplyTo":"17b57e7f-7f3c-abab-1da6-d2e5c9ff893d@gmail.com","subject":"Re: [PATCH] commit: replace rebase/sequence booleans with single pick_state enum","fromName":"Ben Curtis","fromEmail":"nospam@nowsci.com","sentAt":"2020-01-18T16:34:20Z","receivedAt":"2020-01-18T16:34:33Z","isPatch":true,"sender":{"key":"nospam@nowsci.com","avatar":null},"body":"On Fri, 2020-01-17 at 20:01 +0000, Phillip Wood wrote:\n> Hi Ben\n> \n> On 17/01/2020 13:45, Ben Curtis via GitGitGadget wrote:\n> > From: Ben Curtis <nospam@nowsci.com>\n> > \n> > In 116a408,\n> \n> That commit is no longer in pu, it has been replaced by 430b75f720 \n> (\"commit: give correct advice for empty commit during a rebase\", \n> 2019-12-06). There is now a preparatory commit 8d57f75749 (\"commit:\n> use \n> enum value for multiple cherry-picks\", 2019-12-06) which replaces\n> the \n> booleans with an enum. I need to reroll the series \n> (pw/advise-rebase-skip) that contains them, if you've got any\n> comments \n> please let me know.\n> \n> Best Wishes\n> \n> Phillip\n> \n\nHi Phillip,\n\nThank you for the feedback, I assume that means my patch is no longer\nrequired?\n\nAlso, is there a formal issue assignment method with `git`? I hopped on\nthis particular issue on GitGitGadget to get my feet wet here but was\nnot sure if there was a separate maintained list to track overlap like\nthe above.\n\nThanks!\nBen\n\n"},{"id":"390109","messageId":"bfe569ea-93c7-382f-18fe-d81d8f9cbf4f@gmail.com","threadId":"52658","inReplyTo":"bb0e232924305697596743d3fa93d5bf3b0e8d4d.camel@nowsci.com","subject":"Re: [PATCH] commit: replace rebase/sequence booleans with single pick_state enum","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-01-20T17:09:48Z","receivedAt":"2020-01-20T17:09:53Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ben\n\n[Cc'ing dscho as it relates to issue management on gitgitgadget]\n\nOn 18/01/2020 16:34, Ben Curtis wrote:\n> On Fri, 2020-01-17 at 20:01 +0000, Phillip Wood wrote:\n>> Hi Ben\n>>\n>> On 17/01/2020 13:45, Ben Curtis via GitGitGadget wrote:\n>>> From: Ben Curtis <nospam@nowsci.com>\n>>>\n>>> In 116a408,\n>>\n>> That commit is no longer in pu, it has been replaced by 430b75f720\n>> (\"commit: give correct advice for empty commit during a rebase\",\n>> 2019-12-06). There is now a preparatory commit 8d57f75749 (\"commit:\n>> use\n>> enum value for multiple cherry-picks\", 2019-12-06) which replaces\n>> the\n>> booleans with an enum. I need to reroll the series\n>> (pw/advise-rebase-skip) that contains them, if you've got any\n>> comments\n>> please let me know.\n>>\n>> Best Wishes\n>>\n>> Phillip\n>>\n> \n> Hi Phillip,\n> \n> Thank you for the feedback, I assume that means my patch is no longer\n> required?\n\nUnfortunately yes\n\n> Also, is there a formal issue assignment method with `git`? I hopped on\n> this particular issue on GitGitGadget to get my feet wet here but was\n> not sure if there was a separate maintained list to track overlap like\n> the above.\n\nUnfortunately there is no formal issue management. There is the mailing \nlist which is where patches are picked up but it does not provide any \nissue management. In practice when an issue is reported on the list \nthere is either a fix posted relatively quickly or someone notes it down \nsomewhere and may work on it later. There is a convention of adding \n#leftoverbits to an email in the hope that someone will search for that \nand find things but I've never seen someone reference that when \nsubmitting a new patch and if the fix comes in a different email thread \nthen there's no way to see that the issue has been fixed.\n\nThere is gitgitgadet's list of issues but not everyone uses it (and it's \nonly triaged by a small subset of people so there's no guarantee that a \nfeature requested there will be accepted once it gets submitted to the \nmailing list). If someone posts directly to the mailing list then they \nprobably wont see that there is an issue open there. Further confusion \nis provided by https://bugs.chromium.org/p/git/issues/list which has a \ndifferent list of issues.\n\nThe best thing would be to check the history of the 'pu' branch before \nstarting work on an issue to see if it has already been fixed.\n\nI'm really sorry that you've had a bad experience, the idea of the \ngitgitgadget issue tracker is to make it easier for new contributors, \nnot to waste their time. I hope it wont put you off making another \ncontribution.\n\nBest Wishes\n\nPhillip\n\n> Thanks!\n> Ben\n> \n"},{"id":"390128","messageId":"nycvar.QRO.7.76.6.2001202210520.46@tvgsbejvaqbjf.bet","threadId":"52658","inReplyTo":"bfe569ea-93c7-382f-18fe-d81d8f9cbf4f@gmail.com","subject":"Re: [PATCH] commit: replace rebase/sequence booleans with single pick_state enum","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-01-20T21:11:49Z","receivedAt":"2020-01-20T21:12:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Mon, 20 Jan 2020, Phillip Wood wrote:\n\n> [Cc'ing dscho as it relates to issue management on gitgitgadget]\n\nDuplicating efforts is sadly a thing we cannot prevent, not even should we\nagree on a single issue tracker (we won't).\n\nCiao,\nDscho\n"}]}