All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ramkumar Ramachandra <artagnon@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Git List <git@vger.kernel.org>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Christian Couder <chriscool@tuxfamily.org>,
	Daniel Barkalow <barkalow@iabervon.org>,
	Jeff King <peff@peff.net>
Subject: [PATCH 5/7] revert: Make the argument parser responsible for setup_revisions
Date: Sun, 14 Aug 2011 14:03:07 +0530	[thread overview]
Message-ID: <1313310789-10216-6-git-send-email-artagnon@gmail.com> (raw)
In-Reply-To: <1313310789-10216-1-git-send-email-artagnon@gmail.com>

Currently, prepare_revs calls setup_revisions with the (argc, argv) in
the opts structure filled in by parse_args.  As a result, prepare_revs
has to take up the responsibility of erroring out and printing usage
information when (argc, argv) is malformed.  Since parse_args is doing
this for other kinds of parse errors anyway, give it the task of
calling setup_revisions as well.

Based-on-patch-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/revert.c |   53 +++++++++++++++++++++++++++++------------------------
 1 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 8faca07..e8bdeea 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -54,13 +54,14 @@ struct replay_opts {
 	int allow_rerere_auto;
 
 	int mainline;
-	int commit_argc;
-	const char **commit_argv;
 
 	/* Merge strategy */
 	const char *strategy;
 	const char **xopts;
 	size_t xopts_nr, xopts_alloc;
+
+	/* Only used by REPLAY_NONE */
+	struct rev_info *revs;
 };
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -163,9 +164,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 			die(_("program error"));
 	}
 
-	opts->commit_argc = parse_options(argc, argv, NULL, options, usage_str,
-					PARSE_OPT_KEEP_ARGV0 |
-					PARSE_OPT_KEEP_UNKNOWN);
+	argc = parse_options(argc, argv, NULL, options, usage_str,
+			PARSE_OPT_KEEP_ARGV0 |
+			PARSE_OPT_KEEP_UNKNOWN);
 
 	/* Check for incompatible subcommands */
 	verify_opt_mutually_compatible(me,
@@ -200,9 +201,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 				NULL);
 	}
 
-	else if (opts->commit_argc < 2)
-		usage_with_options(usage_str, options);
-
 	if (opts->allow_ff)
 		verify_opt_compatible(me, "--ff",
 				"--signoff", opts->signoff,
@@ -210,7 +208,20 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 				"-x", opts->record_origin,
 				"--edit", opts->edit,
 				NULL);
-	opts->commit_argv = argv;
+
+	if (opts->subcommand == REPLAY_NONE) {
+		opts->revs = xmalloc(sizeof(*opts->revs));
+		init_revisions(opts->revs, NULL);
+		opts->revs->no_walk = 1;
+		if (argc < 2)
+			usage_with_options(usage_str, options);
+		argc = setup_revisions(argc, argv, opts->revs, NULL);
+	} else
+		opts->revs = NULL;
+
+	/* Forbid stray command-line arguments */
+	if (argc > 1)
+		usage_with_options(usage_str, options);
 }
 
 struct commit_message {
@@ -612,23 +623,15 @@ static int do_pick_commit(struct commit *commit, enum replay_action action,
 	return res;
 }
 
-static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)
+static void prepare_revs(struct replay_opts *opts)
 {
-	int argc;
-
-	init_revisions(revs, NULL);
-	revs->no_walk = 1;
 	if (opts->action != REPLAY_REVERT)
-		revs->reverse = 1;
+		opts->revs->reverse ^= 1;
 
-	argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
-	if (argc > 1)
-		usage(*revert_or_cherry_pick_usage(opts));
-
-	if (prepare_revision_walk(revs))
+	if (prepare_revision_walk(opts->revs))
 		die(_("revision walk setup failed"));
 
-	if (!revs->commits)
+	if (!opts->revs->commits)
 		die(_("empty commit set passed"));
 }
 
@@ -823,14 +826,13 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
 static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
 				struct replay_opts *opts)
 {
-	struct rev_info revs;
 	struct commit *commit;
 	struct replay_insn_list **next;
 
-	prepare_revs(&revs, opts);
+	prepare_revs(opts);
 
 	next = todo_list;
-	while ((commit = get_revision(&revs)))
+	while ((commit = get_revision(opts->revs)))
 		next = replay_insn_list_append(opts->action, commit, next);
 }
 
@@ -954,6 +956,9 @@ static int pick_revisions(struct replay_opts *opts)
 	struct replay_insn_list *todo_list = NULL;
 	unsigned char sha1[20];
 
+	if (opts->subcommand == REPLAY_NONE)
+		assert(opts->revs);
+
 	read_and_refresh_cache(opts);
 
 	/*
-- 
1.7.6.351.gb35ac.dirty

  parent reply	other threads:[~2011-08-14  8:36 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-14  8:33 [PATCH v2 0/7] Generalized sequencer foundations Ramkumar Ramachandra
2011-08-14  8:33 ` [PATCH 1/7] revert: Free memory after get_message call Ramkumar Ramachandra
2011-08-14 16:15   ` Jonathan Nieder
2011-08-14  8:33 ` [PATCH 2/7] revert: Fix buffer overflow in insn sheet parser Ramkumar Ramachandra
2011-08-14 11:58   ` Jonathan Nieder
2011-08-14 14:07     ` Ramkumar Ramachandra
2011-08-14  8:33 ` [PATCH 3/7] revert: Make commit descriptions in insn sheet optional Ramkumar Ramachandra
2011-08-14 16:09   ` Jonathan Nieder
2011-08-14 16:21     ` Ramkumar Ramachandra
2011-08-14  8:33 ` [PATCH 4/7] revert: Allow mixed pick and revert instructions Ramkumar Ramachandra
2011-08-14 12:12   ` Jonathan Nieder
2011-08-14 14:06     ` Ramkumar Ramachandra
2011-08-14 14:28       ` Jonathan Nieder
2011-08-14  8:33 ` Ramkumar Ramachandra [this message]
2011-08-14 12:52   ` [PATCH 5/7] revert: Make the argument parser responsible for setup_revisions Jonathan Nieder
2011-08-14 13:43     ` Ramkumar Ramachandra
2011-08-14  8:33 ` [PATCH 6/7] sequencer: Expose API to cherry-picking machinery Ramkumar Ramachandra
2011-08-14 13:13   ` Jonathan Nieder
2011-08-14 13:57     ` Ramkumar Ramachandra
2011-08-14 15:22       ` Jonathan Nieder
2011-08-16 17:51         ` Junio C Hamano
2011-08-16 18:16           ` [PATCH v2] revert: plug memory leak in "cherry-pick root commit" codepath Jonathan Nieder
2011-08-16 18:27             ` Jonathan Nieder
2011-08-16 18:31             ` Jeff King
2011-08-16 18:56               ` Jonathan Nieder
2011-08-14  8:33 ` [PATCH 7/7] sequencer: Remove sequencer state after final commit Ramkumar Ramachandra
2011-08-14 16:04   ` Jonathan Nieder
2011-08-14 16:37     ` Ramkumar Ramachandra
2011-08-14 16:48       ` Jonathan Nieder
2011-08-14 21:13     ` Junio C Hamano
2011-08-14 21:32       ` Jonathan Nieder
2011-08-14 22:30         ` Junio C Hamano
2011-08-15 18:55           ` Junio C Hamano
2011-08-17 20:23             ` Ramkumar Ramachandra
2011-08-18 18:51             ` Ramkumar Ramachandra
2011-08-18 19:18               ` Jonathan Nieder
2011-08-18 19:50                 ` Ramkumar Ramachandra
2011-08-18 20:05                   ` Jonathan Nieder
2011-08-18 20:06                   ` Ramkumar Ramachandra
2011-08-18 20:12                     ` Jonathan Nieder
2011-08-18 20:22                   ` Junio C Hamano
2011-08-18 20:42                 ` Junio C Hamano
2011-08-19  9:08                   ` Ramkumar Ramachandra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1313310789-10216-6-git-send-email-artagnon@gmail.com \
    --to=artagnon@gmail.com \
    --cc=barkalow@iabervon.org \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.