[PATCH v3 7/9] sequencer: simplify pick_one_commit()
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Jul 15, 2026, 15:22 UTC
- Message-ID
- <7c1642b0a49027a8851aa5985f4020d8e76414d1.1784128921.git.phillip.wood@dunelm.org.uk>
- In-Reply-To
- <cover.1784128921.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Unless we're rebasing, all we do in pick_one_commit() is call do_pick_commit() and return its result. Simplify the code by returning early if we're not rebasing so that we don't have to repeatedly call is_rebase_i() in the rest of the function. Note that there are a couple of conditions that do not call is_rebase_i() but they check for either an "edit" or a "fixup" command, both of which imply we're rebasing.
The only block that does not return early is the one guarded by "!res". Move the return into that block to make it clear that after recording the commit as rewritten, all we do is return from the function.
As the conditional blocks are all mutually exclusive (either the conditions are mutually exclusive, or an earlier conditional block that would match a later one contains a "return" statement) chain them together with "else if" to make that clear.
While we could remove "res" from the conditions below "if (!res)" they are left alone because, when we start using an enum in the next commit, it makes it clear that these clauses are handling cases where there are conflicts.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- sequencer.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/sequencer.c b/sequencer.c index 8f3eed205e7..9016af9b5d7 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4966,12 +4966,14 @@ static int pick_one_commit(struct repository *r, res = do_pick_commit(r, item, opts, is_final_fixup(todo_list), check_todo); - if (is_rebase_i(opts) && res < 0) { + if (!is_rebase_i(opts)) + return res; + + if (res < 0) { /* Reschedule */ *reschedule = 1; return -1; - } - if (item->command == TODO_EDIT) { + } else if (item->command == TODO_EDIT) { struct commit *commit = item->commit; if (!res) { if (!opts->verbose) @@ -4981,14 +4983,14 @@ static int pick_one_commit(struct repository *r, } return error_with_patch(r, commit, arg, item->arg_len, opts, res, !res); - } - if (is_rebase_i(opts) && !res) + } else if (!res) { record_in_rewritten(&item->commit->object.oid, peek_command(todo_list, 1)); - if (res && is_fixup(item->command)) { + return 0; + } else if (res && is_fixup(item->command)) { return error_failed_squash(r, item->commit, opts, item->arg_len, arg); - } else if (res && is_rebase_i(opts)) { + } else if (res) { int to_amend = 0; struct object_id oid; @@ -5008,7 +5010,8 @@ static int pick_one_commit(struct repository *r, return error_with_patch(r, item->commit, arg, item->arg_len, opts, res, to_amend); } - return res; + + BUG("Unhandled return value from do_pick_commit()"); } static int pick_commits(struct repository *r,
-- 2.54.0.200.gfd8d68259e3