Home » Mailing list archives » git » 2007 » October » 16

Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.

Previous thread: none

Next thread: none
[view original message]
From: Pierre Habouzit
Subject: [PATCH 05/25] Add shortcuts for very often used options.
Date: Tuesday, October 16, 2007 - 1:39 am

It helps with consistency of the help strings e.g.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 parse-options.c |   21 +++++++++++++++++++++
 parse-options.h |   11 +++++++++++
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index d156594..63f5d4e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -225,3 +225,24 @@ void usage_with_options(const char * const *usagestr,
 
 	exit(129);
 }
+
+/*----- some often used options -----*/
+#include "cache.h"
+int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset)
+{
+	int v;
+
+	if (!arg) {
+		v = unset ? 0 : DEFAULT_ABBREV;
+	} else {
+		v = strtol(arg, (char **)&arg, 10);
+		if (*arg)
+			return opterror(opt, "expects a numerical value", 0);
+		if (v && v < MINIMUM_ABBREV)
+			v = MINIMUM_ABBREV;
+		else if (v > 40)
+			v = 40;
+	}
+	*(int *)(opt->value) = v;
+	return 0;
+}
diff --git a/parse-options.h b/parse-options.h
index 3006a76..a0343bd 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -39,4 +39,15 @@ extern int parse_options(int argc, const char **argv,
 extern NORETURN void usage_with_options(const char * const *usagestr,
                                         const struct option *options);
 
+/*----- some often used options -----*/
+extern int parse_opt_abbrev_cb(const struct option *, const char *, int);
+
+#define OPT__VERBOSE(var)  OPT_BOOLEAN('v', "verbose", (var), "be verbose")
+#define OPT__QUIET(var)    OPT_BOOLEAN('q', "quiet",   (var), "be quiet")
+#define OPT__DRY_RUN(var)  OPT_BOOLEAN('n', "dry-run", (var), "dry run")
+#define OPT__ABBREV(var)  \
+	{ OPTION_CALLBACK, 0, "abbrev", (var), "n", \
+	  "use <n> digits to display sha's", \
+	  PARSE_OPT_OPTARG, &parse_opt_abbrev_cb, 0 }
+
 #endif
-- 
1.5.3.4.1231.g62b9a

-


[ message continues ]
[view original message]
From: Pierre Habouzit
Subject: [PATCH 06/25] parse-options: Allow abbreviated options when unambiguous
Date: Tuesday, October 16, 2007 - 1:39 am

From: Johannes Schindelin <Johannes.Schindelin@gmx.de>

When there is an option "--amend", the option parser now recognizes
"--am" for that option, provided that there is no other option beginning
with "--am".

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 parse-options.c          |   37 +++++++++++++++++++++++++++++++++++++
 t/t0040-parse-options.sh |   23 +++++++++++++++++++++++
 2 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 63f5d4e..7084dee 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -96,6 +96,13 @@ static int parse_short_opt(struct optparse_t *p, const struct option *options)
 static int parse_long_opt(struct optparse_t *p, const char *arg,
                           const struct option *options)
 {
+	const char *arg_end = strchr(arg, '=');
+	const struct option *abbrev_option = NULL;
+	int abbrev_flags = 0;
+
+	if (!arg_end)
+		arg_end = arg + strlen(arg);
+
 	for (; options->type != OPTION_END; options++) {
 		const char *rest;
 		int flags = 0;
@@ -105,10 +112,38 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,
 
 		rest = skip_prefix(arg, options->long_name);
 		if (!rest) {
+			/* abbreviated? */
+			if (!strncmp(options->long_name, arg, arg_end - arg)) {
+is_abbreviated:
+				if (abbrev_option)
+					return error("Ambiguous option: %s "
+						"(could be --%s%s or --%s%s)",
+						arg,
+						(flags & OPT_UNSET) ?
+							"no-" : "",
+						options->long_name,
+						(abbrev_flags & OPT_UNSET) ?
+							"no-" : "",
+						abbrev_option->long_name);
+				if (!(flags & OPT_UNSET) && *arg_end)
+					p->opt = arg_end + 1;
+				abbrev_option = options;
+				abbrev_flags = flags;
+				continue;
+			}
+			/* negated and abbreviated very much? */
+			if (!prefixcmp("no-", arg)) {
+				flags |= OPT_UNSET;
+				goto is_abbreviated;
+			}
+			/* negated? */
 			if (strncmp(arg, "no-", 3))
 				continue;
 			flags |= OPT_UNSET;
 			rest = ...
[ message continues ]
[view original message]
From: Pierre Habouzit
Subject: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.
Date: Tuesday, October 16, 2007 - 1:39 am

* add the possibility to use callbacks to parse some options, this can
  help implementing new options kinds with great flexibility. struct option
  gains a callback pointer and a `defval' where callbacks user can put
  either integers or pointers. callbacks also can use the `value' pointer
  for anything, preferably to the pointer to the final storage for the value
  though.

* add a `flag' member to struct option to make explicit that this option may
  have an optional argument. The semantics depends on the option type. For
  INTEGERS, it means that if the switch is not used in its
  --long-form=<value> form, and that there is no token after it or that the
  token does not starts with a digit, then it's assumed that the switch has
  no argument. For STRING or CALLBACK it works the same, except that the
  condition is that the next atom starts with a dash. This is needed to
  implement backward compatible behaviour with existing ways to parse the
  command line. Its use for new options is discouraged.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 parse-options.c |   47 +++++++++++++++++++++++++++++++++++++++++------
 parse-options.h |   16 ++++++++++++++++
 2 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 7084dee..f4ba9ce 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -39,7 +39,8 @@ static int opterror(const struct option *opt, const char *reason, int flags)
 static int get_value(struct optparse_t *p,
                      const struct option *opt, int flags)
 {
-	const char *s;
+	const char *s, *arg;
+	arg = p->opt ? p->opt : (p->argc > 1 ? p->argv[1] : NULL);
 
 	if (p->opt && (flags & OPT_UNSET))
 		return opterror(opt, "takes no value", flags);
@@ -60,18 +61,42 @@ static int get_value(struct optparse_t *p,
 			*(const char **)opt->value = (const char *)NULL;
 			return 0;
 		}
-		if (!p->opt && p->argc <= 1)
+		if (opt->flags & PARSE_OPT_OPTARG && (!arg || *arg == '-')) {
+			*(const char ...
[ message continues ]
[view original message]
From: Pierre Habouzit
Subject: Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.
Date: Tuesday, October 16, 2007 - 1:45 am

This bit is to allow to aggregate options with arguments together when
the argument is numeric.

    +#if 0
    +		/* can be used to understand -A1B1 like -A1 -B1 */
    +		if (flag & OPT_SHORT && opt->opt && isdigit(*opt->opt)) {
    +			*(int *)opt->value =3D strtol(opt->opt, (char **)&opt->opt, 10);
    +			return 0;
    +		}
    +#endif

I'm not a huge fan, but people may like it. Feel free to keep the
chunk, drop it, or enable it to your liking.


--=20
=C2=B7O=C2=B7  Pierre Habouzit
=C2=B7=C2=B7O                                                madcoder@debia=
n.org
OOO                                                http://www.madism.org

[ message continues ]
[view original message]
From: Johannes Schindelin
Subject: Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.
Date: Tuesday, October 16, 2007 - 6:18 am

Hi,


FWIW I like it.  It allows me to aggregate options such as -M30 with other 
short options.

Ciao,
Dscho

-


[ message continues ]
[view original message]
From: René Scharfe
Subject: Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.
Date: Tuesday, October 16, 2007 - 9:38 am

I don't like it, it complicates number options with unit suffixes (e.g.
--windows-memory of git-pack-objects).

René
-


[ message continues ]
[view original message]
From: Johannes Schindelin
Subject: Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.
Date: Tuesday, October 16, 2007 - 9:44 am

Hi,


Why?  It only means that you cannot say -W10mxabc instead of -W10m xabc.  

Remember: this is a special case for OPT_INTEGER.  Nothing to do with 
OPT_SIZE, which you'd probably implement as a callback.

Ciao,
Dscho

-


[ message continues ]
[view original message]
From: Pierre Habouzit
Subject: Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.
Date: Tuesday, October 16, 2007 - 9:53 am

Yeah but the point is that you can't migrate an option currently being
an integer to an OPT_SIZE because of that (see my other mail). Meaning
that once an argument is of type OPT_INTEGER you can't change it's type
in the future _AT ALL_ without breaking backward compatibility badly.
I'd say it's a rather sucky design.

--=20
=C2=B7O=C2=B7  Pierre Habouzit
=C2=B7=C2=B7O                                                madcoder@debia=
n.org
OOO                                                http://www.madism.org

[ message continues ]
[view original message]
From: Nicolas Pitre
Subject: Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.
Date: Tuesday, October 16, 2007 - 10:21 am

And what's the point of supporting so criptic arguments?
It doesn't have to go that far.


Nicolas
-


[ message continues ]
[view original message]
From: René Scharfe
Subject: Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.
Date: Tuesday, October 16, 2007 - 10:04 am

You mean I need to take a look at the actual patch to get a bit more
context? ;-)  Now that I did, I retract my comment.

Pierre, FYI: I didn't see your patches coming through the NNTP gateway
of gmane.org, which is my way of reading this list.  Its web interface
doesn't show them, either, so it's probably not caused by my news reader.

René
-


[ message continues ]
[view original message]
From: Nicolas Pitre
Subject: Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.
Date: Tuesday, October 16, 2007 - 9:44 am

On Tue, 16 Oct 2007, Ren
[ message continues ]
[view original message]
From: Pierre Habouzit
Subject: Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.
Date: Tuesday, October 16, 2007 - 9:50 am

Oh yeah, you're right, well you example is not an issue, but indeed
you pointed out a real probable issue:

  With that chunk, if an option that takes now an integer, becomes an
option with a suffix, we would then break backward compatibility. I'm
not sure I'm clear, but if you had a -S option (for size) that for now
gets sizes in kilooctet, git foo -S1000. Then if we decide that it's
worth understanting M/G/.. suffixes for this option, we would make the
type of the option be a CALLBACK using git_parse_ulong. This would mean
that with such a change:

before:
  git foo -S1000a would mean git foo -S1000 -a

After:
  git foo -S1000a would be rejected because 'a' isn't a valid size
  suffix.

  This of course become worse if we take git foo -S1000k where the
breakage would be silent.


  This is a very strong argument _against_ this chunk IMO.

--=20
=C2=B7O=C2=B7  Pierre Habouzit
=C2=B7=C2=B7O                                                madcoder@debia=
n.org
OOO                                                http://www.madism.org

[ message continues ]
[view original message]
From: Shawn O. Pearce
Subject: Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.
Date: Tuesday, October 16, 2007 - 9:44 pm

Since everyone (including myself) is apparently strongly against this
hunk I removed it when I cherry-picked this series from Pierre into
my tree.  The series will be in my pu tonight, but minus this hunk.

-- 
Shawn.
-


[ message continues ]
[view original message]
From: Johannes Schindelin
Subject: Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.
Date: Wednesday, October 17, 2007 - 11:00 am

Hi,

> > On Tue, Oct 16, 2007 at 04:38:36PM +0000, Ren
[ message continues ]
Previous thread: none

Next thread: none