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

[PATCH 1/5] diff: parse detached options like -S foo

From
Matthieu Moy <matthieu.moy@imag.fr>
Date
Jul 29, 2010, 08:20 UTC
Message-ID
<1280391629-30017-2-git-send-email-Matthieu.Moy@imag.fr>
In-Reply-To
<1280391629-30017-1-git-send-email-Matthieu.Moy@imag.fr>

Change the option parsing logic in revision.c to accept detached forms like `-S foo' in addition to `-Sfoo'. The rest of git already accepted this form, but revision.c still used its own option parsing.

This patch does not handle --stat-name-width and --stat-width, which are special-cases where diff_long_opt do not apply. They are handled in a separate patch to ease review.

Original patch by Matthieu Moy, plus refactoring by Jonathan Nieder.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 diff.c                       |   87 ++++++++++++++++++++++++++++++++++--------
 diff.h                       |    7 +++
 t/t4013-diff-various.sh      |    5 ++
 t/t4013/diff.log_-S_F_master |    7 +++
 t/t4202-log.sh               |   12 ++---
 5 files changed, 95 insertions(+), 23 deletions(-)
 create mode 100644 t/t4013/diff.log_-S_F_master
diff --git a/diff.c b/diff.c
index 17873f3..d89ea20 100644
--- a/diff.c
+++ b/diff.c
@@ -2990,9 +2990,50 @@ static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *va
 
 static int diff_scoreopt_parse(const char *opt);
 
+static inline int short_opt(char opt, const char **argv,
+			    const char **optarg)
+{
+	const char *arg = argv[0];
+	if (arg[0] != '-' || arg[1] != opt)
+		return 0;
+	if (arg[2] != '\0') {
+		*optarg = arg + strlen("-c");
+		return 1;
+	}
+	if (!argv[1])
+		die("Option '%c' requires a value", opt);
+	*optarg = argv[1];
+	return 2;
+}
+
+int diff_long_opt(const char *opt, const char **argv,
+		  const char **optarg)
+{
+	const char *arg = argv[0];
+	if (arg[0] != '-' || arg[1] != '-')
+		return 0;
+	arg += strlen("--");
+	if (prefixcmp(arg, opt))
+		return 0;
+	arg += strlen(opt);
+	if (*arg == '=') { /* Sticky form: --option=value */
+		*optarg = arg + 1;
+		return 1;
+	}
+	if (*arg != '\0')
+		return 0;
+	/* detached form: --option value */
+	if (!argv[1])
+		die("Option '--%s' requires a value", opt);
+	*optarg = argv[1];
+	return 2;
+}
+
 int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 {
 	const char *arg = av[0];
+	const char *optarg;
+	int argcount;
 
 	/* Output format options */
 	if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch"))
@@ -3149,10 +3190,11 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		else
 			die("bad --word-diff argument: %s", type);
 	}
-	else if (!prefixcmp(arg, "--word-diff-regex=")) {
+	else if ((argcount = diff_long_opt("word-diff-regex", av, &optarg))) {
 		if (options->word_diff == DIFF_WORDS_NONE)
 			options->word_diff = DIFF_WORDS_PLAIN;
-		options->word_regex = arg + 18;
+		options->word_regex = optarg;
+		return argcount;
 	}
 	else if (!strcmp(arg, "--exit-code"))
 		DIFF_OPT_SET(options, EXIT_WITH_STATUS);
@@ -3180,18 +3222,26 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 	/* misc options */
 	else if (!strcmp(arg, "-z"))
 		options->line_termination = 0;
-	else if (!prefixcmp(arg, "-l"))
-		options->rename_limit = strtoul(arg+2, NULL, 10);
-	else if (!prefixcmp(arg, "-S"))
-		options->pickaxe = arg + 2;
+	else if ((argcount = short_opt('l', av, &optarg))) {
+		options->rename_limit = strtoul(optarg, NULL, 10);
+		return argcount;
+	}
+	else if ((argcount = short_opt('S', av, &optarg))) {
+		options->pickaxe = optarg;
+		return argcount;
+	}
 	else if (!strcmp(arg, "--pickaxe-all"))
 		options->pickaxe_opts = DIFF_PICKAXE_ALL;
 	else if (!strcmp(arg, "--pickaxe-regex"))
 		options->pickaxe_opts = DIFF_PICKAXE_REGEX;
-	else if (!prefixcmp(arg, "-O"))
-		options->orderfile = arg + 2;
-	else if (!prefixcmp(arg, "--diff-filter="))
-		options->filter = arg + 14;
+	else if ((argcount = short_opt('O', av, &optarg))) {
+		options->orderfile = optarg;
+		return argcount;
+	}
+	else if ((argcount = diff_long_opt("diff-filter", av, &optarg))) {
+		options->filter = optarg;
+		return argcount;
+	}
 	else if (!strcmp(arg, "--abbrev"))
 		options->abbrev = DEFAULT_ABBREV;
 	else if (!prefixcmp(arg, "--abbrev=")) {
@@ -3201,20 +3251,25 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		else if (40 < options->abbrev)
 			options->abbrev = 40;
 	}
-	else if (!prefixcmp(arg, "--src-prefix="))
-		options->a_prefix = arg + 13;
-	else if (!prefixcmp(arg, "--dst-prefix="))
-		options->b_prefix = arg + 13;
+	else if ((argcount = diff_long_opt("src-prefix", av, &optarg))) {
+		options->a_prefix = optarg;
+		return argcount;
+	}
+	else if ((argcount = diff_long_opt("dst-prefix", av, &optarg))) {
+		options->b_prefix = optarg;
+		return argcount;
+	}
 	else if (!strcmp(arg, "--no-prefix"))
 		options->a_prefix = options->b_prefix = "";
 	else if (opt_arg(arg, '\0', "inter-hunk-context",
 			 &options->interhunkcontext))
 		;
-	else if (!prefixcmp(arg, "--output=")) {
-		options->file = fopen(arg + strlen("--output="), "w");
+	else if ((argcount = diff_long_opt("output", av, &optarg))) {
+		options->file = fopen(optarg, "w");
 		if (!options->file)
 			die_errno("Could not open '%s'", arg + strlen("--output="));
 		options->close_file = 1;
+		return argcount;
 	} else
 		return 0;
 	return 1;
diff --git a/diff.h b/diff.h
index 063d10a..3b11732 100644
--- a/diff.h
+++ b/diff.h
@@ -214,6 +214,13 @@ extern void diff_unmerge(struct diff_options *,
 #define DIFF_SETUP_USE_CACHE		2
 #define DIFF_SETUP_USE_SIZE_CACHE	4
 
+/*
+ * Poor man's alternative to parse-option, to allow both sticky form
+ * (--option=value) and detached form (--option value).
+ */
+extern int diff_long_opt(const char *opt, const char **argv,
+			 const char **optarg);
+
 extern int git_diff_basic_config(const char *var, const char *value, void *cb);
 extern int git_diff_ui_config(const char *var, const char *value, void *cb);
 extern int diff_use_color_default;
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index dae6358..19857f4 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -208,6 +208,7 @@ log -p --first-parent master
 log -m -p --first-parent master
 log -m -p master
 log -SF master
+log -S F master
 log -SF -p master
 log --decorate --all
 log --decorate=full --all
@@ -282,4 +283,8 @@ diff master master^ side
 diff --dirstat master~1 master~2
 EOF
 
+test_expect_success 'log -S requires an argument' '
+	test_must_fail git log -S
+'
+
 test_done
diff --git a/t/t4013/diff.log_-S_F_master b/t/t4013/diff.log_-S_F_master
new file mode 100644
index 0000000..978d2b4
--- /dev/null
+++ b/t/t4013/diff.log_-S_F_master
@@ -0,0 +1,7 @@
+$ git log -S F master
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+$
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 2230e60..36870c5 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -100,13 +100,11 @@ test_expect_success 'oneline' '
 
 test_expect_success 'diff-filter=A' '
 
-	actual=$(git log --pretty="format:%s" --diff-filter=A HEAD) &&
-	expect=$(echo fifth ; echo fourth ; echo third ; echo initial) &&
-	test "$actual" = "$expect" || {
-		echo Oops
-		echo "Actual: $actual"
-		false
-	}
+	git log --pretty="format:%s" --diff-filter=A HEAD > actual &&
+	git log --pretty="format:%s" --diff-filter A HEAD > actual-detached &&
+	printf "fifth\nfourth\nthird\ninitial" > expect &&
+	test_cmp expect actual &&
+	test_cmp expect actual-detached
 
 '
 

-- 
1.7.2.21.ge9796
Previous: Matthieu MoyNext: Matthieu Moy
Message 2 of 25 in “log and diff: accept detached forms (--option value)”
  1. 0/5 log and diff: accept detached forms (--option value)Matthieu Moy, Jul 29, 2010
  2. 1/5 diff: parse detached options like -S fooMatthieu Moy, Jul 29, 2010
  3. 2/5 diff: split off a function for --stat-* option parsingMatthieu Moy, Jul 29, 2010
  4. 3/5 diff: parse detached options --stat-width n, --stat-name-width nMatthieu Moy, Jul 29, 2010
  5. 4/5 log: parse detached options like git log --grep fooMatthieu Moy, Jul 29, 2010
  6. Jonathan NiederJul 29, 2010
  7. Matthieu MoyJul 30, 2010
  8. 1/5 diff: parse detached options like -S fooMatthieu Moy, Jul 30, 2010
  9. Junio C HamanoAug 2, 2010
  10. Matthieu MoyAug 2, 2010
  11. Jonathan NiederAug 2, 2010
  12. 2/5 diff: split off a function for --stat-* option parsingMatthieu Moy, Jul 30, 2010
  13. 3/5 diff: parse detached options --stat-width n, --stat-name-width nMatthieu Moy, Jul 30, 2010
  14. Junio C HamanoAug 2, 2010
  15. Matthieu MoyAug 2, 2010
  16. 4/5 log: parse detached options like git log --grep fooMatthieu Moy, Jul 30, 2010
  17. Junio C HamanoAug 2, 2010
  18. Matthieu MoyAug 2, 2010
  19. Junio C HamanoAug 2, 2010
  20. Matthieu MoyAug 3, 2010
  21. Junio C HamanoAug 3, 2010
  22. Matthieu MoyAug 3, 2010
  23. Miles BaderAug 4, 2010
  24. 5/5 log: parse detached option for --globMatthieu Moy, Jul 30, 2010
  25. 5/5 log: parse detached option for --globMatthieu Moy, Jul 29, 2010

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.