* [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
* [PATCH 2/4] Some better parse-options documentation.
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-2-git-send-email-madcoder@debian.org>
---
parse-options.h | 37 +++++++++++++++++++++++++++++++++++--
1 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/parse-options.h b/parse-options.h
index 3a470e5..65bce6e 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -22,6 +22,41 @@ enum parse_opt_option_flags {
struct option;
typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
+/*
+ * `type`::
+ * holds the type of the option, you must have an OPTION_END last in your
+ * array.
+ *
+ * `short_name`::
+ * the character to use as a short option name, '\0' if none.
+ *
+ * `long_name`::
+ * the long option name, without the leading dashes, NULL if none.
+ *
+ * `value`::
+ * stores pointers to the values to be filled.
+ *
+ * `argh`::
+ * token to explain the kind of argument this option wants. Keep it
+ * homogenous across the repository.
+ *
+ * `help`::
+ * the short help associated to what the option does.
+ * Must never be NULL (except for OPTION_END).
+ * OPTION_GROUP uses this pointer to store the group header.
+ *
+ * `flags`::
+ * mask of parse_opt_option_flags.
+ * PARSE_OPT_OPTARG: says that the argument is optionnal (not for BOOLEANs)
+ * PARSE_OPT_NOARG: says that this option takes no argument, for CALLBACKs
+ *
+ * `callback`::
+ * pointer to the callback to use for OPTION_CALLBACK.
+ *
+ * `defval`::
+ * default value to fill (*->value) with for PARSE_OPT_OPTARG.
+ * CALLBACKS can use it like they want.
+ */
struct option {
enum parse_opt_type type;
int short_name;
@@ -32,8 +67,6 @@ struct option {
int flags;
parse_opt_cb *callback;
- /* holds default value for PARSE_OPT_OPTARG,
- though callbacks can use it like they want */
intptr_t defval;
};
--
1.5.3.5.1531.g59008
^ permalink raw reply related
* Re: [StGit PATCH] Cogito is deprecated, so don't point to it
From: Karl Hasselström @ 2007-11-05 12:00 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Catalin Marinas, git, David Kågedal
In-Reply-To: <vpqejf510ci.fsf@bauges.imag.fr>
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).
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [StGit PATCH] Cogito is deprecated, so don't point to it
From: David Kågedal @ 2007-11-05 11:52 UTC (permalink / raw)
To: git
In-Reply-To: <vpqejf510ci.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> 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?
Karl has been working hard lately to make sure that stg doesn't get
confused when you use git directly.
--
David Kågedal
^ permalink raw reply
* [PATCH 2/2] git-svn: t9114: verify merge commit message in test
From: Eric Wong @ 2007-11-05 11:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Eric Wong
In-Reply-To: <1194261708-32256-1-git-send-email-normalperson@yhbt.net>
It's possible that we end up with an incorrect commit message
in this test after making changes to fix the clobber bug
in dcommit.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
t/t9114-git-svn-dcommit-merge.sh | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index d6ca955..225060b 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -86,4 +86,9 @@ test_expect_success 'verify post-merge ancestry' "
git cat-file commit refs/heads/svn^ | grep '^friend$'
"
+test_expect_success 'verify merge commit message' "
+ git rev-list --pretty=raw -1 refs/heads/svn | \
+ grep \" Merge branch 'merge' into svn\"
+ "
+
test_done
--
1.5.3.5.566.gc207
^ permalink raw reply related
* [PATCH 1/2] git-svn: fix dcommit clobbering when committing a series of diffs
From: Eric Wong @ 2007-11-05 11:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Eric Wong
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>
---
Junio: please apply this to maint, thanks.
git-svn.perl | 47 ++++++++++++++++++++++--
t/t9106-git-svn-dcommit-clobber-series.sh | 56 +++++++++++++++++++++++++++++
2 files changed, 99 insertions(+), 4 deletions(-)
create mode 100755 t/t9106-git-svn-dcommit-clobber-series.sh
diff --git a/git-svn.perl b/git-svn.perl
index 4900f57..62a4a69 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -406,7 +406,8 @@ sub cmd_dcommit {
"If these changes depend on each other, re-running ",
"without --no-rebase will be required."
}
- foreach my $d (@$linear_refs) {
+ while (1) {
+ my $d = shift @$linear_refs or last;
unless (defined $last_rev) {
(undef, $last_rev, undef) = cmt_metadata("$d~1");
unless (defined $last_rev) {
@@ -439,14 +440,14 @@ sub cmd_dcommit {
# we always want to rebase against the current HEAD,
# not any head that was passed to us
- my @diff = command('diff-tree', 'HEAD',
+ my @diff = command('diff-tree', $d,
$gs->refname, '--');
my @finish;
if (@diff) {
@finish = rebase_cmd();
- print STDERR "W: HEAD and ", $gs->refname,
+ print STDERR "W: $d and ", $gs->refname,
" differ, using @finish:\n",
- "@diff";
+ join("\n", @diff), "\n";
} else {
print "No changes between current HEAD and ",
$gs->refname,
@@ -455,6 +456,45 @@ sub cmd_dcommit {
@finish = qw/reset --mixed/;
}
command_noisy(@finish, $gs->refname);
+ if (@diff) {
+ @refs = ();
+ my ($url_, $rev_, $uuid_, $gs_) =
+ working_head_info($head, \@refs);
+ my ($linear_refs_, $parents_) =
+ linearize_history($gs_, \@refs);
+ if (scalar(@$linear_refs) !=
+ scalar(@$linear_refs_)) {
+ fatal "# of revisions changed ",
+ "\nbefore:\n",
+ join("\n", @$linear_refs),
+ "\n\nafter:\n",
+ join("\n", @$linear_refs_), "\n",
+ 'If you are attempting to commit ',
+ "merges, try running:\n\t",
+ 'git rebase --interactive',
+ '--preserve-merges ',
+ $gs->refname,
+ "\nBefore dcommitting";
+ }
+ if ($url_ ne $url) {
+ fatal "URL mismatch after rebase: ",
+ "$url_ != $url";
+ }
+ if ($uuid_ ne $uuid) {
+ fatal "uuid mismatch after rebase: ",
+ "$uuid_ != $uuid";
+ }
+ # remap parents
+ my (%p, @l, $i);
+ for ($i = 0; $i < scalar @$linear_refs; $i++) {
+ my $new = $linear_refs_->[$i] or next;
+ $p{$new} =
+ $parents->{$linear_refs->[$i]};
+ push @l, $new;
+ }
+ $parents = \%p;
+ $linear_refs = \@l;
+ }
$last_rev = $cmt_rev;
}
}
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' "
+ echo hi > life &&
+ git update-index --add life &&
+ git commit -m hi-life &&
+ echo bye >> life &&
+ git commit -m bye-life life
+ "
+
+test_expect_success 'change file but in unrelated area' "
+ test x\"\`sed -n -e 4p < file\`\" = x4 &&
+ test x\"\`sed -n -e 7p < file\`\" = x7 &&
+ perl -i -p -e 's/^4\$/4444/' file &&
+ perl -i -p -e 's/^7\$/7777/' file &&
+ test x\"\`sed -n -e 4p < file\`\" = x4444 &&
+ test x\"\`sed -n -e 7p < file\`\" = x7777 &&
+ git commit -m '4 => 4444, 7 => 7777' file &&
+ git svn dcommit &&
+ svn up tmp &&
+ cd tmp &&
+ test x\"\`sed -n -e 4p < file\`\" = x4444 &&
+ test x\"\`sed -n -e 7p < file\`\" = x7777 &&
+ test x\"\`sed -n -e 58p < file\`\" = x5588 &&
+ test x\"\`sed -n -e 61p < file\`\" = x6611
+ "
+
+test_done
--
1.5.3.5.566.gc207
^ permalink raw reply related
* Re: [ANNOUNCE] cgit v0.7
From: Lars Hjemli @ 2007-11-05 10:59 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git list
In-Reply-To: <1194222569-13948-1-git-send-email-jnareb@gmail.com>
On Nov 5, 2007 1:29 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> Lars Hjemli wrote:
> > cgit v0.7 (a fast webinterface for git) is now available
>
> * Very nice cgit logo,
Thanks!
> but no favicon. Perhaps pacman head and G,
> or pacman head (like in logo) and +/-...
I've never cared much about favicons, but I guess cgit could provide one.
>
> * Providing reference with full sha1 of referenced object for tags
> list is not IMVHO a good design: what is interesting is type of
> tag, if it is signed it's first line, and if it is lightweight
> pointing to tag then perhaps commit subject.
Yes, the full sha1 is not very interesting. But I'm not sure what to
replace it with: the first line of annotated tags is very often
identical to tag name. Maybe it should just abbreviate the sha1?
>
> * Nice diffstat in commit view; the diff view is better, although I
> wouldn't lump from-file / to-file diff header together with git
> diff header and extended git diff header.
I've tried to make the diff look similar to 'git log -p' output, but I
agree the first line per file is probably overkill.
>
> * I like the sidebar very much, although I'm not sure how it would
> work for larger projects (more branches, much more tags).
How do you think it works out with http://hjemli.net/git/xorg/xserver/
? It's got an impressive number of branches and tags ;-)
> Also the
> search textbox is not very visible; I'd rather it have "groove"
> view.
Agreed, it's probably useless trying to style input-controls: the
result is heavily browser/platform dependent.
> * I like separate 'mirrors' section, although I think it rather
> clashes badly with notion of forks (alternates).
Well, it's only a section header, i.e. a parameter in cgitrc
> * I'm not sure if it wouln't be beter to provide -n/+m lines changed
> instead of nn likes changed column.
Maybe. I think it used to be -n/+m, but then I changed it; don't
remember why...
>
> * Nice submodule support!
Heh, it's a simple hack, but thanks anyway. It probably needs to be
configurable per repo though.
>
> By the way, Freedesktop provides besides standard gitweb interface
> also cgit interface at
> http://cgit.freedesktop.org/
> Take a look at how such site looks like with large number of projects
> (perhaps sidebar is noot such a good idea then?), and with large
> projects.
Actually, the filtered branch/tag lists was done partly because of
freedesktop.org. I think it has worked out nicely (but
cgit.freedesktop.org needs to run the latest cgit). Also, the width of
project descriptions is configurable, so it can take up much less
space and leave room for the sidebar.
Thanks for the comments, you've made my day!
--
larsh
^ permalink raw reply
* Re: [RFC PATCH] OSX Mail.app IMAP cache support for git-mailsplit?
From: Johannes Schindelin @ 2007-11-05 10:39 UTC (permalink / raw)
To: Michael Cohen; +Cc: git
In-Reply-To: <7B209F05-B720-41D6-AE98-39FAFF04B9F6@mac.com>
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 ;-)
On Mon, 5 Nov 2007, Michael Cohen wrote:
> On Nov 4, 2007, at 1:49 AM, Michael J. Cohen wrote:
>
> > Trivially, adding support for checking for Messages/ inside the specified
> > Maildir after cur/ is found not to exist would be enough to make this work.
>
> my repo at git://home.325i.org/git-osxmail.git should have that portion.
>
> unsure as to whether to make it an option, a fallback, a config value, or
> whatever...
A fallback would be sufficient.
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.
- you probably forgot to remove the original "if (populate...)...". That
means that populate would be called _twice_, even if successful.
- 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.
Ciao,
Dscho
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.
^ permalink raw reply
* Re: [PATCH] t7501-commit.sh: Add test case for fixing author in amend commit.
From: Johannes Schindelin @ 2007-11-05 10:24 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: gitster, git
In-Reply-To: <1194234165-9498-1-git-send-email-krh@redhat.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 713 bytes --]
Hi,
On Sun, 4 Nov 2007, Kristian Høgsberg wrote:
> +test_tick
>
> test_expect_success 'partial commit that involves removal (3)' '
We usually put the test_tick into the test case. IOW
test_expect_success 'message' '
test_tick &&
...
> '
>
> +oldtick=$GIT_AUTHOR_DATE
> +test_tick
> +
> +author="The Real Author <someguy@his.email.org>"
> +committer="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE"
> +
> +test_expect_success 'amend commit to fix author' '
Same here.
BTW: is this committer mangling really necessary? I thought only
GIT_COMMITTER_DATE was relevant. And that is easily replaced by
sed -e "s/^\(committer.* \)[0-9][0-9]*$/\1$GIT_COMMITTER_DATE/"
Ciao,
Dscho
^ permalink raw reply
* Re: [StGit PATCH] Cogito is deprecated, so don't point to it
From: Matthieu Moy @ 2007-11-05 9:57 UTC (permalink / raw)
To: Karl Hasselström; +Cc: Catalin Marinas, git
In-Reply-To: <20071105030608.6033.35208.stgit@yoghurt>
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?
--
Matthieu
^ permalink raw reply
* [PATCH] git-daemon: fix remote port number in log entry
From: Gerrit Pape @ 2007-11-05 9:16 UTC (permalink / raw)
To: git, Junio C Hamano
The port number in struct sockaddr_in needs to be converted from network
byte order to host byte order (on some architectures).
Signed-off-by: Gerrit Pape <pape@smarden.org>
---
daemon.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/daemon.c b/daemon.c
index 660e155..b8df980 100644
--- a/daemon.c
+++ b/daemon.c
@@ -540,7 +540,7 @@ static int execute(struct sockaddr *addr)
if (addr->sa_family == AF_INET) {
struct sockaddr_in *sin_addr = (void *) addr;
inet_ntop(addr->sa_family, &sin_addr->sin_addr, addrbuf, sizeof(addrbuf));
- port = sin_addr->sin_port;
+ port = ntohs(sin_addr->sin_port);
#ifndef NO_IPV6
} else if (addr && addr->sa_family == AF_INET6) {
struct sockaddr_in6 *sin6_addr = (void *) addr;
@@ -550,7 +550,7 @@ static int execute(struct sockaddr *addr)
inet_ntop(AF_INET6, &sin6_addr->sin6_addr, buf, sizeof(addrbuf) - 1);
strcat(buf, "]");
- port = sin6_addr->sin6_port;
+ port = ntohs(sin6_addr->sin6_port);
#endif
}
loginfo("Connection from %s:%d", addrbuf, port);
--
1.5.3.5
^ permalink raw reply related
* Re: [PATCH] Use parseopts in builtin-fetch
From: Pierre Habouzit @ 2007-11-05 8:55 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711042233590.7357@iabervon.org>
[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]
On Mon, Nov 05, 2007 at 03:35:34AM +0000, Daniel Barkalow wrote:
> I mostly did this and the next one for practice with the API. I'm
> impressed that "git fetch -vv" is even handled correctly without anything
> special.
About that: OPTION_BOOLEAN increments the associated variable, to
support this case specifically.
The last thing that really miss in parse-options is a way to recurse
into a sub-array of struct option, to be able to port the generic diff
and revision arguments.
Though, there is a difficulty here that I've not yet found how to
circumvent tastefully: right now options take an absolute pointer to
_the_ variable that will be filled with values. I need to be able to
relocate such a structure for sub-arrays for quite obvious reasons, and
that is quite hard to achieve without hazardous APIs. I currently lean
in the direction of simply memdup-ing the array and do fix-ups on
*values pointers. Though how to do that in a graceful way is not obvious
to me yet :)
This is the issue that currently prevents me from migrating any log
and diff based, though git diff -wbBCM that doesn't work is irritating me
more and more every day, so I'm confident I'll find a solution soon :)
--
·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] Use parseopts in builtin-fetch
From: Pierre Habouzit @ 2007-11-05 8:43 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711042233590.7357@iabervon.org>
[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]
On Mon, Nov 05, 2007 at 03:35:34AM +0000, Daniel Barkalow wrote:
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
> I mostly did this and the next one for practice with the API. I'm
> impressed that "git fetch -vv" is even handled correctly without anything
> special. Now that I've done it, assuming I did it right, it might as well
> get added to the series.
I believe the same patches (or very similar ones) are in pu but are
not in next yet because they conflict with the builtin-fetch recent
series.
see http://git.madism.org/?p=git.git;a=blobdiff;f=builtin-fetch.c;h=12b1c4;hp=6b1750d;hb=7407915;hpb=61610e6
> + OPT_BOOLEAN('q', "quiet", &quiet, "fetch silently"),
there is an OPT__QUIET(&quiet) for this one.
> + i = 1;
> if (i < argc) {
> int j = 0;
> refs = xcalloc(argc - i + 1, sizeof(const char *));
this is wrong, you meant i = 0, and frankly, it's better to just strip
i altogether.
--
·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 0/3] more terse push output
From: Steffen Prohaska @ 2007-11-05 7:09 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, Nicolas Pitre, Johannes Schindelin
In-Reply-To: <20071105050517.GA6244@sigill.intra.peff.net>
On Nov 5, 2007, at 6:05 AM, Jeff King wrote:
> - the 'ref is not up-to-date, maybe you need to push' message has
> gone
> away in favor of the terse '[rejected] ... (non-fast forward)'. I
> know there was some discussion recently of enhancing that message.
> Is this perhaps too terse?
I like the general idea of the terse output.
If we want a suggestion to the user, we could put it as
a summary. If a ref was rejected send-pack could print a
concluding message:
---
To file:///tmp/parent
+ f3325dc...3b91d1c hasforce -> mirror/hasforce (forced update)
f3325dc..bb022dc master -> mirror/master
! [rejected] needsforce -> mirror/needsforce (non-fast forward)
* [new branch] newbranch -> mirror/newbranch
* [new tag] v1.0 -> v1.0
Counting objects: 5, done.
Writing objects: 100% (3/3), done.
Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
warning: some refs were rejected.
Maybe they are not up-to-date and you want to pull or rebase first?
Or you may need to force an update?
---
In this way the terse output would not be disrupted and the
suggestion would be given to the user nonetheless.
I propose to use "warning" because it is not a real error. push
updates all other refs as expected. It only rejects some
refs. An error would be appropriate only after push learnt
transactions, that is either completely succeeds or doesn't
update any ref at all.
Steffen
^ permalink raw reply
* [RFC PATCH] OSX Mail.app IMAP cache support for git-mailsplit?
From: Michael Cohen @ 2007-11-05 6:36 UTC (permalink / raw)
To: Michael J. Cohen
In-Reply-To: <06FE21A2-20D0-4AAA-B0C7-35783C604B68@mac.com>
On Nov 4, 2007, at 1:49 AM, Michael J. Cohen wrote:
> Trivially, adding support for checking for Messages/ inside the
> specified Maildir after cur/ is found not to exist would be enough
> to make this work.
my repo at git://home.325i.org/git-osxmail.git should have that portion.
unsure as to whether to make it an option, a fallback, a config value,
or whatever...
-mjc
^ permalink raw reply
* [PATCH 3/3] send-pack: require --verbose to show update of tracking refs
From: Jeff King @ 2007-11-05 5:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nicolas Pitre, Johannes Schindelin
In-Reply-To: <20071105050517.GA6244@sigill.intra.peff.net>
This is really an uninteresting detail, and it just takes
attention away from the actual push updates and posssible
errors.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-send-pack.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index d74cc3c..c1fd3f5 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -195,7 +195,8 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)
return;
if (!remote_find_tracking(remote, &rs)) {
- fprintf(stderr, "updating local tracking ref '%s'\n", rs.dst);
+ if (args.verbose)
+ fprintf(stderr, "updating local tracking ref '%s'\n", rs.dst);
if (is_null_sha1(ref->peer_ref->new_sha1)) {
if (delete_ref(rs.dst, NULL))
error("Failed to delete");
--
1.5.3.5.1530.g7353
^ permalink raw reply related
* [PATCH 2/3] receive-pack: don't mention successful updates
From: Jeff King @ 2007-11-05 5:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nicolas Pitre, Johannes Schindelin
In-Reply-To: <20071105050517.GA6244@sigill.intra.peff.net>
The proposed updates are already shown to the user by
send-pack, so there's no point. We continue to show errors,
since they are unexpected.
Signed-off-by: Jeff King <peff@peff.net>
---
receive-pack.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/receive-pack.c b/receive-pack.c
index 38e35c0..ed44b89 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -204,8 +204,6 @@ static const char *update(struct command *cmd)
error("failed to delete %s", name);
return "failed to delete";
}
- fprintf(stderr, "%s: %s -> deleted\n", name,
- sha1_to_hex(old_sha1));
return NULL; /* good */
}
else {
@@ -217,8 +215,6 @@ static const char *update(struct command *cmd)
if (write_ref_sha1(lock, new_sha1, "push")) {
return "failed to write"; /* error() already called */
}
- fprintf(stderr, "%s: %s -> %s\n", name,
- sha1_to_hex(old_sha1), sha1_to_hex(new_sha1));
return NULL; /* good */
}
}
--
1.5.3.5.1530.g7353
^ 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