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

[PATCH 5/7] revert: Make the argument parser responsible for setup_revisions

From
Ramkumar Ramachandra <artagnon@gmail.com>
Date
Aug 14, 2011, 08:33 UTC
Message-ID
<1313310789-10216-6-git-send-email-artagnon@gmail.com>
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
Previous: Jonathan NiederNext: Jonathan Nieder
Message 14 of 43 in “Generalized sequencer foundations”
  1. 0/7 Generalized sequencer foundationsRamkumar Ramachandra, Aug 14, 2011
  2. 1/7 revert: Free memory after get_message callRamkumar Ramachandra, Aug 14, 2011
  3. Jonathan NiederAug 14, 2011
  4. 2/7 revert: Fix buffer overflow in insn sheet parserRamkumar Ramachandra, Aug 14, 2011
  5. Jonathan NiederAug 14, 2011
  6. Ramkumar RamachandraAug 14, 2011
  7. 3/7 revert: Make commit descriptions in insn sheet optionalRamkumar Ramachandra, Aug 14, 2011
  8. Jonathan NiederAug 14, 2011
  9. Ramkumar RamachandraAug 14, 2011
  10. 4/7 revert: Allow mixed pick and revert instructionsRamkumar Ramachandra, Aug 14, 2011
  11. Jonathan NiederAug 14, 2011
  12. Ramkumar RamachandraAug 14, 2011
  13. Jonathan NiederAug 14, 2011
  14. 5/7 revert: Make the argument parser responsible for setup_revisionsRamkumar Ramachandra, Aug 14, 2011
  15. Jonathan NiederAug 14, 2011
  16. Ramkumar RamachandraAug 14, 2011
  17. 6/7 sequencer: Expose API to cherry-picking machineryRamkumar Ramachandra, Aug 14, 2011
  18. Jonathan NiederAug 14, 2011
  19. Ramkumar RamachandraAug 14, 2011
  20. Jonathan NiederAug 14, 2011
  21. Junio C HamanoAug 16, 2011
  22. revert: plug memory leak in "cherry-pick root commit" codepathJonathan Nieder, Aug 16, 2011
  23. Jonathan NiederAug 16, 2011
  24. Jeff KingAug 16, 2011
  25. Jonathan NiederAug 16, 2011
  26. 7/7 sequencer: Remove sequencer state after final commitRamkumar Ramachandra, Aug 14, 2011
  27. Jonathan NiederAug 14, 2011
  28. Ramkumar RamachandraAug 14, 2011
  29. Jonathan NiederAug 14, 2011
  30. Junio C HamanoAug 14, 2011
  31. Jonathan NiederAug 14, 2011
  32. Junio C HamanoAug 14, 2011
  33. Junio C HamanoAug 15, 2011
  34. Ramkumar RamachandraAug 17, 2011
  35. Ramkumar RamachandraAug 18, 2011
  36. Jonathan NiederAug 18, 2011
  37. Ramkumar RamachandraAug 18, 2011
  38. Jonathan NiederAug 18, 2011
  39. Ramkumar RamachandraAug 18, 2011
  40. Jonathan NiederAug 18, 2011
  41. Junio C HamanoAug 18, 2011
  42. Junio C HamanoAug 18, 2011
  43. Ramkumar RamachandraAug 19, 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.