git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Stefan Beller <stefanbeller@googlemail.com>,
	Siddharth Agarwal <sid0@fb.com>
Subject: [PATCH v2 1/2] parse-options: refactor human-friendly-integer parser out of pack-objects
Date: Wed, 22 Jan 2014 11:58:04 -0800	[thread overview]
Message-ID: <1390420685-18449-2-git-send-email-gitster@pobox.com> (raw)
In-Reply-To: <1390420685-18449-1-git-send-email-gitster@pobox.com>

We only had code to understand unit suffixes such as g/m/k (as in
2g/400m/8k) in the configuration parser but not in the command line
option parser.  "git pack-objects" worked it around by having a
custom extension to the parse-options API; make it available to
other callers.

The new macro is not called OPT_ULONG() but OPT_HUM_ULONG(), as it
would be bizzarre to have ULONG that understands human friendly
units while INTEGER that does not.  It is not named with a shorter
"OPT_HULONG", primarily to avoid having to name a future parallel
for parsing human friendly integer "OPT_HINT".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/pack-objects.c | 25 ++++---------------------
 parse-options.c        | 17 +++++++++++++++++
 parse-options.h        |  5 +++++
 3 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f069462..2fa8e1e 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2417,23 +2417,6 @@ static int option_parse_unpack_unreachable(const struct option *opt,
 	return 0;
 }
 
-static int option_parse_ulong(const struct option *opt,
-			      const char *arg, int unset)
-{
-	if (unset)
-		die(_("option %s does not accept negative form"),
-		    opt->long_name);
-
-	if (!git_parse_ulong(arg, opt->value))
-		die(_("unable to parse value '%s' for option %s"),
-		    arg, opt->long_name);
-	return 0;
-}
-
-#define OPT_ULONG(s, l, v, h) \
-	{ OPTION_CALLBACK, (s), (l), (v), "n", (h),	\
-	  PARSE_OPT_NONEG, option_parse_ulong }
-
 int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 {
 	int use_internal_rev_list = 0;
@@ -2455,16 +2438,16 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 		{ OPTION_CALLBACK, 0, "index-version", NULL, N_("version[,offset]"),
 		  N_("write the pack index file in the specified idx format version"),
 		  0, option_parse_index_version },
-		OPT_ULONG(0, "max-pack-size", &pack_size_limit,
-			  N_("maximum size of each output pack file")),
+		OPT_HUM_ULONG(0, "max-pack-size", &pack_size_limit,
+			      N_("maximum size of each output pack file")),
 		OPT_BOOL(0, "local", &local,
 			 N_("ignore borrowed objects from alternate object store")),
 		OPT_BOOL(0, "incremental", &incremental,
 			 N_("ignore packed objects")),
 		OPT_INTEGER(0, "window", &window,
 			    N_("limit pack window by objects")),
-		OPT_ULONG(0, "window-memory", &window_memory_limit,
-			  N_("limit pack window by memory in addition to object limit")),
+		OPT_HUM_ULONG(0, "window-memory", &window_memory_limit,
+		      N_("limit pack window by memory in addition to object limit")),
 		OPT_INTEGER(0, "depth", &depth,
 			    N_("maximum length of delta chain allowed in the resulting pack")),
 		OPT_BOOL(0, "reuse-delta", &reuse_delta,
diff --git a/parse-options.c b/parse-options.c
index c2cbca2..948ade7 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -136,6 +136,23 @@ static int get_value(struct parse_opt_ctx_t *p,
 			return opterror(opt, "expects a numerical value", flags);
 		return 0;
 
+	case OPTION_ULONG:
+		if (unset) {
+			*(unsigned long *)opt->value = 0;
+		} else if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
+			*(unsigned long *)opt->value = opt->defval;
+		} else if (get_arg(p, opt, flags, &arg)) {
+			return -1;
+		} else if (opt->flags & PARSE_OPT_HUMAN_NUMBER) {
+			if (!git_parse_ulong(arg, (unsigned long *)opt->value))
+				return opterror(opt, "expects a numerical value", flags);
+		} else {
+			*(unsigned long *)opt->value = strtoul(arg, (char **)&s, 10);
+			if (*s)
+				return opterror(opt, "expects a numerical value", flags);
+		}
+		return 0;
+
 	default:
 		die("should not happen, someone must be hit on the forehead");
 	}
diff --git a/parse-options.h b/parse-options.h
index 9b94596..d65ecdb 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -16,6 +16,7 @@ enum parse_opt_type {
 	/* options with arguments (usually) */
 	OPTION_STRING,
 	OPTION_INTEGER,
+	OPTION_ULONG,
 	OPTION_CALLBACK,
 	OPTION_LOWLEVEL_CALLBACK,
 	OPTION_FILENAME
@@ -40,6 +41,7 @@ enum parse_opt_option_flags {
 	PARSE_OPT_LASTARG_DEFAULT = 16,
 	PARSE_OPT_NODASH = 32,
 	PARSE_OPT_LITERAL_ARGHELP = 64,
+	PARSE_OPT_HUMAN_NUMBER = 128,
 	PARSE_OPT_SHELL_EVAL = 256
 };
 
@@ -131,6 +133,9 @@ struct option {
 #define OPT_SET_PTR(s, l, v, h, p)  { OPTION_SET_PTR, (s), (l), (v), NULL, \
 				      (h), PARSE_OPT_NOARG, NULL, (p) }
 #define OPT_INTEGER(s, l, v, h)     { OPTION_INTEGER, (s), (l), (v), N_("n"), (h) }
+#define OPT_ULONG(s, l, v, h)       { OPTION_ULONG, (s), (l), (v), N_("n"), (h) }
+#define OPT_HUM_ULONG(s, l, v, h)   { OPTION_ULONG, (s), (l), (v), N_("n"), (h), \
+				      PARSE_OPT_HUMAN_NUMBER }
 #define OPT_STRING(s, l, v, a, h)   { OPTION_STRING,  (s), (l), (v), (a), (h) }
 #define OPT_STRING_LIST(s, l, v, a, h) \
 				    { OPTION_CALLBACK, (s), (l), (v), (a), \
-- 
1.9-rc0-151-ga5225c0

  reply	other threads:[~2014-01-22 19:58 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-21 22:48 git repack --max-pack-size broken in git-next Siddharth Agarwal
2014-01-21 23:01 ` Junio C Hamano
2014-01-21 23:26   ` Junio C Hamano
2014-01-22 19:58   ` [PATCH v2 0/2] Fix "repack --window-memory=4g" regression in 1.8.4 Junio C Hamano
2014-01-22 19:58     ` Junio C Hamano [this message]
2014-01-22 19:58     ` [PATCH v2 2/2] repack: accept larger window-memory and max-pack-size Junio C Hamano
2014-01-23  1:06       ` Jeff King
2014-01-23  1:26         ` Jeff King
2014-01-23  1:27           ` [PATCH 1/3] repack: fix typo in max-pack-size option Jeff King
2014-01-23  1:28           ` [PATCH 2/3] repack: make parsed string options const-correct Jeff King
2014-01-23  1:30           ` [PATCH 3/3] repack: propagate pack-objects options as strings Jeff King
2014-01-23  1:38             ` Jeff King
2014-01-23 18:37           ` [PATCH v2 2/2] repack: accept larger window-memory and max-pack-size Junio C Hamano
2014-01-22 22:33     ` [PATCH v2 0/2] Fix "repack --window-memory=4g" regression in 1.8.4 Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1390420685-18449-2-git-send-email-gitster@pobox.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=sid0@fb.com \
    --cc=stefanbeller@googlemail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).