git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] Use OPT_SET_INT_F() for cmdline option specification
@ 2018-05-20  8:12 Nguyễn Thái Ngọc Duy
  2018-05-20  9:15 ` Martin Ågren
  2018-05-20 15:42 ` [PATCH v2] " Nguyễn Thái Ngọc Duy
  0 siblings, 2 replies; 4+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-20  8:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

The only thing these commands need is extra parseopt flags which can be
passed in by OPT_SET_INT_F() and it is a bit more compact than full
struct initialization.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 archive.c              |  6 ++----
 builtin/am.c           | 12 ++++++------
 builtin/branch.c       |  4 ++--
 builtin/difftool.c     |  9 ++++-----
 builtin/fetch.c        |  6 +++---
 builtin/grep.c         |  6 +++---
 builtin/log.c          |  6 +++---
 builtin/ls-files.c     |  6 +++---
 builtin/merge.c        |  6 +++---
 builtin/notes.c        | 12 ++++++------
 builtin/pack-objects.c | 24 ++++++++++++------------
 11 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/archive.c b/archive.c
index 93ab175b0b..4fe7bec60c 100644
--- a/archive.c
+++ b/archive.c
@@ -411,11 +411,9 @@ static void parse_treeish_arg(const char **argv,
 }
 
 #define OPT__COMPR(s, v, h, p) \
-	{ OPTION_SET_INT, (s), NULL, (v), NULL, (h), \
-	  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, (p) }
+	OPT_SET_INT_F(s, NULL, v, h, p, PARSE_OPT_NONEG)
 #define OPT__COMPR_HIDDEN(s, v, p) \
-	{ OPTION_SET_INT, (s), NULL, (v), NULL, "", \
-	  PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN, NULL, (p) }
+	OPT_SET_INT_F(s, NULL, v, "", p, PARSE_OPT_NONEG | PARSE_OPT_HIDDEN)
 
 static int parse_archive_args(int argc, const char **argv,
 		const struct archiver **ar, struct archiver_args *args,
diff --git a/builtin/am.c b/builtin/am.c
index d834f9e62b..666287b497 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2231,12 +2231,12 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 			N_("pass -b flag to git-mailinfo"), KEEP_NON_PATCH),
 		OPT_BOOL('m', "message-id", &state.message_id,
 			N_("pass -m flag to git-mailinfo")),
-		{ OPTION_SET_INT, 0, "keep-cr", &keep_cr, NULL,
-		  N_("pass --keep-cr flag to git-mailsplit for mbox format"),
-		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},
-		{ OPTION_SET_INT, 0, "no-keep-cr", &keep_cr, NULL,
-		  N_("do not pass --keep-cr flag to git-mailsplit independent of am.keepcr"),
-		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0},
+		OPT_SET_INT_F(0, "keep-cr", &keep_cr,
+			N_("pass --keep-cr flag to git-mailsplit for mbox format"),
+			1, PARSE_OPT_NONEG),
+		OPT_SET_INT_F(0, "no-keep-cr", &keep_cr,
+			N_("do not pass --keep-cr flag to git-mailsplit for mbox format"),
+			0, PARSE_OPT_NONEG),
 		OPT_BOOL('c', "scissors", &state.scissors,
 			N_("strip everything before a scissors line")),
 		OPT_PASSTHRU_ARGV(0, "whitespace", &state.git_apply_opts, N_("action"),
diff --git a/builtin/branch.c b/builtin/branch.c
index efc9ac1922..cc089f9efb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -592,8 +592,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT__QUIET(&quiet, N_("suppress informational messages")),
 		OPT_SET_INT('t', "track",  &track, N_("set up tracking mode (see git-pull(1))"),
 			BRANCH_TRACK_EXPLICIT),
-		{ OPTION_SET_INT, 0, "set-upstream", &track, NULL, N_("do not use"),
-			PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, BRANCH_TRACK_OVERRIDE },
+		OPT_SET_INT_F(0, "set-upstream", &track, N_("do not use"),
+			BRANCH_TRACK_OVERRIDE, PARSE_OPT_HIDDEN),
 		OPT_STRING('u', "set-upstream-to", &new_upstream, N_("upstream"), N_("change the upstream info")),
 		OPT_BOOL(0, "unset-upstream", &unset_upstream, N_("Unset the upstream info")),
 		OPT__COLOR(&branch_use_color, N_("use colored output")),
diff --git a/builtin/difftool.c b/builtin/difftool.c
index aad0e073ee..c439b64207 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -695,12 +695,11 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)
 			 N_("use `diff.guitool` instead of `diff.tool`")),
 		OPT_BOOL('d', "dir-diff", &dir_diff,
 			 N_("perform a full-directory diff")),
-		{ OPTION_SET_INT, 'y', "no-prompt", &prompt, NULL,
+		OPT_SET_INT_F('y', "no-prompt", &prompt,
 			N_("do not prompt before launching a diff tool"),
-			PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0},
-		{ OPTION_SET_INT, 0, "prompt", &prompt, NULL, NULL,
-			PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN,
-			NULL, 1 },
+			0, PARSE_OPT_NONEG),
+		OPT_SET_INT_F(0, "prompt", &prompt, NULL,
+			1, PARSE_OPT_NONEG | PARSE_OPT_HIDDEN),
 		OPT_BOOL(0, "symlinks", &symlinks,
 			 N_("use symlinks in dir-diff mode")),
 		OPT_STRING('t', "tool", &difftool_cmd, N_("<tool>"),
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7ee83ac0f8..b6530a5ce0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -156,9 +156,9 @@ static struct option builtin_fetch_options[] = {
 			N_("deepen history of shallow clone, excluding rev")),
 	OPT_INTEGER(0, "deepen", &deepen_relative,
 		    N_("deepen history of shallow clone")),
-	{ OPTION_SET_INT, 0, "unshallow", &unshallow, NULL,
-		   N_("convert to a complete repository"),
-		   PARSE_OPT_NONEG | PARSE_OPT_NOARG, NULL, 1 },
+	OPT_SET_INT_F(0, "unshallow", &unshallow,
+		      N_("convert to a complete repository"),
+		      1, PARSE_OPT_NONEG),
 	{ OPTION_STRING, 0, "submodule-prefix", &submodule_prefix, N_("dir"),
 		   N_("prepend this to submodule path output"), PARSE_OPT_HIDDEN },
 	{ OPTION_CALLBACK, 0, "recurse-submodules-default",
diff --git a/builtin/grep.c b/builtin/grep.c
index 6e7bc76785..850bcffd99 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -885,9 +885,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			   N_("indicate hit with exit status without output")),
 		OPT_BOOL(0, "all-match", &opt.all_match,
 			N_("show only matches from files that match all patterns")),
-		{ OPTION_SET_INT, 0, "debug", &opt.debug, NULL,
-		  N_("show parse tree for grep expression"),
-		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1 },
+		OPT_SET_INT_F(0, "debug", &opt.debug,
+			      N_("show parse tree for grep expression"),
+			      1, PARSE_OPT_HIDDEN),
 		OPT_GROUP(""),
 		{ OPTION_STRING, 'O', "open-files-in-pager", &show_in_pager,
 			N_("pager"), N_("show matching files in the pager"),
diff --git a/builtin/log.c b/builtin/log.c
index 71f68a3e4f..8f27921505 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1474,9 +1474,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			 N_("output all-zero hash in From header")),
 		OPT_BOOL(0, "ignore-if-in-upstream", &ignore_if_in_upstream,
 			 N_("don't include a patch matching a commit upstream")),
-		{ OPTION_SET_INT, 'p', "no-stat", &use_patch_format, NULL,
-		  N_("show patch format instead of default (patch + stat)"),
-		  PARSE_OPT_NONEG | PARSE_OPT_NOARG, NULL, 1},
+		OPT_SET_INT_F('p', "no-stat", &use_patch_format,
+			      N_("show patch format instead of default (patch + stat)"),
+			      1, PARSE_OPT_NONEG),
 		OPT_GROUP(N_("Messaging")),
 		{ OPTION_CALLBACK, 0, "add-header", NULL, N_("header"),
 			    N_("add email header"), 0, header_callback },
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index a71f6bd088..40d9138c46 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -556,9 +556,9 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 		{ OPTION_CALLBACK, 0, "exclude-standard", &dir, NULL,
 			N_("add the standard git exclusions"),
 			PARSE_OPT_NOARG, option_parse_exclude_standard },
-		{ OPTION_SET_INT, 0, "full-name", &prefix_len, NULL,
-			N_("make the output relative to the project top directory"),
-			PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL },
+		OPT_SET_INT_F(0, "full-name", &prefix_len,
+			      N_("make the output relative to the project top directory"),
+			      0, PARSE_OPT_NONEG),
 		OPT_BOOL(0, "recurse-submodules", &recurse_submodules,
 			N_("recurse through submodules")),
 		OPT_BOOL(0, "error-unmatch", &error_unmatch,
diff --git a/builtin/merge.c b/builtin/merge.c
index 9db5a2cf16..4be71dd81b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -213,9 +213,9 @@ static struct option builtin_merge_options[] = {
 	OPT_BOOL('e', "edit", &option_edit,
 		N_("edit message before committing")),
 	OPT_SET_INT(0, "ff", &fast_forward, N_("allow fast-forward (default)"), FF_ALLOW),
-	{ OPTION_SET_INT, 0, "ff-only", &fast_forward, NULL,
-		N_("abort if fast-forward is not possible"),
-		PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, FF_ONLY },
+	OPT_SET_INT_F(0, "ff-only", &fast_forward,
+		      N_("abort if fast-forward is not possible"),
+		      FF_ONLY, PARSE_OPT_NONEG),
 	OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
 	OPT_BOOL(0, "verify-signatures", &verify_signatures,
 		N_("verify that the named commit has a valid GPG signature")),
diff --git a/builtin/notes.c b/builtin/notes.c
index e5bf80eef1..4d34994c34 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -778,13 +778,13 @@ static int merge(int argc, const char **argv, const char *prefix)
 			   N_("resolve notes conflicts using the given strategy "
 			      "(manual/ours/theirs/union/cat_sort_uniq)")),
 		OPT_GROUP(N_("Committing unmerged notes")),
-		{ OPTION_SET_INT, 0, "commit", &do_commit, NULL,
-			N_("finalize notes merge by committing unmerged notes"),
-			PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},
+		OPT_SET_INT_F(0, "commit", &do_commit,
+			      N_("finalize notes merge by committing unmerged notes"),
+			      1, PARSE_OPT_NONEG),
 		OPT_GROUP(N_("Aborting notes merge resolution")),
-		{ OPTION_SET_INT, 0, "abort", &do_abort, NULL,
-			N_("abort notes merge"),
-			PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},
+		OPT_SET_INT_F(0, "abort", &do_abort,
+			      N_("abort notes merge"),
+			      1, PARSE_OPT_NONEG),
 		OPT_END()
 	};
 
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 24b1c6c5dd..5134c655c7 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2992,18 +2992,18 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			 N_("do not create an empty pack output")),
 		OPT_BOOL(0, "revs", &use_internal_rev_list,
 			 N_("read revision arguments from standard input")),
-		{ OPTION_SET_INT, 0, "unpacked", &rev_list_unpacked, NULL,
-		  N_("limit the objects to those that are not yet packed"),
-		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
-		{ OPTION_SET_INT, 0, "all", &rev_list_all, NULL,
-		  N_("include objects reachable from any reference"),
-		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
-		{ OPTION_SET_INT, 0, "reflog", &rev_list_reflog, NULL,
-		  N_("include objects referred by reflog entries"),
-		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
-		{ OPTION_SET_INT, 0, "indexed-objects", &rev_list_index, NULL,
-		  N_("include objects referred to by the index"),
-		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
+		OPT_SET_INT_F(0, "unpacked", &rev_list_unpacked,
+			      N_("limit the objects to those that are not yet packed"),
+			      1, PARSE_OPT_NONEG),
+		OPT_SET_INT_F(0, "all", &rev_list_all,
+			      N_("include objects reachable from any reference"),
+			      1, PARSE_OPT_NONEG),
+		OPT_SET_INT_F(0, "reflog", &rev_list_reflog,
+			      N_("include objects referred by reflog entries"),
+			      1, PARSE_OPT_NONEG),
+		OPT_SET_INT_F(0, "indexed-objects", &rev_list_index,
+			      N_("include objects referred to by the index"),
+			      1, PARSE_OPT_NONEG),
 		OPT_BOOL(0, "stdout", &pack_to_stdout,
 			 N_("output pack to stdout")),
 		OPT_BOOL(0, "include-tag", &include_tag,
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Use OPT_SET_INT_F() for cmdline option specification
  2018-05-20  8:12 [PATCH] Use OPT_SET_INT_F() for cmdline option specification Nguyễn Thái Ngọc Duy
@ 2018-05-20  9:15 ` Martin Ågren
  2018-05-20 15:40   ` Duy Nguyen
  2018-05-20 15:42 ` [PATCH v2] " Nguyễn Thái Ngọc Duy
  1 sibling, 1 reply; 4+ messages in thread
From: Martin Ågren @ 2018-05-20  9:15 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: Git Mailing List

On 20 May 2018 at 10:12, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> The only thing these commands need is extra parseopt flags which can be
> passed in by OPT_SET_INT_F() and it is a bit more compact than full
> struct initialization.

> diff --git a/archive.c b/archive.c
> index 93ab175b0b..4fe7bec60c 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -411,11 +411,9 @@ static void parse_treeish_arg(const char **argv,
>  }
>
>  #define OPT__COMPR(s, v, h, p) \
> -       { OPTION_SET_INT, (s), NULL, (v), NULL, (h), \
> -         PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, (p) }
> +       OPT_SET_INT_F(s, NULL, v, h, p, PARSE_OPT_NONEG)
>  #define OPT__COMPR_HIDDEN(s, v, p) \
> -       { OPTION_SET_INT, (s), NULL, (v), NULL, "", \
> -         PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN, NULL, (p) }
> +       OPT_SET_INT_F(s, NULL, v, "", p, PARSE_OPT_NONEG | PARSE_OPT_HIDDEN)

Right. We have NULLs in the fifth and the second-to-last positions, and
we use PARSE_OPT_NOARG.  By switching to OPT_SET_INT_F we get those for
free.

Do we want to keep "(s)" instead of "s", just to be safe? And same for
"(v)", "(p)". Macro expansion always makes me paranoid.

> diff --git a/builtin/am.c b/builtin/am.c
> index d834f9e62b..666287b497 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -2231,12 +2231,12 @@ int cmd_am(int argc, const char **argv, const char *prefix)
>                         N_("pass -b flag to git-mailinfo"), KEEP_NON_PATCH),
>                 OPT_BOOL('m', "message-id", &state.message_id,
>                         N_("pass -m flag to git-mailinfo")),
> -               { OPTION_SET_INT, 0, "keep-cr", &keep_cr, NULL,
> -                 N_("pass --keep-cr flag to git-mailsplit for mbox format"),
> -                 PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},
> -               { OPTION_SET_INT, 0, "no-keep-cr", &keep_cr, NULL,
> -                 N_("do not pass --keep-cr flag to git-mailsplit independent of am.keepcr"),
> -                 PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0},
> +               OPT_SET_INT_F(0, "keep-cr", &keep_cr,
> +                       N_("pass --keep-cr flag to git-mailsplit for mbox format"),
> +                       1, PARSE_OPT_NONEG),
> +               OPT_SET_INT_F(0, "no-keep-cr", &keep_cr,
> +                       N_("do not pass --keep-cr flag to git-mailsplit for mbox format"),
> +                       0, PARSE_OPT_NONEG),

I found `-w` and `--word-diff` useful. You actually change the N_("...")
for `--no-keep-cr` here: [-independent of am.keepcr-]{+for mbox format+}
Copy-paste mistake?

Other than that, `--word-diff` has a very structured appearance and
nothing stood out. The ordering is different (f goes at the end in the
post-image), which makes the diff busier than it would have had to be.
(That's obviously nothing this patch can do anything about.)

Martin

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Use OPT_SET_INT_F() for cmdline option specification
  2018-05-20  9:15 ` Martin Ågren
@ 2018-05-20 15:40   ` Duy Nguyen
  0 siblings, 0 replies; 4+ messages in thread
From: Duy Nguyen @ 2018-05-20 15:40 UTC (permalink / raw)
  To: Martin Ågren; +Cc: Git Mailing List

On Sun, May 20, 2018 at 11:15 AM, Martin Ågren <martin.agren@gmail.com> wrote:
> On 20 May 2018 at 10:12, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> The only thing these commands need is extra parseopt flags which can be
>> passed in by OPT_SET_INT_F() and it is a bit more compact than full
>> struct initialization.
>
>> diff --git a/archive.c b/archive.c
>> index 93ab175b0b..4fe7bec60c 100644
>> --- a/archive.c
>> +++ b/archive.c
>> @@ -411,11 +411,9 @@ static void parse_treeish_arg(const char **argv,
>>  }
>>
>>  #define OPT__COMPR(s, v, h, p) \
>> -       { OPTION_SET_INT, (s), NULL, (v), NULL, (h), \
>> -         PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, (p) }
>> +       OPT_SET_INT_F(s, NULL, v, h, p, PARSE_OPT_NONEG)
>>  #define OPT__COMPR_HIDDEN(s, v, p) \
>> -       { OPTION_SET_INT, (s), NULL, (v), NULL, "", \
>> -         PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN, NULL, (p) }
>> +       OPT_SET_INT_F(s, NULL, v, "", p, PARSE_OPT_NONEG | PARSE_OPT_HIDDEN)
>
> Right. We have NULLs in the fifth and the second-to-last positions, and
> we use PARSE_OPT_NOARG.  By switching to OPT_SET_INT_F we get those for
> free.
>
> Do we want to keep "(s)" instead of "s", just to be safe? And same for
> "(v)", "(p)". Macro expansion always makes me paranoid.

They are still wrapped in () in the end by OPT_SET_INT_F() so the
expanded struct initialization  is the same. I think we're fine here.

>> diff --git a/builtin/am.c b/builtin/am.c
>> index d834f9e62b..666287b497 100644
>> --- a/builtin/am.c
>> +++ b/builtin/am.c
>> @@ -2231,12 +2231,12 @@ int cmd_am(int argc, const char **argv, const char *prefix)
>>                         N_("pass -b flag to git-mailinfo"), KEEP_NON_PATCH),
>>                 OPT_BOOL('m', "message-id", &state.message_id,
>>                         N_("pass -m flag to git-mailinfo")),
>> -               { OPTION_SET_INT, 0, "keep-cr", &keep_cr, NULL,
>> -                 N_("pass --keep-cr flag to git-mailsplit for mbox format"),
>> -                 PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},
>> -               { OPTION_SET_INT, 0, "no-keep-cr", &keep_cr, NULL,
>> -                 N_("do not pass --keep-cr flag to git-mailsplit independent of am.keepcr"),
>> -                 PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0},
>> +               OPT_SET_INT_F(0, "keep-cr", &keep_cr,
>> +                       N_("pass --keep-cr flag to git-mailsplit for mbox format"),
>> +                       1, PARSE_OPT_NONEG),
>> +               OPT_SET_INT_F(0, "no-keep-cr", &keep_cr,
>> +                       N_("do not pass --keep-cr flag to git-mailsplit for mbox format"),
>> +                       0, PARSE_OPT_NONEG),
>
> I found `-w` and `--word-diff` useful. You actually change the N_("...")
> for `--no-keep-cr` here: [-independent of am.keepcr-]{+for mbox format+}
> Copy-paste mistake?

Oops. You're correct. Fixed in v2.

>
> Other than that, `--word-diff` has a very structured appearance and
> nothing stood out. The ordering is different (f goes at the end in the
> post-image), which makes the diff busier than it would have had to be.
> (That's obviously nothing this patch can do anything about.)
>
> Martin
-- 
Duy

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] Use OPT_SET_INT_F() for cmdline option specification
  2018-05-20  8:12 [PATCH] Use OPT_SET_INT_F() for cmdline option specification Nguyễn Thái Ngọc Duy
  2018-05-20  9:15 ` Martin Ågren
@ 2018-05-20 15:42 ` Nguyễn Thái Ngọc Duy
  1 sibling, 0 replies; 4+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-05-20 15:42 UTC (permalink / raw)
  To: git; +Cc: Martin Ågren, Nguyễn Thái Ngọc Duy

The only thing these commands need is extra parseopt flag which can be
passed in by OPT_SET_INT_F() and it is a bit more compact than full
struct initialization.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Changes from v1

    diff --git a/builtin/am.c b/builtin/am.c
    index 666287b497..a1ff235fbc 100644
    --- a/builtin/am.c
    +++ b/builtin/am.c
    @@ -2235,7 +2235,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
     			N_("pass --keep-cr flag to git-mailsplit for mbox format"),
     			1, PARSE_OPT_NONEG),
     		OPT_SET_INT_F(0, "no-keep-cr", &keep_cr,
    -			N_("do not pass --keep-cr flag to git-mailsplit for mbox format"),
    +			N_("do not pass --keep-cr flag to git-mailsplit independent of am.keepcr"),
     			0, PARSE_OPT_NONEG),
     		OPT_BOOL('c', "scissors", &state.scissors,
     			N_("strip everything before a scissors line")),

 archive.c              |  6 ++----
 builtin/am.c           | 12 ++++++------
 builtin/branch.c       |  4 ++--
 builtin/difftool.c     |  9 ++++-----
 builtin/fetch.c        |  6 +++---
 builtin/grep.c         |  6 +++---
 builtin/log.c          |  6 +++---
 builtin/ls-files.c     |  6 +++---
 builtin/merge.c        |  6 +++---
 builtin/notes.c        | 12 ++++++------
 builtin/pack-objects.c | 24 ++++++++++++------------
 11 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/archive.c b/archive.c
index 93ab175b0b..4fe7bec60c 100644
--- a/archive.c
+++ b/archive.c
@@ -411,11 +411,9 @@ static void parse_treeish_arg(const char **argv,
 }
 
 #define OPT__COMPR(s, v, h, p) \
-	{ OPTION_SET_INT, (s), NULL, (v), NULL, (h), \
-	  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, (p) }
+	OPT_SET_INT_F(s, NULL, v, h, p, PARSE_OPT_NONEG)
 #define OPT__COMPR_HIDDEN(s, v, p) \
-	{ OPTION_SET_INT, (s), NULL, (v), NULL, "", \
-	  PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN, NULL, (p) }
+	OPT_SET_INT_F(s, NULL, v, "", p, PARSE_OPT_NONEG | PARSE_OPT_HIDDEN)
 
 static int parse_archive_args(int argc, const char **argv,
 		const struct archiver **ar, struct archiver_args *args,
diff --git a/builtin/am.c b/builtin/am.c
index d834f9e62b..a1ff235fbc 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2231,12 +2231,12 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 			N_("pass -b flag to git-mailinfo"), KEEP_NON_PATCH),
 		OPT_BOOL('m', "message-id", &state.message_id,
 			N_("pass -m flag to git-mailinfo")),
-		{ OPTION_SET_INT, 0, "keep-cr", &keep_cr, NULL,
-		  N_("pass --keep-cr flag to git-mailsplit for mbox format"),
-		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},
-		{ OPTION_SET_INT, 0, "no-keep-cr", &keep_cr, NULL,
-		  N_("do not pass --keep-cr flag to git-mailsplit independent of am.keepcr"),
-		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0},
+		OPT_SET_INT_F(0, "keep-cr", &keep_cr,
+			N_("pass --keep-cr flag to git-mailsplit for mbox format"),
+			1, PARSE_OPT_NONEG),
+		OPT_SET_INT_F(0, "no-keep-cr", &keep_cr,
+			N_("do not pass --keep-cr flag to git-mailsplit independent of am.keepcr"),
+			0, PARSE_OPT_NONEG),
 		OPT_BOOL('c', "scissors", &state.scissors,
 			N_("strip everything before a scissors line")),
 		OPT_PASSTHRU_ARGV(0, "whitespace", &state.git_apply_opts, N_("action"),
diff --git a/builtin/branch.c b/builtin/branch.c
index efc9ac1922..cc089f9efb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -592,8 +592,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT__QUIET(&quiet, N_("suppress informational messages")),
 		OPT_SET_INT('t', "track",  &track, N_("set up tracking mode (see git-pull(1))"),
 			BRANCH_TRACK_EXPLICIT),
-		{ OPTION_SET_INT, 0, "set-upstream", &track, NULL, N_("do not use"),
-			PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, BRANCH_TRACK_OVERRIDE },
+		OPT_SET_INT_F(0, "set-upstream", &track, N_("do not use"),
+			BRANCH_TRACK_OVERRIDE, PARSE_OPT_HIDDEN),
 		OPT_STRING('u', "set-upstream-to", &new_upstream, N_("upstream"), N_("change the upstream info")),
 		OPT_BOOL(0, "unset-upstream", &unset_upstream, N_("Unset the upstream info")),
 		OPT__COLOR(&branch_use_color, N_("use colored output")),
diff --git a/builtin/difftool.c b/builtin/difftool.c
index aad0e073ee..c439b64207 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -695,12 +695,11 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)
 			 N_("use `diff.guitool` instead of `diff.tool`")),
 		OPT_BOOL('d', "dir-diff", &dir_diff,
 			 N_("perform a full-directory diff")),
-		{ OPTION_SET_INT, 'y', "no-prompt", &prompt, NULL,
+		OPT_SET_INT_F('y', "no-prompt", &prompt,
 			N_("do not prompt before launching a diff tool"),
-			PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0},
-		{ OPTION_SET_INT, 0, "prompt", &prompt, NULL, NULL,
-			PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN,
-			NULL, 1 },
+			0, PARSE_OPT_NONEG),
+		OPT_SET_INT_F(0, "prompt", &prompt, NULL,
+			1, PARSE_OPT_NONEG | PARSE_OPT_HIDDEN),
 		OPT_BOOL(0, "symlinks", &symlinks,
 			 N_("use symlinks in dir-diff mode")),
 		OPT_STRING('t', "tool", &difftool_cmd, N_("<tool>"),
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7ee83ac0f8..b6530a5ce0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -156,9 +156,9 @@ static struct option builtin_fetch_options[] = {
 			N_("deepen history of shallow clone, excluding rev")),
 	OPT_INTEGER(0, "deepen", &deepen_relative,
 		    N_("deepen history of shallow clone")),
-	{ OPTION_SET_INT, 0, "unshallow", &unshallow, NULL,
-		   N_("convert to a complete repository"),
-		   PARSE_OPT_NONEG | PARSE_OPT_NOARG, NULL, 1 },
+	OPT_SET_INT_F(0, "unshallow", &unshallow,
+		      N_("convert to a complete repository"),
+		      1, PARSE_OPT_NONEG),
 	{ OPTION_STRING, 0, "submodule-prefix", &submodule_prefix, N_("dir"),
 		   N_("prepend this to submodule path output"), PARSE_OPT_HIDDEN },
 	{ OPTION_CALLBACK, 0, "recurse-submodules-default",
diff --git a/builtin/grep.c b/builtin/grep.c
index 6e7bc76785..850bcffd99 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -885,9 +885,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			   N_("indicate hit with exit status without output")),
 		OPT_BOOL(0, "all-match", &opt.all_match,
 			N_("show only matches from files that match all patterns")),
-		{ OPTION_SET_INT, 0, "debug", &opt.debug, NULL,
-		  N_("show parse tree for grep expression"),
-		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1 },
+		OPT_SET_INT_F(0, "debug", &opt.debug,
+			      N_("show parse tree for grep expression"),
+			      1, PARSE_OPT_HIDDEN),
 		OPT_GROUP(""),
 		{ OPTION_STRING, 'O', "open-files-in-pager", &show_in_pager,
 			N_("pager"), N_("show matching files in the pager"),
diff --git a/builtin/log.c b/builtin/log.c
index 71f68a3e4f..8f27921505 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1474,9 +1474,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			 N_("output all-zero hash in From header")),
 		OPT_BOOL(0, "ignore-if-in-upstream", &ignore_if_in_upstream,
 			 N_("don't include a patch matching a commit upstream")),
-		{ OPTION_SET_INT, 'p', "no-stat", &use_patch_format, NULL,
-		  N_("show patch format instead of default (patch + stat)"),
-		  PARSE_OPT_NONEG | PARSE_OPT_NOARG, NULL, 1},
+		OPT_SET_INT_F('p', "no-stat", &use_patch_format,
+			      N_("show patch format instead of default (patch + stat)"),
+			      1, PARSE_OPT_NONEG),
 		OPT_GROUP(N_("Messaging")),
 		{ OPTION_CALLBACK, 0, "add-header", NULL, N_("header"),
 			    N_("add email header"), 0, header_callback },
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index a71f6bd088..40d9138c46 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -556,9 +556,9 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 		{ OPTION_CALLBACK, 0, "exclude-standard", &dir, NULL,
 			N_("add the standard git exclusions"),
 			PARSE_OPT_NOARG, option_parse_exclude_standard },
-		{ OPTION_SET_INT, 0, "full-name", &prefix_len, NULL,
-			N_("make the output relative to the project top directory"),
-			PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL },
+		OPT_SET_INT_F(0, "full-name", &prefix_len,
+			      N_("make the output relative to the project top directory"),
+			      0, PARSE_OPT_NONEG),
 		OPT_BOOL(0, "recurse-submodules", &recurse_submodules,
 			N_("recurse through submodules")),
 		OPT_BOOL(0, "error-unmatch", &error_unmatch,
diff --git a/builtin/merge.c b/builtin/merge.c
index 9db5a2cf16..4be71dd81b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -213,9 +213,9 @@ static struct option builtin_merge_options[] = {
 	OPT_BOOL('e', "edit", &option_edit,
 		N_("edit message before committing")),
 	OPT_SET_INT(0, "ff", &fast_forward, N_("allow fast-forward (default)"), FF_ALLOW),
-	{ OPTION_SET_INT, 0, "ff-only", &fast_forward, NULL,
-		N_("abort if fast-forward is not possible"),
-		PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, FF_ONLY },
+	OPT_SET_INT_F(0, "ff-only", &fast_forward,
+		      N_("abort if fast-forward is not possible"),
+		      FF_ONLY, PARSE_OPT_NONEG),
 	OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
 	OPT_BOOL(0, "verify-signatures", &verify_signatures,
 		N_("verify that the named commit has a valid GPG signature")),
diff --git a/builtin/notes.c b/builtin/notes.c
index e5bf80eef1..4d34994c34 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -778,13 +778,13 @@ static int merge(int argc, const char **argv, const char *prefix)
 			   N_("resolve notes conflicts using the given strategy "
 			      "(manual/ours/theirs/union/cat_sort_uniq)")),
 		OPT_GROUP(N_("Committing unmerged notes")),
-		{ OPTION_SET_INT, 0, "commit", &do_commit, NULL,
-			N_("finalize notes merge by committing unmerged notes"),
-			PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},
+		OPT_SET_INT_F(0, "commit", &do_commit,
+			      N_("finalize notes merge by committing unmerged notes"),
+			      1, PARSE_OPT_NONEG),
 		OPT_GROUP(N_("Aborting notes merge resolution")),
-		{ OPTION_SET_INT, 0, "abort", &do_abort, NULL,
-			N_("abort notes merge"),
-			PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},
+		OPT_SET_INT_F(0, "abort", &do_abort,
+			      N_("abort notes merge"),
+			      1, PARSE_OPT_NONEG),
 		OPT_END()
 	};
 
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 24b1c6c5dd..5134c655c7 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2992,18 +2992,18 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			 N_("do not create an empty pack output")),
 		OPT_BOOL(0, "revs", &use_internal_rev_list,
 			 N_("read revision arguments from standard input")),
-		{ OPTION_SET_INT, 0, "unpacked", &rev_list_unpacked, NULL,
-		  N_("limit the objects to those that are not yet packed"),
-		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
-		{ OPTION_SET_INT, 0, "all", &rev_list_all, NULL,
-		  N_("include objects reachable from any reference"),
-		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
-		{ OPTION_SET_INT, 0, "reflog", &rev_list_reflog, NULL,
-		  N_("include objects referred by reflog entries"),
-		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
-		{ OPTION_SET_INT, 0, "indexed-objects", &rev_list_index, NULL,
-		  N_("include objects referred to by the index"),
-		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
+		OPT_SET_INT_F(0, "unpacked", &rev_list_unpacked,
+			      N_("limit the objects to those that are not yet packed"),
+			      1, PARSE_OPT_NONEG),
+		OPT_SET_INT_F(0, "all", &rev_list_all,
+			      N_("include objects reachable from any reference"),
+			      1, PARSE_OPT_NONEG),
+		OPT_SET_INT_F(0, "reflog", &rev_list_reflog,
+			      N_("include objects referred by reflog entries"),
+			      1, PARSE_OPT_NONEG),
+		OPT_SET_INT_F(0, "indexed-objects", &rev_list_index,
+			      N_("include objects referred to by the index"),
+			      1, PARSE_OPT_NONEG),
 		OPT_BOOL(0, "stdout", &pack_to_stdout,
 			 N_("output pack to stdout")),
 		OPT_BOOL(0, "include-tag", &include_tag,
-- 
2.17.0.705.g3525833791


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-05-20 15:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-20  8:12 [PATCH] Use OPT_SET_INT_F() for cmdline option specification Nguyễn Thái Ngọc Duy
2018-05-20  9:15 ` Martin Ågren
2018-05-20 15:40   ` Duy Nguyen
2018-05-20 15:42 ` [PATCH v2] " Nguyễn Thái Ngọc Duy

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).