[07/20] diff-parseopt: convert -O

Commit Message

Duy Nguyen March 20, 2019, 11:46 a.m. UTC

Signed-off-by: Nguyễn Thái Ngọc Duy <[e-mail]>
---
 diff.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

Comments

Martin Ågren March 20, 2019, 8:26 p.m. UTC | #1

On Wed, 20 Mar 2019 at 12:48, Nguyễn Thái Ngọc Duy <[e-mail]> wrote:
> +               OPT_FILENAME('O', NULL, &options->orderfile,
> +                            N_("override diff.orderFile configuration variable")),

This doesn't really tell me *why* I would want to provide -O. Or put
another way, there are lots of --foo-bar for overriding some baz.fooBar
config item, but they're not described like this. The primary purpose of
--foo-bar, at least to me, would be to solve some actual problem, like
"fooing the bar", whatever that means.

Gleaning at the manpage, an alternative would be "control the order in
which files appear in the output", but maybe you avoided that
deliberately? "provide an orderfile"?


Martin

Patch

diff --git a/diff.c b/diff.c
index 639b166c79..21c7a6b1a1 100644
--- a/diff.c
+++ b/diff.c
@@ -4617,22 +4617,6 @@  static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *va
 	return 1;
 }
 
-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 + 2;
-		return 1;
-	}
-	if (!argv[1])
-		die("Option '%c' requires a value", opt);
-	*optarg = argv[1];
-	return 2;
-}
-
 int parse_long_opt(const char *opt, const char **argv,
 		   const char **optarg)
 {
@@ -5397,6 +5381,8 @@  static void prep_parse_options(struct diff_options *options)
 		OPT_BIT_F(0, "pickaxe-regex", &options->pickaxe_opts,
 			  N_("treat <string> in -S as extended POSIX regular expression"),
 			  DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG),
+		OPT_FILENAME('O', NULL, &options->orderfile,
+			     N_("override diff.orderFile configuration variable")),
 		{ OPTION_CALLBACK, 0, "output", options, N_("<file>"),
 		  N_("Output to a specific file"),
 		  PARSE_OPT_NONEG, NULL, 0, diff_opt_output },
@@ -5449,10 +5435,7 @@  int diff_opt_parse(struct diff_options *options,
 	}
 
 	/* misc options */
-	else if ((argcount = short_opt('O', av, &optarg))) {
-		options->orderfile = prefix_filename(prefix, optarg);
-		return argcount;
-	} else if (skip_prefix(arg, "--find-object=", &arg))
+	else if (skip_prefix(arg, "--find-object=", &arg))
 		return parse_objfind_opt(options, arg);
 	else if ((argcount = parse_long_opt("diff-filter", av, &optarg))) {
 		int offending = parse_diff_filter_opt(optarg, options);