git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 6/6] revert: Simplify passing command-line arguments around

From
Ramkumar Ramachandra <artagnon@gmail.com>
Date
Oct 8, 2011, 17:36 UTC
Message-ID
<1318095407-26429-7-git-send-email-artagnon@gmail.com>
In-Reply-To
<1318095407-26429-1-git-send-email-artagnon@gmail.com>
From: Jonathan Nieder <jrnieder@gmail.com>

The chief command-line argument parser, parse_args, currently parses arguments into an (argc, argv) and lets prepare_revs, a later function, turn it into a structure. Change this so that callers of the cherry-pick machinery will have to fill in an options structure instead of crafting command-line arguments.

[rr: minor improvements, commit message]
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/revert.c |   53 +++++++++++++++++++++++++++++------------------------
 1 files changed, 29 insertions(+), 24 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index a9dd210..a72b20d 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 {
@@ -607,23 +618,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"));
 }
 
@@ -817,14 +820,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);
 }
 
@@ -948,6 +950,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.4.1
Previous: Ramkumar RamachandraNext: Tay Ray Chuan
Message 10 of 16 in “Sequencer fixups mini-series”
  1. 0/6 Sequencer fixups mini-seriesRamkumar Ramachandra, Oct 8, 2011
  2. 1/6 revert: Free memory after get_message callRamkumar Ramachandra, Oct 8, 2011
  3. 2/6 revert: Simplify getting commit subjectRamkumar Ramachandra, Oct 8, 2011
  4. 3/6 revert: Fix buffer overflow in insn sheet parserRamkumar Ramachandra, Oct 8, 2011
  5. 4/6 revert: Make commit descriptions in insn sheet optionalRamkumar Ramachandra, Oct 8, 2011
  6. Junio C HamanoOct 10, 2011
  7. Junio C HamanoOct 12, 2011
  8. Ramkumar RamachandraOct 17, 2011
  9. 5/6 revert: Allow mixed pick and revert instructionsRamkumar Ramachandra, Oct 8, 2011
  10. 6/6 revert: Simplify passing command-line arguments aroundRamkumar Ramachandra, Oct 8, 2011
  11. Tay Ray ChuanOct 9, 2011
  12. Ramkumar RamachandraOct 9, 2011
  13. Jonathan NiederOct 9, 2011
  14. Ramkumar RamachandraOct 9, 2011
  15. Jonathan NiederOct 9, 2011
  16. Ramkumar RamachandraOct 17, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.