Hi Alban
sorry for the slow reply, I think you're probably off-list for a while
by now
On 21/03/2019 21:13, Alban Gruin wrote:
Hi Phillip,
It’s nice to see your work on this on the list.
Le 19/03/2019 à 20:03, Phillip Wood a écrit :
From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
In order to run `rebase -i` without forking `rebase--interactive` it
will be convenient to use the same structure when parsing the options in
cmd_rebase() and cmd_rebase__interactive().
Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
---
builtin/rebase.c | 203 ++++++++++++++++++++++++++---------------------
1 file changed, 112 insertions(+), 91 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index c93f2aa629..33a2495032 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -50,6 +50,73 @@ enum rebase_type {
REBASE_PRESERVE_MERGES
};
+struct rebase_options {
+ enum rebase_type type;
+ const char *state_dir;
+ struct commit *upstream;
+ const char *upstream_name;
+ const char *upstream_arg;
+ char *head_name;
+ struct object_id orig_head;
+ struct commit *onto;
+ const char *onto_name;
+ const char *revisions;
+ const char *switch_to;
+ int root;
+ struct object_id *squash_onto;
+ struct commit *restrict_revision;
+ int dont_finish_rebase;
+ enum {
+ REBASE_NO_QUIET = 1<<0,
+ REBASE_VERBOSE = 1<<1,
+ REBASE_DIFFSTAT = 1<<2,
+ REBASE_FORCE = 1<<3,
+ REBASE_INTERACTIVE_EXPLICIT = 1<<4,
+ } flags;
+ struct argv_array git_am_opts;
+ const char *action;
+ int signoff;
+ int allow_rerere_autoupdate;
+ int keep_empty;
+ int autosquash;
+ char *gpg_sign_opt;
+ int autostash;
+ char *cmd;
+ int allow_empty_message;
+ int rebase_merges, rebase_cousins;
+ char *strategy, *strategy_opts;
+ struct strbuf git_format_patch_opt;
+ int reschedule_failed_exec;
+};
+
+#define REBASE_OPTIONS_INIT { \
+ .type = REBASE_UNSPECIFIED, \
+ .flags = REBASE_NO_QUIET, \
+ .git_am_opts = ARGV_ARRAY_INIT, \
+ .git_format_patch_opt = STRBUF_INIT \
+ }
+
+static struct replay_opts get_replay_opts(const struct rebase_options *opts)
+{
+ struct replay_opts replay = REPLAY_OPTS_INIT;
+
+ sequencer_init_config(&replay);
+
+ replay.action = REPLAY_INTERACTIVE_REBASE;
+ replay.signoff = opts->signoff;
+ replay.allow_ff = !(opts->flags & REBASE_FORCE);
+ if (opts->allow_rerere_autoupdate)
+ replay.allow_rerere_auto = opts->allow_rerere_autoupdate;
+ replay.allow_empty = 1;
+ replay.allow_empty_message = opts->allow_empty_message;
+ replay.verbose = opts->flags & REBASE_VERBOSE;
+ replay.reschedule_failed_exec = opts->reschedule_failed_exec;
+ replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt);
+ replay.strategy = opts->strategy;
+
+ return replay;
+}
+
I wonder if `struct rebase_options` and `struct replay_options` could be
merged, or at least have `replay_options` used in `rebase_options`,
instead of converting one to the other. I think it would make things
simpler and cleaner, but I don’t know how hard it would be, or if my
assumption is correct.
I did consider doing that, but there are a few subtle differences in the
way the options are stored in each struct (eg the gpg option has a
leading -S in struct rebase_options but not struct replay_options) and I
think it would be a bit of a faff to align them as it would mean messing
with the code uses them and the code that reads/writes the various state
files (we cannot change the on-disk format without breaking things -
sometimes users start a rebase with one version of git bundled with
something like tig and then continue on the command-line with a
different version). I'd prefer to leave it as a future cleanup once this
series has been merged.
Best Wishes
Phillip
Cheers,
Alban
