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

[PATCH 5/8] revert: Catch incompatible command-line options early

From
Ramkumar Ramachandra <artagnon@gmail.com>
Date
May 11, 2011, 08:00 UTC
Message-ID
<1305100822-20470-6-git-send-email-artagnon@gmail.com>
In-Reply-To
<1305100822-20470-1-git-send-email-artagnon@gmail.com>

Earlier, incompatible command-line options used to be caught in pick_commits after parse_args has parsed the options and populated the options structure; a lot of unncessary work has already been done, and significant amount of cleanup is required to die at this stage. Instead, hand over this responsibility to parse_args so that the program can die early. Also write a die_opt_incompabile function to handle incompatible options in a general manner; it will be used more extensively as more command-line options are introduced later in the series.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 I think we _should_ die when an error in command-line parsing occurs,
 since it should always be the toplevel caller.  Thanks to Junio for
 redesigning die_opt_incompatible in a sane manner.
 builtin/revert.c |   37 ++++++++++++++++++++++++++-----------
 1 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 288c898..0fe87e8 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -80,10 +80,29 @@ static int option_parse_x(const struct option *opt,
 	return 0;
 }
 
+static void die_opt_incompatible(const char *me, const char *base_opt, ...)
+{
+	const char *this_opt;
+	int this_opt_set;
+	va_list ap;
+
+	va_start(ap, base_opt);
+	while (1) {
+		if (!(this_opt = va_arg(ap, const char *)))
+			break;
+		if ((this_opt_set = va_arg(ap, int)))
+			die(_("%s: %s cannot be used with %s"),
+				me, this_opt, base_opt);
+	}
+	va_end(ap);
+}
+
 static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 {
 	const char *const *usage_str = revert_or_cherry_pick_usage(opts);
+	const char *me = (opts->action == REVERT ? "revert" : "cherry-pick");
 	int noop;
+
 	struct option options[] = {
 		OPT_BOOLEAN('n', "no-commit", &(opts->no_commit), "don't automatically commit"),
 		OPT_BOOLEAN('e', "edit", &(opts->edit), "edit the commit message"),
@@ -121,6 +140,13 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 	if (opts->commit_argc < 2)
 		usage_with_options(usage_str, options);
 
+	if (opts->allow_ff)
+		die_opt_incompatible(me, "--ff",
+				"--signoff", opts->signoff,
+				"--no-commit", opts->no_commit,
+				"-x", opts->no_replay,
+				"--edit", opts->edit,
+				NULL);
 	opts->commit_argv = argv;
 }
 
@@ -609,17 +635,6 @@ static int pick_commits(struct replay_opts *opts)
 	struct commit *commit;
 	int res;
 
-	if (opts->allow_ff) {
-		if (opts->signoff)
-			die(_("cherry-pick --ff cannot be used with --signoff"));
-		if (opts->no_commit)
-			die(_("cherry-pick --ff cannot be used with --no-commit"));
-		if (opts->no_replay)
-			die(_("cherry-pick --ff cannot be used with -x"));
-		if (opts->edit)
-			die(_("cherry-pick --ff cannot be used with --edit"));
-	}
-
 	if ((res = read_and_refresh_cache(opts)) ||
 		(res = prepare_revs(&revs, opts)))
 		return res;
-- 
1.7.5.GIT
Previous: Jonathan NiederNext: Jonathan Nieder
Message 20 of 39 in “Sequencer Foundations”
  1. 0/8 Sequencer FoundationsRamkumar Ramachandra, May 11, 2011
  2. 1/8 revert: Improve error handling by cascading errors upwardsRamkumar Ramachandra, May 11, 2011
  3. Jonathan NiederMay 11, 2011
  4. Ramkumar RamachandraMay 13, 2011
  5. Ramkumar RamachandraMay 19, 2011
  6. 2/8 revert: Make "commit" and "me" local variablesRamkumar Ramachandra, May 11, 2011
  7. Jonathan NiederMay 11, 2011
  8. Ramkumar RamachandraMay 13, 2011
  9. Daniel BarkalowMay 13, 2011
  10. 3/8 revert: Introduce a struct to parse command-line options intoRamkumar Ramachandra, May 11, 2011
  11. Jonathan NiederMay 11, 2011
  12. Ramkumar RamachandraMay 13, 2011
  13. Jonathan NiederMay 13, 2011
  14. Ramkumar RamachandraMay 13, 2011
  15. 4/8 revert: Separate cmdline argument handling from the functional codeRamkumar Ramachandra, May 11, 2011
  16. Jonathan NiederMay 11, 2011
  17. Ramkumar RamachandraMay 13, 2011
  18. Ramkumar RamachandraMay 13, 2011
  19. Jonathan NiederMay 13, 2011
  20. 5/8 revert: Catch incompatible command-line options earlyRamkumar Ramachandra, May 11, 2011
  21. Jonathan NiederMay 11, 2011
  22. Ramkumar RamachandraMay 13, 2011
  23. 6/8 revert: Introduce head, todo, done files to persist stateRamkumar Ramachandra, May 11, 2011
  24. Jonathan NiederMay 11, 2011
  25. Ramkumar RamachandraMay 13, 2011
  26. 7/8 revert: Implement parsing --continue, --abort and --skipRamkumar Ramachandra, May 11, 2011
  27. Jonathan NiederMay 11, 2011
  28. Ramkumar RamachandraMay 13, 2011
  29. Jonathan NiederMay 13, 2011
  30. 8/8 revert: Implement --abort processingRamkumar Ramachandra, May 11, 2011
  31. Jonathan NiederMay 11, 2011
  32. Christian CouderMay 12, 2011
  33. Jonathan NiederMay 12, 2011
  34. Jonathan NiederMay 12, 2011
  35. Christian CouderMay 13, 2011
  36. Jonathan NiederMay 13, 2011
  37. Christian CouderMay 16, 2011
  38. Jonathan NiederMay 19, 2011
  39. Ramkumar RamachandraMay 20, 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.