Hi,
On 12/14/2016 08:29 PM, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
>> -/* We will introduce the 'interactive rebase' mode later */
>> 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();
>> }
>
> This obviously makes the assumption made by 39784cd362 ("sequencer:
> remove useless get_dir() function", 2016-12-07) invalid, where the
> "remove useless" thought that the callers of this function wants a
> single answer that does not depend on opts.
>
> I'll revert that commit from the sb/sequencer-abort-safety topic (as
> the topic is in 'next' already) to keep this one. Please holler if
> that is a mistaken decision.
It seems to be the right decision if one wants to keep the internal-path
backwards-compatibility of "git rebase -i" (and it seems that Dscho
wants to keep it).
However, maintaining more than one directory (like "sequencer" for
sequencer state and "rebase-merge" for rebase todo and log) can cause
the necessity to be even more careful when hacking on sequencer... For
example, the cleanup code must delete both of them, not only one of them.
Hence, I think that it's a wiser choice to keep one directory for
sequencer state *and* additional files.
If we (a) save everything in "sequencer", we break the internal-path
backwards-compatbility but we can get rid of get_dir()...
If we (b) save everything in "rebase-merge" when using rebase -i and
save everything in "sequencer" for pick/revert, we are 100%
backwards-compatible, but we have to change every occurrence of
git_path_seq_dir() to get_dir() and the GIT_PATH_FUNC definitions of
git_path_{todo,opts,head}_file must be replaced by definitions using
get_dir().
Best
Stephan