Re: [PATCH 2/2] pull --rebase: add --[no-]autostash flag
Hello Eric,
I tried out this approach and here's the result.
---
diff --git a/builtin/pull.c b/builtin/pull.c
index b5b0255..0ce007d 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static char *opt_commit;
static char *opt_edit;
static char *opt_ff;
static char *opt_verify_signatures;
+static int opt_autostash = -1;
static int config_autostash;
static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
@@ -150,6 +151,8 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "verify-signatures", &opt_verify_signatures, NULL,
N_("verify that the named commit has a valid GPG signature"),
PARSE_OPT_NOARG),
+ OPT_BOOL(0, "autostash", &opt_autostash,
+ N_("automatically stash/stash pop before and after rebase")),
OPT_PASSTHRU_ARGV('s', "strategy", &opt_strategies, N_("strategy"),
N_("merge strategy to use"),
0),
@@ -801,6 +804,7 @@ static int run_rebase(const unsigned char *curr_head,
argv_array_pushv(&args, opt_strategy_opts.argv);
if (opt_gpg_sign)
argv_array_push(&args, opt_gpg_sign);
+ argv_array_push(&args, opt_autostash ? "--autostash" :
"--no-autostash");
argv_array_push(&args, "--onto");
argv_array_push(&args, sha1_to_hex(merge_head));
@@ -846,12 +850,20 @@ int cmd_pull(int argc, const char **argv, const
char *prefix)
if (get_sha1("HEAD", orig_head))
hashclr(orig_head);
+ if(!opt_rebase && opt_autostash != -1)
+ die(_("--[no-]autostash option is only valid with --rebase."));
+
if (opt_rebase) {
int autostash = config_autostash;
if (is_null_sha1(orig_head) && !is_cache_unborn())
die(_("Updating an unborn branch with changes
added to the index."));
+ if (opt_autostash != -1)
+ autostash = opt_autostash;
+ else
+ opt_autostash = config_autostash;
+
if (!autostash)
die_on_unclean_work_tree(prefix);
---
This way of implementation looks a bit less clean to me than
the previous one because we are using "opt_autostash" to pass
the "--[no-]autostash" flag to git-rebase, thus if user does not
specify anything about stashing in command line then config_autostash
value has to be used ( i.e. opt_autostash = config_autostash).
To do this an "else" case has to be introduced in the code. This
might effect the readability of the code because the reader might
wonder why "opt_autostash" is used to assign value to "autostash"
in one case, and opt_autostash = config_autostash in other case.
But I agree that this way I won't be touching the changes I made
in patch 1/2.
I would like to know your view on above mentioned issue.
Also I made a mistake in patch 1/2 which I will correct in the next
version along with other changes suggested by you.
Thanks,
Mehul
Mehul Jain <mehul.jain2029@gmail.com>