* Re: [PATCH] Implement selectable group ownership in git-init
From: Francesco Pretto @ 2007-11-05 15:09 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Junio C Hamano, git
In-Reply-To: <8EF5148D-C1F0-4329-A221-82D0B7E9932C@wincent.com>
Wincent Colaiuta ha scritto:
>
> using the administration tools designed for managing access, just like
> every other SCM (and every server, and every piece of software which can
> be accessed by many on a multi-user system).
>
I don't agree in general: in SCMs and other multi-user softwares, the access
control configuration can be safely postponed just because it's in their
standard usage pattern that the access should be conditioned by a daemon
to be configured later. It's not the case of git, just because git is very
tied to *nix permissions.
But as it is now, it could seems that it's good to put committers in the (for
example) git group, just because you have a git administrative account
git:git . This is caused, imo, by the fact that the flow of creating a shared
repository for a specific work/project group with git-init run by an
administrative user (as it should be) is something like this:
- Do it wrong;
- Fix it immediately.
I don't like the "Do it wrong" part. I'm trying to produce a sane and
transparent patch to implement the selectable group just in case of repository
first initialization. Why do I care so much of first time users? Dunno, but
I think it's important.
^ permalink raw reply
* Re: [PATCH 1/2] git-svn: fix dcommit clobbering when committing a series of diffs
From: Peter Baumann @ 2007-11-05 14:22 UTC (permalink / raw)
To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <1194261708-32256-1-git-send-email-normalperson@yhbt.net>
On Mon, Nov 05, 2007 at 03:21:47AM -0800, Eric Wong wrote:
> Our revision number sent to SVN is set to the last revision we
> committed if we've made any previous commits in a dcommit
> invocation.
>
> Although our SVN Editor code uses the delta of two (old) trees
> to generate information to send upstream, it'll still send
> complete resultant files upstream; even if the tree they're
> based against is out-of-date.
>
> The combination of sending a file that does not include the
> latest changes, but set with a revision number of a commit we
> just made will cause SVN to accept the resultant file even if it
> was generated against an old tree.
>
> More trouble was caused when fixing this because we were
> rebasing uncessarily at times. We used git-diff-tree to check
> the imported SVN revision against our HEAD, not the last tree we
> committed to SVN. The unnecessary rebasing caused merge commits
> upstream to SVN to fail.
>
> Signed-off-by: Eric Wong <normalperson@yhbt.net>
> ---
>
[...]
> diff --git a/t/t9106-git-svn-dcommit-clobber-series.sh b/t/t9106-git-svn-dcommit-clobber-series.sh
> new file mode 100755
> index 0000000..4216a88
> --- /dev/null
> +++ b/t/t9106-git-svn-dcommit-clobber-series.sh
> @@ -0,0 +1,56 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2007 Eric Wong
> +test_description='git-svn dcommit clobber series'
> +. ./lib-git-svn.sh
> +
> +test_expect_success 'initialize repo' "
> + mkdir import &&
> + cd import &&
> + awk 'BEGIN { for (i = 1; i < 64; i++) { print i } }' > file
> + svn import -m 'initial' . $svnrepo &&
> + cd .. &&
> + git svn init $svnrepo &&
> + git svn fetch &&
> + test -e file
> + "
> +
> +test_expect_success '(supposedly) non-conflicting change from SVN' "
> + test x\"\`sed -n -e 58p < file\`\" = x58 &&
> + test x\"\`sed -n -e 61p < file\`\" = x61 &&
> + svn co $svnrepo tmp &&
> + cd tmp &&
> + perl -i -p -e 's/^58\$/5588/' file &&
> + perl -i -p -e 's/^61\$/6611/' file &&
> + test x\"\`sed -n -e 58p < file\`\" = x5588 &&
> + test x\"\`sed -n -e 61p < file\`\" = x6611 &&
> + svn commit -m '58 => 5588, 61 => 6611' &&
> + cd ..
> + "
> +
> +test_expect_success 'unrelated some unrelated changes to git' "
The first unrelated seems odd here.
-Peter
> + echo hi > life &&
> + git update-index --add life &&
> + git commit -m hi-life &&
> + echo bye >> life &&
> + git commit -m bye-life life
> + "
> +
[...]
^ permalink raw reply
* Re: [PATCH] parseopt: introduce OPT_RECURSE to specify shared options
From: Pierre Habouzit @ 2007-11-05 13:53 UTC (permalink / raw)
To: Johannes Schindelin, git, gitster
In-Reply-To: <20071105123923.GC25574@artemis.corp>
[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]
On Mon, Nov 05, 2007 at 12:39:23PM +0000, Pierre Habouzit wrote:
> I like the kind of code that I allow to write better (I tend to
> dislike big fat global variables), though it's obvious that Johannes
> patch is a lot simpler and I like that.
We discussed it further, and what came out is that instead of
supporting quite complicated recursion mechanisms (or even a non so
complicated one), we can just define an
OPT__DIFFOPTIONS(diffopts)/OPT__REVOPTIONS(rev) macro that would inline
the needed options.
That's an idea I had but dismissed. Though, maybe it's not _that_
ugly, is clearly simpler, and one can argue that it's in the logical
continuation of OPT__QUIET and friends. What do you think ?
If it's the road we decide to take, then my documentation patch (2/4),
the parseopt fix (my 1/4 or johannes 1/3) and Johannes usage generator
enhancement (his 3/3) are still to be taken.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] Implement selectable group ownership in git-init
From: Wincent Colaiuta @ 2007-11-05 13:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Francesco Pretto, git
In-Reply-To: <7vabpvx8uu.fsf@gitster.siamese.dyndns.org>
El 3/11/2007, a las 23:27, Junio C Hamano escribió:
> I think what the patch attempts to achieve may be good, but only
> to reduce a few keystrokes of doing the "chgrp". Is it really
> worth it, I have to wonder...
I think this proposal adds unnecessary clutter to the codebase for
something that can easily be achieved (and *should*) using chown,
chgrp, or "sudo -u" etc.
The permissions of your Git installation and your repositories are
completely outside of the domain of Git itself, and should be
controlled using the administration tools designed for managing
access, just like every other SCM (and every server, and every piece
of software which can be accessed by many on a multi-user system).
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH 2/3] parseopt: introduce OPT_RECURSE to specify shared options
From: Johannes Schindelin @ 2007-11-05 13:46 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Pierre Habouzit
In-Reply-To: <Pine.LNX.4.64.0711051315300.4362@racer.site>
Hi,
On Mon, 5 Nov 2007, Johannes Schindelin wrote:
> The diff options should not need to be defined in every user of the
> diffcore. This provides the framework:
>
> extern struct option *diff_options;
>
> struct option options[] = {
> OPT_RECURSE(diff_options),
> ...
> OPT_END(),
> };
After kicking this around a bit more on IRC, we had another idea. Instead
of introducing OPT_RECURSE(), do something like OPT__QUIET(), only this
time in diff.h:
#define OPT__DIFF(opt) \
OPT_BOOLEAN('p', NULL, &opt.format_patch, "show a patch"), \
...
Pierre said this feels a bit "80s", so I'd like to hear other people's
opinions.
Hmm?
Ciao,
Dscho
^ permalink raw reply
* [PATCH 3/3] parseopt: do not list options with the same name twice
From: Johannes Schindelin @ 2007-11-05 13:15 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711051237420.4362@racer.site>
The option parser will take the first exact match, and happily ignore
it when a later option has the same name. For example, when you have
something like
OPT_BOOLEAN('i', NULL, &troz, "blah"),
OPT_BOOLEAN('i', NULL, &brain, "blub"),
in your options array, a command line "prog -i" will increment the
variable "troz", and leave the variable "brain" alone.
This behavior is all good and well, especially since we plan to
have recursive options, e.g. for the diff options, and in some cases
options should be overridden (see for example format-patch's "-n").
This patch matches the usage to this behavior: whenever an option name
was overridden by another option, it is no longer shown. In the
example above, that means that the usage would only show
-i blah
and not print anything about "blub".
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Okay, my plan was borked: the options struct is passed as a
const. So this is plan B.
parse-options.c | 37 ++++++++++++++++++++++++++-----------
test-parse-options.c | 2 ++
2 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index ed8e951..83a221b 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1,5 +1,6 @@
#include "git-compat-util.h"
#include "parse-options.h"
+#include "path-list.h"
#define OPT_SHORT 1
#define OPT_UNSET 2
@@ -261,15 +262,18 @@ int parse_options(int argc, const char **argv, const struct option *options,
#define USAGE_OPTS_WIDTH 24
#define USAGE_GAP 2
-static void usage_show_options(const struct option *opts)
+static void usage_show_options(const struct option *opts,
+ char *seen_short_names, struct path_list *seen_long_names)
{
for (; opts->type != OPTION_END; opts++) {
- size_t pos;
+ const char *before_option = " ";
+ size_t pos = 0;
int pad;
if (opts->type == OPTION_RECURSE) {
const struct option *sub = opts->value;
- usage_show_options(sub);
+ usage_show_options(sub,
+ seen_short_names, seen_long_names);
continue;
}
@@ -280,13 +284,19 @@ static void usage_show_options(const struct option *opts)
continue;
}
- pos = fprintf(stderr, " ");
- if (opts->short_name)
- pos += fprintf(stderr, "-%c", opts->short_name);
- if (opts->long_name && opts->short_name)
- pos += fprintf(stderr, ", ");
- if (opts->long_name)
- pos += fprintf(stderr, "--%s", opts->long_name);
+ if (opts->short_name && !seen_short_names[opts->short_name]) {
+ pos += fprintf(stderr, "%s-%c",
+ before_option, opts->short_name);
+ seen_short_names[opts->short_name] = 1;
+ before_option = ", ";
+ }
+ if (opts->long_name && !path_list_has_path(seen_long_names,
+ opts->long_name)) {
+ pos += fprintf(stderr, "%s--%s",
+ before_option, opts->long_name);
+ path_list_insert(opts->long_name, seen_long_names);
+ } else if (*before_option != ',')
+ continue;
switch (opts->type) {
case OPTION_INTEGER:
@@ -329,6 +339,9 @@ static void usage_show_options(const struct option *opts)
void usage_with_options(const char * const *usagestr,
const struct option *opts)
{
+ char seen_short_names[256];
+ struct path_list seen_long_names = { NULL, 0, 0, 0 };
+
fprintf(stderr, "usage: %s\n", *usagestr++);
while (*usagestr && **usagestr)
fprintf(stderr, " or: %s\n", *usagestr++);
@@ -338,7 +351,9 @@ void usage_with_options(const char * const *usagestr,
if (opts->type != OPTION_GROUP)
fputc('\n', stderr);
- usage_show_options(opts);
+ memset(seen_short_names, 0, sizeof(seen_short_names));
+ usage_show_options(opts, seen_short_names, &seen_long_names);
+ path_list_clear(&seen_long_names, 0);
fputc('\n', stderr);
diff --git a/test-parse-options.c b/test-parse-options.c
index ee64fb3..56f6f24 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -16,6 +16,7 @@ int main(int argc, const char **argv)
struct option sub[] = {
OPT_BOOLEAN('n', "narf", &narf, "what are we doing tonight?"),
OPT_INTEGER('z', "zort", &zort, "try to take over the world"),
+ OPT_INTEGER('j', NULL, &boolean, "ignored option"),
OPT_END(),
};
struct option options[] = {
@@ -27,6 +28,7 @@ int main(int argc, const char **argv)
OPT_STRING('s', "string", &string, "string", "get a string"),
OPT_STRING(0, "string2", &string, "str", "get another string"),
OPT_STRING(0, "st", &string, "st", "get another string (pervert ordering)"),
+ OPT_INTEGER(0, "string", &boolean, "ignored option"),
OPT_END(),
};
int i;
--
1.5.3.5.1549.g91a3
^ permalink raw reply related
* [PATCH 2/3] parseopt: introduce OPT_RECURSE to specify shared options
From: Johannes Schindelin @ 2007-11-05 13:15 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711051237420.4362@racer.site>
The diff options should not need to be defined in every user of the
diffcore. This provides the framework:
extern struct option *diff_options;
struct option options[] = {
OPT_RECURSE(diff_options),
...
OPT_END(),
};
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
parse-options.c | 68 +++++++++++++++++++++++++++++++++++-----------
parse-options.h | 2 +
t/t0040-parse-options.sh | 26 +++++++++++++++++
test-parse-options.c | 10 +++++++
4 files changed, 90 insertions(+), 16 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 15b32f7..ed8e951 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -4,6 +4,8 @@
#define OPT_SHORT 1
#define OPT_UNSET 2
+#define OPT_NOT_FOUND -2
+
struct optparse_t {
const char **argv;
int argc;
@@ -111,8 +113,14 @@ static int parse_short_opt(struct optparse_t *p, const struct option *options)
p->opt = p->opt[1] ? p->opt + 1 : NULL;
return get_value(p, options, OPT_SHORT);
}
+ if (options->type == OPTION_RECURSE) {
+ const struct option *sub = options->value;
+ int result = parse_short_opt(p, sub);
+ if (result != OPT_NOT_FOUND)
+ return result;
+ }
}
- return error("unknown switch `%c'", *p->opt);
+ return OPT_NOT_FOUND;
}
static int parse_long_opt(struct optparse_t *p, const char *arg,
@@ -129,6 +137,13 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,
const char *rest;
int flags = 0;
+ if (options->type == OPTION_RECURSE) {
+ const struct option *sub = options->value;
+ int result = parse_long_opt(p, arg, sub);
+ if (result != OPT_NOT_FOUND)
+ return result;
+ }
+
if (!options->long_name)
continue;
@@ -187,14 +202,14 @@ is_abbreviated:
abbrev_option->long_name);
if (abbrev_option)
return get_value(p, abbrev_option, abbrev_flags);
- return error("unknown option `%s'", arg);
+ return OPT_NOT_FOUND;
}
int parse_options(int argc, const char **argv, const struct option *options,
const char * const usagestr[], int flags)
{
struct optparse_t args = { argv + 1, argc - 1, NULL };
- int j = 0;
+ int j = 0, result;
for (; args.argc; args.argc--, args.argv++) {
const char *arg = args.argv[0];
@@ -209,8 +224,13 @@ int parse_options(int argc, const char **argv, const struct option *options,
do {
if (*args.opt == 'h')
usage_with_options(usagestr, options);
- if (parse_short_opt(&args, options) < 0)
+ result = parse_short_opt(&args, options);
+ if (result < 0) {
+ if (result == OPT_NOT_FOUND)
+ error("unknown switch `%c'",
+ arg[1]);
usage_with_options(usagestr, options);
+ }
} while (args.opt);
continue;
}
@@ -225,8 +245,12 @@ int parse_options(int argc, const char **argv, const struct option *options,
if (!strcmp(arg + 2, "help"))
usage_with_options(usagestr, options);
- if (parse_long_opt(&args, arg + 2, options))
+ result = parse_long_opt(&args, arg + 2, options);
+ if (result) {
+ if (result == OPT_NOT_FOUND)
+ error("unknown option `%s'", arg + 2);
usage_with_options(usagestr, options);
+ }
}
memmove(argv + j, args.argv, args.argc * sizeof(*argv));
@@ -237,22 +261,18 @@ int parse_options(int argc, const char **argv, const struct option *options,
#define USAGE_OPTS_WIDTH 24
#define USAGE_GAP 2
-void usage_with_options(const char * const *usagestr,
- const struct option *opts)
+static void usage_show_options(const struct option *opts)
{
- fprintf(stderr, "usage: %s\n", *usagestr++);
- while (*usagestr && **usagestr)
- fprintf(stderr, " or: %s\n", *usagestr++);
- while (*usagestr)
- fprintf(stderr, " %s\n", *usagestr++);
-
- if (opts->type != OPTION_GROUP)
- fputc('\n', stderr);
-
for (; opts->type != OPTION_END; opts++) {
size_t pos;
int pad;
+ if (opts->type == OPTION_RECURSE) {
+ const struct option *sub = opts->value;
+ usage_show_options(sub);
+ continue;
+ }
+
if (opts->type == OPTION_GROUP) {
fputc('\n', stderr);
if (*opts->help)
@@ -304,6 +324,22 @@ void usage_with_options(const char * const *usagestr,
}
fprintf(stderr, "%*s%s\n", pad + USAGE_GAP, "", opts->help);
}
+}
+
+void usage_with_options(const char * const *usagestr,
+ const struct option *opts)
+{
+ fprintf(stderr, "usage: %s\n", *usagestr++);
+ while (*usagestr && **usagestr)
+ fprintf(stderr, " or: %s\n", *usagestr++);
+ while (*usagestr)
+ fprintf(stderr, " %s\n", *usagestr++);
+
+ if (opts->type != OPTION_GROUP)
+ fputc('\n', stderr);
+
+ usage_show_options(opts);
+
fputc('\n', stderr);
exit(129);
diff --git a/parse-options.h b/parse-options.h
index 3a470e5..9ec1e35 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -8,6 +8,7 @@ enum parse_opt_type {
OPTION_STRING,
OPTION_INTEGER,
OPTION_CALLBACK,
+ OPTION_RECURSE,
};
enum parse_opt_flags {
@@ -44,6 +45,7 @@ struct option {
#define OPT_STRING(s, l, v, a, h) { OPTION_STRING, (s), (l), (v), (a), (h) }
#define OPT_CALLBACK(s, l, v, a, h, f) \
{ OPTION_CALLBACK, (s), (l), (v), (a), (h), 0, (f) }
+#define OPT_RECURSE(options) { OPTION_RECURSE, 0, NULL, options }
/* parse_options() will filter out the processed options and leave the
* non-option argments in argv[].
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 462fdf2..9c3efaa 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -13,6 +13,8 @@ usage: test-parse-options <options>
-b, --boolean get a boolean
-i, --integer <n> get a integer
-j <n> get a integer, too
+ -n, --narf what are we doing tonight?
+ -z, --zort <n> try to take over the world
string options
-s, --string <string>
@@ -29,6 +31,8 @@ test_expect_success 'test help' '
'
cat > expect << EOF
+narf: 0
+zort: 1
boolean: 2
integer: 1729
string: 123
@@ -40,6 +44,8 @@ test_expect_success 'short options' '
test ! -s output.err
'
cat > expect << EOF
+narf: 0
+zort: 1
boolean: 2
integer: 1729
string: 321
@@ -53,6 +59,8 @@ test_expect_success 'long options' '
'
cat > expect << EOF
+narf: 0
+zort: 1
boolean: 1
integer: 13
string: 123
@@ -69,6 +77,8 @@ test_expect_success 'intermingled arguments' '
'
cat > expect << EOF
+narf: 0
+zort: 1
boolean: 0
integer: 2
string: (not set)
@@ -92,6 +102,22 @@ test_expect_failure 'ambiguously abbreviated option' '
'
cat > expect << EOF
+narf: 1
+zort: 5
+boolean: 0
+integer: 3
+string: (not set)
+EOF
+
+test_expect_success 'recurse works' '
+ test-parse-options --narf -z5 --int=3 > output 2> output.err &&
+ test ! -s output.err &&
+ git diff expect output
+'
+
+cat > expect << EOF
+narf: 0
+zort: 1
boolean: 0
integer: 0
string: 123
diff --git a/test-parse-options.c b/test-parse-options.c
index 4d3e2ec..ee64fb3 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -1,6 +1,8 @@
#include "cache.h"
#include "parse-options.h"
+static int narf = 0;
+static int zort = 1;
static int boolean = 0;
static int integer = 0;
static char *string = NULL;
@@ -11,10 +13,16 @@ int main(int argc, const char **argv)
"test-parse-options <options>",
NULL
};
+ struct option sub[] = {
+ OPT_BOOLEAN('n', "narf", &narf, "what are we doing tonight?"),
+ OPT_INTEGER('z', "zort", &zort, "try to take over the world"),
+ OPT_END(),
+ };
struct option options[] = {
OPT_BOOLEAN('b', "boolean", &boolean, "get a boolean"),
OPT_INTEGER('i', "integer", &integer, "get a integer"),
OPT_INTEGER('j', NULL, &integer, "get a integer, too"),
+ OPT_RECURSE(sub),
OPT_GROUP("string options"),
OPT_STRING('s', "string", &string, "string", "get a string"),
OPT_STRING(0, "string2", &string, "str", "get another string"),
@@ -25,6 +33,8 @@ int main(int argc, const char **argv)
argc = parse_options(argc, argv, options, usage, 0);
+ printf("narf: %d\n", narf);
+ printf("zort: %d\n", zort);
printf("boolean: %d\n", boolean);
printf("integer: %d\n", integer);
printf("string: %s\n", string ? string : "(not set)");
--
1.5.3.5.1549.g91a3
^ permalink raw reply related
* [PATCH 1/3] parse-options: abbreviation engine fix.
From: Johannes Schindelin @ 2007-11-05 13:15 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711051237420.4362@racer.site>
When an option could be an ambiguous abbreviation of two options, the code
used to error out. Even if an exact match would have occured later.
Test and original patch by Pierre Habouzit.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
parse-options.c | 33 +++++++++++++++++++++------------
t/t0040-parse-options.sh | 13 +++++++++++++
test-parse-options.c | 1 +
3 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index cc09c98..15b32f7 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -119,8 +119,8 @@ 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;
+ const struct option *abbrev_option = NULL, *ambiguous_option = NULL;
+ int abbrev_flags = 0, ambiguous_flags = 0;
if (!arg_end)
arg_end = arg + strlen(arg);
@@ -137,16 +137,16 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,
/* 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 (abbrev_option) {
+ /*
+ * If this is abbreviated, it is
+ * ambiguous. So when there is no
+ * exact match later, we need to
+ * error out.
+ */
+ ambiguous_option = abbrev_option;
+ ambiguous_flags = abbrev_flags;
+ }
if (!(flags & OPT_UNSET) && *arg_end)
p->opt = arg_end + 1;
abbrev_option = options;
@@ -176,6 +176,15 @@ is_abbreviated:
}
return get_value(p, options, flags);
}
+
+ if (ambiguous_option)
+ return error("Ambiguous option: %s "
+ "(could be --%s%s or --%s%s)",
+ arg,
+ (ambiguous_flags & OPT_UNSET) ? "no-" : "",
+ ambiguous_option->long_name,
+ (abbrev_flags & OPT_UNSET) ? "no-" : "",
+ abbrev_option->long_name);
if (abbrev_option)
return get_value(p, abbrev_option, abbrev_flags);
return error("unknown option `%s'", arg);
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index ae49424..462fdf2 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -18,6 +18,7 @@ string options
-s, --string <string>
get a string
--string2 <str> get another string
+ --st <st> get another string (pervert ordering)
EOF
@@ -90,4 +91,16 @@ test_expect_failure 'ambiguously abbreviated option' '
test $? != 129
'
+cat > expect << EOF
+boolean: 0
+integer: 0
+string: 123
+EOF
+
+test_expect_success 'non ambiguous option (after two options it abbreviates)' '
+ test-parse-options --st 123 > output 2> output.err &&
+ test ! -s output.err &&
+ git diff expect output
+'
+
test_done
diff --git a/test-parse-options.c b/test-parse-options.c
index 277cfe4..4d3e2ec 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -18,6 +18,7 @@ int main(int argc, const char **argv)
OPT_GROUP("string options"),
OPT_STRING('s', "string", &string, "string", "get a string"),
OPT_STRING(0, "string2", &string, "str", "get another string"),
+ OPT_STRING(0, "st", &string, "st", "get another string (pervert ordering)"),
OPT_END(),
};
int i;
--
1.5.3.5.1549.g91a3
^ permalink raw reply related
* Re: [StGit PATCH] Cogito is deprecated, so don't point to it
From: Karl Hasselström @ 2007-11-05 13:11 UTC (permalink / raw)
To: David Kågedal; +Cc: Matthieu Moy, git, Catalin Marinas
In-Reply-To: <87zlxsannq.fsf@lysator.liu.se>
On 2007-11-05 13:21:13 +0100, David Kågedal wrote:
> Don't forget your new assimilate implementation.
That's in Catalin's master branch already, so there can't possibly be
a release that has this patch and not the assimilate improvements. But
yes, that's what makes it possible to easily recover from just about
anything the git commands can do.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [PATCH] errors: "strict subset" -> "ancestor"
From: Wincent Colaiuta @ 2007-11-05 13:06 UTC (permalink / raw)
To: David Symonds; +Cc: Steffen Prohaska, J. Bruce Fields, Junio C Hamano, git
In-Reply-To: <ee77f5c20711030014x23ac6206rec81fe5968992147@mail.gmail.com>
El 3/11/2007, a las 8:14, David Symonds escribió:
> On 11/3/07, Steffen Prohaska <prohaska@zib.de> wrote:
>>
>> On Nov 3, 2007, at 3:39 AM, J. Bruce Fields wrote:
>>
>>> diff --git a/send-pack.c b/send-pack.c
>>> index 5e127a1..b74fd45 100644
>>> --- a/send-pack.c
>>> +++ b/send-pack.c
>>> @@ -297,9 +297,9 @@ static int send_pack(int in, int out, struct
>>> remote *remote, int nr_refspec, cha
>>> * commits at the remote end and likely
>>> * we were not up to date to begin
>>> with.
>>> */
>>> - error("remote '%s' is not a strict "
>>> - "subset of local ref '%s'. "
>>> - "maybe you are not up-to-date
>>> and "
>>> + error("remote '%s' is not an
>>> ancestor of\n"
>>> + " local '%s'.\n"
>>
>> Two spaces in a row after local and before '%s'.
>
> So? That's presumably to align the remote and local strings.
Kind of: it aligns "error:" with "local":
error: remote 'refs/heads/master' is not an ancestor of
local 'refs/heads/master'.
Personal I think it would be better to align the right edges of
"remote" and "local" so that it looks like the following; this more
clearly shows the correspondence between the remote and local refs:
error: remote 'refs/heads/master' is not an ancestor of
local 'refs/heads/master'.
Or alternatively:
error: remote 'refs/heads/master' is not an ancestor of
local 'refs/heads/master'.
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] parse-options: abbreviation engine fix.
From: Pierre Habouzit @ 2007-11-05 12:59 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711051230020.4362@racer.site>
[-- Attachment #1: Type: text/plain, Size: 1689 bytes --]
On lun, nov 05, 2007 at 12:34:00 +0000, Johannes Schindelin wrote:
>
> When an option could be an ambiguous abbreviation of two options, the code
> used to error out. Even if an exact match would have occured later.
>
> Test and original patch by Pierre Habouzit.
>
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> ---
>
> On Mon, 5 Nov 2007, Pierre Habouzit wrote:
>
> > When we had at least two long option then followed by another
> > one that was a prefix of both of them, then the abbreviation
> > detector failed.
>
> Yeah, I assumed that you would never do such a thing, but I agree
> that with recursing option parsing it is much more likely.
>
> It took me some time to understand your patch, and that the moving
> of the UNSET handling was unnecessary.
yeah, that's because I dislike where it's done. Each time I read it,
I'm under the impression that it clobbers p->opt if this case is not the
one used.
In fact, I believe that for code clarity we should do what you
proposed earlier: disallow `=` in option names (that's rather sane
anyway) and drop the `rest` variable altogether, just use your current
arg_end and prefixcmp/strncmps. That will incidentally also allow to
remove skip_prefix while we're at it.
This way we could set p->opt first thing in the function and be done
with it instead of doing it two different ways in the function, hence
making the reader assume one is wrong or at least questionable.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty
From: Jakub Narebski @ 2007-11-05 12:58 UTC (permalink / raw)
To: git
In-Reply-To: <20071105124920.17726.qmail@746e9cce42b49f.315fe32.mid.smarden.org>
[Cc: Gerrit Pape <pape@smarden.org>, git@vger.kernel.org]
Gerrit Pape wrote:
> + char *sub[] = { "cur", "new" };
[...]
> + for (i = 0; i < 2; ++i) {
Wouldn't it be better to use sizeof(sub)/sizeof(sub[0]) or it's macro
equivalent ARRAY_SIZE(sub) instead of hardcoding 2 to avoid errors?
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* [PATCH] Add support for Apple Mail's Maildir format to builtin-mailsplit.
From: Michael J. Cohen @ 2007-11-05 12:57 UTC (permalink / raw)
To: Git Mailing List
Add support for Apple Mail's Maildir format to builtin-mailsplit.
Currently, mailsplit only checks for the directory 'cur' inside of the
specified directory, whereas Apple Mail's Maildirs have a Messages
directory.
Signed-off-by: Michael Cohen <mjc@kernel.org>
---
Another patch will follow to allow builtin-mailinfo to parse Apple
Mail's extra header and footers.
builtin-mailsplit.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
index 74b0470..6256351 100644
--- a/builtin-mailsplit.c
+++ b/builtin-mailsplit.c
@@ -129,8 +129,11 @@ static int split_maildir(const char *maildir,
const char *dir,
struct path_list list = {NULL, 0, 0, 1};
snprintf(curdir, sizeof(curdir), "%s/cur", maildir);
- if (populate_maildir_list(&list, curdir) < 0)
- goto out;
+ if (populate_maildir_list(&list, curdir) < 0) {
+ snprintf(curdir, sizeof(curdir), "%s/Messages", maildir);
+ if (populate_maildir_list(&list, curdir) < 0)
+ goto out;
+ }
for (i = 0; i < list.nr; i++) {
FILE *f;
--
1.5.3.5.562.g45f4-dirty
^ permalink raw reply related
* [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty
From: Gerrit Pape @ 2007-11-05 12:49 UTC (permalink / raw)
To: Fernando J. Pereda, git, Junio C Hamano
In-Reply-To: <20071026160118.GA5076@ferdyx.org>
When saving patches to a maildir with e.g. mutt, the files are put into
the new/ subdirectory of the maildir, not cur/. This makes git-am state
"Nothing to do.". This patch lets git-mailsplit additional check new/
after reading cur/.
This was reported by Joey Hess through
http://bugs.debian.org/447396
Signed-off-by: Gerrit Pape <pape@smarden.org>
---
On Fri, Oct 26, 2007 at 06:01:18PM +0200, Fernando J. Pereda wrote:
> By that reasoning, you should make it parse both cur/ and new/.
Okay.
builtin-mailsplit.c | 36 ++++++++++++++++++++----------------
1 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
index 74b0470..79e8ee0 100644
--- a/builtin-mailsplit.c
+++ b/builtin-mailsplit.c
@@ -101,19 +101,26 @@ static int populate_maildir_list(struct path_list *list, const char *path)
{
DIR *dir;
struct dirent *dent;
+ char name[PATH_MAX];
+ char *sub[] = { "cur", "new" };
+ int i;
- if ((dir = opendir(path)) == NULL) {
- error("cannot opendir %s (%s)", path, strerror(errno));
- return -1;
- }
+ for (i = 0; i < 2; ++i) {
+ snprintf(name, sizeof(name), "%s/%s", path, sub[i]);
+ if ((dir = opendir(name)) == NULL) {
+ error("cannot opendir %s (%s)", name, strerror(errno));
+ return -1;
+ }
- while ((dent = readdir(dir)) != NULL) {
- if (dent->d_name[0] == '.')
- continue;
- path_list_insert(dent->d_name, list);
- }
+ while ((dent = readdir(dir)) != NULL) {
+ if (dent->d_name[0] == '.')
+ continue;
+ snprintf(name, sizeof(name), "%s/%s", sub[i], dent->d_name);
+ path_list_insert(name, list);
+ }
- closedir(dir);
+ closedir(dir);
+ }
return 0;
}
@@ -122,19 +129,17 @@ static int split_maildir(const char *maildir, const char *dir,
int nr_prec, int skip)
{
char file[PATH_MAX];
- char curdir[PATH_MAX];
char name[PATH_MAX];
int ret = -1;
int i;
struct path_list list = {NULL, 0, 0, 1};
- snprintf(curdir, sizeof(curdir), "%s/cur", maildir);
- if (populate_maildir_list(&list, curdir) < 0)
+ if (populate_maildir_list(&list, maildir) < 0)
goto out;
for (i = 0; i < list.nr; i++) {
FILE *f;
- snprintf(file, sizeof(file), "%s/%s", curdir, list.items[i].path);
+ snprintf(file, sizeof(file), "%s/%s", maildir, list.items[i].path);
f = fopen(file, "r");
if (!f) {
error("cannot open mail %s (%s)", file, strerror(errno));
@@ -152,10 +157,9 @@ static int split_maildir(const char *maildir, const char *dir,
fclose(f);
}
- path_list_clear(&list, 1);
-
ret = skip;
out:
+ path_list_clear(&list, 1);
return ret;
}
--
1.5.3.5
^ permalink raw reply related
* Re: [PATCH] parse-options: abbreviation engine fix.
From: Johannes Schindelin @ 2007-11-05 12:38 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711051230020.4362@racer.site>
Hi,
On Mon, 5 Nov 2007, Johannes Schindelin wrote:
> +cat > expect << EOF
> +boolean: 0
> +integer: 2
> +string: 123
> +EOF
> +
> +test_expect_failure 'non ambiguous option (after two options it abbreviates)' '
> + test-parse-options --st 123 &&
> + test ! -s output.err &&
> + git diff expect output
> +'
> +
Aaargh! Yet another instance of test_expect_failure being wrong, wrong,
wrong.
I'll come up with a replacement patch.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] parseopt: introduce OPT_RECURSE to specify shared options
From: Pierre Habouzit @ 2007-11-05 12:39 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <Pine.LNX.4.64.0711051209061.4362@racer.site>
[-- Attachment #1: Type: text/plain, Size: 2886 bytes --]
On Mon, Nov 05, 2007 at 12:09:41PM +0000, Johannes Schindelin wrote:
>
> The diff options should not need to be defined in every user of the
> diffcore. This provides the framework:
>
> extern struct option *diff_options;
>
> struct option options[] = {
> OPT_RECURSE(diff_options),
> ...
> OPT_END(),
> };
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> This is not yet clever enough to show the correct usage when
> some sub option uses the same short or long name as one that
> has already been printed.
>
> Add the superlevel options struct as another parameter to
> usage_show_options(), and write an unset_option() function
> which takes a short name, a long name, and that superlevel
> options struct, and unsets all options which match.
Alternatively you can have a 256-bit bitfield to mark short options
that have already been seen and a hashtable of long options already
printed out, and while outputting an option, one should check that
either the short option or the long option is new.
This is nicer as some of the struct option may live in .rodata
> parse-options.c | 68 +++++++++++++++++++++++++++++++++++-----------
> parse-options.h | 2 +
> t/t0040-parse-options.sh | 24 ++++++++++++++++
> test-parse-options.c | 10 +++++++
> 4 files changed, 88 insertions(+), 16 deletions(-)
Okay, we discussed this with Johannes on IRC. I came up with the
relocation thing because I feared that the msys port (and maybe other ?)
that are about to use (or already do) threads would step on each other
toes while recursing into a sub-array of options.
Johannes thinks that this never happens in our codebase, hence that my
patches are an overkill.
The likely users of this feature are currently diff options (diff.c
diff_opt_parse) and revisions (builtin-log.c setup_revisions).
Using Johannes patch, we will have to export a global struct
diff_option (resp. struct rev_info) from diff.c (resp. revisions.c) and
no function (or almost) would take struct diff_option (resp struct
rev_info) as an argument because everyone would work on the global
variable[0].
With my patches, we can work like we do now, with a more functional
approach.
I like the kind of code that I allow to write better (I tend to
dislike big fat global variables), though it's obvious that Johannes
patch is a lot simpler and I like that.
[0] actually we don't *need* to remove the struct diff_options argument
from many functions except from diff_opt_parse, it's just that for
real, everybody will work on the same global structure anyway.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH] parse-options: abbreviation engine fix.
From: Johannes Schindelin @ 2007-11-05 12:34 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Junio C Hamano, git
In-Reply-To: <1194264204-3475-2-git-send-email-madcoder@debian.org>
When an option could be an ambiguous abbreviation of two options, the code
used to error out. Even if an exact match would have occured later.
Test and original patch by Pierre Habouzit.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
On Mon, 5 Nov 2007, Pierre Habouzit wrote:
> When we had at least two long option then followed by another
> one that was a prefix of both of them, then the abbreviation
> detector failed.
Yeah, I assumed that you would never do such a thing, but I agree
that with recursing option parsing it is much more likely.
It took me some time to understand your patch, and that the moving
of the UNSET handling was unnecessary.
IMHO this patch is easier to read.
parse-options.c | 33 +++++++++++++++++++++------------
t/t0040-parse-options.sh | 13 +++++++++++++
test-parse-options.c | 1 +
3 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index cc09c98..15b32f7 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -119,8 +119,8 @@ 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;
+ const struct option *abbrev_option = NULL, *ambiguous_option = NULL;
+ int abbrev_flags = 0, ambiguous_flags = 0;
if (!arg_end)
arg_end = arg + strlen(arg);
@@ -137,16 +137,16 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,
/* 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 (abbrev_option) {
+ /*
+ * If this is abbreviated, it is
+ * ambiguous. So when there is no
+ * exact match later, we need to
+ * error out.
+ */
+ ambiguous_option = abbrev_option;
+ ambiguous_flags = abbrev_flags;
+ }
if (!(flags & OPT_UNSET) && *arg_end)
p->opt = arg_end + 1;
abbrev_option = options;
@@ -176,6 +176,15 @@ is_abbreviated:
}
return get_value(p, options, flags);
}
+
+ if (ambiguous_option)
+ return error("Ambiguous option: %s "
+ "(could be --%s%s or --%s%s)",
+ arg,
+ (ambiguous_flags & OPT_UNSET) ? "no-" : "",
+ ambiguous_option->long_name,
+ (abbrev_flags & OPT_UNSET) ? "no-" : "",
+ abbrev_option->long_name);
if (abbrev_option)
return get_value(p, abbrev_option, abbrev_flags);
return error("unknown option `%s'", arg);
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index ae49424..ee758e5 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -18,6 +18,7 @@ string options
-s, --string <string>
get a string
--string2 <str> get another string
+ --st <st> get another string (pervert ordering)
EOF
@@ -90,4 +91,16 @@ test_expect_failure 'ambiguously abbreviated option' '
test $? != 129
'
+cat > expect << EOF
+boolean: 0
+integer: 2
+string: 123
+EOF
+
+test_expect_failure 'non ambiguous option (after two options it abbreviates)' '
+ test-parse-options --st 123 &&
+ test ! -s output.err &&
+ git diff expect output
+'
+
test_done
diff --git a/test-parse-options.c b/test-parse-options.c
index 277cfe4..4d3e2ec 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -18,6 +18,7 @@ int main(int argc, const char **argv)
OPT_GROUP("string options"),
OPT_STRING('s', "string", &string, "string", "get a string"),
OPT_STRING(0, "string2", &string, "str", "get another string"),
+ OPT_STRING(0, "st", &string, "st", "get another string (pervert ordering)"),
OPT_END(),
};
int i;
--
1.5.3.5.1549.g91a3
^ permalink raw reply related
* Re: [StGit PATCH] Cogito is deprecated, so don't point to it
From: David Kågedal @ 2007-11-05 12:21 UTC (permalink / raw)
To: Matthieu Moy, Karl Hasselström; +Cc: git, Catalin Marinas
In-Reply-To: <20071105120014.GA17406@diana.vm.bytemark.co.uk>
Karl Hasselström <kha@treskal.com> writes:
> On 2007-11-05 10:57:17 +0100, Matthieu Moy wrote:
>
>> Karl Hasselström <kha@treskal.com> writes:
>>
>> > -directly). For standard SCM operations, either use plain GIT commands
>> > -or the Cogito tool but it is not recommended to mix them with the
>> > -StGIT commands.
>> > +directly). For standard SCM operations, use plain GIT commands.
>>
>> Doesn't the "but it is not recommended to mix them with the StGIT
>> commands." part still hold?
>
> I'm not sure it ever held. Except possibly during merge resolution,
> but that mismatch is going away with the patch series by David Kågedal
> that's sitting in kha/experimental (which changes StGit to use exactly
> the same conflict representation as git).
Don't forget your new assimilate implementation.
--
David Kågedal
^ permalink raw reply
* Re: [StGit PATCH] Cogito is deprecated, so don't point to it
From: Karl Hasselström @ 2007-11-05 12:16 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Catalin Marinas, git, David Kågedal
In-Reply-To: <20071105120014.GA17406@diana.vm.bytemark.co.uk>
On 2007-11-05 13:00:14 +0100, Karl Hasselström wrote:
> On 2007-11-05 10:57:17 +0100, Matthieu Moy wrote:
>
> > Doesn't the "but it is not recommended to mix them with the StGIT
> > commands." part still hold?
>
> I'm not sure it ever held. Except possibly during merge resolution,
> but that mismatch is going away with the patch series by David
> Kågedal that's sitting in kha/experimental (which changes StGit to
> use exactly the same conflict representation as git).
Mmm. Then there's of course the fact that if you use git commands to
change the branch ref (e.g. committing, resetting, and so on) you need
to run assimilate to deconfuse StGit. But it'll tell you so.
So I guess the (or at least my) conclusion is that we ought to have an
StGit User Manual where the user can learn these things, preferably
with examples and pretty DAG graphs. Half a sentence doesn't buy us
that much.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* [PATCH] parseopt: introduce OPT_RECURSE to specify shared options
From: Johannes Schindelin @ 2007-11-05 12:09 UTC (permalink / raw)
To: git, gitster
The diff options should not need to be defined in every user of the
diffcore. This provides the framework:
extern struct option *diff_options;
struct option options[] = {
OPT_RECURSE(diff_options),
...
OPT_END(),
};
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
This is not yet clever enough to show the correct usage when
some sub option uses the same short or long name as one that
has already been printed.
Add the superlevel options struct as another parameter to
usage_show_options(), and write an unset_option() function
which takes a short name, a long name, and that superlevel
options struct, and unsets all options which match.
Then teach usage_show_options() to skip those options which
have a short name '\0' and a long name NULL.
But I leave that as an exercise to the reader.
parse-options.c | 68 +++++++++++++++++++++++++++++++++++-----------
parse-options.h | 2 +
t/t0040-parse-options.sh | 24 ++++++++++++++++
test-parse-options.c | 10 +++++++
4 files changed, 88 insertions(+), 16 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index cc09c98..66f79a1 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -4,6 +4,8 @@
#define OPT_SHORT 1
#define OPT_UNSET 2
+#define OPT_NOT_FOUND -2
+
struct optparse_t {
const char **argv;
int argc;
@@ -111,8 +113,14 @@ static int parse_short_opt(struct optparse_t *p, const struct option *options)
p->opt = p->opt[1] ? p->opt + 1 : NULL;
return get_value(p, options, OPT_SHORT);
}
+ if (options->type == OPTION_RECURSE) {
+ const struct option *sub = options->value;
+ int result = parse_short_opt(p, sub);
+ if (result != OPT_NOT_FOUND)
+ return result;
+ }
}
- return error("unknown switch `%c'", *p->opt);
+ return OPT_NOT_FOUND;
}
static int parse_long_opt(struct optparse_t *p, const char *arg,
@@ -129,6 +137,13 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,
const char *rest;
int flags = 0;
+ if (options->type == OPTION_RECURSE) {
+ const struct option *sub = options->value;
+ int result = parse_long_opt(p, arg, sub);
+ if (result != OPT_NOT_FOUND)
+ return result;
+ }
+
if (!options->long_name)
continue;
@@ -178,14 +193,14 @@ is_abbreviated:
}
if (abbrev_option)
return get_value(p, abbrev_option, abbrev_flags);
- return error("unknown option `%s'", arg);
+ return OPT_NOT_FOUND;
}
int parse_options(int argc, const char **argv, const struct option *options,
const char * const usagestr[], int flags)
{
struct optparse_t args = { argv + 1, argc - 1, NULL };
- int j = 0;
+ int j = 0, result;
for (; args.argc; args.argc--, args.argv++) {
const char *arg = args.argv[0];
@@ -200,8 +215,13 @@ int parse_options(int argc, const char **argv, const struct option *options,
do {
if (*args.opt == 'h')
usage_with_options(usagestr, options);
- if (parse_short_opt(&args, options) < 0)
+ result = parse_short_opt(&args, options);
+ if (result < 0) {
+ if (result == OPT_NOT_FOUND)
+ error("unknown switch `%c'",
+ arg[1]);
usage_with_options(usagestr, options);
+ }
} while (args.opt);
continue;
}
@@ -216,8 +236,12 @@ int parse_options(int argc, const char **argv, const struct option *options,
if (!strcmp(arg + 2, "help"))
usage_with_options(usagestr, options);
- if (parse_long_opt(&args, arg + 2, options))
+ result = parse_long_opt(&args, arg + 2, options);
+ if (result) {
+ if (result == OPT_NOT_FOUND)
+ error("unknown option `%s'", arg + 2);
usage_with_options(usagestr, options);
+ }
}
memmove(argv + j, args.argv, args.argc * sizeof(*argv));
@@ -228,22 +252,18 @@ int parse_options(int argc, const char **argv, const struct option *options,
#define USAGE_OPTS_WIDTH 24
#define USAGE_GAP 2
-void usage_with_options(const char * const *usagestr,
- const struct option *opts)
+static void usage_show_options(const struct option *opts)
{
- fprintf(stderr, "usage: %s\n", *usagestr++);
- while (*usagestr && **usagestr)
- fprintf(stderr, " or: %s\n", *usagestr++);
- while (*usagestr)
- fprintf(stderr, " %s\n", *usagestr++);
-
- if (opts->type != OPTION_GROUP)
- fputc('\n', stderr);
-
for (; opts->type != OPTION_END; opts++) {
size_t pos;
int pad;
+ if (opts->type == OPTION_RECURSE) {
+ const struct option *sub = opts->value;
+ usage_show_options(sub);
+ continue;
+ }
+
if (opts->type == OPTION_GROUP) {
fputc('\n', stderr);
if (*opts->help)
@@ -295,6 +315,22 @@ void usage_with_options(const char * const *usagestr,
}
fprintf(stderr, "%*s%s\n", pad + USAGE_GAP, "", opts->help);
}
+}
+
+void usage_with_options(const char * const *usagestr,
+ const struct option *opts)
+{
+ fprintf(stderr, "usage: %s\n", *usagestr++);
+ while (*usagestr && **usagestr)
+ fprintf(stderr, " or: %s\n", *usagestr++);
+ while (*usagestr)
+ fprintf(stderr, " %s\n", *usagestr++);
+
+ if (opts->type != OPTION_GROUP)
+ fputc('\n', stderr);
+
+ usage_show_options(opts);
+
fputc('\n', stderr);
exit(129);
diff --git a/parse-options.h b/parse-options.h
index 3a470e5..9ec1e35 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -8,6 +8,7 @@ enum parse_opt_type {
OPTION_STRING,
OPTION_INTEGER,
OPTION_CALLBACK,
+ OPTION_RECURSE,
};
enum parse_opt_flags {
@@ -44,6 +45,7 @@ struct option {
#define OPT_STRING(s, l, v, a, h) { OPTION_STRING, (s), (l), (v), (a), (h) }
#define OPT_CALLBACK(s, l, v, a, h, f) \
{ OPTION_CALLBACK, (s), (l), (v), (a), (h), 0, (f) }
+#define OPT_RECURSE(options) { OPTION_RECURSE, 0, NULL, options }
/* parse_options() will filter out the processed options and leave the
* non-option argments in argv[].
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index ae49424..de2d285 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -13,6 +13,8 @@ usage: test-parse-options <options>
-b, --boolean get a boolean
-i, --integer <n> get a integer
-j <n> get a integer, too
+ -n, --narf what are we doing tonight?
+ -z, --zort <n> try to take over the world
string options
-s, --string <string>
@@ -28,6 +30,8 @@ test_expect_success 'test help' '
'
cat > expect << EOF
+narf: 0
+zort: 1
boolean: 2
integer: 1729
string: 123
@@ -39,6 +43,8 @@ test_expect_success 'short options' '
test ! -s output.err
'
cat > expect << EOF
+narf: 0
+zort: 1
boolean: 2
integer: 1729
string: 321
@@ -52,6 +58,8 @@ test_expect_success 'long options' '
'
cat > expect << EOF
+narf: 0
+zort: 1
boolean: 1
integer: 13
string: 123
@@ -68,6 +76,8 @@ test_expect_success 'intermingled arguments' '
'
cat > expect << EOF
+narf: 0
+zort: 1
boolean: 0
integer: 2
string: (not set)
@@ -90,4 +100,18 @@ test_expect_failure 'ambiguously abbreviated option' '
test $? != 129
'
+cat > expect << EOF
+narf: 1
+zort: 5
+boolean: 0
+integer: 3
+string: (not set)
+EOF
+
+test_expect_success 'recurse works' '
+ test-parse-options --narf -z5 --int=3 > output 2> output.err &&
+ test ! -s output.err &&
+ git diff expect output
+'
+
test_done
diff --git a/test-parse-options.c b/test-parse-options.c
index 277cfe4..24613f8 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -1,6 +1,8 @@
#include "cache.h"
#include "parse-options.h"
+static int narf = 0;
+static int zort = 1;
static int boolean = 0;
static int integer = 0;
static char *string = NULL;
@@ -11,10 +13,16 @@ int main(int argc, const char **argv)
"test-parse-options <options>",
NULL
};
+ struct option sub[] = {
+ OPT_BOOLEAN('n', "narf", &narf, "what are we doing tonight?"),
+ OPT_INTEGER('z', "zort", &zort, "try to take over the world"),
+ OPT_END(),
+ };
struct option options[] = {
OPT_BOOLEAN('b', "boolean", &boolean, "get a boolean"),
OPT_INTEGER('i', "integer", &integer, "get a integer"),
OPT_INTEGER('j', NULL, &integer, "get a integer, too"),
+ OPT_RECURSE(sub),
OPT_GROUP("string options"),
OPT_STRING('s', "string", &string, "string", "get a string"),
OPT_STRING(0, "string2", &string, "str", "get another string"),
@@ -24,6 +32,8 @@ int main(int argc, const char **argv)
argc = parse_options(argc, argv, options, usage, 0);
+ printf("narf: %d\n", narf);
+ printf("zort: %d\n", zort);
printf("boolean: %d\n", boolean);
printf("integer: %d\n", integer);
printf("string: %s\n", string ? string : "(not set)");
--
1.5.3.5.1549.g91a3
^ permalink raw reply related
* Re: [RFC PATCH] OSX Mail.app IMAP cache support for git-mailsplit?
From: Michael Cohen @ 2007-11-05 12:09 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0711051034060.4362@racer.site>
On Nov 5, 2007, at 5:39 AM, Johannes Schindelin wrote:
> Hi,
>
> you have a very weird mail setting; I had to add the git list back
> to the
> Cc. This is just annoying enough for me to write an extra paragraph
> to
> annoy you back ;-)
Have to get used to this; thank you. :)
> Several comments (your patch not inlined, since you did not inline it
> either):
>
> - there needs to be a space between the ) and the { in the first if
> line.
Doh. done.
> - you probably forgot to remove the original "if (populate...)...".
> That
> means that populate would be called _twice_, even if successful.
good catch.
> - git is written in C. Therefore, "//" as a way to comment out is
> wrong.
> - if you still return -1 when the dir could not be opened, I wonder
> what
> the rationale is to comment the error out.
More work needs to be done in there and in builtin-mailinfo.c to
massage the mail format that Apple is using. Also what I think I need
to do there is check the path that is being tested and print something
like "%s/cur could not be found, trying alternate path" on the first
test?
> P.S.: You might want to send patches as these right away, without
> asking
> if anybody cares (you'll see that very soon), but rather in accord
> with
> Documentation/SubmittingPatches.
Thanks for that.
^ permalink raw reply
* [PATCH 4/4] Implement OPTION_SUBARRAY handling.
From: Pierre Habouzit @ 2007-11-05 12:03 UTC (permalink / raw)
To: gitster, Junio C Hamano; +Cc: git, Pierre Habouzit
In-Reply-To: <1194264204-3475-4-git-send-email-madcoder@debian.org>
Basically, an OPTION_SUBARRAY is a pointer to a new `struct option` array.
This array should start with an OPTION_BASEOFFSET used to relocate ->value
pointers.
The sizeof() of the struct used for the BASEOFFSET entry is also stored so
that a subarray can also have global side effects (->values pointers that
are not in the memory range of the BASE struct are not relocated).
Arbitrary nesting of subarrays is supported, though no checks that there is
a subarray loop is performed.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
parse-options.c | 78 +++++++++++++++++++++++++++++++++++++++------
parse-options.h | 5 +++
t/t0040-parse-options.sh | 31 +++++++++++++++++-
test-parse-options.c | 25 ++++++++++++++-
4 files changed, 126 insertions(+), 13 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 5cea511..001d1e5 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -4,6 +4,12 @@
#define OPT_SHORT 1
#define OPT_UNSET 2
+struct optreloc_t {
+ const char *base;
+ const char *end;
+ intptr_t offs;
+};
+
struct optparse_t {
const char **argv;
int argc;
@@ -11,6 +17,8 @@ struct optparse_t {
const struct option *abbrev_option, *conflict_option;
int abbrev_flags, conflict_flags;
+
+ struct optreloc_t reloc;
};
static inline const char *get_arg(struct optparse_t *p)
@@ -39,10 +47,32 @@ static int opterror(const struct option *opt, const char *reason, int flags)
return error("option `%s' %s", opt->long_name, reason);
}
+static void *reloc_value(struct optparse_t *p, const struct option *opt)
+{
+ char *value = opt->value;
+ if (value >= p->reloc.base && value < p->reloc.end)
+ return value + p->reloc.offs;
+ return value;
+}
+
+static const struct option *reloc_option(struct optparse_t *p,
+ const struct option *opt,
+ struct option *buf)
+{
+ char *value = opt->value;
+ if (value >= p->reloc.base && value < p->reloc.end) {
+ *buf = *opt;
+ buf->value = value + p->reloc.offs;
+ return buf;
+ }
+ return opt;
+}
+
static int get_value(struct optparse_t *p,
const struct option *opt, int flags)
{
const char *s, *arg;
+ struct option tmp;
arg = p->opt ? p->opt : (p->argc > 1 ? p->argv[1] : NULL);
if (p->opt && (flags & OPT_UNSET))
@@ -53,26 +83,27 @@ static int get_value(struct optparse_t *p,
if (!(flags & OPT_SHORT) && p->opt)
return opterror(opt, "takes no value", flags);
if (flags & OPT_UNSET)
- *(int *)opt->value = 0;
+ *(int *)reloc_value(p, opt) = 0;
else
- (*(int *)opt->value)++;
+ (*(int *)reloc_value(p, opt))++;
return 0;
case OPTION_STRING:
if (flags & OPT_UNSET) {
- *(const char **)opt->value = (const char *)NULL;
+ *(const char **)reloc_value(p, opt) = (const char *)NULL;
return 0;
}
if (opt->flags & PARSE_OPT_OPTARG && (!arg || *arg == '-')) {
- *(const char **)opt->value = (const char *)opt->defval;
+ *(const char **)reloc_value(p, opt) = (const char *)opt->defval;
return 0;
}
if (!arg)
return opterror(opt, "requires a value", flags);
- *(const char **)opt->value = get_arg(p);
+ *(const char **)reloc_value(p, opt) = get_arg(p);
return 0;
case OPTION_CALLBACK:
+ opt = reloc_option(p, opt, &tmp);
if (flags & OPT_UNSET)
return (*opt->callback)(opt, NULL, 1);
if (opt->flags & PARSE_OPT_NOARG) {
@@ -88,16 +119,16 @@ static int get_value(struct optparse_t *p,
case OPTION_INTEGER:
if (flags & OPT_UNSET) {
- *(int *)opt->value = 0;
+ *(int *)reloc_value(p, opt) = 0;
return 0;
}
if (opt->flags & PARSE_OPT_OPTARG && (!arg || !isdigit(*arg))) {
- *(int *)opt->value = opt->defval;
+ *(int *)reloc_value(p, opt) = opt->defval;
return 0;
}
if (!arg)
return opterror(opt, "requires a value", flags);
- *(int *)opt->value = strtol(get_arg(p), (char **)&s, 10);
+ *(int *)reloc_value(p, opt) = strtol(get_arg(p), (char **)&s, 10);
if (*s)
return opterror(opt, "expects a numerical value", flags);
return 0;
@@ -107,9 +138,24 @@ static int get_value(struct optparse_t *p,
}
}
+static const struct option *prepare_recursion(struct optparse_t *p,
+ const struct option *opt)
+{
+ const struct option *subarray = (const struct option *)opt->defval;
+ if (subarray->type != OPTION_BASEOFFSET)
+ die("subarray does not begins with a relocation stanza");
+ p->reloc.base = subarray->value;
+ p->reloc.end = p->reloc.base + subarray->defval;
+ p->reloc.offs = (const char *)opt->value - p->reloc.base;
+ return ++subarray;
+}
+
static int parse_short_opt(struct optparse_t *p, const struct option *options,
int level)
{
+ struct optreloc_t reloc;
+ int res;
+
for (;; options++) {
switch (options->type) {
case OPTION_END:
@@ -118,7 +164,11 @@ static int parse_short_opt(struct optparse_t *p, const struct option *options,
case OPTION_BASEOFFSET:
continue;
case OPTION_SUBARRAY:
- die("unsupported yet");
+ reloc = p->reloc;
+ res = parse_short_opt(p, prepare_recursion(p, options), level + 1);
+ p->reloc = reloc;
+ if (!res)
+ return 0;
break;
default:
if (options->short_name != *p->opt)
@@ -141,15 +191,21 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,
arg_end = arg + strlen(arg);
for (; options->type != OPTION_END; options++) {
+ struct optreloc_t reloc;
const char *rest;
- int flags = 0;
+ int res, flags = 0;
switch (options->type) {
case OPTION_GROUP:
case OPTION_BASEOFFSET:
continue;
case OPTION_SUBARRAY:
- die("unsupported yet");
+ reloc = p->reloc;
+ res = parse_long_opt(p, arg, prepare_recursion(p, options),
+ level + 1);
+ p->reloc = reloc;
+ if (!res)
+ return 0;
break;
default:
break;
diff --git a/parse-options.h b/parse-options.h
index 6668924..4f5a241 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -84,6 +84,11 @@ struct option {
#define OPT_CALLBACK(s, l, v, a, h, f) \
{ OPTION_CALLBACK, (s), (l), (v), (a), (h), 0, (f) }
+#define OPT_BASEOFFS(v) \
+ { OPTION_BASEOFFSET, 0, NULL, (v), NULL, NULL, 0, NULL, sizeof(*(v)) }
+#define OPT_SUBARRAY(v, sub) \
+ { OPTION_SUBARRAY, 0, NULL, (v), NULL, NULL, 0, NULL, (intptr_t)sub }
+
/* parse_options() will filter out the processed options and leave the
* non-option argments in argv[].
* Returns the number of arguments left in argv[].
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index ee758e5..c7a754e 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -20,6 +20,10 @@ string options
--string2 <str> get another string
--st <st> get another string (pervert ordering)
+test subarray
+ --incr-reloc increment a relocated integer
+ --incr-fixed increment a fixed integer
+
EOF
test_expect_success 'test help' '
@@ -32,6 +36,8 @@ cat > expect << EOF
boolean: 2
integer: 1729
string: 123
+diverted i: 0
+fixed i: 0
EOF
test_expect_success 'short options' '
@@ -43,6 +49,8 @@ cat > expect << EOF
boolean: 2
integer: 1729
string: 321
+diverted i: 0
+fixed i: 0
EOF
test_expect_success 'long options' '
@@ -56,6 +64,8 @@ cat > expect << EOF
boolean: 1
integer: 13
string: 123
+diverted i: 0
+fixed i: 0
arg 00: a1
arg 01: b1
arg 02: --boolean
@@ -72,6 +82,8 @@ cat > expect << EOF
boolean: 0
integer: 2
string: (not set)
+diverted i: 0
+fixed i: 0
EOF
test_expect_success 'unambiguously abbreviated option' '
@@ -93,11 +105,28 @@ test_expect_failure 'ambiguously abbreviated option' '
cat > expect << EOF
boolean: 0
+integer: 0
+string: (not set)
+diverted i: 1
+fixed i: 2
+EOF
+
+test_expect_success 'subarrays and partial relocation of options' '
+ test-parse-options --incr-reloc --incr-fixed --incr-fixed > output 2> output.err &&
+ test ! -s output.err &&
+ git diff expect output
+'
+
+test_done
+cat > expect << EOF
+boolean: 0
integer: 2
string: 123
+diverted i: 0
+fixed i: 0
EOF
-test_expect_failure 'non ambiguous option (after two options it abbreviates)' '
+test_expect_failure 'non ambiguous option (after two options it abbreviates, across subarray)' '
test-parse-options --st 123 &&
test ! -s output.err &&
git diff expect output
diff --git a/test-parse-options.c b/test-parse-options.c
index 4d3e2ec..5f740da 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -1,16 +1,34 @@
#include "cache.h"
#include "parse-options.h"
+struct reloc_me_please {
+ int integer;
+};
+
static int boolean = 0;
static int integer = 0;
static char *string = NULL;
+static int reloc_integer = 0;
+static int fixed_integer = 0;
+
+static const struct option subopts[] = {
+ OPT_BASEOFFS(&reloc_integer),
+ OPT_STRING(0, "st", &string, "st", "get another string (pervert ordering)"),
+ OPT_GROUP("test subarray"),
+ OPT_BOOLEAN(0, "incr-reloc", &reloc_integer, "increment a relocated integer"),
+ OPT_BOOLEAN(0, "incr-fixed", &fixed_integer, "increment a fixed integer"),
+ OPT_END(),
+};
+
int main(int argc, const char **argv)
{
const char *usage[] = {
"test-parse-options <options>",
NULL
};
+ int diverted_integer = 0;
+
struct option options[] = {
OPT_BOOLEAN('b', "boolean", &boolean, "get a boolean"),
OPT_INTEGER('i', "integer", &integer, "get a integer"),
@@ -18,7 +36,7 @@ int main(int argc, const char **argv)
OPT_GROUP("string options"),
OPT_STRING('s', "string", &string, "string", "get a string"),
OPT_STRING(0, "string2", &string, "str", "get another string"),
- OPT_STRING(0, "st", &string, "st", "get another string (pervert ordering)"),
+ OPT_SUBARRAY(&diverted_integer, subopts),
OPT_END(),
};
int i;
@@ -28,9 +46,14 @@ int main(int argc, const char **argv)
printf("boolean: %d\n", boolean);
printf("integer: %d\n", integer);
printf("string: %s\n", string ? string : "(not set)");
+ printf("diverted i: %d\n", diverted_integer);
+ printf("fixed i: %d\n", fixed_integer);
for (i = 0; i < argc; i++)
printf("arg %02d: %s\n", i, argv[i]);
+ if (reloc_integer)
+ die("reloc_integer should not ever change");
+
return 0;
}
--
1.5.3.5.1531.g59008
^ permalink raw reply related
* [PATCH 3/4] Add OPTION_BASEOFFSET/OPTION_SUBARRAY.
From: Pierre Habouzit @ 2007-11-05 12:03 UTC (permalink / raw)
To: gitster, Junio C Hamano; +Cc: git, Pierre Habouzit
In-Reply-To: <1194264204-3475-3-git-send-email-madcoder@debian.org>
Currently make the implementation die() if SUBARRAYs are encountered.
Refactor the rest of the code to be ready for recursion.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
parse-options.c | 114 ++++++++++++++++++++++++++++++++++++++-----------------
parse-options.h | 6 +++
2 files changed, 85 insertions(+), 35 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index d2e32c1..5cea511 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -8,6 +8,9 @@ struct optparse_t {
const char **argv;
int argc;
const char *opt;
+
+ const struct option *abbrev_option, *conflict_option;
+ int abbrev_flags, conflict_flags;
};
static inline const char *get_arg(struct optparse_t *p)
@@ -104,23 +107,35 @@ static int get_value(struct optparse_t *p,
}
}
-static int parse_short_opt(struct optparse_t *p, const struct option *options)
+static int parse_short_opt(struct optparse_t *p, const struct option *options,
+ int level)
{
- for (; options->type != OPTION_END; options++) {
- if (options->short_name == *p->opt) {
+ for (;; options++) {
+ switch (options->type) {
+ case OPTION_END:
+ return level > 0 ? -1 : error("unknown switch `%c'", *p->opt);
+ case OPTION_GROUP:
+ case OPTION_BASEOFFSET:
+ continue;
+ case OPTION_SUBARRAY:
+ die("unsupported yet");
+ break;
+ default:
+ if (options->short_name != *p->opt)
+ continue;
p->opt = p->opt[1] ? p->opt + 1 : NULL;
return get_value(p, options, OPT_SHORT);
}
}
- return error("unknown switch `%c'", *p->opt);
}
static int parse_long_opt(struct optparse_t *p, const char *arg,
- const struct option *options)
+ const struct option *options, int level)
{
const char *arg_end = strchr(arg, '=');
- const struct option *abbrev_option = NULL, *conflict_option = NULL;
- int abbrev_flags = 0, conflict_flags = 0;
+
+ if (level == 0)
+ p->conflict_option = p->abbrev_option = NULL;
if (!arg_end)
arg_end = arg + strlen(arg);
@@ -129,6 +144,17 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,
const char *rest;
int flags = 0;
+ switch (options->type) {
+ case OPTION_GROUP:
+ case OPTION_BASEOFFSET:
+ continue;
+ case OPTION_SUBARRAY:
+ die("unsupported yet");
+ break;
+ default:
+ break;
+ }
+
if (!options->long_name)
continue;
@@ -138,13 +164,16 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,
rest = skip_prefix(arg, options->long_name);
if (!rest) {
+ /* negated and abbreviated very much? */
+ if (!prefixcmp("no-", arg))
+ die("--n,--no,--no- are never proper abbreviated options");
/* abbreviated? */
if (!strncmp(options->long_name, arg, arg_end - arg)) {
is_abbreviated:
- conflict_option = abbrev_option;
- conflict_flags = abbrev_flags;
- abbrev_option = options;
- abbrev_flags = flags;
+ p->conflict_option = p->abbrev_option;
+ p->conflict_flags = p->abbrev_flags;
+ p->abbrev_option = options;
+ p->abbrev_flags = flags;
continue;
}
/* negated? */
@@ -167,16 +196,18 @@ is_abbreviated:
}
return get_value(p, options, flags);
}
- if (conflict_option)
+ if (level > 0)
+ return -1;
+ if (p->conflict_option)
return error("Ambiguous option: %s (could be --%s%s or --%s%s)",
- arg, (conflict_flags & OPT_UNSET) ? "no-" : "",
- conflict_option->long_name,
- (abbrev_flags & OPT_UNSET) ? "no-" : "",
- abbrev_option->long_name);
- if (abbrev_option) {
- if (!(abbrev_flags & OPT_UNSET) && *arg_end)
+ arg, (p->conflict_flags & OPT_UNSET) ? "no-" : "",
+ p->conflict_option->long_name,
+ (p->abbrev_flags & OPT_UNSET) ? "no-" : "",
+ p->abbrev_option->long_name);
+ if (p->abbrev_option) {
+ if (!(p->abbrev_flags & OPT_UNSET) && *arg_end)
p->opt = arg_end + 1;
- return get_value(p, abbrev_option, abbrev_flags);
+ return get_value(p, p->abbrev_option, p->abbrev_flags);
}
return error("unknown option `%s'", arg);
}
@@ -200,7 +231,7 @@ int parse_options(int argc, const char **argv, const struct option *options,
do {
if (*args.opt == 'h')
usage_with_options(usagestr, options);
- if (parse_short_opt(&args, options) < 0)
+ if (parse_short_opt(&args, options, 0) < 0)
usage_with_options(usagestr, options);
} while (args.opt);
continue;
@@ -216,7 +247,7 @@ int parse_options(int argc, const char **argv, const struct option *options,
if (!strcmp(arg + 2, "help"))
usage_with_options(usagestr, options);
- if (parse_long_opt(&args, arg + 2, options))
+ if (parse_long_opt(&args, arg + 2, options, 0))
usage_with_options(usagestr, options);
}
@@ -228,27 +259,27 @@ int parse_options(int argc, const char **argv, const struct option *options,
#define USAGE_OPTS_WIDTH 24
#define USAGE_GAP 2
-void usage_with_options(const char * const *usagestr,
- const struct option *opts)
+static void dump_options(const struct option *opts)
{
- fprintf(stderr, "usage: %s\n", *usagestr++);
- while (*usagestr && **usagestr)
- fprintf(stderr, " or: %s\n", *usagestr++);
- while (*usagestr)
- fprintf(stderr, " %s\n", *usagestr++);
-
- if (opts->type != OPTION_GROUP)
- fputc('\n', stderr);
-
- for (; opts->type != OPTION_END; opts++) {
+ for (;; opts++) {
size_t pos;
int pad;
- if (opts->type == OPTION_GROUP) {
+ switch (opts->type) {
+ case OPTION_END:
+ return;
+ case OPTION_GROUP:
fputc('\n', stderr);
if (*opts->help)
fprintf(stderr, "%s\n", opts->help);
continue;
+ case OPTION_BASEOFFSET:
+ continue;
+ case OPTION_SUBARRAY:
+ dump_options((struct option *)opts->defval);
+ continue;
+ default:
+ break;
}
pos = fprintf(stderr, " ");
@@ -295,8 +326,21 @@ void usage_with_options(const char * const *usagestr,
}
fprintf(stderr, "%*s%s\n", pad + USAGE_GAP, "", opts->help);
}
- fputc('\n', stderr);
+}
+void usage_with_options(const char * const *usagestr,
+ const struct option *opts)
+{
+ fprintf(stderr, "usage: %s\n", *usagestr++);
+ while (*usagestr && **usagestr)
+ fprintf(stderr, " or: %s\n", *usagestr++);
+ while (*usagestr)
+ fprintf(stderr, " %s\n", *usagestr++);
+
+ if (opts->type != OPTION_GROUP)
+ fputc('\n', stderr);
+ dump_options(opts);
+ fputc('\n', stderr);
exit(129);
}
diff --git a/parse-options.h b/parse-options.h
index 65bce6e..6668924 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -4,6 +4,8 @@
enum parse_opt_type {
OPTION_END,
OPTION_GROUP,
+ OPTION_BASEOFFSET,
+ OPTION_SUBARRAY,
OPTION_BOOLEAN,
OPTION_STRING,
OPTION_INTEGER,
@@ -35,6 +37,7 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
*
* `value`::
* stores pointers to the values to be filled.
+ * BASEOFFSET use it to store the offset wrt which the struct was filled.
*
* `argh`::
* token to explain the kind of argument this option wants. Keep it
@@ -56,6 +59,9 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
* `defval`::
* default value to fill (*->value) with for PARSE_OPT_OPTARG.
* CALLBACKS can use it like they want.
+ * SUBARRAYs use it to store the subarray address.
+ * BASEOFFSET use it to store the sizeof the struct used to fill the array.
+ * Any `value` that does not points into it is not relocated.
*/
struct option {
enum parse_opt_type type;
--
1.5.3.5.1531.g59008
^ permalink raw reply related
* proposal for an OPTION_SUBARRAY (recursive parser)
From: Pierre Habouzit @ 2007-11-05 12:03 UTC (permalink / raw)
To: gitster; +Cc: git
While working on OPTION_SUBARRAY I noticed a quite bad bug in how
abbrevations are dealt with, here is the fix, should be applied to next:
[PATCH 1/4] parse-options: abbreviation engine fix.
Here is a better documentation of the struct option, only touch
comments, safe for next:
[PATCH 2/4] Some better parse-options documentation.
Here is a 2 patch series that implement recursion and option relocation
of struct options when nested:
[PATCH 3/4] Add OPTION_BASEOFFSET/OPTION_SUBARRAY.
[PATCH 4/4] Implement OPTION_SUBARRAY handling.
Those two patch have nor real reason to go anywhere else than pu yet, I
post them for review and comments, and will probably base diff/log
migration on those soon.
^ permalink raw reply
* [PATCH 1/4] parse-options: abbreviation engine fix.
From: Pierre Habouzit @ 2007-11-05 12:03 UTC (permalink / raw)
To: gitster, Junio C Hamano; +Cc: git, Pierre Habouzit
In-Reply-To: <1194264204-3475-1-git-send-email-madcoder@debian.org>
When we had at least two long option then followed by another one that was a
prefix of both of them, then the abbreviation detector failed.
Fix the issue, add a test.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
parse-options.c | 48 +++++++++++++++++++++++-----------------------
t/t0040-parse-options.sh | 13 ++++++++++++
test-parse-options.c | 1 +
3 files changed, 38 insertions(+), 24 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index cc09c98..d2e32c1 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -119,8 +119,8 @@ 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;
+ const struct option *abbrev_option = NULL, *conflict_option = NULL;
+ int abbrev_flags = 0, conflict_flags = 0;
if (!arg_end)
arg_end = arg + strlen(arg);
@@ -132,42 +132,33 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,
if (!options->long_name)
continue;
+ /* special case {n,no,no-} that always conflict */
+ if (!prefixcmp("no-", arg))
+ die("`--{n,no,no-}' cannot be abbreviated forms of options");
+
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;
+ conflict_option = abbrev_option;
+ conflict_flags = abbrev_flags;
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 = skip_prefix(arg + 3, options->long_name);
+ arg += 3;
+ rest = skip_prefix(arg, options->long_name);
/* abbreviated and negated? */
- if (!rest && !prefixcmp(options->long_name, arg + 3))
- goto is_abbreviated;
- if (!rest)
+ if (!rest) {
+ if (!strncmp(options->long_name, arg, arg_end - arg))
+ goto is_abbreviated;
continue;
+ }
}
if (*rest) {
if (*rest != '=')
@@ -176,8 +167,17 @@ is_abbreviated:
}
return get_value(p, options, flags);
}
- if (abbrev_option)
+ if (conflict_option)
+ return error("Ambiguous option: %s (could be --%s%s or --%s%s)",
+ arg, (conflict_flags & OPT_UNSET) ? "no-" : "",
+ conflict_option->long_name,
+ (abbrev_flags & OPT_UNSET) ? "no-" : "",
+ abbrev_option->long_name);
+ if (abbrev_option) {
+ if (!(abbrev_flags & OPT_UNSET) && *arg_end)
+ p->opt = arg_end + 1;
return get_value(p, abbrev_option, abbrev_flags);
+ }
return error("unknown option `%s'", arg);
}
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index ae49424..ee758e5 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -18,6 +18,7 @@ string options
-s, --string <string>
get a string
--string2 <str> get another string
+ --st <st> get another string (pervert ordering)
EOF
@@ -90,4 +91,16 @@ test_expect_failure 'ambiguously abbreviated option' '
test $? != 129
'
+cat > expect << EOF
+boolean: 0
+integer: 2
+string: 123
+EOF
+
+test_expect_failure 'non ambiguous option (after two options it abbreviates)' '
+ test-parse-options --st 123 &&
+ test ! -s output.err &&
+ git diff expect output
+'
+
test_done
diff --git a/test-parse-options.c b/test-parse-options.c
index 277cfe4..4d3e2ec 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -18,6 +18,7 @@ int main(int argc, const char **argv)
OPT_GROUP("string options"),
OPT_STRING('s', "string", &string, "string", "get a string"),
OPT_STRING(0, "string2", &string, "str", "get another string"),
+ OPT_STRING(0, "st", &string, "st", "get another string (pervert ordering)"),
OPT_END(),
};
int i;
--
1.5.3.5.1531.g59008
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox