git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCHv3 0/9] Removing deprecated parsing macros
@ 2013-08-03 11:51 Stefan Beller
  2013-08-03 11:51 ` [PATCHv3 1/9] Remove deprecated OPTION_BOOLEAN for parsing arguments Stefan Beller
                   ` (10 more replies)
  0 siblings, 11 replies; 27+ messages in thread
From: Stefan Beller @ 2013-08-03 11:51 UTC (permalink / raw)
  To: git, gitster, sunshine; +Cc: Stefan Beller

Suggested changes by Eric Sunshine included.

Within the builtin/ folder all occurrences of OPT_BOOLEAN have been removed.
Now we only need to review the usage of it in parse-options as used in
OPT__VERBOSE, OPT__QUIET, OPT__DRY_RUN and OPT__FORCE.
Most likely we could just use OPT_SET_INT there and then OPT_BOOLEAN is 
gone.

The patch 1 and 2 are not intended to change any semantics,
but were the most work, because of the checking for each place not
changing the semantics.

Patch 3 introduces -q the shortform of --quiet for log and format-patch 

Patch 4,5 are unspectacular, just improving readability.

Patch 6 is indeed the only occurence, where I needed to use OPT_COUNTUP
for OPT_BOOLEAN. Personally I'd change it there as well, but it's plumbing.

Patch 7 makes git config a little more flexible (allowing --global 
multiple times).

Patch 8 is a change in the plumbing layer, what I'd call a bugfix,
not urgent, but still.

Patch 9 introduces the new OPT_CMDMODE to revert. Junio suggested to 
change the OPT_CMDMODE a little and use the short parameter as the value
assigned to the command variable. This patch shows, why it might not be a
good idea, as the options there do not have short parameters.


Stefan Beller (9):
  Remove deprecated OPTION_BOOLEAN for parsing arguments
  Replace deprecated OPT_BOOLEAN by OPT_BOOL
  log, format-patch: parsing uses OPT__QUIET
  checkout: remove superfluous local variable
  branch, commit, name-rev: ease up boolean conditions
  hash-object: Replace stdin parsing OPT_BOOLEAN by OPT_COUNTUP
  config parsing options: allow one flag multiple times
  checkout-index: Fix negations of even numbers of -n
  revert: use the OPT_CMDMODE for parsing, reducing code

 Documentation/git-format-patch.txt |  1 +
 builtin/apply.c                    | 24 +++++++--------
 builtin/bisect--helper.c           |  8 ++---
 builtin/blame.c                    |  8 ++---
 builtin/branch.c                   | 13 ++++----
 builtin/check-attr.c               |  8 ++---
 builtin/check-ignore.c             | 12 ++++----
 builtin/checkout-index.c           |  8 ++---
 builtin/checkout.c                 | 27 ++++++++---------
 builtin/clean.c                    |  6 ++--
 builtin/clone.c                    | 23 +++++++-------
 builtin/commit.c                   | 48 ++++++++++++++---------------
 builtin/config.c                   |  8 ++---
 builtin/describe.c                 | 20 ++++++------
 builtin/fast-export.c              | 10 +++---
 builtin/fetch.c                    | 24 +++++++--------
 builtin/fsck.c                     | 16 +++++-----
 builtin/gc.c                       |  4 +--
 builtin/grep.c                     | 38 +++++++++++------------
 builtin/hash-object.c              |  8 ++---
 builtin/log.c                      | 17 +++++------
 builtin/ls-files.c                 | 24 +++++++--------
 builtin/ls-tree.c                  |  6 ++--
 builtin/merge-base.c               | 10 +++---
 builtin/merge-file.c               |  2 +-
 builtin/merge.c                    | 12 ++++----
 builtin/mv.c                       |  2 +-
 builtin/name-rev.c                 | 14 ++++-----
 builtin/notes.c                    | 12 ++++----
 builtin/push.c                     |  6 ++--
 builtin/remote.c                   | 28 ++++++++---------
 builtin/replace.c                  |  6 ++--
 builtin/reset.c                    |  2 +-
 builtin/rev-parse.c                |  4 +--
 builtin/revert.c                   | 62 +++++++++-----------------------------
 builtin/rm.c                       |  6 ++--
 builtin/shortlog.c                 | 12 ++++----
 builtin/show-branch.c              | 28 ++++++++---------
 builtin/show-ref.c                 | 15 +++++----
 builtin/tag.c                      |  4 +--
 builtin/update-ref.c               |  4 +--
 parse-options.h                    |  5 ++-
 42 files changed, 278 insertions(+), 317 deletions(-)

-- 
1.8.4.rc0.16.g7fca822.dirty

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

* [PATCHv3 1/9] Remove deprecated OPTION_BOOLEAN for parsing arguments
  2013-08-03 11:51 [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
@ 2013-08-03 11:51 ` Stefan Beller
  2013-08-03 11:51 ` [PATCHv3 2/9] Replace deprecated OPT_BOOLEAN by OPT_BOOL Stefan Beller
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: Stefan Beller @ 2013-08-03 11:51 UTC (permalink / raw)
  To: git, gitster, sunshine; +Cc: Stefan Beller

As of b04ba2bb4 OPTION_BOOLEAN was deprecated.
This commit removes all occurrences of OPTION_BOOLEAN.
In b04ba2bb4 Junio suggested to replace it with either
OPTION_SET_INT or OPTION_COUNTUP instead. However a pattern, which
occurred often with the OPTION_BOOLEAN was a hidden boolean parameter.
So I defined OPT_HIDDEN_BOOL as an additional possible parse option
in parse-options.h to make life easy.

The OPT_HIDDEN_BOOL was used in checkout, clone, commit, show-ref.
The only exception, where there was need to fiddle with OPTION_SET_INT
was log and notes. However in these two files there is also a pattern,
so we could think of introducing OPT_NONEG_BOOL.

Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
---
 builtin/checkout.c |  5 ++---
 builtin/clone.c    |  7 +++----
 builtin/commit.c   | 10 ++++------
 builtin/log.c      |  4 ++--
 builtin/notes.c    |  8 ++++----
 builtin/show-ref.c |  5 ++---
 parse-options.h    |  5 ++---
 7 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7025938..646a475 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1073,9 +1073,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN('p', "patch", &opts.patch_mode, N_("select hunks interactively")),
 		OPT_BOOL(0, "ignore-skip-worktree-bits", &opts.ignore_skipworktree,
 			 N_("do not limit pathspecs to sparse entries only")),
-		{ OPTION_BOOLEAN, 0, "guess", &dwim_new_local_branch, NULL,
-		  N_("second guess 'git checkout no-such-branch'"),
-		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
+		OPT_HIDDEN_BOOL(0, "guess", &dwim_new_local_branch,
+				N_("second guess 'git checkout no-such-branch'")),
 		OPT_END(),
 	};
 
diff --git a/builtin/clone.c b/builtin/clone.c
index 430307b..e7b0b13 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -64,10 +64,9 @@ static struct option builtin_clone_options[] = {
 		 N_("force progress reporting")),
 	OPT_BOOLEAN('n', "no-checkout", &option_no_checkout,
 		    N_("don't create a checkout")),
-	OPT_BOOLEAN(0, "bare", &option_bare, N_("create a bare repository")),
-	{ OPTION_BOOLEAN, 0, "naked", &option_bare, NULL,
-		N_("create a bare repository"),
-		PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
+	OPT_BOOL(0, "bare", &option_bare, N_("create a bare repository")),
+	OPT_HIDDEN_BOOL(0, "naked", &option_bare,
+			N_("create a bare repository")),
 	OPT_BOOLEAN(0, "mirror", &option_mirror,
 		    N_("create a mirror repository (implies bare)")),
 	OPT_BOOL('l', "local", &option_local,
diff --git a/builtin/commit.c b/builtin/commit.c
index 003bd7d..b64a083 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1448,12 +1448,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, N_("mode"), N_("show untracked files, optional modes: all, normal, no. (Default: all)"), PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
 		/* end commit contents options */
 
-		{ OPTION_BOOLEAN, 0, "allow-empty", &allow_empty, NULL,
-		  N_("ok to record an empty change"),
-		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
-		{ OPTION_BOOLEAN, 0, "allow-empty-message", &allow_empty_message, NULL,
-		  N_("ok to record a change with an empty message"),
-		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
+		OPT_HIDDEN_BOOL(0, "allow-empty", &allow_empty,
+				N_("ok to record an empty change")),
+		OPT_HIDDEN_BOOL(0, "allow-empty-message", &allow_empty_message,
+				N_("ok to record a change with an empty message")),
 
 		OPT_END()
 	};
diff --git a/builtin/log.c b/builtin/log.c
index 2625f98..05e374d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1183,9 +1183,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			    N_("don't output binary diffs")),
 		OPT_BOOLEAN(0, "ignore-if-in-upstream", &ignore_if_in_upstream,
 			    N_("don't include a patch matching a commit upstream")),
-		{ OPTION_BOOLEAN, 'p', "no-stat", &use_patch_format, NULL,
+		{ 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 },
+		  PARSE_OPT_NONEG | PARSE_OPT_NOARG, NULL, 1},
 		OPT_GROUP(N_("Messaging")),
 		{ OPTION_CALLBACK, 0, "add-header", NULL, N_("header"),
 			    N_("add email header"), 0, header_callback },
diff --git a/builtin/notes.c b/builtin/notes.c
index e4100c4..8f63cb2 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -739,13 +739,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_BOOLEAN, 0, "commit", &do_commit, NULL,
+		{ OPTION_SET_INT, 0, "commit", &do_commit, NULL,
 			N_("finalize notes merge by committing unmerged notes"),
-			PARSE_OPT_NOARG | PARSE_OPT_NONEG },
+			PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},
 		OPT_GROUP(N_("Aborting notes merge resolution")),
-		{ OPTION_BOOLEAN, 0, "abort", &do_abort, NULL,
+		{ OPTION_SET_INT, 0, "abort", &do_abort, NULL,
 			N_("abort notes merge"),
-			PARSE_OPT_NOARG | PARSE_OPT_NONEG },
+			PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},
 		OPT_END()
 	};
 
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 87806ad..18680bb 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -169,9 +169,8 @@ static const struct option show_ref_options[] = {
 	OPT_BOOLEAN(0, "heads", &heads_only, N_("only show heads (can be combined with tags)")),
 	OPT_BOOLEAN(0, "verify", &verify, N_("stricter reference checking, "
 		    "requires exact ref path")),
-	{ OPTION_BOOLEAN, 'h', NULL, &show_head, NULL,
-	  N_("show the HEAD reference, even if it would be filtered out"),
-	  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
+	OPT_HIDDEN_BOOL('h', NULL, &show_head,
+			N_("show the HEAD reference, even if it would be filtered out")),
 	OPT_BOOLEAN(0, "head", &show_head,
 	  N_("show the HEAD reference, even if it would be filtered out")),
 	OPT_BOOLEAN('d', "dereference", &deref_tags,
diff --git a/parse-options.h b/parse-options.h
index 2404e06..aeab9aa 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -22,9 +22,6 @@ enum parse_opt_type {
 	OPTION_FILENAME
 };
 
-/* Deprecated synonym */
-#define OPTION_BOOLEAN OPTION_COUNTUP
-
 enum parse_opt_flags {
 	PARSE_OPT_KEEP_DASHDASH = 1,
 	PARSE_OPT_STOP_AT_NON_OPTION = 2,
@@ -129,6 +126,8 @@ struct option {
 #define OPT_SET_INT(s, l, v, h, i)  { OPTION_SET_INT, (s), (l), (v), NULL, \
 				      (h), PARSE_OPT_NOARG, NULL, (i) }
 #define OPT_BOOL(s, l, v, h)        OPT_SET_INT(s, l, v, h, 1)
+#define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \
+				      (h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1}
 #define OPT_SET_PTR(s, l, v, h, p)  { OPTION_SET_PTR, (s), (l), (v), NULL, \
 				      (h), PARSE_OPT_NOARG, NULL, (p) }
 #define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \
-- 
1.8.4.rc0.16.g7fca822.dirty

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

* [PATCHv3 2/9] Replace deprecated OPT_BOOLEAN by OPT_BOOL
  2013-08-03 11:51 [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
  2013-08-03 11:51 ` [PATCHv3 1/9] Remove deprecated OPTION_BOOLEAN for parsing arguments Stefan Beller
@ 2013-08-03 11:51 ` Stefan Beller
  2013-08-03 11:51 ` [PATCHv3 3/9] log, format-patch: parsing uses OPT__QUIET Stefan Beller
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: Stefan Beller @ 2013-08-03 11:51 UTC (permalink / raw)
  To: git, gitster, sunshine; +Cc: Stefan Beller

This task emerged from b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
2011-09-27). All occurrences of the respective variables have
been reviewed and none of them relied on the counting up mechanism,
but all of them were using the variable as a true boolean.

This patch does not change semantics of any command intentionally.

Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
---
 builtin/apply.c          | 24 ++++++++++++------------
 builtin/bisect--helper.c |  8 ++++----
 builtin/blame.c          |  8 ++++----
 builtin/branch.c         | 10 +++++-----
 builtin/check-attr.c     |  8 ++++----
 builtin/check-ignore.c   | 12 ++++++------
 builtin/checkout-index.c |  6 +++---
 builtin/checkout.c       | 10 +++++-----
 builtin/clean.c          |  6 +++---
 builtin/clone.c          | 16 ++++++++--------
 builtin/commit.c         | 36 ++++++++++++++++++------------------
 builtin/config.c         |  2 +-
 builtin/describe.c       | 20 ++++++++++----------
 builtin/fast-export.c    | 10 +++++-----
 builtin/fetch.c          | 24 ++++++++++++------------
 builtin/fsck.c           | 16 ++++++++--------
 builtin/gc.c             |  4 ++--
 builtin/grep.c           | 38 +++++++++++++++++++-------------------
 builtin/hash-object.c    |  6 +++---
 builtin/log.c            |  8 ++++----
 builtin/ls-files.c       | 24 ++++++++++++------------
 builtin/ls-tree.c        |  6 +++---
 builtin/merge-base.c     | 10 +++++-----
 builtin/merge-file.c     |  2 +-
 builtin/merge.c          | 12 ++++++------
 builtin/mv.c             |  2 +-
 builtin/name-rev.c       | 12 ++++++------
 builtin/notes.c          |  4 ++--
 builtin/push.c           |  6 +++---
 builtin/remote.c         | 28 ++++++++++++++--------------
 builtin/replace.c        |  6 +++---
 builtin/reset.c          |  2 +-
 builtin/rev-parse.c      |  4 ++--
 builtin/rm.c             |  6 +++---
 builtin/shortlog.c       | 12 ++++++------
 builtin/show-branch.c    | 28 ++++++++++++++--------------
 builtin/show-ref.c       | 10 +++++-----
 builtin/tag.c            |  4 ++--
 builtin/update-ref.c     |  4 ++--
 39 files changed, 227 insertions(+), 227 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 50912c9..ef32e4f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4363,23 +4363,23 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
 		{ OPTION_CALLBACK, 'p', NULL, NULL, N_("num"),
 			N_("remove <num> leading slashes from traditional diff paths"),
 			0, option_parse_p },
-		OPT_BOOLEAN(0, "no-add", &no_add,
+		OPT_BOOL(0, "no-add", &no_add,
 			N_("ignore additions made by the patch")),
-		OPT_BOOLEAN(0, "stat", &diffstat,
+		OPT_BOOL(0, "stat", &diffstat,
 			N_("instead of applying the patch, output diffstat for the input")),
 		OPT_NOOP_NOARG(0, "allow-binary-replacement"),
 		OPT_NOOP_NOARG(0, "binary"),
-		OPT_BOOLEAN(0, "numstat", &numstat,
+		OPT_BOOL(0, "numstat", &numstat,
 			N_("show number of added and deleted lines in decimal notation")),
-		OPT_BOOLEAN(0, "summary", &summary,
+		OPT_BOOL(0, "summary", &summary,
 			N_("instead of applying the patch, output a summary for the input")),
-		OPT_BOOLEAN(0, "check", &check,
+		OPT_BOOL(0, "check", &check,
 			N_("instead of applying the patch, see if the patch is applicable")),
-		OPT_BOOLEAN(0, "index", &check_index,
+		OPT_BOOL(0, "index", &check_index,
 			N_("make sure the patch is applicable to the current index")),
-		OPT_BOOLEAN(0, "cached", &cached,
+		OPT_BOOL(0, "cached", &cached,
 			N_("apply a patch without touching the working tree")),
-		OPT_BOOLEAN(0, "apply", &force_apply,
+		OPT_BOOL(0, "apply", &force_apply,
 			N_("also apply the patch (use with --stat/--summary/--check)")),
 		OPT_BOOL('3', "3way", &threeway,
 			 N_( "attempt three-way merge if a patch does not apply")),
@@ -4399,13 +4399,13 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
 		{ OPTION_CALLBACK, 0, "ignore-whitespace", NULL, NULL,
 			N_("ignore changes in whitespace when finding context"),
 			PARSE_OPT_NOARG, option_parse_space_change },
-		OPT_BOOLEAN('R', "reverse", &apply_in_reverse,
+		OPT_BOOL('R', "reverse", &apply_in_reverse,
 			N_("apply the patch in reverse")),
-		OPT_BOOLEAN(0, "unidiff-zero", &unidiff_zero,
+		OPT_BOOL(0, "unidiff-zero", &unidiff_zero,
 			N_("don't expect at least one line of context")),
-		OPT_BOOLEAN(0, "reject", &apply_with_reject,
+		OPT_BOOL(0, "reject", &apply_with_reject,
 			N_("leave the rejected hunks in corresponding *.rej files")),
-		OPT_BOOLEAN(0, "allow-overlap", &allow_overlap,
+		OPT_BOOL(0, "allow-overlap", &allow_overlap,
 			N_("allow overlapping hunks")),
 		OPT__VERBOSE(&apply_verbosely, N_("be verbose")),
 		OPT_BIT(0, "inaccurate-eof", &options,
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index e3884e3..3324229 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -13,10 +13,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	int next_all = 0;
 	int no_checkout = 0;
 	struct option options[] = {
-		OPT_BOOLEAN(0, "next-all", &next_all,
-			    N_("perform 'git bisect next'")),
-		OPT_BOOLEAN(0, "no-checkout", &no_checkout,
-			    N_("update BISECT_HEAD instead of checking out the current commit")),
+		OPT_BOOL(0, "next-all", &next_all,
+			 N_("perform 'git bisect next'")),
+		OPT_BOOL(0, "no-checkout", &no_checkout,
+			 N_("update BISECT_HEAD instead of checking out the current commit")),
 		OPT_END()
 	};
 
diff --git a/builtin/blame.c b/builtin/blame.c
index 079dcd3..f932112 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2273,10 +2273,10 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	static const char *revs_file = NULL;
 	static const char *contents_from = NULL;
 	static const struct option options[] = {
-		OPT_BOOLEAN(0, "incremental", &incremental, N_("Show blame entries as we find them, incrementally")),
-		OPT_BOOLEAN('b', NULL, &blank_boundary, N_("Show blank SHA-1 for boundary commits (Default: off)")),
-		OPT_BOOLEAN(0, "root", &show_root, N_("Do not treat root commits as boundaries (Default: off)")),
-		OPT_BOOLEAN(0, "show-stats", &show_stats, N_("Show work cost statistics")),
+		OPT_BOOL(0, "incremental", &incremental, N_("Show blame entries as we find them, incrementally")),
+		OPT_BOOL('b', NULL, &blank_boundary, N_("Show blank SHA-1 for boundary commits (Default: off)")),
+		OPT_BOOL(0, "root", &show_root, N_("Do not treat root commits as boundaries (Default: off)")),
+		OPT_BOOL(0, "show-stats", &show_stats, N_("Show work cost statistics")),
 		OPT_BIT(0, "score-debug", &output_option, N_("Show output score for blame entries"), OUTPUT_SHOW_SCORE),
 		OPT_BIT('f', "show-name", &output_option, N_("Show original filename (Default: auto)"), OUTPUT_SHOW_NAME),
 		OPT_BIT('n', "show-number", &output_option, N_("Show original linenumber (Default: off)"), OUTPUT_SHOW_NUMBER),
diff --git a/builtin/branch.c b/builtin/branch.c
index 0836890..4daed0b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -797,7 +797,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT( 0, "set-upstream",  &track, N_("change upstream info"),
 			BRANCH_TRACK_OVERRIDE),
 		OPT_STRING('u', "set-upstream-to", &new_upstream, "upstream", "change the upstream info"),
-		OPT_BOOLEAN(0, "unset-upstream", &unset_upstream, "Unset the upstream info"),
+		OPT_BOOL(0, "unset-upstream", &unset_upstream, "Unset the upstream info"),
 		OPT__COLOR(&branch_use_color, N_("use colored output")),
 		OPT_SET_INT('r', "remotes",     &kinds, N_("act on remote-tracking branches"),
 			REF_REMOTE_BRANCH),
@@ -822,10 +822,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('D', NULL, &delete, N_("delete branch (even if not merged)"), 2),
 		OPT_BIT('m', "move", &rename, N_("move/rename a branch and its reflog"), 1),
 		OPT_BIT('M', NULL, &rename, N_("move/rename a branch, even if target exists"), 2),
-		OPT_BOOLEAN(0, "list", &list, N_("list branch names")),
-		OPT_BOOLEAN('l', "create-reflog", &reflog, N_("create the branch's reflog")),
-		OPT_BOOLEAN(0, "edit-description", &edit_description,
-			    N_("edit the description for the branch")),
+		OPT_BOOL(0, "list", &list, N_("list branch names")),
+		OPT_BOOL('l', "create-reflog", &reflog, N_("create the branch's reflog")),
+		OPT_BOOL(0, "edit-description", &edit_description,
+			 N_("edit the description for the branch")),
 		OPT__FORCE(&force_create, N_("force creation (when already exists)")),
 		{
 			OPTION_CALLBACK, 0, "no-merged", &merge_filter_ref,
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 075d01d..6e5cd88 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -16,10 +16,10 @@ NULL
 static int null_term_line;
 
 static const struct option check_attr_options[] = {
-	OPT_BOOLEAN('a', "all", &all_attrs, N_("report all attributes set on file")),
-	OPT_BOOLEAN(0,  "cached", &cached_attrs, N_("use .gitattributes only from the index")),
-	OPT_BOOLEAN(0 , "stdin", &stdin_paths, N_("read file names from stdin")),
-	OPT_BOOLEAN('z', NULL, &null_term_line,
+	OPT_BOOL('a', "all", &all_attrs, N_("report all attributes set on file")),
+	OPT_BOOL(0,  "cached", &cached_attrs, N_("use .gitattributes only from the index")),
+	OPT_BOOL(0 , "stdin", &stdin_paths, N_("read file names from stdin")),
+	OPT_BOOL('z', NULL, &null_term_line,
 		N_("input paths are terminated by a null character")),
 	OPT_END()
 };
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 4a8fc70..c9f0c9b 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -18,12 +18,12 @@ static const struct option check_ignore_options[] = {
 	OPT__QUIET(&quiet, N_("suppress progress reporting")),
 	OPT__VERBOSE(&verbose, N_("be verbose")),
 	OPT_GROUP(""),
-	OPT_BOOLEAN(0, "stdin", &stdin_paths,
-		    N_("read file names from stdin")),
-	OPT_BOOLEAN('z', NULL, &null_term_line,
-		    N_("input paths are terminated by a null character")),
-	OPT_BOOLEAN('n', "non-matching", &show_non_matching,
-		    N_("show non-matching input paths")),
+	OPT_BOOL(0, "stdin", &stdin_paths,
+		 N_("read file names from stdin")),
+	OPT_BOOL('z', NULL, &null_term_line,
+		 N_("input paths are terminated by a null character")),
+	OPT_BOOL('n', "non-matching", &show_non_matching,
+		 N_("show non-matching input paths")),
 	OPT_END()
 };
 
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index b1feda7..aa922ed 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -183,7 +183,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 	int prefix_length;
 	int force = 0, quiet = 0, not_new = 0;
 	struct option builtin_checkout_index_options[] = {
-		OPT_BOOLEAN('a', "all", &all,
+		OPT_BOOL('a', "all", &all,
 			N_("check out all files in the index")),
 		OPT__FORCE(&force, N_("force overwrite of existing files")),
 		OPT__QUIET(&quiet,
@@ -196,9 +196,9 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 		{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
 			N_("paths are separated with NUL character"),
 			PARSE_OPT_NOARG, option_parse_z },
-		OPT_BOOLEAN(0, "stdin", &read_from_stdin,
+		OPT_BOOL(0, "stdin", &read_from_stdin,
 			N_("read list of paths from the standard input")),
-		OPT_BOOLEAN(0, "temp", &to_tempfile,
+		OPT_BOOL(0, "temp", &to_tempfile,
 			N_("write the content to temporary files")),
 		OPT_CALLBACK(0, "prefix", NULL, N_("string"),
 			N_("when creating files, prepend <string>"),
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 646a475..8b48f4a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1056,8 +1056,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 			   N_("create and checkout a new branch")),
 		OPT_STRING('B', NULL, &opts.new_branch_force, N_("branch"),
 			   N_("create/reset and checkout a branch")),
-		OPT_BOOLEAN('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")),
-		OPT_BOOLEAN(0, "detach", &opts.force_detach, N_("detach the HEAD at named commit")),
+		OPT_BOOL('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")),
+		OPT_BOOL(0, "detach", &opts.force_detach, N_("detach the HEAD at named commit")),
 		OPT_SET_INT('t', "track",  &opts.track, N_("set upstream info for new branch"),
 			BRANCH_TRACK_EXPLICIT),
 		OPT_STRING(0, "orphan", &opts.new_orphan_branch, N_("new branch"), N_("new unparented branch")),
@@ -1066,11 +1066,11 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT('3', "theirs", &opts.writeout_stage, N_("checkout their version for unmerged files"),
 			    3),
 		OPT__FORCE(&opts.force, N_("force checkout (throw away local modifications)")),
-		OPT_BOOLEAN('m', "merge", &opts.merge, N_("perform a 3-way merge with the new branch")),
-		OPT_BOOLEAN(0, "overwrite-ignore", &opts.overwrite_ignore, N_("update ignored files (default)")),
+		OPT_BOOL('m', "merge", &opts.merge, N_("perform a 3-way merge with the new branch")),
+		OPT_BOOL(0, "overwrite-ignore", &opts.overwrite_ignore, N_("update ignored files (default)")),
 		OPT_STRING(0, "conflict", &conflict_style, N_("style"),
 			   N_("conflict style (merge or diff3)")),
-		OPT_BOOLEAN('p', "patch", &opts.patch_mode, N_("select hunks interactively")),
+		OPT_BOOL('p', "patch", &opts.patch_mode, N_("select hunks interactively")),
 		OPT_BOOL(0, "ignore-skip-worktree-bits", &opts.ignore_skipworktree,
 			 N_("do not limit pathspecs to sparse entries only")),
 		OPT_HIDDEN_BOOL(0, "guess", &dwim_new_local_branch,
diff --git a/builtin/clean.c b/builtin/clean.c
index dba8387..df0f61e 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -851,12 +851,12 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		OPT__DRY_RUN(&dry_run, N_("dry run")),
 		OPT__FORCE(&force, N_("force")),
 		OPT_BOOL('i', "interactive", &interactive, N_("interactive cleaning")),
-		OPT_BOOLEAN('d', NULL, &remove_directories,
+		OPT_BOOL('d', NULL, &remove_directories,
 				N_("remove whole directories")),
 		{ OPTION_CALLBACK, 'e', "exclude", &exclude_list, N_("pattern"),
 		  N_("add <pattern> to ignore rules"), PARSE_OPT_NONEG, exclude_cb },
-		OPT_BOOLEAN('x', NULL, &ignored, N_("remove ignored files, too")),
-		OPT_BOOLEAN('X', NULL, &ignored_only,
+		OPT_BOOL('x', NULL, &ignored, N_("remove ignored files, too")),
+		OPT_BOOL('X', NULL, &ignored_only,
 				N_("remove only ignored files")),
 		OPT_END()
 	};
diff --git a/builtin/clone.c b/builtin/clone.c
index e7b0b13..ca3eb68 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -62,22 +62,22 @@ static struct option builtin_clone_options[] = {
 	OPT__VERBOSITY(&option_verbosity),
 	OPT_BOOL(0, "progress", &option_progress,
 		 N_("force progress reporting")),
-	OPT_BOOLEAN('n', "no-checkout", &option_no_checkout,
-		    N_("don't create a checkout")),
+	OPT_BOOL('n', "no-checkout", &option_no_checkout,
+		 N_("don't create a checkout")),
 	OPT_BOOL(0, "bare", &option_bare, N_("create a bare repository")),
 	OPT_HIDDEN_BOOL(0, "naked", &option_bare,
 			N_("create a bare repository")),
-	OPT_BOOLEAN(0, "mirror", &option_mirror,
-		    N_("create a mirror repository (implies bare)")),
+	OPT_BOOL(0, "mirror", &option_mirror,
+		 N_("create a mirror repository (implies bare)")),
 	OPT_BOOL('l', "local", &option_local,
 		N_("to clone from a local repository")),
-	OPT_BOOLEAN(0, "no-hardlinks", &option_no_hardlinks,
+	OPT_BOOL(0, "no-hardlinks", &option_no_hardlinks,
 		    N_("don't use local hardlinks, always copy")),
-	OPT_BOOLEAN('s', "shared", &option_shared,
+	OPT_BOOL('s', "shared", &option_shared,
 		    N_("setup as shared repository")),
-	OPT_BOOLEAN(0, "recursive", &option_recursive,
+	OPT_BOOL(0, "recursive", &option_recursive,
 		    N_("initialize submodules in the clone")),
-	OPT_BOOLEAN(0, "recurse-submodules", &option_recursive,
+	OPT_BOOL(0, "recurse-submodules", &option_recursive,
 		    N_("initialize submodules in the clone")),
 	OPT_STRING(0, "template", &option_template, N_("template-directory"),
 		   N_("directory from which templates will be used")),
diff --git a/builtin/commit.c b/builtin/commit.c
index b64a083..c20426b 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1209,14 +1209,14 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT(0, "long", &status_format,
 			    N_("show status in long format (default)"),
 			    STATUS_FORMAT_LONG),
-		OPT_BOOLEAN('z', "null", &s.null_termination,
-			    N_("terminate entries with NUL")),
+		OPT_BOOL('z', "null", &s.null_termination,
+			 N_("terminate entries with NUL")),
 		{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg,
 		  N_("mode"),
 		  N_("show untracked files, optional modes: all, normal, no. (Default: all)"),
 		  PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
-		OPT_BOOLEAN(0, "ignored", &show_ignored_in_status,
-			    N_("show ignored files")),
+		OPT_BOOL(0, "ignored", &show_ignored_in_status,
+			 N_("show ignored files")),
 		{ OPTION_STRING, 0, "ignore-submodules", &ignore_submodule_arg, N_("when"),
 		  N_("ignore changes to submodules, optional when: all, dirty, untracked. (Default: all)"),
 		  PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
@@ -1415,24 +1415,24 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_STRING('C', "reuse-message", &use_message, N_("commit"), N_("reuse message from specified commit")),
 		OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")),
 		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
-		OPT_BOOLEAN(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
-		OPT_BOOLEAN('s', "signoff", &signoff, N_("add Signed-off-by:")),
+		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
+		OPT_BOOL('s', "signoff", &signoff, N_("add Signed-off-by:")),
 		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
 		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
 		OPT_STRING(0, "cleanup", &cleanup_arg, N_("default"), N_("how to strip spaces and #comments from message")),
-		OPT_BOOLEAN(0, "status", &include_status, N_("include status in commit message template")),
+		OPT_BOOL(0, "status", &include_status, N_("include status in commit message template")),
 		{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key id"),
 		  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
 		/* end commit message options */
 
 		OPT_GROUP(N_("Commit contents options")),
-		OPT_BOOLEAN('a', "all", &all, N_("commit all changed files")),
-		OPT_BOOLEAN('i', "include", &also, N_("add specified files to index for commit")),
-		OPT_BOOLEAN(0, "interactive", &interactive, N_("interactively add files")),
-		OPT_BOOLEAN('p', "patch", &patch_interactive, N_("interactively add changes")),
-		OPT_BOOLEAN('o', "only", &only, N_("commit only specified files")),
-		OPT_BOOLEAN('n', "no-verify", &no_verify, N_("bypass pre-commit hook")),
-		OPT_BOOLEAN(0, "dry-run", &dry_run, N_("show what would be committed")),
+		OPT_BOOL('a', "all", &all, N_("commit all changed files")),
+		OPT_BOOL('i', "include", &also, N_("add specified files to index for commit")),
+		OPT_BOOL(0, "interactive", &interactive, N_("interactively add files")),
+		OPT_BOOL('p', "patch", &patch_interactive, N_("interactively add changes")),
+		OPT_BOOL('o', "only", &only, N_("commit only specified files")),
+		OPT_BOOL('n', "no-verify", &no_verify, N_("bypass pre-commit hook")),
+		OPT_BOOL(0, "dry-run", &dry_run, N_("show what would be committed")),
 		OPT_SET_INT(0, "short", &status_format, N_("show status concisely"),
 			    STATUS_FORMAT_SHORT),
 		OPT_BOOL(0, "branch", &s.show_branch, N_("show branch information")),
@@ -1441,10 +1441,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT(0, "long", &status_format,
 			    N_("show status in long format (default)"),
 			    STATUS_FORMAT_LONG),
-		OPT_BOOLEAN('z', "null", &s.null_termination,
-			    N_("terminate entries with NUL")),
-		OPT_BOOLEAN(0, "amend", &amend, N_("amend previous commit")),
-		OPT_BOOLEAN(0, "no-post-rewrite", &no_post_rewrite, N_("bypass post-rewrite hook")),
+		OPT_BOOL('z', "null", &s.null_termination,
+			 N_("terminate entries with NUL")),
+		OPT_BOOL(0, "amend", &amend, N_("amend previous commit")),
+		OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass post-rewrite hook")),
 		{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, N_("mode"), N_("show untracked files, optional modes: all, normal, no. (Default: all)"), PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
 		/* end commit contents options */
 
diff --git a/builtin/config.c b/builtin/config.c
index 4010c43..da12fdb 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -75,7 +75,7 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "bool-or-int", &types, N_("value is --bool or --int"), TYPE_BOOL_OR_INT),
 	OPT_BIT(0, "path", &types, N_("value is a path (file or directory name)"), TYPE_PATH),
 	OPT_GROUP(N_("Other")),
-	OPT_BOOLEAN('z', "null", &end_null, N_("terminate values with NUL byte")),
+	OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
 	OPT_BOOL(0, "includes", &respect_includes, N_("respect include directives on lookup")),
 	OPT_END(),
 };
diff --git a/builtin/describe.c b/builtin/describe.c
index 7d73722..c94e5c3 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -406,12 +406,12 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 {
 	int contains = 0;
 	struct option options[] = {
-		OPT_BOOLEAN(0, "contains",   &contains, N_("find the tag that comes after the commit")),
-		OPT_BOOLEAN(0, "debug",      &debug, N_("debug search strategy on stderr")),
-		OPT_BOOLEAN(0, "all",        &all, N_("use any ref")),
-		OPT_BOOLEAN(0, "tags",       &tags, N_("use any tag, even unannotated")),
-		OPT_BOOLEAN(0, "long",       &longformat, N_("always use long format")),
-		OPT_BOOLEAN(0, "first-parent", &first_parent, N_("only follow first parent")),
+		OPT_BOOL(0, "contains",   &contains, N_("find the tag that comes after the commit")),
+		OPT_BOOL(0, "debug",      &debug, N_("debug search strategy on stderr")),
+		OPT_BOOL(0, "all",        &all, N_("use any ref")),
+		OPT_BOOL(0, "tags",       &tags, N_("use any tag, even unannotated")),
+		OPT_BOOL(0, "long",       &longformat, N_("always use long format")),
+		OPT_BOOL(0, "first-parent", &first_parent, N_("only follow first parent")),
 		OPT__ABBREV(&abbrev),
 		OPT_SET_INT(0, "exact-match", &max_candidates,
 			    N_("only output exact matches"), 0),
@@ -419,11 +419,11 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 			    N_("consider <n> most recent tags (default: 10)")),
 		OPT_STRING(0, "match",       &pattern, N_("pattern"),
 			   N_("only consider tags matching <pattern>")),
-		OPT_BOOLEAN(0, "always",     &always,
-			   N_("show abbreviated commit object as fallback")),
+		OPT_BOOL(0, "always",        &always,
+			N_("show abbreviated commit object as fallback")),
 		{OPTION_STRING, 0, "dirty",  &dirty, N_("mark"),
-			   N_("append <mark> on dirty working tree (default: \"-dirty\")"),
-		 PARSE_OPT_OPTARG, NULL, (intptr_t) "-dirty"},
+			N_("append <mark> on dirty working tree (default: \"-dirty\")"),
+			PARSE_OPT_OPTARG, NULL, (intptr_t) "-dirty"},
 		OPT_END(),
 	};
 
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d1d68e9..32b0c3f 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -674,11 +674,11 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 			     N_("Dump marks to this file")),
 		OPT_STRING(0, "import-marks", &import_filename, N_("file"),
 			     N_("Import marks from this file")),
-		OPT_BOOLEAN(0, "fake-missing-tagger", &fake_missing_tagger,
-			     N_("Fake a tagger when tags lack one")),
-		OPT_BOOLEAN(0, "full-tree", &full_tree,
-			     N_("Output full tree for each commit")),
-		OPT_BOOLEAN(0, "use-done-feature", &use_done_feature,
+		OPT_BOOL(0, "fake-missing-tagger", &fake_missing_tagger,
+			 N_("Fake a tagger when tags lack one")),
+		OPT_BOOL(0, "full-tree", &full_tree,
+			 N_("Output full tree for each commit")),
+		OPT_BOOL(0, "use-done-feature", &use_done_feature,
 			     N_("Use the done feature to terminate the stream")),
 		OPT_BOOL(0, "no-data", &no_data, N_("Skip output of blob data")),
 		OPT_END()
diff --git a/builtin/fetch.c b/builtin/fetch.c
index d784b2e..99afed0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -56,28 +56,28 @@ static int option_parse_recurse_submodules(const struct option *opt,
 
 static struct option builtin_fetch_options[] = {
 	OPT__VERBOSITY(&verbosity),
-	OPT_BOOLEAN(0, "all", &all,
-		    N_("fetch from all remotes")),
-	OPT_BOOLEAN('a', "append", &append,
-		    N_("append to .git/FETCH_HEAD instead of overwriting")),
+	OPT_BOOL(0, "all", &all,
+		 N_("fetch from all remotes")),
+	OPT_BOOL('a', "append", &append,
+		 N_("append to .git/FETCH_HEAD instead of overwriting")),
 	OPT_STRING(0, "upload-pack", &upload_pack, N_("path"),
 		   N_("path to upload pack on remote end")),
 	OPT__FORCE(&force, N_("force overwrite of local branch")),
-	OPT_BOOLEAN('m', "multiple", &multiple,
-		    N_("fetch from multiple remotes")),
+	OPT_BOOL('m', "multiple", &multiple,
+		 N_("fetch from multiple remotes")),
 	OPT_SET_INT('t', "tags", &tags,
 		    N_("fetch all tags and associated objects"), TAGS_SET),
 	OPT_SET_INT('n', NULL, &tags,
 		    N_("do not fetch all tags (--no-tags)"), TAGS_UNSET),
-	OPT_BOOLEAN('p', "prune", &prune,
-		    N_("prune remote-tracking branches no longer on remote")),
+	OPT_BOOL('p', "prune", &prune,
+		 N_("prune remote-tracking branches no longer on remote")),
 	{ OPTION_CALLBACK, 0, "recurse-submodules", NULL, N_("on-demand"),
 		    N_("control recursive fetching of submodules"),
 		    PARSE_OPT_OPTARG, option_parse_recurse_submodules },
-	OPT_BOOLEAN(0, "dry-run", &dry_run,
-		    N_("dry run")),
-	OPT_BOOLEAN('k', "keep", &keep, N_("keep downloaded pack")),
-	OPT_BOOLEAN('u', "update-head-ok", &update_head_ok,
+	OPT_BOOL(0, "dry-run", &dry_run,
+		 N_("dry run")),
+	OPT_BOOL('k', "keep", &keep, N_("keep downloaded pack")),
+	OPT_BOOL('u', "update-head-ok", &update_head_ok,
 		    N_("allow updating of HEAD ref")),
 	OPT_BOOL(0, "progress", &progress, N_("force progress reporting")),
 	OPT_STRING(0, "depth", &depth, N_("depth"),
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 9909b6d..39fa5e8 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -611,15 +611,15 @@ static char const * const fsck_usage[] = {
 
 static struct option fsck_opts[] = {
 	OPT__VERBOSE(&verbose, N_("be verbose")),
-	OPT_BOOLEAN(0, "unreachable", &show_unreachable, N_("show unreachable objects")),
+	OPT_BOOL(0, "unreachable", &show_unreachable, N_("show unreachable objects")),
 	OPT_BOOL(0, "dangling", &show_dangling, N_("show dangling objects")),
-	OPT_BOOLEAN(0, "tags", &show_tags, N_("report tags")),
-	OPT_BOOLEAN(0, "root", &show_root, N_("report root nodes")),
-	OPT_BOOLEAN(0, "cache", &keep_cache_objects, N_("make index objects head nodes")),
-	OPT_BOOLEAN(0, "reflogs", &include_reflogs, N_("make reflogs head nodes (default)")),
-	OPT_BOOLEAN(0, "full", &check_full, N_("also consider packs and alternate objects")),
-	OPT_BOOLEAN(0, "strict", &check_strict, N_("enable more strict checking")),
-	OPT_BOOLEAN(0, "lost-found", &write_lost_and_found,
+	OPT_BOOL(0, "tags", &show_tags, N_("report tags")),
+	OPT_BOOL(0, "root", &show_root, N_("report root nodes")),
+	OPT_BOOL(0, "cache", &keep_cache_objects, N_("make index objects head nodes")),
+	OPT_BOOL(0, "reflogs", &include_reflogs, N_("make reflogs head nodes (default)")),
+	OPT_BOOL(0, "full", &check_full, N_("also consider packs and alternate objects")),
+	OPT_BOOL(0, "strict", &check_strict, N_("enable more strict checking")),
+	OPT_BOOL(0, "lost-found", &write_lost_and_found,
 				N_("write dangling objects in .git/lost-found")),
 	OPT_BOOL(0, "progress", &show_progress, N_("show progress")),
 	OPT_END(),
diff --git a/builtin/gc.c b/builtin/gc.c
index 6be6c8d..c4f7390 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -178,8 +178,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		{ OPTION_STRING, 0, "prune", &prune_expire, N_("date"),
 			N_("prune unreferenced objects"),
 			PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },
-		OPT_BOOLEAN(0, "aggressive", &aggressive, N_("be more thorough (increased runtime)")),
-		OPT_BOOLEAN(0, "auto", &auto_gc, N_("enable auto-gc mode")),
+		OPT_BOOL(0, "aggressive", &aggressive, N_("be more thorough (increased runtime)")),
+		OPT_BOOL(0, "auto", &auto_gc, N_("enable auto-gc mode")),
 		OPT_END()
 	};
 
diff --git a/builtin/grep.c b/builtin/grep.c
index d3b3b1d..7877e77 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -638,20 +638,20 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	int pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;
 
 	struct option options[] = {
-		OPT_BOOLEAN(0, "cached", &cached,
+		OPT_BOOL(0, "cached", &cached,
 			N_("search in index instead of in the work tree")),
 		OPT_NEGBIT(0, "no-index", &use_index,
 			 N_("find in contents not managed by git"), 1),
-		OPT_BOOLEAN(0, "untracked", &untracked,
+		OPT_BOOL(0, "untracked", &untracked,
 			N_("search in both tracked and untracked files")),
 		OPT_SET_INT(0, "exclude-standard", &opt_exclude,
 			    N_("search also in ignored files"), 1),
 		OPT_GROUP(""),
-		OPT_BOOLEAN('v', "invert-match", &opt.invert,
+		OPT_BOOL('v', "invert-match", &opt.invert,
 			N_("show non-matching lines")),
-		OPT_BOOLEAN('i', "ignore-case", &opt.ignore_case,
+		OPT_BOOL('i', "ignore-case", &opt.ignore_case,
 			N_("case insensitive matching")),
-		OPT_BOOLEAN('w', "word-regexp", &opt.word_regexp,
+		OPT_BOOL('w', "word-regexp", &opt.word_regexp,
 			N_("match patterns only at word boundaries")),
 		OPT_SET_INT('a', "text", &opt.binary,
 			N_("process binary files as text"), GREP_BINARY_TEXT),
@@ -675,26 +675,26 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			    N_("use Perl-compatible regular expressions"),
 			    GREP_PATTERN_TYPE_PCRE),
 		OPT_GROUP(""),
-		OPT_BOOLEAN('n', "line-number", &opt.linenum, N_("show line numbers")),
+		OPT_BOOL('n', "line-number", &opt.linenum, N_("show line numbers")),
 		OPT_NEGBIT('h', NULL, &opt.pathname, N_("don't show filenames"), 1),
 		OPT_BIT('H', NULL, &opt.pathname, N_("show filenames"), 1),
 		OPT_NEGBIT(0, "full-name", &opt.relative,
 			N_("show filenames relative to top directory"), 1),
-		OPT_BOOLEAN('l', "files-with-matches", &opt.name_only,
+		OPT_BOOL('l', "files-with-matches", &opt.name_only,
 			N_("show only filenames instead of matching lines")),
-		OPT_BOOLEAN(0, "name-only", &opt.name_only,
+		OPT_BOOL(0, "name-only", &opt.name_only,
 			N_("synonym for --files-with-matches")),
-		OPT_BOOLEAN('L', "files-without-match",
+		OPT_BOOL('L', "files-without-match",
 			&opt.unmatch_name_only,
 			N_("show only the names of files without match")),
-		OPT_BOOLEAN('z', "null", &opt.null_following_name,
+		OPT_BOOL('z', "null", &opt.null_following_name,
 			N_("print NUL after filenames")),
-		OPT_BOOLEAN('c', "count", &opt.count,
+		OPT_BOOL('c', "count", &opt.count,
 			N_("show the number of matches instead of matching lines")),
 		OPT__COLOR(&opt.color, N_("highlight matches")),
-		OPT_BOOLEAN(0, "break", &opt.file_break,
+		OPT_BOOL(0, "break", &opt.file_break,
 			N_("print empty line between matches from different files")),
-		OPT_BOOLEAN(0, "heading", &opt.heading,
+		OPT_BOOL(0, "heading", &opt.heading,
 			N_("show filename only once above matches from same file")),
 		OPT_GROUP(""),
 		OPT_CALLBACK('C', "context", &opt, N_("n"),
@@ -706,9 +706,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			N_("show <n> context lines after matches")),
 		OPT_NUMBER_CALLBACK(&opt, N_("shortcut for -C NUM"),
 			context_callback),
-		OPT_BOOLEAN('p', "show-function", &opt.funcname,
+		OPT_BOOL('p', "show-function", &opt.funcname,
 			N_("show a line with the function name before matches")),
-		OPT_BOOLEAN('W', "function-context", &opt.funcbody,
+		OPT_BOOL('W', "function-context", &opt.funcbody,
 			N_("show the surrounding function")),
 		OPT_GROUP(""),
 		OPT_CALLBACK('f', NULL, &opt, N_("file"),
@@ -718,7 +718,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		{ OPTION_CALLBACK, 0, "and", &opt, NULL,
 		  N_("combine patterns specified with -e"),
 		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, and_callback },
-		OPT_BOOLEAN(0, "or", &dummy, ""),
+		OPT_BOOL(0, "or", &dummy, ""),
 		{ OPTION_CALLBACK, 0, "not", &opt, NULL, "",
 		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, not_callback },
 		{ OPTION_CALLBACK, '(', NULL, &opt, NULL, "",
@@ -729,7 +729,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		  close_callback },
 		OPT__QUIET(&opt.status_only,
 			   N_("indicate hit with exit status without output")),
-		OPT_BOOLEAN(0, "all-match", &opt.all_match,
+		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"),
@@ -738,8 +738,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		{ OPTION_STRING, 'O', "open-files-in-pager", &show_in_pager,
 			N_("pager"), N_("show matching files in the pager"),
 			PARSE_OPT_OPTARG, NULL, (intptr_t)default_pager },
-		OPT_BOOLEAN(0, "ext-grep", &external_grep_allowed__ignored,
-			    N_("allow calling of grep(1) (ignored by this build)")),
+		OPT_BOOL(0, "ext-grep", &external_grep_allowed__ignored,
+			 N_("allow calling of grep(1) (ignored by this build)")),
 		{ OPTION_CALLBACK, 0, "help-all", &options, NULL, N_("show usage"),
 		  PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, help_callback },
 		OPT_END()
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 8d184f1..4aea5bb 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -70,10 +70,10 @@ static const char *vpath;
 
 static const struct option hash_object_options[] = {
 	OPT_STRING('t', NULL, &type, N_("type"), N_("object type")),
-	OPT_BOOLEAN('w', NULL, &write_object, N_("write the object into the object database")),
+	OPT_BOOL('w', NULL, &write_object, N_("write the object into the object database")),
 	OPT_BOOLEAN( 0 , "stdin", &hashstdin, N_("read the object from stdin")),
-	OPT_BOOLEAN( 0 , "stdin-paths", &stdin_paths, N_("read file names from stdin")),
-	OPT_BOOLEAN( 0 , "no-filters", &no_filters, N_("store file as is without filters")),
+	OPT_BOOL( 0 , "stdin-paths", &stdin_paths, N_("read file names from stdin")),
+	OPT_BOOL( 0 , "no-filters", &no_filters, N_("store file as is without filters")),
 	OPT_STRING( 0 , "path", &vpath, N_("file"), N_("process file as it were from this path")),
 	OPT_END()
 };
diff --git a/builtin/log.c b/builtin/log.c
index 05e374d..1dafbd0 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1179,10 +1179,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		{ OPTION_CALLBACK, 'k', "keep-subject", &rev, NULL,
 			    N_("don't strip/add [PATCH]"),
 			    PARSE_OPT_NOARG | PARSE_OPT_NONEG, keep_callback },
-		OPT_BOOLEAN(0, "no-binary", &no_binary_diff,
-			    N_("don't output binary diffs")),
-		OPT_BOOLEAN(0, "ignore-if-in-upstream", &ignore_if_in_upstream,
-			    N_("don't include a patch matching a commit upstream")),
+		OPT_BOOL(0, "no-binary", &no_binary_diff,
+			 N_("don't output binary diffs")),
+		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},
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 5cf3e31..963ccc9 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -461,24 +461,24 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 		{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
 			N_("paths are separated with NUL character"),
 			PARSE_OPT_NOARG, option_parse_z },
-		OPT_BOOLEAN('t', NULL, &show_tag,
+		OPT_BOOL('t', NULL, &show_tag,
 			N_("identify the file status with tags")),
-		OPT_BOOLEAN('v', NULL, &show_valid_bit,
+		OPT_BOOL('v', NULL, &show_valid_bit,
 			N_("use lowercase letters for 'assume unchanged' files")),
-		OPT_BOOLEAN('c', "cached", &show_cached,
+		OPT_BOOL('c', "cached", &show_cached,
 			N_("show cached files in the output (default)")),
-		OPT_BOOLEAN('d', "deleted", &show_deleted,
+		OPT_BOOL('d', "deleted", &show_deleted,
 			N_("show deleted files in the output")),
-		OPT_BOOLEAN('m', "modified", &show_modified,
+		OPT_BOOL('m', "modified", &show_modified,
 			N_("show modified files in the output")),
-		OPT_BOOLEAN('o', "others", &show_others,
+		OPT_BOOL('o', "others", &show_others,
 			N_("show other files in the output")),
 		OPT_BIT('i', "ignored", &dir.flags,
 			N_("show ignored files in the output"),
 			DIR_SHOW_IGNORED),
-		OPT_BOOLEAN('s', "stage", &show_stage,
+		OPT_BOOL('s', "stage", &show_stage,
 			N_("show staged contents' object name in the output")),
-		OPT_BOOLEAN('k', "killed", &show_killed,
+		OPT_BOOL('k', "killed", &show_killed,
 			N_("show files on the filesystem that need to be removed")),
 		OPT_BIT(0, "directory", &dir.flags,
 			N_("show 'other' directories' name only"),
@@ -486,9 +486,9 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 		OPT_NEGBIT(0, "empty-directory", &dir.flags,
 			N_("don't show empty directories"),
 			DIR_HIDE_EMPTY_DIRECTORIES),
-		OPT_BOOLEAN('u', "unmerged", &show_unmerged,
+		OPT_BOOL('u', "unmerged", &show_unmerged,
 			N_("show unmerged files in the output")),
-		OPT_BOOLEAN(0, "resolve-undo", &show_resolve_undo,
+		OPT_BOOL(0, "resolve-undo", &show_resolve_undo,
 			    N_("show resolve-undo information")),
 		{ OPTION_CALLBACK, 'x', "exclude", &exclude_list, N_("pattern"),
 			N_("skip files matching pattern"),
@@ -504,12 +504,12 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 		{ 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_BOOLEAN(0, "error-unmatch", &error_unmatch,
+		OPT_BOOL(0, "error-unmatch", &error_unmatch,
 			N_("if any <file> is not in the index, treat this as an error")),
 		OPT_STRING(0, "with-tree", &with_tree, N_("tree-ish"),
 			N_("pretend that paths removed since <tree-ish> are still present")),
 		OPT__ABBREV(&abbrev),
-		OPT_BOOLEAN(0, "debug", &debug_mode, N_("show debugging data")),
+		OPT_BOOL(0, "debug", &debug_mode, N_("show debugging data")),
 		OPT_END()
 	};
 
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index fb76e38..de88563 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -138,9 +138,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 			LS_NAME_ONLY),
 		OPT_SET_INT(0, "full-name", &chomp_prefix,
 			    N_("use full path names"), 0),
-		OPT_BOOLEAN(0, "full-tree", &full_tree,
-			    N_("list entire tree; not just current directory "
-			       "(implies --full-name)")),
+		OPT_BOOL(0, "full-tree", &full_tree,
+			 N_("list entire tree; not just current directory "
+			    "(implies --full-name)")),
 		OPT__ABBREV(&abbrev),
 		OPT_END()
 	};
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index 0c4cd2f..e88eb93 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -95,11 +95,11 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)
 	int is_ancestor = 0;
 
 	struct option options[] = {
-		OPT_BOOLEAN('a', "all", &show_all, N_("output all common ancestors")),
-		OPT_BOOLEAN(0, "octopus", &octopus, N_("find ancestors for a single n-way merge")),
-		OPT_BOOLEAN(0, "independent", &reduce, N_("list revs not reachable from others")),
-		OPT_BOOLEAN(0, "is-ancestor", &is_ancestor,
-			    N_("is the first one ancestor of the other?")),
+		OPT_BOOL('a', "all", &show_all, N_("output all common ancestors")),
+		OPT_BOOL(0, "octopus", &octopus, N_("find ancestors for a single n-way merge")),
+		OPT_BOOL(0, "independent", &reduce, N_("list revs not reachable from others")),
+		OPT_BOOL(0, "is-ancestor", &is_ancestor,
+			 N_("is the first one ancestor of the other?")),
 		OPT_END()
 	};
 
diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index c0570f2..844f84f 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -30,7 +30,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	int quiet = 0;
 	int prefixlen = 0;
 	struct option options[] = {
-		OPT_BOOLEAN('p', "stdout", &to_stdout, N_("send results to standard output")),
+		OPT_BOOL('p', "stdout", &to_stdout, N_("send results to standard output")),
 		OPT_SET_INT(0, "diff3", &xmp.style, N_("use a diff3 based merge"), XDL_MERGE_DIFF3),
 		OPT_SET_INT(0, "ours", &xmp.favor, N_("for conflicts, use our version"),
 			    XDL_MERGE_FAVOR_OURS),
diff --git a/builtin/merge.c b/builtin/merge.c
index 34a6166..a8cf4a2 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -197,15 +197,15 @@ static struct option builtin_merge_options[] = {
 	{ OPTION_CALLBACK, 'n', NULL, NULL, NULL,
 		N_("do not show a diffstat at the end of the merge"),
 		PARSE_OPT_NOARG, option_parse_n },
-	OPT_BOOLEAN(0, "stat", &show_diffstat,
+	OPT_BOOL(0, "stat", &show_diffstat,
 		N_("show a diffstat at the end of the merge")),
-	OPT_BOOLEAN(0, "summary", &show_diffstat, N_("(synonym to --stat)")),
+	OPT_BOOL(0, "summary", &show_diffstat, N_("(synonym to --stat)")),
 	{ OPTION_INTEGER, 0, "log", &shortlog_len, N_("n"),
 	  N_("add (at most <n>) entries from shortlog to merge commit message"),
 	  PARSE_OPT_OPTARG, NULL, DEFAULT_MERGE_LOG_LEN },
-	OPT_BOOLEAN(0, "squash", &squash,
+	OPT_BOOL(0, "squash", &squash,
 		N_("create a single commit instead of doing a merge")),
-	OPT_BOOLEAN(0, "commit", &option_commit,
+	OPT_BOOL(0, "commit", &option_commit,
 		N_("perform a commit if the merge succeeds (default)")),
 	OPT_BOOL('e', "edit", &option_edit,
 		N_("edit message before committing")),
@@ -224,12 +224,12 @@ static struct option builtin_merge_options[] = {
 		N_("merge commit message (for a non-fast-forward merge)"),
 		option_parse_message),
 	OPT__VERBOSITY(&verbosity),
-	OPT_BOOLEAN(0, "abort", &abort_current_merge,
+	OPT_BOOL(0, "abort", &abort_current_merge,
 		N_("abort the current in-progress merge")),
 	OPT_SET_INT(0, "progress", &show_progress, N_("force progress reporting"), 1),
 	{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key id"),
 	  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
-	OPT_BOOLEAN(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
+	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
 	OPT_END()
 };
 
diff --git a/builtin/mv.c b/builtin/mv.c
index 034fec9..be6fa77 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -62,7 +62,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 		OPT__VERBOSE(&verbose, N_("be verbose")),
 		OPT__DRY_RUN(&show_only, N_("dry run")),
 		OPT__FORCE(&force, N_("force move/rename even if target exists")),
-		OPT_BOOLEAN('k', NULL, &ignore_errors, N_("skip move/rename errors")),
+		OPT_BOOL('k', NULL, &ignore_errors, N_("skip move/rename errors")),
 		OPT_END(),
 	};
 	const char **source, **destination, **dest_path;
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 0aaa19e..a908a34 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -310,15 +310,15 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 	int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
 	struct name_ref_data data = { 0, 0, NULL };
 	struct option opts[] = {
-		OPT_BOOLEAN(0, "name-only", &data.name_only, N_("print only names (no SHA-1)")),
-		OPT_BOOLEAN(0, "tags", &data.tags_only, N_("only use tags to name the commits")),
+		OPT_BOOL(0, "name-only", &data.name_only, N_("print only names (no SHA-1)")),
+		OPT_BOOL(0, "tags", &data.tags_only, N_("only use tags to name the commits")),
 		OPT_STRING(0, "refs", &data.ref_filter, N_("pattern"),
 				   N_("only use refs matching <pattern>")),
 		OPT_GROUP(""),
-		OPT_BOOLEAN(0, "all", &all, N_("list all commits reachable from all refs")),
-		OPT_BOOLEAN(0, "stdin", &transform_stdin, N_("read from stdin")),
-		OPT_BOOLEAN(0, "undefined", &allow_undefined, N_("allow to print `undefined` names")),
-		OPT_BOOLEAN(0, "always",     &always,
+		OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
+		OPT_BOOL(0, "stdin", &transform_stdin, N_("read from stdin")),
+		OPT_BOOL(0, "undefined", &allow_undefined, N_("allow to print `undefined` names (default)")),
+		OPT_BOOL(0, "always",     &always,
 			   N_("show abbreviated commit object as fallback")),
 		{
 			/* A Hidden OPT_BOOL */
diff --git a/builtin/notes.c b/builtin/notes.c
index 8f63cb2..d459e23 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -483,7 +483,7 @@ static int copy(int argc, const char **argv, const char *prefix)
 	const char *rewrite_cmd = NULL;
 	struct option options[] = {
 		OPT__FORCE(&force, N_("replace existing notes")),
-		OPT_BOOLEAN(0, "stdin", &from_stdin, N_("read objects from stdin")),
+		OPT_BOOL(0, "stdin", &from_stdin, N_("read objects from stdin")),
 		OPT_STRING(0, "for-rewrite", &rewrite_cmd, N_("command"),
 			   N_("load rewriting config for <command> (implies "
 			      "--stdin)")),
@@ -853,7 +853,7 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)
 		OPT_BIT(0, "ignore-missing", &flag,
 			N_("attempt to remove non-existent note is not an error"),
 			IGNORE_MISSING),
-		OPT_BOOLEAN(0, "stdin", &from_stdin,
+		OPT_BOOL(0, "stdin", &from_stdin,
 			    N_("read object names from the standard input")),
 		OPT_END()
 	};
diff --git a/builtin/push.c b/builtin/push.c
index 6d36c24..bebaf3d 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -446,15 +446,15 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BIT( 0 , "all", &flags, N_("push all refs"), TRANSPORT_PUSH_ALL),
 		OPT_BIT( 0 , "mirror", &flags, N_("mirror all refs"),
 			    (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)),
-		OPT_BOOLEAN( 0, "delete", &deleterefs, N_("delete refs")),
-		OPT_BOOLEAN( 0 , "tags", &tags, N_("push tags (can't be used with --all or --mirror)")),
+		OPT_BOOL( 0, "delete", &deleterefs, N_("delete refs")),
+		OPT_BOOL( 0 , "tags", &tags, N_("push tags (can't be used with --all or --mirror)")),
 		OPT_BIT('n' , "dry-run", &flags, N_("dry run"), TRANSPORT_PUSH_DRY_RUN),
 		OPT_BIT( 0,  "porcelain", &flags, N_("machine-readable output"), TRANSPORT_PUSH_PORCELAIN),
 		OPT_BIT('f', "force", &flags, N_("force updates"), TRANSPORT_PUSH_FORCE),
 		{ OPTION_CALLBACK, 0, "recurse-submodules", &flags, N_("check"),
 			N_("control recursive pushing of submodules"),
 			PARSE_OPT_OPTARG, option_parse_recurse_submodules },
-		OPT_BOOLEAN( 0 , "thin", &thin, N_("use thin pack")),
+		OPT_BOOL( 0 , "thin", &thin, N_("use thin pack")),
 		OPT_STRING( 0 , "receive-pack", &receivepack, "receive-pack", N_("receive pack program")),
 		OPT_STRING( 0 , "exec", &receivepack, "receive-pack", N_("receive pack program")),
 		OPT_BIT('u', "set-upstream", &flags, N_("set upstream for git pull/status"),
diff --git a/builtin/remote.c b/builtin/remote.c
index 5e54d36..eaac3e2 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -160,7 +160,7 @@ static int add(int argc, const char **argv)
 	int i;
 
 	struct option options[] = {
-		OPT_BOOLEAN('f', "fetch", &fetch, N_("fetch the remote branches")),
+		OPT_BOOL('f', "fetch", &fetch, N_("fetch the remote branches")),
 		OPT_SET_INT(0, "tags", &fetch_tags,
 			    N_("import all tags and associated objects when fetching"),
 			    TAGS_SET),
@@ -1088,7 +1088,7 @@ static int show(int argc, const char **argv)
 {
 	int no_query = 0, result = 0, query_flag = 0;
 	struct option options[] = {
-		OPT_BOOLEAN('n', NULL, &no_query, N_("do not query remotes")),
+		OPT_BOOL('n', NULL, &no_query, N_("do not query remotes")),
 		OPT_END()
 	};
 	struct ref_states states;
@@ -1195,10 +1195,10 @@ static int set_head(int argc, const char **argv)
 	char *head_name = NULL;
 
 	struct option options[] = {
-		OPT_BOOLEAN('a', "auto", &opt_a,
-			    N_("set refs/remotes/<name>/HEAD according to remote")),
-		OPT_BOOLEAN('d', "delete", &opt_d,
-			    N_("delete refs/remotes/<name>/HEAD")),
+		OPT_BOOL('a', "auto", &opt_a,
+			 N_("set refs/remotes/<name>/HEAD according to remote")),
+		OPT_BOOL('d', "delete", &opt_d,
+			 N_("delete refs/remotes/<name>/HEAD")),
 		OPT_END()
 	};
 	argc = parse_options(argc, argv, NULL, options, builtin_remote_sethead_usage,
@@ -1317,8 +1317,8 @@ static int update(int argc, const char **argv)
 {
 	int i, prune = 0;
 	struct option options[] = {
-		OPT_BOOLEAN('p', "prune", &prune,
-			    N_("prune remotes after fetching")),
+		OPT_BOOL('p', "prune", &prune,
+			 N_("prune remotes after fetching")),
 		OPT_END()
 	};
 	const char **fetch_argv;
@@ -1404,7 +1404,7 @@ static int set_branches(int argc, const char **argv)
 {
 	int add_mode = 0;
 	struct option options[] = {
-		OPT_BOOLEAN('\0', "add", &add_mode, N_("add branch")),
+		OPT_BOOL('\0', "add", &add_mode, N_("add branch")),
 		OPT_END()
 	};
 
@@ -1432,11 +1432,11 @@ static int set_url(int argc, const char **argv)
 	int urlset_nr;
 	struct strbuf name_buf = STRBUF_INIT;
 	struct option options[] = {
-		OPT_BOOLEAN('\0', "push", &push_mode,
-			    N_("manipulate push URLs")),
-		OPT_BOOLEAN('\0', "add", &add_mode,
-			    N_("add URL")),
-		OPT_BOOLEAN('\0', "delete", &delete_mode,
+		OPT_BOOL('\0', "push", &push_mode,
+			 N_("manipulate push URLs")),
+		OPT_BOOL('\0', "add", &add_mode,
+			 N_("add URL")),
+		OPT_BOOL('\0', "delete", &delete_mode,
 			    N_("delete URLs")),
 		OPT_END()
 	};
diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..11b0a55 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -118,9 +118,9 @@ int cmd_replace(int argc, const char **argv, const char *prefix)
 {
 	int list = 0, delete = 0, force = 0;
 	struct option options[] = {
-		OPT_BOOLEAN('l', NULL, &list, N_("list replace refs")),
-		OPT_BOOLEAN('d', NULL, &delete, N_("delete replace refs")),
-		OPT_BOOLEAN('f', NULL, &force, N_("replace the ref if it exists")),
+		OPT_BOOL('l', NULL, &list, N_("list replace refs")),
+		OPT_BOOL('d', NULL, &delete, N_("delete replace refs")),
+		OPT_BOOL('f', NULL, &force, N_("replace the ref if it exists")),
 		OPT_END()
 	};
 
diff --git a/builtin/reset.c b/builtin/reset.c
index afa6e02..0905815 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -258,7 +258,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 				N_("reset HEAD, index and working tree"), MERGE),
 		OPT_SET_INT(0, "keep", &reset_type,
 				N_("reset HEAD but keep local changes"), KEEP),
-		OPT_BOOLEAN('p', "patch", &patch_mode, N_("select hunks interactively")),
+		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
 		OPT_END()
 	};
 
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index de894c7..67e8ccf 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -346,9 +346,9 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 		NULL
 	};
 	static struct option parseopt_opts[] = {
-		OPT_BOOLEAN(0, "keep-dashdash", &keep_dashdash,
+		OPT_BOOL(0, "keep-dashdash", &keep_dashdash,
 					N_("keep the `--` passed as an arg")),
-		OPT_BOOLEAN(0, "stop-at-non-option", &stop_at_non_option,
+		OPT_BOOL(0, "stop-at-non-option", &stop_at_non_option,
 					N_("stop parsing after the "
 					   "first non-option argument")),
 		OPT_END(),
diff --git a/builtin/rm.c b/builtin/rm.c
index 18916e0..ce28fa9 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -268,10 +268,10 @@ static int ignore_unmatch = 0;
 static struct option builtin_rm_options[] = {
 	OPT__DRY_RUN(&show_only, N_("dry run")),
 	OPT__QUIET(&quiet, N_("do not list removed files")),
-	OPT_BOOLEAN( 0 , "cached",         &index_only, N_("only remove from the index")),
+	OPT_BOOL( 0 , "cached",         &index_only, N_("only remove from the index")),
 	OPT__FORCE(&force, N_("override the up-to-date check")),
-	OPT_BOOLEAN('r', NULL,             &recursive,  N_("allow recursive removal")),
-	OPT_BOOLEAN( 0 , "ignore-unmatch", &ignore_unmatch,
+	OPT_BOOL('r', NULL,             &recursive,  N_("allow recursive removal")),
+	OPT_BOOL( 0 , "ignore-unmatch", &ignore_unmatch,
 				N_("exit with a zero status even if nothing matched")),
 	OPT_END(),
 };
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 1434f8f..ae73d17 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -224,12 +224,12 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 	int nongit = !startup_info->have_repository;
 
 	static const struct option options[] = {
-		OPT_BOOLEAN('n', "numbered", &log.sort_by_number,
-			    N_("sort output according to the number of commits per author")),
-		OPT_BOOLEAN('s', "summary", &log.summary,
-			    N_("Suppress commit descriptions, only provides commit count")),
-		OPT_BOOLEAN('e', "email", &log.email,
-			    N_("Show the email address of each author")),
+		OPT_BOOL('n', "numbered", &log.sort_by_number,
+			 N_("sort output according to the number of commits per author")),
+		OPT_BOOL('s', "summary", &log.summary,
+			 N_("Suppress commit descriptions, only provides commit count")),
+		OPT_BOOL('e', "email", &log.email,
+			 N_("Show the email address of each author")),
 		{ OPTION_CALLBACK, 'w', NULL, &log, N_("w[,i1[,i2]]"),
 			N_("Linewrap output"), PARSE_OPT_OPTARG, &parse_wrap_args },
 		OPT_END(),
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 9788eb1..001f29c 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -646,30 +646,30 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 	int dense = 1;
 	const char *reflog_base = NULL;
 	struct option builtin_show_branch_options[] = {
-		OPT_BOOLEAN('a', "all", &all_heads,
-			    N_("show remote-tracking and local branches")),
-		OPT_BOOLEAN('r', "remotes", &all_remotes,
-			    N_("show remote-tracking branches")),
+		OPT_BOOL('a', "all", &all_heads,
+			 N_("show remote-tracking and local branches")),
+		OPT_BOOL('r', "remotes", &all_remotes,
+			 N_("show remote-tracking branches")),
 		OPT__COLOR(&showbranch_use_color,
 			    N_("color '*!+-' corresponding to the branch")),
 		{ OPTION_INTEGER, 0, "more", &extra, N_("n"),
 			    N_("show <n> more commits after the common ancestor"),
 			    PARSE_OPT_OPTARG, NULL, (intptr_t)1 },
 		OPT_SET_INT(0, "list", &extra, N_("synonym to more=-1"), -1),
-		OPT_BOOLEAN(0, "no-name", &no_name, N_("suppress naming strings")),
-		OPT_BOOLEAN(0, "current", &with_current_branch,
-			    N_("include the current branch")),
-		OPT_BOOLEAN(0, "sha1-name", &sha1_name,
-			    N_("name commits with their object names")),
-		OPT_BOOLEAN(0, "merge-base", &merge_base,
-			    N_("show possible merge bases")),
-		OPT_BOOLEAN(0, "independent", &independent,
+		OPT_BOOL(0, "no-name", &no_name, N_("suppress naming strings")),
+		OPT_BOOL(0, "current", &with_current_branch,
+			 N_("include the current branch")),
+		OPT_BOOL(0, "sha1-name", &sha1_name,
+			 N_("name commits with their object names")),
+		OPT_BOOL(0, "merge-base", &merge_base,
+			 N_("show possible merge bases")),
+		OPT_BOOL(0, "independent", &independent,
 			    N_("show refs unreachable from any other ref")),
 		OPT_SET_INT(0, "topo-order", &sort_order,
 			    N_("show commits in topological order"),
 			    REV_SORT_IN_GRAPH_ORDER),
-		OPT_BOOLEAN(0, "topics", &topics,
-			    N_("show only commits not on the first branch")),
+		OPT_BOOL(0, "topics", &topics,
+			 N_("show only commits not on the first branch")),
 		OPT_SET_INT(0, "sparse", &dense,
 			    N_("show merges reachable from only one tip"), 0),
 		OPT_SET_INT(0, "date-order", &sort_order,
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 18680bb..9f3f5e3 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -165,15 +165,15 @@ static int help_callback(const struct option *opt, const char *arg, int unset)
 }
 
 static const struct option show_ref_options[] = {
-	OPT_BOOLEAN(0, "tags", &tags_only, N_("only show tags (can be combined with heads)")),
-	OPT_BOOLEAN(0, "heads", &heads_only, N_("only show heads (can be combined with tags)")),
-	OPT_BOOLEAN(0, "verify", &verify, N_("stricter reference checking, "
+	OPT_BOOL(0, "tags", &tags_only, N_("only show tags (can be combined with heads)")),
+	OPT_BOOL(0, "heads", &heads_only, N_("only show heads (can be combined with tags)")),
+	OPT_BOOL(0, "verify", &verify, N_("stricter reference checking, "
 		    "requires exact ref path")),
 	OPT_HIDDEN_BOOL('h', NULL, &show_head,
 			N_("show the HEAD reference, even if it would be filtered out")),
-	OPT_BOOLEAN(0, "head", &show_head,
+	OPT_BOOL(0, "head", &show_head,
 	  N_("show the HEAD reference, even if it would be filtered out")),
-	OPT_BOOLEAN('d', "dereference", &deref_tags,
+	OPT_BOOL('d', "dereference", &deref_tags,
 		    N_("dereference tags into object IDs")),
 	{ OPTION_CALLBACK, 's', "hash", &abbrev, N_("n"),
 	  N_("only show SHA1 hash using <n> digits"),
diff --git a/builtin/tag.c b/builtin/tag.c
index d8ae5aa..b577af5 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -450,12 +450,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		OPT_CMDMODE('v', "verify", &cmdmode, N_("verify tags"), 'v'),
 
 		OPT_GROUP(N_("Tag creation options")),
-		OPT_BOOLEAN('a', "annotate", &annotate,
+		OPT_BOOL('a', "annotate", &annotate,
 					N_("annotated tag, needs a message")),
 		OPT_CALLBACK('m', "message", &msg, N_("message"),
 			     N_("tag message"), parse_msg_arg),
 		OPT_FILENAME('F', "file", &msgfile, N_("read message from file")),
-		OPT_BOOLEAN('s', "sign", &opt.sign, N_("annotated and GPG-signed tag")),
+		OPT_BOOL('s', "sign", &opt.sign, N_("annotated and GPG-signed tag")),
 		OPT_STRING(0, "cleanup", &cleanup_arg, N_("mode"),
 			N_("how to strip spaces and #comments from message")),
 		OPT_STRING('u', "local-user", &keyid, N_("key id"),
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 51d2684..7484d36 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -16,8 +16,8 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
 	int delete = 0, no_deref = 0, flags = 0;
 	struct option options[] = {
 		OPT_STRING( 'm', NULL, &msg, N_("reason"), N_("reason of the update")),
-		OPT_BOOLEAN('d', NULL, &delete, N_("delete the reference")),
-		OPT_BOOLEAN( 0 , "no-deref", &no_deref,
+		OPT_BOOL('d', NULL, &delete, N_("delete the reference")),
+		OPT_BOOL( 0 , "no-deref", &no_deref,
 					N_("update <refname> not the one it points to")),
 		OPT_END(),
 	};
-- 
1.8.4.rc0.16.g7fca822.dirty

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

* [PATCHv3 3/9] log, format-patch: parsing uses OPT__QUIET
  2013-08-03 11:51 [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
  2013-08-03 11:51 ` [PATCHv3 1/9] Remove deprecated OPTION_BOOLEAN for parsing arguments Stefan Beller
  2013-08-03 11:51 ` [PATCHv3 2/9] Replace deprecated OPT_BOOLEAN by OPT_BOOL Stefan Beller
@ 2013-08-03 11:51 ` Stefan Beller
  2013-08-03 11:51 ` [PATCHv3 4/9] checkout: remove superfluous local variable Stefan Beller
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: Stefan Beller @ 2013-08-03 11:51 UTC (permalink / raw)
  To: git, gitster, sunshine; +Cc: Stefan Beller

This patch allows users to use the short form -q on
log and format-patch, which was non possible before.

Also the documentation of format-patch mentions -q now.

The documentation of log doesn't even talk about --quiet, so I'll leave
that for more experienced git contributors. ;)
It doesn't seem to change the default behavior, but in combination
with --stat for example it suppresses the actual stats.
however the only relevant code in log is
	if (quiet)
		rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;

Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
---
 Documentation/git-format-patch.txt | 1 +
 builtin/log.c                      | 5 ++---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index e394276..9e0ef0e 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -242,6 +242,7 @@ configuration options in linkgit:git-notes[1] to use this workflow).
 Note that the leading character does not have to be a dot; for example,
 you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.
 
+-q::
 --quiet::
 	Do not print the names of the generated files to standard output.
 
diff --git a/builtin/log.c b/builtin/log.c
index 1dafbd0..ed4dec4 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -121,7 +121,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	static struct line_opt_callback_data line_cb = {NULL, NULL, STRING_LIST_INIT_DUP};
 
 	const struct option builtin_log_options[] = {
-		OPT_BOOL(0, "quiet", &quiet, N_("suppress diff output")),
+		OPT__QUIET(&quiet, N_("suppress diff output")),
 		OPT_BOOL(0, "source", &source, N_("show source")),
 		OPT_BOOL(0, "use-mailmap", &mailmap, N_("Use mail map file")),
 		{ OPTION_CALLBACK, 0, "decorate", NULL, NULL, N_("decorate options"),
@@ -1210,8 +1210,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			    PARSE_OPT_OPTARG, thread_callback },
 		OPT_STRING(0, "signature", &signature, N_("signature"),
 			    N_("add a signature")),
-		OPT_BOOLEAN(0, "quiet", &quiet,
-			    N_("don't print the patch filenames")),
+		OPT__QUIET(&quiet, N_("don't print the patch filenames")),
 		OPT_END()
 	};
 
-- 
1.8.4.rc0.16.g7fca822.dirty

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

* [PATCHv3 4/9] checkout: remove superfluous local variable
  2013-08-03 11:51 [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
                   ` (2 preceding siblings ...)
  2013-08-03 11:51 ` [PATCHv3 3/9] log, format-patch: parsing uses OPT__QUIET Stefan Beller
@ 2013-08-03 11:51 ` Stefan Beller
  2013-08-03 11:51 ` [PATCHv3 5/9] branch, commit, name-rev: ease up boolean conditions Stefan Beller
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: Stefan Beller @ 2013-08-03 11:51 UTC (permalink / raw)
  To: git, gitster, sunshine; +Cc: Stefan Beller

Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
---
 builtin/checkout.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 8b48f4a..ed39cec 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -228,8 +228,6 @@ static int checkout_paths(const struct checkout_opts *opts,
 	int flag;
 	struct commit *head;
 	int errs = 0;
-	int stage = opts->writeout_stage;
-	int merge = opts->merge;
 	int newfd;
 	struct lock_file *lock_file;
 
@@ -327,8 +325,8 @@ static int checkout_paths(const struct checkout_opts *opts,
 				continue;
 			if (opts->force) {
 				warning(_("path '%s' is unmerged"), ce->name);
-			} else if (stage) {
-				errs |= check_stage(stage, ce, pos);
+			} else if (opts->writeout_stage) {
+				errs |= check_stage(opts->writeout_stage, ce, pos);
 			} else if (opts->merge) {
 				errs |= check_stages((1<<2) | (1<<3), ce, pos);
 			} else {
@@ -352,9 +350,9 @@ static int checkout_paths(const struct checkout_opts *opts,
 				errs |= checkout_entry(ce, &state, NULL);
 				continue;
 			}
-			if (stage)
-				errs |= checkout_stage(stage, ce, pos, &state);
-			else if (merge)
+			if (opts->writeout_stage)
+				errs |= checkout_stage(opts->writeout_stage, ce, pos, &state);
+			else if (opts->merge)
 				errs |= checkout_merged(pos, &state);
 			pos = skip_same_name(ce, pos) - 1;
 		}
-- 
1.8.4.rc0.16.g7fca822.dirty

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

* [PATCHv3 5/9] branch, commit, name-rev: ease up boolean conditions
  2013-08-03 11:51 [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
                   ` (3 preceding siblings ...)
  2013-08-03 11:51 ` [PATCHv3 4/9] checkout: remove superfluous local variable Stefan Beller
@ 2013-08-03 11:51 ` Stefan Beller
  2013-08-03 11:51 ` [PATCHv3 6/9] hash-object: Replace stdin parsing OPT_BOOLEAN by OPT_COUNTUP Stefan Beller
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: Stefan Beller @ 2013-08-03 11:51 UTC (permalink / raw)
  To: git, gitster, sunshine; +Cc: Stefan Beller

Now that the variables are readin by OPT_BOOL, which makes sure
to have the values being 0 or 1 after reading, we do not need
the double negation to map any other value to 1 for integer
variables.

Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
---
 builtin/branch.c   | 3 ++-
 builtin/commit.c   | 2 +-
 builtin/name-rev.c | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 4daed0b..33ba1fb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -872,7 +872,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (with_commit || merge_filter != NO_FILTER)
 		list = 1;
 
-	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1)
+	if (delete + rename + force_create + list + unset_upstream +
+	    !!new_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
 	if (abbrev == -1)
diff --git a/builtin/commit.c b/builtin/commit.c
index c20426b..b0f86c8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1072,7 +1072,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
 	if (patch_interactive)
 		interactive = 1;
 
-	if (!!also + !!only + !!all + !!interactive > 1)
+	if (also + only + all + interactive > 1)
 		die(_("Only one of --include/--only/--all/--interactive/--patch can be used."));
 	if (argc == 0 && (also || (only && !amend)))
 		die(_("No paths with --include/--only does not make sense."));
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index a908a34..20fcf8c 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -331,7 +331,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
-	if (!!all + !!transform_stdin + !!argc > 1) {
+	if (all + transform_stdin + !!argc > 1) {
 		error("Specify either a list, or --all, not both!");
 		usage_with_options(name_rev_usage, opts);
 	}
-- 
1.8.4.rc0.16.g7fca822.dirty

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

* [PATCHv3 6/9] hash-object: Replace stdin parsing OPT_BOOLEAN by OPT_COUNTUP
  2013-08-03 11:51 [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
                   ` (4 preceding siblings ...)
  2013-08-03 11:51 ` [PATCHv3 5/9] branch, commit, name-rev: ease up boolean conditions Stefan Beller
@ 2013-08-03 11:51 ` Stefan Beller
  2013-08-05 18:50   ` Junio C Hamano
  2013-08-03 11:51 ` [PATCHv3 7/9] config parsing options: allow one flag multiple times Stefan Beller
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Stefan Beller @ 2013-08-03 11:51 UTC (permalink / raw)
  To: git, gitster, sunshine; +Cc: Stefan Beller

This task emerged from b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
2011-09-27). hash-object is a plumbing layer command, so better
not change the input/output behavior for now.

Unfortunately we have these lines relying on the count up mechanism of
OPT_BOOLEAN:

	if (hashstdin > 1)
		errstr = "Multiple --stdin arguments are not supported";

Maybe later, when the plumbing is refined (git 2.0?), we can drop that
error message and replace the OPT_COUNTUP by OPT_BOOL.

Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
---
 builtin/hash-object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 4aea5bb..d7fcf4c 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -71,7 +71,7 @@ static const char *vpath;
 static const struct option hash_object_options[] = {
 	OPT_STRING('t', NULL, &type, N_("type"), N_("object type")),
 	OPT_BOOL('w', NULL, &write_object, N_("write the object into the object database")),
-	OPT_BOOLEAN( 0 , "stdin", &hashstdin, N_("read the object from stdin")),
+	OPT_COUNTUP( 0 , "stdin", &hashstdin, N_("read the object from stdin")),
 	OPT_BOOL( 0 , "stdin-paths", &stdin_paths, N_("read file names from stdin")),
 	OPT_BOOL( 0 , "no-filters", &no_filters, N_("store file as is without filters")),
 	OPT_STRING( 0 , "path", &vpath, N_("file"), N_("process file as it were from this path")),
-- 
1.8.4.rc0.16.g7fca822.dirty

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

* [PATCHv3 7/9] config parsing options: allow one flag multiple times
  2013-08-03 11:51 [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
                   ` (5 preceding siblings ...)
  2013-08-03 11:51 ` [PATCHv3 6/9] hash-object: Replace stdin parsing OPT_BOOLEAN by OPT_COUNTUP Stefan Beller
@ 2013-08-03 11:51 ` Stefan Beller
  2013-08-05 18:52   ` Junio C Hamano
  2013-08-03 11:51 ` [PATCHv3 8/9] checkout-index: Fix negations of even numbers of -n Stefan Beller
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Stefan Beller @ 2013-08-03 11:51 UTC (permalink / raw)
  To: git, gitster, sunshine; +Cc: Stefan Beller

This task emerged from b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
2011-09-27).

This commit introduces a change for the users, after this patch
you can pass one of the config level flags multiple times:
Before:
	$ git config --global --global --list
	error: only one config file at a time.
	usage: ...

Afterwards this will work. This is due to the following check in the code:
	if (use_global_config + use_system_config + use_local_config +
	    !!given_config_file + !!given_config_blob > 1) {
		error("only one config file at a time.");
		usage_with_options(builtin_config_usage, builtin_config_options);
	}

With OPT_BOOL instead of OPT_BOOLEAN the variables use_global_config,
use_system_config, use_local_config will only have the value 0 if the
command line option was not passed or 1 no matter how often the
respective command line option was passed.

Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
---
 builtin/config.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index da12fdb..4ab9e9a 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -50,9 +50,9 @@ static int respect_includes = -1;
 
 static struct option builtin_config_options[] = {
 	OPT_GROUP(N_("Config file location")),
-	OPT_BOOLEAN(0, "global", &use_global_config, N_("use global config file")),
-	OPT_BOOLEAN(0, "system", &use_system_config, N_("use system config file")),
-	OPT_BOOLEAN(0, "local", &use_local_config, N_("use repository config file")),
+	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
+	OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
+	OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")),
 	OPT_STRING('f', "file", &given_config_file, N_("file"), N_("use given config file")),
 	OPT_STRING(0, "blob", &given_config_blob, N_("blob-id"), N_("read config from given blob object")),
 	OPT_GROUP(N_("Action")),
-- 
1.8.4.rc0.16.g7fca822.dirty

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

* [PATCHv3 8/9] checkout-index: Fix negations of even numbers of -n
  2013-08-03 11:51 [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
                   ` (6 preceding siblings ...)
  2013-08-03 11:51 ` [PATCHv3 7/9] config parsing options: allow one flag multiple times Stefan Beller
@ 2013-08-03 11:51 ` Stefan Beller
  2013-08-03 11:51 ` [PATCHv3 9/9] revert: use the OPT_CMDMODE for parsing, reducing code Stefan Beller
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: Stefan Beller @ 2013-08-03 11:51 UTC (permalink / raw)
  To: git, gitster, sunshine; +Cc: Stefan Beller

The --no-create was parsed with OPT_BOOLEAN, which has a counting up
logic implemented. Since b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
2011-09-27) the OPT_BOOLEAN is deprecated and is only a define:
	/* Deprecated synonym */
	#define OPTION_BOOLEAN OPTION_COUNTUP

However the variable not_new, which can be counted up by giving
--no-create multiple times, is used to set a bit in the struct checkout
bitfield (defined in cache.h:969, declared at builtin/checkout-index.c:19):

	state.not_new = not_new;

When assigning a value other than 0 or 1 to a bit, all leading digits but
the last are ignored and only the last bit is used for setting the bit
variable.

Hence the following:
	# in git.git:
	$ git status
	# working directory clean
	rm COPYING
	$ git status
	# deleted:    COPYING
	$ git checkout-index -a -n
	$ git status
	# deleted:    COPYING
	# which is expected as we're telling git to not restore or create
	# files, however:
	$ git checkout-index -a -n -n
	$ git status
	# working directory clean, COPYING is restored again!
	# That's the bug, we're fixing here.

By restraining the variable not_new to a value being definitely 0 or 1
by the macro OPT_BOOL the bug is fixed.

Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
---
 builtin/checkout-index.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index aa922ed..69e167b 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -188,7 +188,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 		OPT__FORCE(&force, N_("force overwrite of existing files")),
 		OPT__QUIET(&quiet,
 			N_("no warning for existing files and files not in index")),
-		OPT_BOOLEAN('n', "no-create", &not_new,
+		OPT_BOOL('n', "no-create", &not_new,
 			N_("don't checkout new files")),
 		{ OPTION_CALLBACK, 'u', "index", &newfd, NULL,
 			N_("update stat information in the index file"),
-- 
1.8.4.rc0.16.g7fca822.dirty

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

* [PATCHv3 9/9] revert: use the OPT_CMDMODE for parsing, reducing code
  2013-08-03 11:51 [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
                   ` (7 preceding siblings ...)
  2013-08-03 11:51 ` [PATCHv3 8/9] checkout-index: Fix negations of even numbers of -n Stefan Beller
@ 2013-08-03 11:51 ` Stefan Beller
  2013-08-03 11:55 ` [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
  2013-08-06  6:39 ` Junio C Hamano
  10 siblings, 0 replies; 27+ messages in thread
From: Stefan Beller @ 2013-08-03 11:51 UTC (permalink / raw)
  To: git, gitster, sunshine; +Cc: Stefan Beller

The revert command comes with their own implementation of checking
for exclusiveness of parameters.
Now that the OPT_CMDMODE is in place, we can also rely on that macro
instead of cooking that solution for each command itself.

This commit also replaces OPT_BOOLEAN, which was deprecated by b04ba2bb
(parse-options: deprecate OPT_BOOLEAN, 2011-09-27). Instead OPT_BOOL is
used.

Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
---
 builtin/revert.c | 62 ++++++++++++++------------------------------------------
 1 file changed, 15 insertions(+), 47 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 1d2648b..8e87acd 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -71,44 +71,19 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)
 		die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
 }
 
-LAST_ARG_MUST_BE_NULL
-static void verify_opt_mutually_compatible(const char *me, ...)
-{
-	const char *opt1, *opt2 = NULL;
-	va_list ap;
-
-	va_start(ap, me);
-	while ((opt1 = va_arg(ap, const char *))) {
-		if (va_arg(ap, int))
-			break;
-	}
-	if (opt1) {
-		while ((opt2 = va_arg(ap, const char *))) {
-			if (va_arg(ap, int))
-				break;
-		}
-	}
-	va_end(ap);
-
-	if (opt1 && opt2)
-		die(_("%s: %s cannot be used with %s"),	me, opt1, opt2);
-}
-
 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 = action_name(opts);
-	int remove_state = 0;
-	int contin = 0;
-	int rollback = 0;
+	int cmd = 0;
 	struct option options[] = {
-		OPT_BOOLEAN(0, "quit", &remove_state, N_("end revert or cherry-pick sequence")),
-		OPT_BOOLEAN(0, "continue", &contin, N_("resume revert or cherry-pick sequence")),
-		OPT_BOOLEAN(0, "abort", &rollback, N_("cancel revert or cherry-pick sequence")),
-		OPT_BOOLEAN('n', "no-commit", &opts->no_commit, N_("don't automatically commit")),
-		OPT_BOOLEAN('e', "edit", &opts->edit, N_("edit the commit message")),
+		OPT_CMDMODE(0, "quit", &cmd, N_("end revert or cherry-pick sequence"), 'q'),
+		OPT_CMDMODE(0, "continue", &cmd, N_("resume revert or cherry-pick sequence"), 'c'),
+		OPT_CMDMODE(0, "abort", &cmd, N_("cancel revert or cherry-pick sequence"), 'a'),
+		OPT_BOOL('n', "no-commit", &opts->no_commit, N_("don't automatically commit")),
+		OPT_BOOL('e', "edit", &opts->edit, N_("edit the commit message")),
 		OPT_NOOP_NOARG('r', NULL),
-		OPT_BOOLEAN('s', "signoff", &opts->signoff, N_("add Signed-off-by:")),
+		OPT_BOOL('s', "signoff", &opts->signoff, N_("add Signed-off-by:")),
 		OPT_INTEGER('m', "mainline", &opts->mainline, N_("parent number")),
 		OPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto),
 		OPT_STRING(0, "strategy", &opts->strategy, N_("strategy"), N_("merge strategy")),
@@ -124,11 +99,11 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 
 	if (opts->action == REPLAY_PICK) {
 		struct option cp_extra[] = {
-			OPT_BOOLEAN('x', NULL, &opts->record_origin, N_("append commit name")),
-			OPT_BOOLEAN(0, "ff", &opts->allow_ff, N_("allow fast-forward")),
-			OPT_BOOLEAN(0, "allow-empty", &opts->allow_empty, N_("preserve initially empty commits")),
-			OPT_BOOLEAN(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")),
-			OPT_BOOLEAN(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")),
+			OPT_BOOL('x', NULL, &opts->record_origin, N_("append commit name")),
+			OPT_BOOL(0, "ff", &opts->allow_ff, N_("allow fast-forward")),
+			OPT_BOOL(0, "allow-empty", &opts->allow_empty, N_("preserve initially empty commits")),
+			OPT_BOOL(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")),
+			OPT_BOOL(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")),
 			OPT_END(),
 		};
 		if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))
@@ -139,23 +114,16 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 			PARSE_OPT_KEEP_ARGV0 |
 			PARSE_OPT_KEEP_UNKNOWN);
 
-	/* Check for incompatible subcommands */
-	verify_opt_mutually_compatible(me,
-				"--quit", remove_state,
-				"--continue", contin,
-				"--abort", rollback,
-				NULL);
-
 	/* implies allow_empty */
 	if (opts->keep_redundant_commits)
 		opts->allow_empty = 1;
 
 	/* Set the subcommand */
-	if (remove_state)
+	if (cmd == 'q')
 		opts->subcommand = REPLAY_REMOVE_STATE;
-	else if (contin)
+	else if (cmd == 'c')
 		opts->subcommand = REPLAY_CONTINUE;
-	else if (rollback)
+	else if (cmd == 'a')
 		opts->subcommand = REPLAY_ROLLBACK;
 	else
 		opts->subcommand = REPLAY_NONE;
-- 
1.8.4.rc0.16.g7fca822.dirty

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

* Re: [PATCHv3 0/9] Removing deprecated parsing macros
  2013-08-03 11:51 [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
                   ` (8 preceding siblings ...)
  2013-08-03 11:51 ` [PATCHv3 9/9] revert: use the OPT_CMDMODE for parsing, reducing code Stefan Beller
@ 2013-08-03 11:55 ` Stefan Beller
  2013-08-06  6:39 ` Junio C Hamano
  10 siblings, 0 replies; 27+ messages in thread
From: Stefan Beller @ 2013-08-03 11:55 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, gitster, sunshine

[-- Attachment #1: Type: text/plain, Size: 172 bytes --]

On 08/03/2013 01:51 PM, Stefan Beller wrote:
> Suggested changes by Eric Sunshine included.
> 

The patches still apply on top of origin/jc/parseopt-command-modes



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

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

* Re: [PATCHv3 6/9] hash-object: Replace stdin parsing OPT_BOOLEAN by OPT_COUNTUP
  2013-08-03 11:51 ` [PATCHv3 6/9] hash-object: Replace stdin parsing OPT_BOOLEAN by OPT_COUNTUP Stefan Beller
@ 2013-08-05 18:50   ` Junio C Hamano
  0 siblings, 0 replies; 27+ messages in thread
From: Junio C Hamano @ 2013-08-05 18:50 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, sunshine

Stefan Beller <stefanbeller@googlemail.com> writes:

> This task emerged from b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
> 2011-09-27). hash-object is a plumbing layer command, so better
> not change the input/output behavior for now.
>
> Unfortunately we have these lines relying on the count up mechanism of
> OPT_BOOLEAN:
>
> 	if (hashstdin > 1)
> 		errstr = "Multiple --stdin arguments are not supported";
>
> Maybe later, when the plumbing is refined (git 2.0?), we can drop that
> error message and replace the OPT_COUNTUP by OPT_BOOL.

I am of two minds about that as a future direction.

The original motivation of this is that it was too easy to see

	git hash-object Makefile COPYING

to work as expected, and extend that knowledge to expect this

	git hash-objects --stdin --stdin

to somehow work without thinking things through.

So it is not about refining plumbing, but is about educating users.
Because a popular system will always have influx of users yet to be
educated, I do not think it makes sense to drop this safety.

The patch itself, and others so far except 1 and 2 which are too big
for me to carefully review right now, seem to make sense.

Thanks.

> Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
> ---
>  builtin/hash-object.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/hash-object.c b/builtin/hash-object.c
> index 4aea5bb..d7fcf4c 100644
> --- a/builtin/hash-object.c
> +++ b/builtin/hash-object.c
> @@ -71,7 +71,7 @@ static const char *vpath;
>  static const struct option hash_object_options[] = {
>  	OPT_STRING('t', NULL, &type, N_("type"), N_("object type")),
>  	OPT_BOOL('w', NULL, &write_object, N_("write the object into the object database")),
> -	OPT_BOOLEAN( 0 , "stdin", &hashstdin, N_("read the object from stdin")),
> +	OPT_COUNTUP( 0 , "stdin", &hashstdin, N_("read the object from stdin")),
>  	OPT_BOOL( 0 , "stdin-paths", &stdin_paths, N_("read file names from stdin")),
>  	OPT_BOOL( 0 , "no-filters", &no_filters, N_("store file as is without filters")),
>  	OPT_STRING( 0 , "path", &vpath, N_("file"), N_("process file as it were from this path")),

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

* Re: [PATCHv3 7/9] config parsing options: allow one flag multiple times
  2013-08-03 11:51 ` [PATCHv3 7/9] config parsing options: allow one flag multiple times Stefan Beller
@ 2013-08-05 18:52   ` Junio C Hamano
  0 siblings, 0 replies; 27+ messages in thread
From: Junio C Hamano @ 2013-08-05 18:52 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, sunshine

Stefan Beller <stefanbeller@googlemail.com> writes:

> This task emerged from b04ba2bb (parse-options: deprecate OPT_BOOLEAN,
> 2011-09-27).
>
> This commit introduces a change for the users, after this patch
> you can pass one of the config level flags multiple times:
> Before:
> 	$ git config --global --global --list
> 	error: only one config file at a time.
> 	usage: ...
>
> Afterwards this will work. This is due to the following check in the code:
> 	if (use_global_config + use_system_config + use_local_config +
> 	    !!given_config_file + !!given_config_blob > 1) {
> 		error("only one config file at a time.");
> 		usage_with_options(builtin_config_usage, builtin_config_options);
> 	}

Of course, you could further lose that "at most one of them" logic
by using CMDMODE.  That will involve updating the logic that
currently looks at these three variables to look at one enum that
can have 4 states (nothing specified, and one of these three
specified), which will be more involved change, but the resulting
code may become simpler (I didn't try---I am just speculating).

Thanks.

>
> With OPT_BOOL instead of OPT_BOOLEAN the variables use_global_config,
> use_system_config, use_local_config will only have the value 0 if the
> command line option was not passed or 1 no matter how often the
> respective command line option was passed.
>
> Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
> ---



>  builtin/config.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index da12fdb..4ab9e9a 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -50,9 +50,9 @@ static int respect_includes = -1;
>  
>  static struct option builtin_config_options[] = {
>  	OPT_GROUP(N_("Config file location")),
> -	OPT_BOOLEAN(0, "global", &use_global_config, N_("use global config file")),
> -	OPT_BOOLEAN(0, "system", &use_system_config, N_("use system config file")),
> -	OPT_BOOLEAN(0, "local", &use_local_config, N_("use repository config file")),
> +	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
> +	OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
> +	OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")),
>  	OPT_STRING('f', "file", &given_config_file, N_("file"), N_("use given config file")),
>  	OPT_STRING(0, "blob", &given_config_blob, N_("blob-id"), N_("read config from given blob object")),
>  	OPT_GROUP(N_("Action")),

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

* Re: [PATCHv3 0/9] Removing deprecated parsing macros
  2013-08-03 11:51 [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
                   ` (9 preceding siblings ...)
  2013-08-03 11:55 ` [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
@ 2013-08-06  6:39 ` Junio C Hamano
  2013-08-06 13:02   ` Stefan Beller
  10 siblings, 1 reply; 27+ messages in thread
From: Junio C Hamano @ 2013-08-06  6:39 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, sunshine

Thanks.  Queued this at the tip of 'pu'.  There seem to be some
fallouts found in the test suite, though.

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

* Re: [PATCHv3 0/9] Removing deprecated parsing macros
  2013-08-06  6:39 ` Junio C Hamano
@ 2013-08-06 13:02   ` Stefan Beller
  2013-08-06 13:07     ` [PATCH] branch, commit, name-rev: ease up boolean conditions Stefan Beller
  2013-08-06 17:20     ` [PATCHv3 0/9] Removing deprecated parsing macros Junio C Hamano
  0 siblings, 2 replies; 27+ messages in thread
From: Stefan Beller @ 2013-08-06 13:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, sunshine

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]

On 08/06/2013 08:39 AM, Junio C Hamano wrote:
> Thanks.  Queued this at the tip of 'pu'.  There seem to be some
> fallouts found in the test suite, though.
> 

Thanks. I am sorry for forgetting 'make test' before sending patches.
And the test suite is correct.

e35ea450 (branch, commit, name-rev: ease up boolean conditions)
is faulty. In builtin/branch.c the variables 'delete' and 'rename' 
are not used as a true boolean but I assumed so. 
These are parsed in via OPT_BIT and depending if you pass -d or -D 
for deletion you have value 1 or 2 in delete.

Hence this change is incorrect:
-	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1)
+	if (delete + rename + force_create + list + unset_upstream +
+	    !!new_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);

The current patch e35ea450 ([PATCHv3 5/9] branch, commit, name-rev: ease up boolean conditions)
can be dropped/reverted and I'll resend it. (The order doesn't matter, 
you can just drop that commit and apply the resend version on top of 
origin/sb/parseopt-boolean-removal

Stefan




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

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

* [PATCH] branch, commit, name-rev: ease up boolean conditions
  2013-08-06 13:02   ` Stefan Beller
@ 2013-08-06 13:07     ` Stefan Beller
  2013-08-06 18:46       ` Eric Sunshine
  2013-08-06 17:20     ` [PATCHv3 0/9] Removing deprecated parsing macros Junio C Hamano
  1 sibling, 1 reply; 27+ messages in thread
From: Stefan Beller @ 2013-08-06 13:07 UTC (permalink / raw)
  To: git, gitster, sunshine; +Cc: Stefan Beller

Now that the variables are readin by OPT_BOOL, which makes sure
to have the values being 0 or 1 after reading, we do not need
the double negation to map any other value to 1 for integer
variables.

Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
---
 builtin/branch.c   | 3 ++-
 builtin/commit.c   | 2 +-
 builtin/name-rev.c | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 4daed0b..0dca694 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -872,7 +872,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (with_commit || merge_filter != NO_FILTER)
 		list = 1;
 
-	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1)
+	if (force_create + list + unset_upstream +
+	    !!delete + !!rename + !!new_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
 	if (abbrev == -1)
diff --git a/builtin/commit.c b/builtin/commit.c
index c20426b..b0f86c8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1072,7 +1072,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
 	if (patch_interactive)
 		interactive = 1;
 
-	if (!!also + !!only + !!all + !!interactive > 1)
+	if (also + only + all + interactive > 1)
 		die(_("Only one of --include/--only/--all/--interactive/--patch can be used."));
 	if (argc == 0 && (also || (only && !amend)))
 		die(_("No paths with --include/--only does not make sense."));
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index a908a34..20fcf8c 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -331,7 +331,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
-	if (!!all + !!transform_stdin + !!argc > 1) {
+	if (all + transform_stdin + !!argc > 1) {
 		error("Specify either a list, or --all, not both!");
 		usage_with_options(name_rev_usage, opts);
 	}
-- 
1.8.4.rc0.16.g7fca822.dirty

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

* Re: [PATCHv3 0/9] Removing deprecated parsing macros
  2013-08-06 13:02   ` Stefan Beller
  2013-08-06 13:07     ` [PATCH] branch, commit, name-rev: ease up boolean conditions Stefan Beller
@ 2013-08-06 17:20     ` Junio C Hamano
  2013-08-06 23:31       ` [PATCH 0/4] Update built-in parseopt macros Junio C Hamano
  1 sibling, 1 reply; 27+ messages in thread
From: Junio C Hamano @ 2013-08-06 17:20 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, sunshine

Stefan Beller <stefanbeller@googlemail.com> writes:

> On 08/06/2013 08:39 AM, Junio C Hamano wrote:
>> Thanks.  Queued this at the tip of 'pu'.  There seem to be some
>> fallouts found in the test suite, though.
>> 
>
> Thanks. I am sorry for forgetting 'make test' before sending patches.
> And the test suite is correct.
>
> e35ea450 (branch, commit, name-rev: ease up boolean conditions)
> is faulty. In builtin/branch.c the variables 'delete' and 'rename' 
> are not used as a true boolean but I assumed so. 
> These are parsed in via OPT_BIT and depending if you pass -d or -D 
> for deletion you have value 1 or 2 in delete.

Likewise for 'rename' that becomes 2 when -M is given.

> Hence this change is incorrect:
> -	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1)
> +	if (delete + rename + force_create + list + unset_upstream +
> +	    !!new_upstream > 1)

As a follow-up to this series, we may want to think about the
following as well:

 - OPT__VERBOSE() is defined in terms of OPT_BOOLEAN().  I think we
   use it to represent increasing levels of verbosity, so we cannot
   turn this into OPT_BOOL().  Its implementation has to become
   OPT_COUNTUP().

 - OPT__QUIET() is defined in terms of OPT_BOOLEAN().  I offhand do
   not know if we have increasing levels of quietness.  The users
   need auditing before we can decide to turn this into either
   OPT_COUNTUP() or OPT_BOOL().

 - OPT__DRY_RUN() should probably become OPT_BOOL().

 - OPT_FORCE(); do we have levels of forcefulness?  If so
   OPT_COUNTUP(), otherwise OPT_BOOL().

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

* Re: [PATCH] branch, commit, name-rev: ease up boolean conditions
  2013-08-06 13:07     ` [PATCH] branch, commit, name-rev: ease up boolean conditions Stefan Beller
@ 2013-08-06 18:46       ` Eric Sunshine
  2013-08-06 20:18         ` Stefan Beller
  0 siblings, 1 reply; 27+ messages in thread
From: Eric Sunshine @ 2013-08-06 18:46 UTC (permalink / raw)
  To: Stefan Beller; +Cc: Git List, Junio C Hamano

On Tue, Aug 6, 2013 at 9:07 AM, Stefan Beller
<stefanbeller@googlemail.com> wrote:
> Now that the variables are readin by OPT_BOOL, which makes sure

Do you mean s/readin/read in/ ?

Or should it be s/readin/set/ ?

> to have the values being 0 or 1 after reading, we do not need
> the double negation to map any other value to 1 for integer
> variables.
>
> Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>

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

* Re: [PATCH] branch, commit, name-rev: ease up boolean conditions
  2013-08-06 18:46       ` Eric Sunshine
@ 2013-08-06 20:18         ` Stefan Beller
  2013-08-06 20:18           ` Stefan Beller
  0 siblings, 1 reply; 27+ messages in thread
From: Stefan Beller @ 2013-08-06 20:18 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Git List, Junio C Hamano

[-- Attachment #1: Type: text/plain, Size: 570 bytes --]

On 08/06/2013 08:46 PM, Eric Sunshine wrote:
> On Tue, Aug 6, 2013 at 9:07 AM, Stefan Beller
> <stefanbeller@googlemail.com> wrote:
>> Now that the variables are readin by OPT_BOOL, which makes sure
> 
> Do you mean s/readin/read in/ ?
> 
> Or should it be s/readin/set/ ?
> 
>> to have the values being 0 or 1 after reading, we do not need
>> the double negation to map any other value to 1 for integer
>> variables.
>>
>> Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>

I think s/readin/set/ is best.
Also s/after reading/after parsing/


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

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

* [PATCH] branch, commit, name-rev: ease up boolean conditions
  2013-08-06 20:18         ` Stefan Beller
@ 2013-08-06 20:18           ` Stefan Beller
  0 siblings, 0 replies; 27+ messages in thread
From: Stefan Beller @ 2013-08-06 20:18 UTC (permalink / raw)
  To: sunshine, git, gitster; +Cc: Stefan Beller

Now that the variables are set by OPT_BOOL, which makes sure
to have the values being 0 or 1 after parsing, we do not need
the double negation to map any other value to 1 for integer
variables.

Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
---
 builtin/branch.c   | 3 ++-
 builtin/commit.c   | 2 +-
 builtin/name-rev.c | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 4daed0b..0dca694 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -872,7 +872,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (with_commit || merge_filter != NO_FILTER)
 		list = 1;
 
-	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1)
+	if (force_create + list + unset_upstream +
+	    !!delete + !!rename + !!new_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
 	if (abbrev == -1)
diff --git a/builtin/commit.c b/builtin/commit.c
index c20426b..b0f86c8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1072,7 +1072,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
 	if (patch_interactive)
 		interactive = 1;
 
-	if (!!also + !!only + !!all + !!interactive > 1)
+	if (also + only + all + interactive > 1)
 		die(_("Only one of --include/--only/--all/--interactive/--patch can be used."));
 	if (argc == 0 && (also || (only && !amend)))
 		die(_("No paths with --include/--only does not make sense."));
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index a908a34..20fcf8c 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -331,7 +331,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
-	if (!!all + !!transform_stdin + !!argc > 1) {
+	if (all + transform_stdin + !!argc > 1) {
 		error("Specify either a list, or --all, not both!");
 		usage_with_options(name_rev_usage, opts);
 	}
-- 
1.8.4.rc0.16.g7fca822.dirty

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

* [PATCH 0/4] Update built-in parseopt macros
  2013-08-06 17:20     ` [PATCHv3 0/9] Removing deprecated parsing macros Junio C Hamano
@ 2013-08-06 23:31       ` Junio C Hamano
  2013-08-06 23:31         ` [PATCH 1/4] OPT__QUIET(): switch from count-up to true bool Junio C Hamano
                           ` (3 more replies)
  0 siblings, 4 replies; 27+ messages in thread
From: Junio C Hamano @ 2013-08-06 23:31 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

As a follow-up to Stefan's series, we may want to think about the
following as well:

 - OPT__VERBOSE() is defined in terms of OPT_BOOLEAN().  I think we
   use it to represent increasing levels of verbosity, so we cannot
   turn this into OPT_BOOL().  Its implementation has to become
   OPT_COUNTUP().

 - OPT__QUIET() is defined in terms of OPT_BOOLEAN().  I offhand do
   not know if we have increasing levels of quietness.  The users
   need auditing before we can decide to turn this into either
   OPT_COUNTUP() or OPT_BOOL().

 - OPT__DRY_RUN() should probably become OPT_BOOL().

 - OPT_FORCE(); do we have levels of forcefulness?  If so
   OPT_COUNTUP(), otherwise OPT_BOOL().

And here is my attempt.  The last one is iffy as I didn't bother
auditing the existing callers.

Junio C Hamano (4):
  OPT__QUIET(): switch from count-up to true bool
  OPT__VERBOSE(): clarify its expected use by using OPT_COUNTUP
  OPT__DRY_RUN(): use OPT_BOOL, not OPT_BOOLEAN
  OPT__FORCE(): clarify its expected use by using OPT_COUNTUP

 parse-options.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
1.8.4-rc1-210-gf6d87e2

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

* [PATCH 1/4] OPT__QUIET(): switch from count-up to true bool
  2013-08-06 23:31       ` [PATCH 0/4] Update built-in parseopt macros Junio C Hamano
@ 2013-08-06 23:31         ` Junio C Hamano
  2013-08-06 23:31         ` [PATCH 2/4] OPT__VERBOSE(): clarify its expected use by using OPT_COUNTUP Junio C Hamano
                           ` (2 subsequent siblings)
  3 siblings, 0 replies; 27+ messages in thread
From: Junio C Hamano @ 2013-08-06 23:31 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

The parseopt parsing for OPT__QUIET() is implemented in terms of
OPT_BOOLEAN aka OPT_COUNTUP, so a user _could_ theoretically have
used it to make "git cmd -q -q" and "git cmd -q" behave differently.

However, no existing user does so (a summary of the audit at the
end).  Use OPT_BOOL to make sure our choices are either 0 or 1.

    builtin/branch.c:

	quiet is passed to create_branch() in branch.c and
	delete_branches().  The former passes it to setup_tracking()
	which is used as a bool to decide use of BRANCH_CONFIG_VERBOSE.
	The latter uses it as a bool to give a single printf() for
	reporting the names of deleted branches.

    builtin/check-ignore.c:

	all users of quiet use it as a bool.

    builtin/checkout-index.c:

	quiet is assigned to state.quite and only the latter is used
	throughout the program.  It is a single-bit bitfield.

    builtin/checkout.c:

	quiet is stored in checkout_opts.quiet which is of type int.  It
	is used in many places:

	- reset_tree() uses it as a bool;

	- merge_working_tree() uses it twice, as a bool at both
	  places;

	- update_refs_for_switch() uses it three times, all as a bool.
	  It also passes it to create_branch() which we already verified
	  above.

	- switch_branches() and switch_unborn_to_new_branch() use it
	  once each, as a bool at both places.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 parse-options.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse-options.h b/parse-options.h
index c378b75..f2b01ee 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -231,7 +231,7 @@ extern int parse_opt_string_list(const struct option *, const char *, int);
 extern int parse_opt_noop_cb(const struct option *, const char *, int);
 
 #define OPT__VERBOSE(var, h)  OPT_BOOLEAN('v', "verbose", (var), (h))
-#define OPT__QUIET(var, h)    OPT_BOOLEAN('q', "quiet",   (var), (h))
+#define OPT__QUIET(var, h)    OPT_BOOL('q', "quiet",   (var), (h))
 #define OPT__VERBOSITY(var) \
 	{ OPTION_CALLBACK, 'v', "verbose", (var), NULL, N_("be more verbose"), \
 	  PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }, \
-- 
1.8.4-rc1-210-gf6d87e2

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

* [PATCH 2/4] OPT__VERBOSE(): clarify its expected use by using OPT_COUNTUP
  2013-08-06 23:31       ` [PATCH 0/4] Update built-in parseopt macros Junio C Hamano
  2013-08-06 23:31         ` [PATCH 1/4] OPT__QUIET(): switch from count-up to true bool Junio C Hamano
@ 2013-08-06 23:31         ` Junio C Hamano
  2013-08-06 23:31         ` [PATCH 3/4] OPT__DRY_RUN(): use OPT_BOOL, not OPT_BOOLEAN Junio C Hamano
  2013-08-06 23:31         ` [RFH/PATCH 4/4] OPT__FORCE(): clarify its expected use by using OPT_COUNTUP Junio C Hamano
  3 siblings, 0 replies; 27+ messages in thread
From: Junio C Hamano @ 2013-08-06 23:31 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

The parseopt parsing for OPT__VERBOSE() is implemented in terms of
OPT_BOOLEAN() and users of it do take advantage of the "counting up"
behaviour to implement increasing levels of verbosity by
differentiating "git cmd -v" and "git cmd -v -v".

Clarify this by explicitly using OPT_COUNTUP() instead.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 parse-options.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse-options.h b/parse-options.h
index f2b01ee..582dd4b 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -230,7 +230,7 @@ extern int parse_opt_tertiary(const struct option *, const char *, int);
 extern int parse_opt_string_list(const struct option *, const char *, int);
 extern int parse_opt_noop_cb(const struct option *, const char *, int);
 
-#define OPT__VERBOSE(var, h)  OPT_BOOLEAN('v', "verbose", (var), (h))
+#define OPT__VERBOSE(var, h)  OPT_COUNTUP('v', "verbose", (var), (h))
 #define OPT__QUIET(var, h)    OPT_BOOL('q', "quiet",   (var), (h))
 #define OPT__VERBOSITY(var) \
 	{ OPTION_CALLBACK, 'v', "verbose", (var), NULL, N_("be more verbose"), \
-- 
1.8.4-rc1-210-gf6d87e2

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

* [PATCH 3/4] OPT__DRY_RUN(): use OPT_BOOL, not OPT_BOOLEAN
  2013-08-06 23:31       ` [PATCH 0/4] Update built-in parseopt macros Junio C Hamano
  2013-08-06 23:31         ` [PATCH 1/4] OPT__QUIET(): switch from count-up to true bool Junio C Hamano
  2013-08-06 23:31         ` [PATCH 2/4] OPT__VERBOSE(): clarify its expected use by using OPT_COUNTUP Junio C Hamano
@ 2013-08-06 23:31         ` Junio C Hamano
  2013-08-06 23:31         ` [RFH/PATCH 4/4] OPT__FORCE(): clarify its expected use by using OPT_COUNTUP Junio C Hamano
  3 siblings, 0 replies; 27+ messages in thread
From: Junio C Hamano @ 2013-08-06 23:31 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

There cannot be "git cmd -n -n" that behaves even less drier than
"git cmd -n", and no existing users of the macro implements such
a semantics (a summary of the audit at the end).

Instead of OPT_BOOLEAN, use OPT_BOOL to clarify.

    builtin/add.c:

	uses "show_only" as a bool.

    builtin/clean.c:

	uses "dry_run" as a bool, and passes it to the parameter
	with the same name of remove_dirs(), which is also used as a
	bool.

    builtin/mv.c:

	uses "show_only" as a bool.

    builtin/notes.c:

	uses "show_only" as a bool.

    builtin/prune.c:

	uses "show_only" as a bool (including its use as a bool to
	decide use of PRUNE_PACKED_DRY_RUN as a parameter to
	prune_packed_objects()).

    builtin/read-tree.c:

	opts.dry_run is used as a bool, in this file and also in
	unpack_trees() that is called from here.

    builtin/remote.c:

	dry_run is passed to prune_remote() and used as a bool.

    builtin/rm.c:

	show_only is used as a bool.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 parse-options.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse-options.h b/parse-options.h
index 582dd4b..78f52c2 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -237,7 +237,7 @@ extern int parse_opt_noop_cb(const struct option *, const char *, int);
 	  PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }, \
 	{ OPTION_CALLBACK, 'q', "quiet", (var), NULL, N_("be more quiet"), \
 	  PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }
-#define OPT__DRY_RUN(var, h)  OPT_BOOLEAN('n', "dry-run", (var), (h))
+#define OPT__DRY_RUN(var, h)  OPT_BOOL('n', "dry-run", (var), (h))
 #define OPT__FORCE(var, h)    OPT_BOOLEAN('f', "force",   (var), (h))
 #define OPT__ABBREV(var)  \
 	{ OPTION_CALLBACK, 0, "abbrev", (var), N_("n"),	\
-- 
1.8.4-rc1-210-gf6d87e2

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

* [RFH/PATCH 4/4] OPT__FORCE(): clarify its expected use by using OPT_COUNTUP
  2013-08-06 23:31       ` [PATCH 0/4] Update built-in parseopt macros Junio C Hamano
                           ` (2 preceding siblings ...)
  2013-08-06 23:31         ` [PATCH 3/4] OPT__DRY_RUN(): use OPT_BOOL, not OPT_BOOLEAN Junio C Hamano
@ 2013-08-06 23:31         ` Junio C Hamano
  2013-08-07  7:33           ` Stefan Beller
  3 siblings, 1 reply; 27+ messages in thread
From: Junio C Hamano @ 2013-08-06 23:31 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

The parseopt parsing for OPT__FORCE() is implemented in terms of
OPT_BOOLEAN() and users of it can take advantage of the "counting
up" behaviour to implement increasing levels of forcefulness by
differentiating "git cmd -f" and "git cmd -f -f".

Clarify this by explicitly using OPT_COUNTUP() instead.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This _should_ be done with a similar audit of existing callers,
   but I ran out of concentration.

 parse-options.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse-options.h b/parse-options.h
index 78f52c2..1eeb0d9 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -238,7 +238,7 @@ extern int parse_opt_noop_cb(const struct option *, const char *, int);
 	{ OPTION_CALLBACK, 'q', "quiet", (var), NULL, N_("be more quiet"), \
 	  PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }
 #define OPT__DRY_RUN(var, h)  OPT_BOOL('n', "dry-run", (var), (h))
-#define OPT__FORCE(var, h)    OPT_BOOLEAN('f', "force",   (var), (h))
+#define OPT__FORCE(var, h)    OPT_COUNTUP('f', "force",   (var), (h))
 #define OPT__ABBREV(var)  \
 	{ OPTION_CALLBACK, 0, "abbrev", (var), N_("n"),	\
 	  N_("use <n> digits to display SHA-1s"),	\
-- 
1.8.4-rc1-210-gf6d87e2

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

* Re: [RFH/PATCH 4/4] OPT__FORCE(): clarify its expected use by using OPT_COUNTUP
  2013-08-06 23:31         ` [RFH/PATCH 4/4] OPT__FORCE(): clarify its expected use by using OPT_COUNTUP Junio C Hamano
@ 2013-08-07  7:33           ` Stefan Beller
  2013-08-07 15:28             ` Junio C Hamano
  0 siblings, 1 reply; 27+ messages in thread
From: Stefan Beller @ 2013-08-07  7:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 4452 bytes --]

On 08/07/2013 01:31 AM, Junio C Hamano wrote:
> The parseopt parsing for OPT__FORCE() is implemented in terms of
> OPT_BOOLEAN() and users of it can take advantage of the "counting
> up" behaviour to implement increasing levels of forcefulness by
> differentiating "git cmd -f" and "git cmd -f -f".
> 
> Clarify this by explicitly using OPT_COUNTUP() instead.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>  * This _should_ be done with a similar audit of existing callers,
>    but I ran out of concentration.
> 
>  parse-options.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/parse-options.h b/parse-options.h
> index 78f52c2..1eeb0d9 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -238,7 +238,7 @@ extern int parse_opt_noop_cb(const struct option *, const char *, int);
>  	{ OPTION_CALLBACK, 'q', "quiet", (var), NULL, N_("be more quiet"), \
>  	  PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }
>  #define OPT__DRY_RUN(var, h)  OPT_BOOL('n', "dry-run", (var), (h))
> -#define OPT__FORCE(var, h)    OPT_BOOLEAN('f', "force",   (var), (h))
> +#define OPT__FORCE(var, h)    OPT_COUNTUP('f', "force",   (var), (h))
>  #define OPT__ABBREV(var)  \
>  	{ OPTION_CALLBACK, 0, "abbrev", (var), N_("n"),	\
>  	  N_("use <n> digits to display SHA-1s"),	\
> 

We need the COUNTUP, because in builtin/clean.c we have
	OPT__FORCE(&force, N_("force")),
	...
	if (force > 1)
		rm_flags = 0;

So a OPT_BOOL definitely doesn't cut it.
Now that I started reviewing the OPT_FORCE parts, I realize
there is still an error in the patch, which needed correction.
(branch, commit, name-rev: ease up boolean conditions):

-	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1)
+	if (force_create + list + unset_upstream +
+	    !!delete + !!rename + !!new_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);

force_create is set via OPT_FORCE as well, so we cannot remove the !! before the force_create,
hence we'd only remove it from list and unset_upstream, which are set by OPT_BOOL.

-- 8< --
From: Stefan Beller <stefanbeller@googlemail.com>
Date: Wed, 7 Aug 2013 09:32:25 +0200
Subject: [PATCH] branch, commit, name-rev: ease up boolean conditions

Now that the variables are set by OPT_BOOL, which makes sure
to have the values being 0 or 1 after parsing, we do not need
the double negation to map any other value to 1 for integer
variables.

Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/branch.c   | 3 ++-
 builtin/commit.c   | 2 +-
 builtin/name-rev.c | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 4daed0b..0903763 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -872,7 +872,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (with_commit || merge_filter != NO_FILTER)
 		list = 1;
 
-	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1)
+	if (!!delete + !!rename + !!force_create + !!new_upstream +
+	    list + unset_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
 	if (abbrev == -1)
diff --git a/builtin/commit.c b/builtin/commit.c
index c20426b..b0f86c8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1072,7 +1072,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
 	if (patch_interactive)
 		interactive = 1;
 
-	if (!!also + !!only + !!all + !!interactive > 1)
+	if (also + only + all + interactive > 1)
 		die(_("Only one of --include/--only/--all/--interactive/--patch can be used."));
 	if (argc == 0 && (also || (only && !amend)))
 		die(_("No paths with --include/--only does not make sense."));
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index a908a34..20fcf8c 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -331,7 +331,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
-	if (!!all + !!transform_stdin + !!argc > 1) {
+	if (all + transform_stdin + !!argc > 1) {
 		error("Specify either a list, or --all, not both!");
 		usage_with_options(name_rev_usage, opts);
 	}
-- 
1.8.4.rc0.16.g7fca822.dirty



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

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

* Re: [RFH/PATCH 4/4] OPT__FORCE(): clarify its expected use by using OPT_COUNTUP
  2013-08-07  7:33           ` Stefan Beller
@ 2013-08-07 15:28             ` Junio C Hamano
  0 siblings, 0 replies; 27+ messages in thread
From: Junio C Hamano @ 2013-08-07 15:28 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git

Stefan Beller <stefanbeller@googlemail.com> writes:

> On 08/07/2013 01:31 AM, Junio C Hamano wrote:
>> The parseopt parsing for OPT__FORCE() is implemented in terms of
>> OPT_BOOLEAN() and users of it can take advantage of the "counting
>> up" behaviour to implement increasing levels of forcefulness by
>> differentiating "git cmd -f" and "git cmd -f -f".
>> 
>> Clarify this by explicitly using OPT_COUNTUP() instead.
>> 
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> 
>>  * This _should_ be done with a similar audit of existing callers,
>>    but I ran out of concentration.
>> 
>>  parse-options.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/parse-options.h b/parse-options.h
>> index 78f52c2..1eeb0d9 100644
>> --- a/parse-options.h
>> +++ b/parse-options.h
>> @@ -238,7 +238,7 @@ extern int parse_opt_noop_cb(const struct option *, const char *, int);
>>  	{ OPTION_CALLBACK, 'q', "quiet", (var), NULL, N_("be more quiet"), \
>>  	  PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }
>>  #define OPT__DRY_RUN(var, h)  OPT_BOOL('n', "dry-run", (var), (h))
>> -#define OPT__FORCE(var, h)    OPT_BOOLEAN('f', "force",   (var), (h))
>> +#define OPT__FORCE(var, h)    OPT_COUNTUP('f', "force",   (var), (h))
>>  #define OPT__ABBREV(var)  \
>>  	{ OPTION_CALLBACK, 0, "abbrev", (var), N_("n"),	\
>>  	  N_("use <n> digits to display SHA-1s"),	\
>> 
>
> We need the COUNTUP, because in builtin/clean.c we have
> 	OPT__FORCE(&force, N_("force")),
> 	...
> 	if (force > 1)
> 		rm_flags = 0;

Good that I marked it as RFH ;-)  Thanks.

> So a OPT_BOOL definitely doesn't cut it.
> Now that I started reviewing the OPT_FORCE parts, I realize
> there is still an error in the patch, which needed correction.
> (branch, commit, name-rev: ease up boolean conditions):
>
> -	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1)
> +	if (force_create + list + unset_upstream +
> +	    !!delete + !!rename + !!new_upstream > 1)
>  		usage_with_options(builtin_branch_usage, options);
>
> force_create is set via OPT_FORCE as well, so we cannot remove the !! before the force_create,
> hence we'd only remove it from list and unset_upstream, which are set by OPT_BOOL.

Good.  Will replace.  Thanks.

> -- 8< --
> From: Stefan Beller <stefanbeller@googlemail.com>
> Date: Wed, 7 Aug 2013 09:32:25 +0200
> Subject: [PATCH] branch, commit, name-rev: ease up boolean conditions
>
> Now that the variables are set by OPT_BOOL, which makes sure
> to have the values being 0 or 1 after parsing, we do not need
> the double negation to map any other value to 1 for integer
> variables.
>
> Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin/branch.c   | 3 ++-
>  builtin/commit.c   | 2 +-
>  builtin/name-rev.c | 2 +-
>  3 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 4daed0b..0903763 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -872,7 +872,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>  	if (with_commit || merge_filter != NO_FILTER)
>  		list = 1;
>  
> -	if (!!delete + !!rename + !!force_create + !!list + !!new_upstream + !!unset_upstream > 1)
> +	if (!!delete + !!rename + !!force_create + !!new_upstream +
> +	    list + unset_upstream > 1)
>  		usage_with_options(builtin_branch_usage, options);
>  
>  	if (abbrev == -1)
> diff --git a/builtin/commit.c b/builtin/commit.c
> index c20426b..b0f86c8 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1072,7 +1072,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
>  	if (patch_interactive)
>  		interactive = 1;
>  
> -	if (!!also + !!only + !!all + !!interactive > 1)
> +	if (also + only + all + interactive > 1)
>  		die(_("Only one of --include/--only/--all/--interactive/--patch can be used."));
>  	if (argc == 0 && (also || (only && !amend)))
>  		die(_("No paths with --include/--only does not make sense."));
> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> index a908a34..20fcf8c 100644
> --- a/builtin/name-rev.c
> +++ b/builtin/name-rev.c
> @@ -331,7 +331,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
>  
>  	git_config(git_default_config, NULL);
>  	argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
> -	if (!!all + !!transform_stdin + !!argc > 1) {
> +	if (all + transform_stdin + !!argc > 1) {
>  		error("Specify either a list, or --all, not both!");
>  		usage_with_options(name_rev_usage, opts);
>  	}

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

end of thread, other threads:[~2013-08-07 15:28 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-03 11:51 [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
2013-08-03 11:51 ` [PATCHv3 1/9] Remove deprecated OPTION_BOOLEAN for parsing arguments Stefan Beller
2013-08-03 11:51 ` [PATCHv3 2/9] Replace deprecated OPT_BOOLEAN by OPT_BOOL Stefan Beller
2013-08-03 11:51 ` [PATCHv3 3/9] log, format-patch: parsing uses OPT__QUIET Stefan Beller
2013-08-03 11:51 ` [PATCHv3 4/9] checkout: remove superfluous local variable Stefan Beller
2013-08-03 11:51 ` [PATCHv3 5/9] branch, commit, name-rev: ease up boolean conditions Stefan Beller
2013-08-03 11:51 ` [PATCHv3 6/9] hash-object: Replace stdin parsing OPT_BOOLEAN by OPT_COUNTUP Stefan Beller
2013-08-05 18:50   ` Junio C Hamano
2013-08-03 11:51 ` [PATCHv3 7/9] config parsing options: allow one flag multiple times Stefan Beller
2013-08-05 18:52   ` Junio C Hamano
2013-08-03 11:51 ` [PATCHv3 8/9] checkout-index: Fix negations of even numbers of -n Stefan Beller
2013-08-03 11:51 ` [PATCHv3 9/9] revert: use the OPT_CMDMODE for parsing, reducing code Stefan Beller
2013-08-03 11:55 ` [PATCHv3 0/9] Removing deprecated parsing macros Stefan Beller
2013-08-06  6:39 ` Junio C Hamano
2013-08-06 13:02   ` Stefan Beller
2013-08-06 13:07     ` [PATCH] branch, commit, name-rev: ease up boolean conditions Stefan Beller
2013-08-06 18:46       ` Eric Sunshine
2013-08-06 20:18         ` Stefan Beller
2013-08-06 20:18           ` Stefan Beller
2013-08-06 17:20     ` [PATCHv3 0/9] Removing deprecated parsing macros Junio C Hamano
2013-08-06 23:31       ` [PATCH 0/4] Update built-in parseopt macros Junio C Hamano
2013-08-06 23:31         ` [PATCH 1/4] OPT__QUIET(): switch from count-up to true bool Junio C Hamano
2013-08-06 23:31         ` [PATCH 2/4] OPT__VERBOSE(): clarify its expected use by using OPT_COUNTUP Junio C Hamano
2013-08-06 23:31         ` [PATCH 3/4] OPT__DRY_RUN(): use OPT_BOOL, not OPT_BOOLEAN Junio C Hamano
2013-08-06 23:31         ` [RFH/PATCH 4/4] OPT__FORCE(): clarify its expected use by using OPT_COUNTUP Junio C Hamano
2013-08-07  7:33           ` Stefan Beller
2013-08-07 15:28             ` Junio C Hamano

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).