Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> static inline int is_rebase_i(const struct replay_opts *opts)
> {
> - return 0;
> + return opts->action == REPLAY_INTERACTIVE_REBASE;
> }
>
> static const char *get_dir(const struct replay_opts *opts)
> {
> + if (is_rebase_i(opts))
> + return rebase_path();
> return git_path_seq_dir();
> }
>
> static const char *get_todo_path(const struct replay_opts *opts)
> {
> + if (is_rebase_i(opts))
> + return rebase_path_todo();
> return git_path_todo_file();
> }
>
> @@ -168,7 +179,15 @@ int sequencer_remove_state(struct replay_opts *opts)
>
> static const char *action_name(const struct replay_opts *opts)
> {
> - return opts->action == REPLAY_REVERT ? N_("revert") : N_("cherry-pick");
> + switch (opts->action) {
> + case REPLAY_REVERT:
> + return N_("revert");
> + case REPLAY_PICK:
> + return N_("cherry-pick");
> + case REPLAY_INTERACTIVE_REBASE:
> + return N_("rebase -i");
> + }
> + die(_("Unknown action: %d"), opts->action);
> }
This case statement which looks perfectly sensible---it says that
there are three equal modes the subsystem operates in.
This is just a mental note and not a suggestion to change anything
immediately, but it makes me wonder if git_dir/get_todo_path would
also want to do so, moving towards retiring is_rebase_i() which is
"everything else vs one oddball which is rebase-i" mindset.
> @@ -395,7 +414,10 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
>
> if (active_cache_changed &&
> write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
> - /* TRANSLATORS: %s will be "revert" or "cherry-pick" */
> + /*
> + * TRANSLATORS: %s will be "revert", "cherry-pick" or
> + * "rebase -i".
> + */
IIRC, the "TRANSLATORS:" comment has to deviate from our coding
style due to tool limitation and has to be done like this:
> + /* TRANSLATORS: %s will be "revert", "cherry-pick" or
> + * "rebase -i".
> + */
> @@ -1204,6 +1226,9 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
> const char *todo_path = get_todo_path(opts);
> int next = todo_list->current, offset, fd;
>
> + if (is_rebase_i(opts))
> + next++;
> +
This is because...? Everybody else counts 0-based while rebase-i
counts from 1 or something?
> fd = hold_lock_file_for_update(&todo_lock, todo_path, 0);
> if (fd < 0)
> return error_errno(_("could not lock '%s'"), todo_path);
Everything else in the patch is understandable. This bit isn't
without explanation, at least to me.