* Re: [PATCH] git-submodule - register module url if adding in place
From: Johannes Schindelin @ 2008-07-08 11:34 UTC (permalink / raw)
To: Mark Levedahl; +Cc: git
In-Reply-To: <1215484630-3784-1-git-send-email-mlevedahl@gmail.com>
Hi,
On Mon, 7 Jul 2008, Mark Levedahl wrote:
> When adding a new submodule in place, meaning the user created the
> submodule as a git repo in the tree first, we don't go through git
> submodule init to register the module. Thus, the url is not stored in
> .git/config, and no git operation will ever do so. In this case, assume
> the url the user supplies to git add is the one that should be
> registered, and do so.
I agree with Sylvain here, namely that this is too dangerous. Imagine
this very valid scenario:
$ git clone <somewhere> abc
$ git submodule add abc
Bummer.
Yes, happened to me.
So I'd like this to be an error, not something that tries to be helpful,
when it clearly cannot be.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/6] revisions: split the pure option parsing out from parse_revisions.
From: Pierre Habouzit @ 2008-07-08 11:25 UTC (permalink / raw)
To: Johannes Sixt, git, torvalds, gitster, peff
In-Reply-To: <20080708110022.GE19202@artemis.madism.org>
[-- Attachment #1: Type: text/plain, Size: 6070 bytes --]
On Tue, Jul 08, 2008 at 11:00:22AM +0000, Pierre Habouzit wrote:
> On Tue, Jul 08, 2008 at 10:51:13AM +0000, Johannes Sixt wrote:
> > Pierre Habouzit schrieb:
> > > +int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
> > > + int *unkc, const char **unkv)
> > > +{
> > > + const char *arg = argv[0];
> > > +
> > > + /* pseudo revision arguments */
> > > + if (!strcmp(arg, "--all") || !strcmp(arg, "--branches") ||
> > > + !strcmp(arg, "--tags") || !strcmp(arg, "--remotes") ||
> > > + !strcmp(arg, "--reflog") || !strcmp(arg, "--not") ||
> > > + !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk"))
> > > + {
> > > + unkv[*unkc++] = arg;
> >
> > + unkv[(*unkc)++] = arg;
>
> Huh right. Good catch, I wonder why the testsuite failed to see that.
Okay, I know why, there is another bug that was hiding this one,
parse_revisions whas doing argv[left++] = arg itself too, which was
fixing this mistake. I've pushed it again, and here is the fixed patch
3/6 for reviewing.
-----------8<-----------
From d0a062617526128c4ec0245614eb9916b7996c38 Mon Sep 17 00:00:00 2001
From: Pierre Habouzit <madcoder@debian.org>
Date: Tue, 8 Jul 2008 11:00:02 +0200
Subject: [PATCH] revisions: split handle_revision_args from parse_revisions.
This new function is meant to only parse non option revision arguments in
a row. IOW it's meant to parse what remains from a parse-opt filtering of
the argument list, with the knowledge of the "--" position (0 means none).
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
revision.c | 97 +++++++++++++++++++++++++++++++++++------------------------
revision.h | 1 +
2 files changed, 58 insertions(+), 40 deletions(-)
diff --git a/revision.c b/revision.c
index 99b3cc9..6958725 100644
--- a/revision.c
+++ b/revision.c
@@ -1241,39 +1241,15 @@ int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
return 1;
}
-/*
- * Parse revision information, filling in the "rev_info" structure,
- * and removing the used arguments from the argument list.
- *
- * Returns the number of arguments left that weren't recognized
- * (which are also moved to the head of the argument list)
- */
-int parse_revisions(int argc, const char **argv, struct rev_info *revs)
+int handle_revision_args(struct rev_info *revs,
+ int argc, const char **argv, int dashdash_pos)
{
- int i, flags, seen_dashdash;
- const char **unrecognized = argv + 1;
- int left = 1;
+ int i, left, flags = 0;
- /* First, search for "--" */
- seen_dashdash = 0;
- for (i = 1; i < argc; i++) {
+ for (left = i = 1; i < argc; i++) {
const char *arg = argv[i];
- if (strcmp(arg, "--"))
- continue;
- argv[i] = NULL;
- argc = i;
- if (argv[i + 1])
- revs->prune_data = get_pathspec(revs->prefix, argv + i + 1);
- seen_dashdash = 1;
- break;
- }
- flags = 0;
- for (i = 1; i < argc; i++) {
- const char *arg = argv[i];
if (*arg == '-') {
- int opts;
-
if (!strcmp(arg, "--all")) {
handle_refs(revs, flags, for_each_ref);
continue;
@@ -1306,23 +1282,22 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)
revs->no_walk = 0;
continue;
}
-
- opts = handle_revision_opt(revs, argc - i, argv + i, NULL, NULL);
- if (opts > 0) {
- i += opts - 1;
- continue;
+ if (i == dashdash_pos) {
+ argv[i] = NULL;
+ argc = i;
+ if (argv[i + 1])
+ revs->prune_data = get_pathspec(revs->prefix, argv + i + 1);
+ break;
}
- if (opts < 0)
- exit(128);
- *unrecognized++ = arg;
- left++;
+
+ argv[left++] = arg;
continue;
}
- if (handle_revision_arg(arg, revs, flags, seen_dashdash)) {
+ if (handle_revision_arg(arg, revs, flags, dashdash_pos)) {
int j;
- if (seen_dashdash || *arg == '^')
- die("bad revision '%s'", arg);
+ if (dashdash_pos || *arg == '^')
+ return error("bad revision '%s'", arg);
/* If we didn't have a "--":
* (1) all filenames must exist;
@@ -1338,9 +1313,51 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)
break;
}
}
+
return left;
}
+/*
+ * Parse revision information, filling in the "rev_info" structure,
+ * and removing the used arguments from the argument list.
+ *
+ * Returns the number of arguments left that weren't recognized
+ * (which are also moved to the head of the argument list)
+ */
+int parse_revisions(int argc, const char **argv, struct rev_info *revs)
+{
+ int i, left, dashdash_pos = 0;
+
+ /* First, filter out revision options and look for "--" */
+ for (left = i = 1; i < argc; i++) {
+ const char *arg = argv[i];
+ int opts;
+
+ if (arg[0] != '-') {
+ argv[left++] = arg;
+ continue;
+ }
+
+ if (!strcmp(arg, "--")) {
+ dashdash_pos = left;
+ memcpy(argv + left, argv + i, sizeof(*argv) * (argc - i));
+ left += argc - i;
+ break;
+ }
+
+ opts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);
+ if (opts > 0) {
+ i += opts - 1;
+ continue;
+ }
+ if (opts < 0)
+ exit(128);
+ }
+ argv[left] = NULL;
+
+ return handle_revision_args(revs, left, argv, dashdash_pos);
+}
+
void setup_revisions(struct rev_info *revs, const char *def)
{
if (revs->def == NULL)
diff --git a/revision.h b/revision.h
index 46ab713..c0f5df0 100644
--- a/revision.h
+++ b/revision.h
@@ -125,6 +125,7 @@ extern int parse_revisions(int argc, const char **argv, struct rev_info *revs);
extern void setup_revisions(struct rev_info *revs, const char *def);
extern int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
int *unkc, const char **unkv);
+extern int handle_revision_args(struct rev_info *revs, int argc, const char **argv, int dashdash_pos);
extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);
extern int prepare_revision_walk(struct rev_info *revs);
--
1.5.6.2.399.g15e15
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply related
* Re: [PATCH] better git-submodule status output
From: Johannes Schindelin @ 2008-07-08 11:21 UTC (permalink / raw)
To: Sylvain Joyeux
Cc: Junio C Hamano, Avery Pennarun, Lars Hjemli, Ping Yin,
Mark Levedahl, git
In-Reply-To: <20080708080042.GB3628@joyeux>
Hi,
On Tue, 8 Jul 2008, Sylvain Joyeux wrote:
> What I was missing is, given the current discussion, a git-submodule
> fetch alike to what Avery was proposing, which fetches all submodules in
> one command. Having to do the fetch/update manually for each submodule
> is really cumbersome -- I have around 30 modules in the superproject.
Did I not suggest "git submodule update" earlier? AFAIR this updates all
submodules that have been initialized.
Hth,
Dscho
^ permalink raw reply
* Re: [PATCH 1/6] revisions: refactor init_revisions and setup_revisions.
From: Pierre Habouzit @ 2008-07-08 11:06 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, torvalds, gitster, peff
In-Reply-To: <alpine.DEB.1.00.0807081258010.4319@eeepc-johanness>
[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]
On Tue, Jul 08, 2008 at 10:59:43AM +0000, Johannes Schindelin wrote:
> Hi,
>
> On Tue, 8 Jul 2008, Pierre Habouzit wrote:
>
> > setup_revisions has been split in two: parse_revisions that does
> > (almost) only argument parsing, to be more like what parse-options can
> > do, and setup_revisions that does the rest.
>
> I do not see the sense of this change, except
Well, it's required to remove "parse_revisions" at once if we one day
reach the goal of having only parse-opt based parsers.
> - blowing up your patch unnecessarily,
> - making it more inconvenient for users, and
It makes it convenient for parse-opt based parser, and it makes it
convenient for people that don't have arguments to parse (and those are
many).
> - adding an opportunity for bugs to creep in.
>
> In other words, I would like to see the semantics of setup_revisions()
> untouched.
FWIW like said I already submitted that patch under different versions
in the past, and I seem to recall that it was welcomed as it splits the
rev_info final fixups from the argument parsing which are quite
different steps.
FWIW I really like the new semantics better. If you're worried about
the fact that people may be confused about the new semantics, fine,
let's rename it then.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH 2/6] revisions: split the pure option parsing out from parse_revisions.
From: Pierre Habouzit @ 2008-07-08 11:00 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, torvalds, gitster, peff
In-Reply-To: <487346A1.4040006@viscovery.net>
[-- Attachment #1: Type: text/plain, Size: 932 bytes --]
On Tue, Jul 08, 2008 at 10:51:13AM +0000, Johannes Sixt wrote:
> Pierre Habouzit schrieb:
> > +int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
> > + int *unkc, const char **unkv)
> > +{
> > + const char *arg = argv[0];
> > +
> > + /* pseudo revision arguments */
> > + if (!strcmp(arg, "--all") || !strcmp(arg, "--branches") ||
> > + !strcmp(arg, "--tags") || !strcmp(arg, "--remotes") ||
> > + !strcmp(arg, "--reflog") || !strcmp(arg, "--not") ||
> > + !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk"))
> > + {
> > + unkv[*unkc++] = arg;
>
> + unkv[(*unkc)++] = arg;
Huh right. Good catch, I wonder why the testsuite failed to see that.
Anyways I pushed a fix on my public repo.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH 1/6] revisions: refactor init_revisions and setup_revisions.
From: Johannes Schindelin @ 2008-07-08 10:59 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git, torvalds, gitster, peff
In-Reply-To: <1215510964-16664-2-git-send-email-madcoder@debian.org>
Hi,
On Tue, 8 Jul 2008, Pierre Habouzit wrote:
> setup_revisions has been split in two: parse_revisions that does
> (almost) only argument parsing, to be more like what parse-options can
> do, and setup_revisions that does the rest.
I do not see the sense of this change, except
- blowing up your patch unnecessarily,
- making it more inconvenient for users, and
- adding an opportunity for bugs to creep in.
In other words, I would like to see the semantics of setup_revisions()
untouched.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/6] revisions: split the pure option parsing out from parse_revisions.
From: Johannes Sixt @ 2008-07-08 10:51 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git, torvalds, gitster, peff
In-Reply-To: <1215510964-16664-3-git-send-email-madcoder@debian.org>
Pierre Habouzit schrieb:
> +int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
> + int *unkc, const char **unkv)
> +{
> + const char *arg = argv[0];
> +
> + /* pseudo revision arguments */
> + if (!strcmp(arg, "--all") || !strcmp(arg, "--branches") ||
> + !strcmp(arg, "--tags") || !strcmp(arg, "--remotes") ||
> + !strcmp(arg, "--reflog") || !strcmp(arg, "--not") ||
> + !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk"))
> + {
> + unkv[*unkc++] = arg;
+ unkv[(*unkc)++] = arg;
Hm?
-- Hannes
^ permalink raw reply
* Re: Re* [PATCH] builtin-rerere: fix conflict markers parsing
From: Olivier Marin @ 2008-07-08 10:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vwsjwvmlk.fsf_-_@gitster.siamese.dyndns.org>
Junio C Hamano a écrit :
> Junio C Hamano <gitster@pobox.com> writes:
>
> I am wondering if a patch like this on top of your patch may make things
> even safer. The idea is the same as the earlier a1b32fd (git-rerere:
> detect unparsable conflicts, 2008-06-22) to fail rerere unless the markers
> are unambiguous.
>
> Thanks to your isspace(buf[7]), it is slightly less likely that this
> safety triggers on false positives.
>
> Thoughts?
My main concern was the error message that most users will not understand
after a "git rebase --continue", for example. So, I tried to remove it and
let things work as before because rerere seems to work even with ambiguous
cases.
But I think your patch is the right thing to do: safe is better.
Olivier.
^ permalink raw reply
* Re: [PATCH v2 2/4] Add git-sequencer prototype documentation
From: Jakub Narebski @ 2008-07-08 10:37 UTC (permalink / raw)
To: Stephan Beyer
Cc: git, Christian Couder, Daniel Barkalow, Johannes Schindelin,
Junio C Hamano
In-Reply-To: <1215277204-20926-1-git-send-email-s-beyer@gmx.net>
On Sat, 5 July 2008, Stephan Beyer wrote:
> +git-sequencer(1)
> +TODO FILE FORMAT
> +edit <commit>::
> +mark <mark>::
> +merge [options] <commit-ish1> <commit-ish2> ... <commit-ishN>::
> +pick [options] <commit>::
> +patch [options] <file>::
> +pause::
> +ref <ref>::
> +reset <commit-ish>::
> +squash [options] <commit>::
> +squash [options] --from <mark>::
A few comments and ideas:
1. Splitting a patch
I cannot comment well on git-sequencer, as I have started using
StGIT patch management interface instead of git-rebase in times when
there were no "git rebase --interactive". Nevertheless working with
StGIT is a bit similar to working with interactive rebase...
I don't find myself wanting to join two patches into one (to squadh
a commit) perhaps because when I want to add something to a commit
(to a patch) I simply go to this patch, edit files, and refresh the
patch. From time to time however I find myself SPLITTING a patch,
for example extracting something added "by the way"/"while at it"
into separate commit (like late separate better documenting
project_index file format from adding optional description field
to project_index file format).
Currently I don't see easy way to do this with git-sequencer.
2. Patch based rebase
git-rebase by default, and for speed, uses git-format-patch / git-am
pipeline (utilizing '--rebasing' option to git-am to avoid changing
commit messages, even if they do not follow commit message conventions).
If you want for plain "git rebase" to use git-sequencer, it should be
easy to support this "engine"; therefore perhaps it would be good
to add some equivalent of "git format-patch" to the TODO file format.
3. Checking rebase
Usually when you are interacting with upstream by sending patches by
email, the last part before sending series of patches is git-rebase
on top of current work. It would be nice if there were some way to
have "git rebase" (via git-sequencer) to check that all commits
(perhaps with some explicitly stated in TODO file exceptions) passes
pre-commit hook (checking for whitespaces and conflict markers), and
if possibly also either test suite, or relevant parts of test suite.
So perhaps extending TODO format by "check <script>" or
"check-all <script>"?
I'm just not sure if it is worth adding to TOD file format...
--
Jakub Narebski
Poland
^ permalink raw reply
* [PATCH] parse-options: add PARSE_OPT_FAKELASTARG flag.
From: Pierre Habouzit @ 2008-07-08 10:34 UTC (permalink / raw)
To: Junio C Hamano, git, Lars Hjemli
In-Reply-To: <20080708101452.GB19202@artemis.madism.org>
[-- Attachment #1: Type: text/plain, Size: 4482 bytes --]
If you set this for a given flag, and the flag appears without a value on
the command line, then the `defval' is used to fake a new argument.
Note that this flag is meaningless in presence of OPTARG or NOARG flags.
(in the current implementation it will be ignored, but don't rely on it).
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
> (3) inspired from (1) and (2), have a flag for options that says
> "I do take an argument, but if I'm the last option on the
> command line, please fake this argument for me.
>
> I really like (3) more FWIW as it doesn't generate ambiguous
> parsers like (2) would, and it's not horrible like (1). And cherry
> on top it's pretty trivial to implement I think.
And here it is, untested though (in the sense that I didn't test the
new feature, but git is not broken with the patch).
parse-options.c | 55 +++++++++++++++++++++++++++----------------------------
parse-options.h | 1 +
2 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index a90b336..a63485c 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -5,17 +5,6 @@
#define OPT_SHORT 1
#define OPT_UNSET 2
-static inline const char *get_arg(struct parse_opt_ctx_t *p)
-{
- if (p->opt) {
- const char *res = p->opt;
- p->opt = NULL;
- return res;
- }
- p->argc--;
- return *++p->argv;
-}
-
static int opterror(const struct option *opt, const char *reason, int flags)
{
if (flags & OPT_SHORT)
@@ -25,8 +14,24 @@ static int opterror(const struct option *opt, const char *reason, int flags)
return error("option `%s' %s", opt->long_name, reason);
}
+static inline int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,
+ int flags, const char **arg)
+{
+ if (p->opt) {
+ *arg = p->opt;
+ p->opt = NULL;
+ } else if (p->argc) {
+ p->argc--;
+ *arg = *++p->argv;
+ } else if (opt->flags & PARSE_OPT_FAKELASTARG) {
+ *arg = (const char *)opt->defval;
+ } else
+ return opterror(opt, "requires a value", flags);
+ return 0;
+}
+
static int get_value(struct parse_opt_ctx_t *p,
- const struct option *opt, int flags)
+ const struct option *opt, int flags)
{
const char *s, *arg;
const int unset = flags & OPT_UNSET;
@@ -52,7 +57,6 @@ static int get_value(struct parse_opt_ctx_t *p,
}
}
- arg = p->opt ? p->opt : (p->argc > 1 ? p->argv[1] : NULL);
switch (opt->type) {
case OPTION_BIT:
if (unset)
@@ -74,17 +78,12 @@ static int get_value(struct parse_opt_ctx_t *p,
return 0;
case OPTION_STRING:
- if (unset) {
+ if (unset)
*(const char **)opt->value = NULL;
- return 0;
- }
- if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
+ else if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
*(const char **)opt->value = (const char *)opt->defval;
- return 0;
- }
- if (!arg)
- return opterror(opt, "requires a value", flags);
- *(const char **)opt->value = get_arg(p);
+ else
+ return get_arg(p, opt, flags, (const char **)opt->value);
return 0;
case OPTION_CALLBACK:
@@ -94,9 +93,9 @@ static int get_value(struct parse_opt_ctx_t *p,
return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
- if (!arg)
- return opterror(opt, "requires a value", flags);
- return (*opt->callback)(opt, get_arg(p), 0) ? (-1) : 0;
+ if (get_arg(p, opt, flags, &arg))
+ return -1;
+ return (*opt->callback)(opt, arg, 0) ? (-1) : 0;
case OPTION_INTEGER:
if (unset) {
@@ -107,9 +106,9 @@ static int get_value(struct parse_opt_ctx_t *p,
*(int *)opt->value = opt->defval;
return 0;
}
- if (!arg)
- return opterror(opt, "requires a value", flags);
- *(int *)opt->value = strtol(get_arg(p), (char **)&s, 10);
+ if (get_arg(p, opt, flags, &arg))
+ return -1;
+ *(int *)opt->value = strtol(arg, (char **)&s, 10);
if (*s)
return opterror(opt, "expects a numerical value", flags);
return 0;
diff --git a/parse-options.h b/parse-options.h
index c5f0b4b..6e9edd1 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -28,6 +28,7 @@ enum parse_opt_option_flags {
PARSE_OPT_NOARG = 2,
PARSE_OPT_NONEG = 4,
PARSE_OPT_HIDDEN = 8,
+ PARSE_OPT_FAKELASTARG = 16,
};
struct option;
--
1.5.6.2.398.g3c3f1.dirty
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply related
* Re: What should "git branch --merged master" do?
From: Pierre Habouzit @ 2008-07-08 10:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Lars Hjemli
In-Reply-To: <7v8wwcx446.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 3075 bytes --]
On Tue, Jul 08, 2008 at 06:49:13AM +0000, Junio C Hamano wrote:
> e8b404c (git-branch: add support for --merged and --no-merged, 2008-04-17)
> introduced "git branch --merged" to show the branches that are contained
> within the current HEAD. I.e. the ones that you can say "git branch -d"
> and do not have to say "-D" to delete.
>
> Currently "git branch --merged master" fails with a message:
>
> fatal: A branch named 'master' already exists.
>
> but --merged and its opposite --no-merged are in spirit very similar to
> another option --contains in that it is meant to be used as a way to
> influence which branches are _reported_, and not about creating a new
> branch.
>
> Perhaps we should change them to default to HEAD (to retain the current
> behaviour) but take a commit, and show branches that are merged to the
> commit or not yet fully merged to the commit, respectively?
>
> Incidentally, "git branch --with" fails without the mandatory commit
> argument, and if we are going to do the above, we probably should default
> the argument to HEAD as well.
>
> Here is an attempt to update --with but I am not happy with it.
>
> The patch makes
>
> $ git branch --contains
>
> work as expected, but breaks
>
> $ git branch --contains master
>
> You need to spell "git branch --contains=master" instead. Which means it
> is a regression and cannot be applied. I suspect I am not using
> parse_options() in an optimal way, but I did not find a way for my
> callback to tell "I consumed the next parameter and it was mine" or "The
> next one is not my optional parameter" back to the parse_options(), so
> probably parse_options() need to be fixed to update this without
> regression, I suspect.
I actually see three ways.
(1) quick and dirty: if we see at argv[argc - 1]
--contains/--merge/--no-merge/... we do:
argv[argc] = "HEAD" and use parse-options with a mandatory
argument for --merge/--contains/... Okay this leaves the issue of
the fact that we must NULL-terminate argv, but that's trivial
(even if quite despisable).
(2) big update of parse-options: we refactor callbacks so that they
can decide if they take their argument or not. This removes the
need for --long-opt=foo for options with optional arguments, but
the refactor will be heavy. That's more or less what you suggest
in your last § but I don't like it for many reasons so I wont
enter the details.
(3) inspired from (1) and (2), have a flag for options that says "I do
take an argument, but if I'm the last option on the command line,
please fake this argument for me.
I really like (3) more FWIW as it doesn't generate ambiguous parsers
like (2) would, and it's not horrible like (1). And cherry on top it's
pretty trivial to implement I think.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: Migration of builtin-blame to parse-option
From: Pierre Habouzit @ 2008-07-08 10:02 UTC (permalink / raw)
To: git; +Cc: torvalds, gitster, peff
In-Reply-To: <1215510964-16664-1-git-send-email-madcoder@debian.org>
[-- Attachment #1: Type: text/plain, Size: 522 bytes --]
While rebasing this branch on the latest next, I had a conflict for
the patch 1 that I fixed, and I also fixed spacing issues in patches 4
and 6. The fixed series is on my public git repo[0], but it's really
like this one, so I don't see the point in flooding again with it ;)
[0] git://git.madism.org/~madcoder/git.git#ph/parseopt
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* [PATCH 2/6] revisions: split the pure option parsing out from parse_revisions.
From: Pierre Habouzit @ 2008-07-08 9:56 UTC (permalink / raw)
To: git; +Cc: torvalds, gitster, peff, Pierre Habouzit
In-Reply-To: <1215510964-16664-2-git-send-email-madcoder@debian.org>
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
revision.c | 579 ++++++++++++++++++++++++++++--------------------------------
revision.h | 2 +
2 files changed, 273 insertions(+), 308 deletions(-)
diff --git a/revision.c b/revision.c
index 3a46ed4..a3792c9 100644
--- a/revision.c
+++ b/revision.c
@@ -974,6 +974,273 @@ static void add_ignore_packed(struct rev_info *revs, const char *name)
revs->ignore_packed[num] = NULL;
}
+int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
+ int *unkc, const char **unkv)
+{
+ const char *arg = argv[0];
+
+ /* pseudo revision arguments */
+ if (!strcmp(arg, "--all") || !strcmp(arg, "--branches") ||
+ !strcmp(arg, "--tags") || !strcmp(arg, "--remotes") ||
+ !strcmp(arg, "--reflog") || !strcmp(arg, "--not") ||
+ !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk"))
+ {
+ unkv[*unkc++] = arg;
+ return 0;
+ }
+
+ if (!prefixcmp(arg, "--max-count=")) {
+ revs->max_count = atoi(arg + 12);
+ }
+ else if (!prefixcmp(arg, "--skip=")) {
+ revs->skip_count = atoi(arg + 7);
+ }
+ /* accept -<digit>, like traditional "head" */
+ else if ((*arg == '-') && isdigit(arg[1])) {
+ revs->max_count = atoi(arg + 1);
+ }
+ else if (!strcmp(arg, "-n")) {
+ if (argc <= 1)
+ return error("-n requires an argument");
+ revs->max_count = atoi(argv[1]);
+ return 2;
+ }
+ else if (!prefixcmp(arg, "-n")) {
+ revs->max_count = atoi(arg + 2);
+ }
+ else if (!prefixcmp(arg, "--max-age=")) {
+ revs->max_age = atoi(arg + 10);
+ }
+ else if (!prefixcmp(arg, "--since=")) {
+ revs->max_age = approxidate(arg + 8);
+ }
+ else if (!prefixcmp(arg, "--after=")) {
+ revs->max_age = approxidate(arg + 8);
+ }
+ else if (!prefixcmp(arg, "--min-age=")) {
+ revs->min_age = atoi(arg + 10);
+ }
+ else if (!prefixcmp(arg, "--before=")) {
+ revs->min_age = approxidate(arg + 9);
+ }
+ else if (!prefixcmp(arg, "--until=")) {
+ revs->min_age = approxidate(arg + 8);
+ }
+ else if (!strcmp(arg, "--first-parent")) {
+ revs->first_parent_only = 1;
+ }
+ else if (!strcmp(arg, "-g") ||
+ !strcmp(arg, "--walk-reflogs")) {
+ init_reflog_walk(&revs->reflog_info);
+ }
+ else if (!strcmp(arg, "--default")) {
+ if (argc <= 1)
+ return error("bad --default argument");
+ revs->def = argv[1];
+ return 2;
+ }
+ else if (!strcmp(arg, "--merge")) {
+ revs->show_merge = 1;
+ }
+ else if (!strcmp(arg, "--topo-order")) {
+ revs->lifo = 1;
+ revs->topo_order = 1;
+ }
+ else if (!strcmp(arg, "--date-order")) {
+ revs->lifo = 0;
+ revs->topo_order = 1;
+ }
+ else if (!prefixcmp(arg, "--early-output")) {
+ int count = 100;
+ switch (arg[14]) {
+ case '=':
+ count = atoi(arg+15);
+ /* Fallthrough */
+ case 0:
+ revs->topo_order = 1;
+ revs->early_output = count;
+ }
+ }
+ else if (!strcmp(arg, "--parents")) {
+ revs->rewrite_parents = 1;
+ revs->print_parents = 1;
+ }
+ else if (!strcmp(arg, "--dense")) {
+ revs->dense = 1;
+ }
+ else if (!strcmp(arg, "--sparse")) {
+ revs->dense = 0;
+ }
+ else if (!strcmp(arg, "--show-all")) {
+ revs->show_all = 1;
+ }
+ else if (!strcmp(arg, "--remove-empty")) {
+ revs->remove_empty_trees = 1;
+ }
+ else if (!strcmp(arg, "--no-merges")) {
+ revs->no_merges = 1;
+ }
+ else if (!strcmp(arg, "--boundary")) {
+ revs->boundary = 1;
+ }
+ else if (!strcmp(arg, "--left-right")) {
+ revs->left_right = 1;
+ }
+ else if (!strcmp(arg, "--cherry-pick")) {
+ revs->cherry_pick = 1;
+ revs->limited = 1;
+ }
+ else if (!strcmp(arg, "--objects")) {
+ revs->tag_objects = 1;
+ revs->tree_objects = 1;
+ revs->blob_objects = 1;
+ }
+ else if (!strcmp(arg, "--objects-edge")) {
+ revs->tag_objects = 1;
+ revs->tree_objects = 1;
+ revs->blob_objects = 1;
+ revs->edge_hint = 1;
+ }
+ else if (!strcmp(arg, "--unpacked")) {
+ revs->unpacked = 1;
+ free(revs->ignore_packed);
+ revs->ignore_packed = NULL;
+ revs->num_ignore_packed = 0;
+ }
+ else if (!prefixcmp(arg, "--unpacked=")) {
+ revs->unpacked = 1;
+ add_ignore_packed(revs, arg+11);
+ }
+ else if (!strcmp(arg, "-r")) {
+ revs->diff = 1;
+ DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
+ }
+ else if (!strcmp(arg, "-t")) {
+ revs->diff = 1;
+ DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
+ DIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE);
+ }
+ else if (!strcmp(arg, "-m")) {
+ revs->ignore_merges = 0;
+ }
+ else if (!strcmp(arg, "-c")) {
+ revs->diff = 1;
+ revs->dense_combined_merges = 0;
+ revs->combine_merges = 1;
+ }
+ else if (!strcmp(arg, "--cc")) {
+ revs->diff = 1;
+ revs->dense_combined_merges = 1;
+ revs->combine_merges = 1;
+ }
+ else if (!strcmp(arg, "-v")) {
+ revs->verbose_header = 1;
+ }
+ else if (!strcmp(arg, "--pretty")) {
+ revs->verbose_header = 1;
+ get_commit_format(arg+8, revs);
+ }
+ else if (!prefixcmp(arg, "--pretty=")) {
+ revs->verbose_header = 1;
+ get_commit_format(arg+9, revs);
+ }
+ else if (!strcmp(arg, "--graph")) {
+ revs->topo_order = 1;
+ revs->rewrite_parents = 1;
+ revs->graph = graph_init(revs);
+ }
+ else if (!strcmp(arg, "--root")) {
+ revs->show_root_diff = 1;
+ }
+ else if (!strcmp(arg, "--no-commit-id")) {
+ revs->no_commit_id = 1;
+ }
+ else if (!strcmp(arg, "--always")) {
+ revs->always_show_header = 1;
+ }
+ else if (!strcmp(arg, "--no-abbrev")) {
+ revs->abbrev = 0;
+ }
+ else if (!strcmp(arg, "--abbrev")) {
+ revs->abbrev = DEFAULT_ABBREV;
+ }
+ else if (!prefixcmp(arg, "--abbrev=")) {
+ revs->abbrev = strtoul(arg + 9, NULL, 10);
+ if (revs->abbrev < MINIMUM_ABBREV)
+ revs->abbrev = MINIMUM_ABBREV;
+ else if (revs->abbrev > 40)
+ revs->abbrev = 40;
+ }
+ else if (!strcmp(arg, "--abbrev-commit")) {
+ revs->abbrev_commit = 1;
+ }
+ else if (!strcmp(arg, "--full-diff")) {
+ revs->diff = 1;
+ revs->full_diff = 1;
+ }
+ else if (!strcmp(arg, "--full-history")) {
+ revs->simplify_history = 0;
+ }
+ else if (!strcmp(arg, "--relative-date")) {
+ revs->date_mode = DATE_RELATIVE;
+ }
+ else if (!strncmp(arg, "--date=", 7)) {
+ revs->date_mode = parse_date_format(arg + 7);
+ }
+ else if (!strcmp(arg, "--log-size")) {
+ revs->show_log_size = 1;
+ }
+ /*
+ * Grepping the commit log
+ */
+ else if (!prefixcmp(arg, "--author=")) {
+ add_header_grep(revs, "author", arg+9);
+ }
+ else if (!prefixcmp(arg, "--committer=")) {
+ add_header_grep(revs, "committer", arg+12);
+ }
+ else if (!prefixcmp(arg, "--grep=")) {
+ add_message_grep(revs, arg+7);
+ }
+ else if (!strcmp(arg, "--extended-regexp") ||
+ !strcmp(arg, "-E")) {
+ if (revs->grep_filter)
+ revs->grep_filter->regflags |= REG_EXTENDED;
+ }
+ else if (!strcmp(arg, "--regexp-ignore-case") ||
+ !strcmp(arg, "-i")) {
+ if (revs->grep_filter)
+ revs->grep_filter->regflags |= REG_ICASE;
+ }
+ else if (!strcmp(arg, "--fixed-strings") ||
+ !strcmp(arg, "-F")) {
+ if (revs->grep_filter)
+ revs->grep_filter->fixed = 1;
+ }
+ else if (!strcmp(arg, "--all-match")) {
+ if (revs->grep_filter)
+ revs->grep_filter->all_match = 1;
+ }
+ else if (!prefixcmp(arg, "--encoding=")) {
+ arg += 11;
+ if (strcmp(arg, "none"))
+ git_log_output_encoding = xstrdup(arg);
+ else
+ git_log_output_encoding = "";
+ }
+ else if (!strcmp(arg, "--reverse")) {
+ revs->reverse ^= 1;
+ }
+ else if (!strcmp(arg, "--children")) {
+ revs->children.name = "children";
+ revs->limited = 1;
+ }
+ else
+ return diff_opt_parse(&revs->diffopt, argv, argc);
+
+ return 1;
+}
+
/*
* Parse revision information, filling in the "rev_info" structure,
* and removing the used arguments from the argument list.
@@ -1006,53 +1273,7 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)
const char *arg = argv[i];
if (*arg == '-') {
int opts;
- if (!prefixcmp(arg, "--max-count=")) {
- revs->max_count = atoi(arg + 12);
- continue;
- }
- if (!prefixcmp(arg, "--skip=")) {
- revs->skip_count = atoi(arg + 7);
- continue;
- }
- /* accept -<digit>, like traditional "head" */
- if ((*arg == '-') && isdigit(arg[1])) {
- revs->max_count = atoi(arg + 1);
- continue;
- }
- if (!strcmp(arg, "-n")) {
- if (argc <= i + 1)
- die("-n requires an argument");
- revs->max_count = atoi(argv[++i]);
- continue;
- }
- if (!prefixcmp(arg, "-n")) {
- revs->max_count = atoi(arg + 2);
- continue;
- }
- if (!prefixcmp(arg, "--max-age=")) {
- revs->max_age = atoi(arg + 10);
- continue;
- }
- if (!prefixcmp(arg, "--since=")) {
- revs->max_age = approxidate(arg + 8);
- continue;
- }
- if (!prefixcmp(arg, "--after=")) {
- revs->max_age = approxidate(arg + 8);
- continue;
- }
- if (!prefixcmp(arg, "--min-age=")) {
- revs->min_age = atoi(arg + 10);
- continue;
- }
- if (!prefixcmp(arg, "--before=")) {
- revs->min_age = approxidate(arg + 9);
- continue;
- }
- if (!prefixcmp(arg, "--until=")) {
- revs->min_age = approxidate(arg + 8);
- continue;
- }
+
if (!strcmp(arg, "--all")) {
handle_refs(revs, flags, for_each_ref);
continue;
@@ -1069,269 +1290,14 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)
handle_refs(revs, flags, for_each_remote_ref);
continue;
}
- if (!strcmp(arg, "--first-parent")) {
- revs->first_parent_only = 1;
- continue;
- }
if (!strcmp(arg, "--reflog")) {
handle_reflog(revs, flags);
continue;
}
- if (!strcmp(arg, "-g") ||
- !strcmp(arg, "--walk-reflogs")) {
- init_reflog_walk(&revs->reflog_info);
- continue;
- }
if (!strcmp(arg, "--not")) {
flags ^= UNINTERESTING;
continue;
}
- if (!strcmp(arg, "--default")) {
- if (++i >= argc)
- die("bad --default argument");
- revs->def = argv[i];
- continue;
- }
- if (!strcmp(arg, "--merge")) {
- revs->show_merge = 1;
- continue;
- }
- if (!strcmp(arg, "--topo-order")) {
- revs->lifo = 1;
- revs->topo_order = 1;
- continue;
- }
- if (!strcmp(arg, "--date-order")) {
- revs->lifo = 0;
- revs->topo_order = 1;
- continue;
- }
- if (!prefixcmp(arg, "--early-output")) {
- int count = 100;
- switch (arg[14]) {
- case '=':
- count = atoi(arg+15);
- /* Fallthrough */
- case 0:
- revs->topo_order = 1;
- revs->early_output = count;
- continue;
- }
- }
- if (!strcmp(arg, "--parents")) {
- revs->rewrite_parents = 1;
- revs->print_parents = 1;
- continue;
- }
- if (!strcmp(arg, "--dense")) {
- revs->dense = 1;
- continue;
- }
- if (!strcmp(arg, "--sparse")) {
- revs->dense = 0;
- continue;
- }
- if (!strcmp(arg, "--show-all")) {
- revs->show_all = 1;
- continue;
- }
- if (!strcmp(arg, "--remove-empty")) {
- revs->remove_empty_trees = 1;
- continue;
- }
- if (!strcmp(arg, "--no-merges")) {
- revs->no_merges = 1;
- continue;
- }
- if (!strcmp(arg, "--boundary")) {
- revs->boundary = 1;
- continue;
- }
- if (!strcmp(arg, "--left-right")) {
- revs->left_right = 1;
- continue;
- }
- if (!strcmp(arg, "--cherry-pick")) {
- revs->cherry_pick = 1;
- revs->limited = 1;
- continue;
- }
- if (!strcmp(arg, "--objects")) {
- revs->tag_objects = 1;
- revs->tree_objects = 1;
- revs->blob_objects = 1;
- continue;
- }
- if (!strcmp(arg, "--objects-edge")) {
- revs->tag_objects = 1;
- revs->tree_objects = 1;
- revs->blob_objects = 1;
- revs->edge_hint = 1;
- continue;
- }
- if (!strcmp(arg, "--unpacked")) {
- revs->unpacked = 1;
- free(revs->ignore_packed);
- revs->ignore_packed = NULL;
- revs->num_ignore_packed = 0;
- continue;
- }
- if (!prefixcmp(arg, "--unpacked=")) {
- revs->unpacked = 1;
- add_ignore_packed(revs, arg+11);
- continue;
- }
- if (!strcmp(arg, "-r")) {
- revs->diff = 1;
- DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
- continue;
- }
- if (!strcmp(arg, "-t")) {
- revs->diff = 1;
- DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
- DIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE);
- continue;
- }
- if (!strcmp(arg, "-m")) {
- revs->ignore_merges = 0;
- continue;
- }
- if (!strcmp(arg, "-c")) {
- revs->diff = 1;
- revs->dense_combined_merges = 0;
- revs->combine_merges = 1;
- continue;
- }
- if (!strcmp(arg, "--cc")) {
- revs->diff = 1;
- revs->dense_combined_merges = 1;
- revs->combine_merges = 1;
- continue;
- }
- if (!strcmp(arg, "-v")) {
- revs->verbose_header = 1;
- continue;
- }
- if (!strcmp(arg, "--pretty")) {
- revs->verbose_header = 1;
- get_commit_format(arg+8, revs);
- continue;
- }
- if (!prefixcmp(arg, "--pretty=")) {
- revs->verbose_header = 1;
- get_commit_format(arg+9, revs);
- continue;
- }
- if (!strcmp(arg, "--graph")) {
- revs->topo_order = 1;
- revs->rewrite_parents = 1;
- revs->graph = graph_init(revs);
- continue;
- }
- if (!strcmp(arg, "--root")) {
- revs->show_root_diff = 1;
- continue;
- }
- if (!strcmp(arg, "--no-commit-id")) {
- revs->no_commit_id = 1;
- continue;
- }
- if (!strcmp(arg, "--always")) {
- revs->always_show_header = 1;
- continue;
- }
- if (!strcmp(arg, "--no-abbrev")) {
- revs->abbrev = 0;
- continue;
- }
- if (!strcmp(arg, "--abbrev")) {
- revs->abbrev = DEFAULT_ABBREV;
- continue;
- }
- if (!prefixcmp(arg, "--abbrev=")) {
- revs->abbrev = strtoul(arg + 9, NULL, 10);
- if (revs->abbrev < MINIMUM_ABBREV)
- revs->abbrev = MINIMUM_ABBREV;
- else if (revs->abbrev > 40)
- revs->abbrev = 40;
- continue;
- }
- if (!strcmp(arg, "--abbrev-commit")) {
- revs->abbrev_commit = 1;
- continue;
- }
- if (!strcmp(arg, "--full-diff")) {
- revs->diff = 1;
- revs->full_diff = 1;
- continue;
- }
- if (!strcmp(arg, "--full-history")) {
- revs->simplify_history = 0;
- continue;
- }
- if (!strcmp(arg, "--relative-date")) {
- revs->date_mode = DATE_RELATIVE;
- continue;
- }
- if (!strncmp(arg, "--date=", 7)) {
- revs->date_mode = parse_date_format(arg + 7);
- continue;
- }
- if (!strcmp(arg, "--log-size")) {
- revs->show_log_size = 1;
- continue;
- }
-
- /*
- * Grepping the commit log
- */
- if (!prefixcmp(arg, "--author=")) {
- add_header_grep(revs, "author", arg+9);
- continue;
- }
- if (!prefixcmp(arg, "--committer=")) {
- add_header_grep(revs, "committer", arg+12);
- continue;
- }
- if (!prefixcmp(arg, "--grep=")) {
- add_message_grep(revs, arg+7);
- continue;
- }
- if (!strcmp(arg, "--extended-regexp") ||
- !strcmp(arg, "-E")) {
- if (revs->grep_filter)
- revs->grep_filter->regflags |= REG_EXTENDED;
- continue;
- }
- if (!strcmp(arg, "--regexp-ignore-case") ||
- !strcmp(arg, "-i")) {
- if (revs->grep_filter)
- revs->grep_filter->regflags |= REG_ICASE;
- continue;
- }
- if (!strcmp(arg, "--fixed-strings") ||
- !strcmp(arg, "-F")) {
- if (revs->grep_filter)
- revs->grep_filter->fixed = 1;
- continue;
- }
- if (!strcmp(arg, "--all-match")) {
- if (revs->grep_filter)
- revs->grep_filter->all_match = 1;
- continue;
- }
- if (!prefixcmp(arg, "--encoding=")) {
- arg += 11;
- if (strcmp(arg, "none"))
- git_log_output_encoding = xstrdup(arg);
- else
- git_log_output_encoding = "";
- continue;
- }
- if (!strcmp(arg, "--reverse")) {
- revs->reverse ^= 1;
- continue;
- }
if (!strcmp(arg, "--no-walk")) {
revs->no_walk = 1;
continue;
@@ -1340,17 +1306,14 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)
revs->no_walk = 0;
continue;
}
- if (!strcmp(arg, "--children")) {
- revs->children.name = "children";
- revs->limited = 1;
- continue;
- }
- opts = diff_opt_parse(&revs->diffopt, argv+i, argc-i);
+ opts = handle_revision_opt(revs, argc - i, argv + i, NULL, NULL);
if (opts > 0) {
i += opts - 1;
continue;
}
+ if (opts < 0)
+ exit(128);
*unrecognized++ = arg;
left++;
continue;
diff --git a/revision.h b/revision.h
index e45b86e..46ab713 100644
--- a/revision.h
+++ b/revision.h
@@ -123,6 +123,8 @@ volatile show_early_output_fn_t show_early_output;
extern void init_revisions(struct rev_info *revs, const char *prefix);
extern int parse_revisions(int argc, const char **argv, struct rev_info *revs);
extern void setup_revisions(struct rev_info *revs, const char *def);
+extern int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
+ int *unkc, const char **unkv);
extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);
extern int prepare_revision_walk(struct rev_info *revs);
--
1.5.6.2.352.g416a6
^ permalink raw reply related
* [PATCH 6/6] git-blame: migrate to incremental parse-option [2/2]
From: Pierre Habouzit @ 2008-07-08 9:56 UTC (permalink / raw)
To: git; +Cc: torvalds, gitster, peff, Pierre Habouzit
In-Reply-To: <1215510964-16664-6-git-send-email-madcoder@debian.org>
Now use handle_revision_args instead of parse_revisions, and simplify the
handling of old-style arguments a lot thanks to the filtering.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
builtin-blame.c | 132 +++++++++++++++++--------------------------------------
1 files changed, 40 insertions(+), 92 deletions(-)
diff --git a/builtin-blame.c b/builtin-blame.c
index 6256341..bbbaea5 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -2262,8 +2262,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
struct scoreboard sb;
struct origin *o;
struct blame_entry *ent;
- int i, seen_dashdash, unk;
- long bottom, top, lno;
+ long dashdash_pos, bottom, top, lno;
const char *final_commit_name = NULL;
enum object_type type;
@@ -2301,6 +2300,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
git_config(git_blame_config, NULL);
init_revisions(&revs, NULL);
save_commit_buffer = 0;
+ dashdash_pos = 0;
parse_options_start(&ctx, argc, argv, PARSE_OPT_KEEP_DASHDASH |
PARSE_OPT_KEEP_ARGV0);
@@ -2311,6 +2311,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
case PARSE_OPT_HELP:
exit(129);
case PARSE_OPT_DONE:
+ if (ctx.argv[0])
+ dashdash_pos = ctx.cpidx;
goto parse_done;
}
@@ -2330,20 +2332,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
parse_done:
argc = parse_options_end(&ctx);
- seen_dashdash = 0;
- for (unk = i = 1; i < argc; i++) {
- const char *arg = argv[i];
- if (*arg != '-')
- break;
- else if (!strcmp("--", arg)) {
- seen_dashdash = 1;
- i++;
- break;
- }
- else
- argv[unk++] = arg;
- }
-
if (!blame_move_score)
blame_move_score = BLAME_DEFAULT_MOVE_SCORE;
if (!blame_copy_score)
@@ -2356,92 +2344,52 @@ parse_done:
*
* The remaining are:
*
- * (1) if seen_dashdash, its either
- * "-options -- <path>" or
- * "-options -- <path> <rev>".
- * but the latter is allowed only if there is no
- * options that we passed to revision machinery.
+ * (1) if dashdash_pos != 0, its either
+ * "blame [revisions] -- <path>" or
+ * "blame -- <path> <rev>"
*
- * (2) otherwise, we may have "--" somewhere later and
- * might be looking at the first one of multiple 'rev'
- * parameters (e.g. " master ^next ^maint -- path").
- * See if there is a dashdash first, and give the
- * arguments before that to revision machinery.
- * After that there must be one 'path'.
+ * (2) otherwise, its one of the two:
+ * "blame [revisions] <path>"
+ * "blame <path> <rev>"
*
- * (3) otherwise, its one of the three:
- * "-options <path> <rev>"
- * "-options <rev> <path>"
- * "-options <path>"
- * but again the first one is allowed only if
- * there is no options that we passed to revision
- * machinery.
+ * Note that we must strip out <path> from the arguments: we do not
+ * want the path pruning but we may want "bottom" processing.
*/
-
- if (seen_dashdash) {
- /* (1) */
- if (argc <= i)
- usage_with_options(blame_opt_usage, options);
- path = add_prefix(prefix, argv[i]);
- if (i + 1 == argc - 1) {
- if (unk != 1)
+ if (dashdash_pos) {
+ switch (argc - dashdash_pos - 1) {
+ case 2: /* (1b) */
+ if (argc != 4)
usage_with_options(blame_opt_usage, options);
- argv[unk++] = argv[i + 1];
+ /* reorder for the new way: <rev> -- <path> */
+ argv[1] = argv[3];
+ argv[3] = argv[2];
+ argv[2] = "--";
+ dashdash_pos = 2;
+ /* FALLTHROUGH */
+ case 1: /* (1a) */
+ path = add_prefix(prefix, argv[--argc]);
+ argv[argc] = NULL;
+ break;
+ default:
+ usage_with_options(blame_opt_usage, options);
}
- else if (i + 1 != argc)
- /* garbage at end */
+ } else {
+ if (argc < 2)
usage_with_options(blame_opt_usage, options);
- }
- else {
- int j;
- for (j = i; !seen_dashdash && j < argc; j++)
- if (!strcmp(argv[j], "--"))
- seen_dashdash = j;
- if (seen_dashdash) {
- /* (2) */
- if (seen_dashdash + 1 != argc - 1)
- usage_with_options(blame_opt_usage, options);
- path = add_prefix(prefix, argv[seen_dashdash + 1]);
- for (j = i; j < seen_dashdash; j++)
- argv[unk++] = argv[j];
+ path = add_prefix(prefix, argv[argc - 1]);
+ if (argc == 3 && !has_path_in_work_tree(path)) { /* (2b) */
+ path = add_prefix(prefix, argv[1]);
+ argv[1] = argv[2];
}
- else {
- /* (3) */
- if (argc <= i)
- usage_with_options(blame_opt_usage, options);
- path = add_prefix(prefix, argv[i]);
- if (i + 1 == argc - 1) {
- final_commit_name = argv[i + 1];
-
- /* if (unk == 1) we could be getting
- * old-style
- */
- if (unk == 1 && !has_path_in_work_tree(path)) {
- path = add_prefix(prefix, argv[i + 1]);
- final_commit_name = argv[i];
- }
- }
- else if (i != argc - 1)
- usage_with_options(blame_opt_usage, options);
+ argv[argc - 1] = "--";
+ dashdash_pos = argc - 1;
- setup_work_tree();
- if (!has_path_in_work_tree(path))
- die("cannot stat path %s: %s",
- path, strerror(errno));
- }
+ setup_work_tree();
+ if (!has_path_in_work_tree(path))
+ die("cannot stat path %s: %s", path, strerror(errno));
}
- if (final_commit_name)
- argv[unk++] = final_commit_name;
-
- /*
- * Now we got rev and path. We do not want the path pruning
- * but we may want "bottom" processing.
- */
- argv[unk++] = "--"; /* terminate the rev name */
- argv[unk] = NULL;
-
- parse_revisions(unk, argv, &revs);
+ handle_revision_args(&revs, argc, argv, dashdash_pos);
setup_revisions(&revs, NULL);
memset(&sb, 0, sizeof(sb));
--
1.5.6.2.352.g416a6
^ permalink raw reply related
* [PATCH 5/6] git-blame: migrate to incremental parse-option [1/2]
From: Pierre Habouzit @ 2008-07-08 9:56 UTC (permalink / raw)
To: git; +Cc: torvalds, gitster, peff, Pierre Habouzit
In-Reply-To: <1215510964-16664-5-git-send-email-madcoder@debian.org>
This step merely moves the parser to an incremental version, still using
parse_revisions.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
builtin-blame.c | 220 +++++++++++++++++++++++++++++-------------------------
1 files changed, 118 insertions(+), 102 deletions(-)
diff --git a/builtin-blame.c b/builtin-blame.c
index 1e26b88..6256341 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -18,24 +18,16 @@
#include "cache-tree.h"
#include "path-list.h"
#include "mailmap.h"
+#include "parse-options.h"
-static char blame_usage[] =
-"git-blame [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-p] [-w] [-L n,m] [-S <revs-file>] [-M] [-C] [-C] [--contents <filename>] [--incremental] [commit] [--] file\n"
-" -c Use the same output mode as git-annotate (Default: off)\n"
-" -b Show blank SHA-1 for boundary commits (Default: off)\n"
-" -l Show long commit SHA1 (Default: off)\n"
-" --root Do not treat root commits as boundaries (Default: off)\n"
-" -t Show raw timestamp (Default: off)\n"
-" -f, --show-name Show original filename (Default: auto)\n"
-" -n, --show-number Show original linenumber (Default: off)\n"
-" -s Suppress author name and timestamp (Default: off)\n"
-" -p, --porcelain Show in a format designed for machine consumption\n"
-" -w Ignore whitespace differences\n"
-" -L n,m Process only line range n,m, counting from 1\n"
-" -M, -C Find line movements within and across files\n"
-" --incremental Show blame entries as we find them, incrementally\n"
-" --contents file Use <file>'s contents as the final image\n"
-" -S revs-file Use revisions from revs-file instead of calling git-rev-list\n";
+static char blame_usage[] = "git-blame [options] [rev-opts] [rev] [--] file";
+
+static const char *blame_opt_usage[] = {
+ blame_usage,
+ "",
+ "[rev-opts] are documented in git-rev-parse(1)",
+ NULL
+};
static int longest_file;
static int longest_author;
@@ -2219,6 +2211,50 @@ static const char *prepare_initial(struct scoreboard *sb)
return final_commit_name;
}
+static int blame_copy_callback(const struct option *option, const char *arg, int unset)
+{
+ int *opt = option->value;
+
+ /*
+ * -C enables copy from removed files;
+ * -C -C enables copy from existing files, but only
+ * when blaming a new file;
+ * -C -C -C enables copy from existing files for
+ * everybody
+ */
+ if (*opt & PICKAXE_BLAME_COPY_HARDER)
+ *opt |= PICKAXE_BLAME_COPY_HARDEST;
+ if (*opt & PICKAXE_BLAME_COPY)
+ *opt |= PICKAXE_BLAME_COPY_HARDER;
+ *opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;
+
+ if (arg)
+ blame_copy_score = parse_score(arg);
+ return 0;
+}
+
+static int blame_move_callback(const struct option *option, const char *arg, int unset)
+{
+ int *opt = option->value;
+
+ *opt |= PICKAXE_BLAME_MOVE;
+
+ if (arg)
+ blame_move_score = parse_score(arg);
+ return 0;
+}
+
+static int blame_bottomtop_callback(const struct option *option, const char *arg, int unset)
+{
+ const char **bottomtop = option->value;
+ if (!arg)
+ return -1;
+ if (*bottomtop)
+ die("More than one '-L n,m' option given");
+ *bottomtop = arg;
+ return 0;
+}
+
int cmd_blame(int argc, const char **argv, const char *prefix)
{
struct rev_info revs;
@@ -2226,98 +2262,79 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
struct scoreboard sb;
struct origin *o;
struct blame_entry *ent;
- int i, seen_dashdash, unk, opt;
+ int i, seen_dashdash, unk;
long bottom, top, lno;
- int output_option = 0;
- int show_stats = 0;
- const char *revs_file = NULL;
const char *final_commit_name = NULL;
enum object_type type;
- const char *bottomtop = NULL;
- const char *contents_from = NULL;
+
+ static const char *bottomtop = NULL;
+ static int output_option = 0, opt = 0;
+ static int show_stats = 0;
+ static const char *revs_file = NULL;
+ static const char *contents_from = NULL;
+ static const struct option options[] = {
+ OPT_BOOLEAN(0, "incremental", &incremental, "Show blame entries as we find them, incrementally"),
+ OPT_BOOLEAN('b', NULL, &blank_boundary, "Show blank SHA-1 for boundary commits (Default: off)"),
+ OPT_BOOLEAN(0, "root", &show_root, "Do not treat root commits as boundaries (Default: off)"),
+ OPT_BOOLEAN(0, "show-stats", &show_stats, "Show work cost statistics"),
+ OPT_BIT(0, "score-debug", &output_option, "Show output score for blame entries", OUTPUT_SHOW_SCORE),
+ OPT_BIT('f', "show-name", &output_option, "Show original filename (Default: auto)", OUTPUT_SHOW_NAME),
+ OPT_BIT('n', "show-number", &output_option, "Show original linenumber (Default: off)", OUTPUT_SHOW_NUMBER),
+ OPT_BIT('p', "porcelain", &output_option, "Show in a format designed for machine consumption", OUTPUT_PORCELAIN),
+ OPT_BIT('c', NULL, &output_option, "Use the same output mode as git-annotate (Default: off)", OUTPUT_ANNOTATE_COMPAT),
+ OPT_BIT('t', NULL, &output_option, "Show raw timestamp (Default: off)", OUTPUT_RAW_TIMESTAMP),
+ OPT_BIT('l', NULL, &output_option, "Show long commit SHA1 (Default: off)", OUTPUT_LONG_OBJECT_NAME),
+ OPT_BIT('s', NULL, &output_option, "Suppress author name and timestamp (Default: off)", OUTPUT_NO_AUTHOR),
+ OPT_BIT('w', NULL, &xdl_opts, "Ignore whitespace differences", XDF_IGNORE_WHITESPACE),
+ OPT_STRING('S', NULL, &revs_file, "file", "Use revisions from <file> instead of calling git-rev-list"),
+ OPT_STRING(0, "contents", &contents_from, "file", "Use <file>'s contents as the final image"),
+ { OPTION_CALLBACK, 'C', NULL, &opt, "score", "Find line copies within and across files", PARSE_OPT_OPTARG, blame_copy_callback },
+ { OPTION_CALLBACK, 'M', NULL, &opt, "score", "Find line movements within and across files", PARSE_OPT_OPTARG, blame_move_callback },
+ OPT_CALLBACK('L', NULL, &bottomtop, "n,m", "Process only line range n,m, counting from 1", blame_bottomtop_callback),
+ OPT_END()
+ };
+
+ struct parse_opt_ctx_t ctx;
cmd_is_annotate = !strcmp(argv[0], "annotate");
git_config(git_blame_config, NULL);
+ init_revisions(&revs, NULL);
save_commit_buffer = 0;
- opt = 0;
+ parse_options_start(&ctx, argc, argv, PARSE_OPT_KEEP_DASHDASH |
+ PARSE_OPT_KEEP_ARGV0);
+ for (;;) {
+ int n;
+
+ switch (parse_options_step(&ctx, options, blame_opt_usage)) {
+ case PARSE_OPT_HELP:
+ exit(129);
+ case PARSE_OPT_DONE:
+ goto parse_done;
+ }
+
+ if (!strcmp(ctx.argv[0], "--reverse")) {
+ ctx.argv[0] = "--children";
+ reverse = 1;
+ }
+ n = handle_revision_opt(&revs, ctx.argc, ctx.argv,
+ &ctx.cpidx, ctx.out);
+ if (n <= 0) {
+ error("unknown option `%s'", ctx.argv[0]);
+ usage_with_options(blame_opt_usage, options);
+ }
+ ctx.argv += n;
+ ctx.argc -= n;
+ }
+parse_done:
+ argc = parse_options_end(&ctx);
+
seen_dashdash = 0;
for (unk = i = 1; i < argc; i++) {
const char *arg = argv[i];
if (*arg != '-')
break;
- else if (!strcmp("-b", arg))
- blank_boundary = 1;
- else if (!strcmp("--root", arg))
- show_root = 1;
- else if (!strcmp("--reverse", arg)) {
- argv[unk++] = "--children";
- reverse = 1;
- }
- else if (!strcmp(arg, "--show-stats"))
- show_stats = 1;
- else if (!strcmp("-c", arg))
- output_option |= OUTPUT_ANNOTATE_COMPAT;
- else if (!strcmp("-t", arg))
- output_option |= OUTPUT_RAW_TIMESTAMP;
- else if (!strcmp("-l", arg))
- output_option |= OUTPUT_LONG_OBJECT_NAME;
- else if (!strcmp("-s", arg))
- output_option |= OUTPUT_NO_AUTHOR;
- else if (!strcmp("-w", arg))
- xdl_opts |= XDF_IGNORE_WHITESPACE;
- else if (!strcmp("-S", arg) && ++i < argc)
- revs_file = argv[i];
- else if (!prefixcmp(arg, "-M")) {
- opt |= PICKAXE_BLAME_MOVE;
- blame_move_score = parse_score(arg+2);
- }
- else if (!prefixcmp(arg, "-C")) {
- /*
- * -C enables copy from removed files;
- * -C -C enables copy from existing files, but only
- * when blaming a new file;
- * -C -C -C enables copy from existing files for
- * everybody
- */
- if (opt & PICKAXE_BLAME_COPY_HARDER)
- opt |= PICKAXE_BLAME_COPY_HARDEST;
- if (opt & PICKAXE_BLAME_COPY)
- opt |= PICKAXE_BLAME_COPY_HARDER;
- opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;
- blame_copy_score = parse_score(arg+2);
- }
- else if (!prefixcmp(arg, "-L")) {
- if (!arg[2]) {
- if (++i >= argc)
- usage(blame_usage);
- arg = argv[i];
- }
- else
- arg += 2;
- if (bottomtop)
- die("More than one '-L n,m' option given");
- bottomtop = arg;
- }
- else if (!strcmp("--contents", arg)) {
- if (++i >= argc)
- usage(blame_usage);
- contents_from = argv[i];
- }
- else if (!strcmp("--incremental", arg))
- incremental = 1;
- else if (!strcmp("--score-debug", arg))
- output_option |= OUTPUT_SHOW_SCORE;
- else if (!strcmp("-f", arg) ||
- !strcmp("--show-name", arg))
- output_option |= OUTPUT_SHOW_NAME;
- else if (!strcmp("-n", arg) ||
- !strcmp("--show-number", arg))
- output_option |= OUTPUT_SHOW_NUMBER;
- else if (!strcmp("-p", arg) ||
- !strcmp("--porcelain", arg))
- output_option |= OUTPUT_PORCELAIN;
else if (!strcmp("--", arg)) {
seen_dashdash = 1;
i++;
@@ -2364,16 +2381,16 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
if (seen_dashdash) {
/* (1) */
if (argc <= i)
- usage(blame_usage);
+ usage_with_options(blame_opt_usage, options);
path = add_prefix(prefix, argv[i]);
if (i + 1 == argc - 1) {
if (unk != 1)
- usage(blame_usage);
+ usage_with_options(blame_opt_usage, options);
argv[unk++] = argv[i + 1];
}
else if (i + 1 != argc)
/* garbage at end */
- usage(blame_usage);
+ usage_with_options(blame_opt_usage, options);
}
else {
int j;
@@ -2383,7 +2400,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
if (seen_dashdash) {
/* (2) */
if (seen_dashdash + 1 != argc - 1)
- usage(blame_usage);
+ usage_with_options(blame_opt_usage, options);
path = add_prefix(prefix, argv[seen_dashdash + 1]);
for (j = i; j < seen_dashdash; j++)
argv[unk++] = argv[j];
@@ -2391,7 +2408,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
else {
/* (3) */
if (argc <= i)
- usage(blame_usage);
+ usage_with_options(blame_opt_usage, options);
path = add_prefix(prefix, argv[i]);
if (i + 1 == argc - 1) {
final_commit_name = argv[i + 1];
@@ -2405,7 +2422,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
}
}
else if (i != argc - 1)
- usage(blame_usage); /* garbage at end */
+ usage_with_options(blame_opt_usage, options);
setup_work_tree();
if (!has_path_in_work_tree(path))
@@ -2424,7 +2441,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
argv[unk++] = "--"; /* terminate the rev name */
argv[unk] = NULL;
- init_revisions(&revs, NULL);
parse_revisions(unk, argv, &revs);
setup_revisions(&revs, NULL);
memset(&sb, 0, sizeof(sb));
--
1.5.6.2.352.g416a6
^ permalink raw reply related
* [PATCH 1/6] revisions: refactor init_revisions and setup_revisions.
From: Pierre Habouzit @ 2008-07-08 9:55 UTC (permalink / raw)
To: git; +Cc: torvalds, gitster, peff, Pierre Habouzit
In-Reply-To: <1215510964-16664-1-git-send-email-madcoder@debian.org>
struct rev_info gains two new field:
* .def to store --default argument;
* .show_merge 1-bit field.
setup_revisions has been split in two: parse_revisions that does (almost)
only argument parsing, to be more like what parse-options can do, and
setup_revisions that does the rest.
Many places had no arguments to pass to setup_revisions, and those don't use
parse_revisions at all.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
builtin-add.c | 2 +-
builtin-blame.c | 3 +-
builtin-commit.c | 4 +-
builtin-diff-files.c | 3 +-
builtin-diff-index.c | 3 +-
builtin-diff-tree.c | 3 +-
builtin-diff.c | 3 +-
builtin-fast-export.c | 3 +-
builtin-fmt-merge-msg.c | 2 +-
builtin-log.c | 7 +++--
builtin-pack-objects.c | 3 +-
builtin-rev-list.c | 3 +-
builtin-revert.c | 2 +-
builtin-shortlog.c | 3 +-
bundle.c | 6 +++-
http-push.c | 3 +-
remote.c | 3 +-
revision.c | 51 +++++++++++++++++++++++------------------------
revision.h | 7 ++++-
upload-pack.c | 4 +-
wt-status.c | 6 ++--
21 files changed, 70 insertions(+), 54 deletions(-)
diff --git a/builtin-add.c b/builtin-add.c
index 9930cf5..8ba9604 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -121,7 +121,7 @@ int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
struct update_callback_data data;
struct rev_info rev;
init_revisions(&rev, prefix);
- setup_revisions(0, NULL, &rev, NULL);
+ setup_revisions(&rev, NULL);
rev.prune_data = pathspec;
rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
rev.diffopt.format_callback = update_callback;
diff --git a/builtin-blame.c b/builtin-blame.c
index cf41511..1e26b88 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -2425,7 +2425,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
argv[unk] = NULL;
init_revisions(&revs, NULL);
- setup_revisions(unk, argv, &revs, NULL);
+ parse_revisions(unk, argv, &revs);
+ setup_revisions(&revs, NULL);
memset(&sb, 0, sizeof(sb));
sb.revs = &revs;
diff --git a/builtin-commit.c b/builtin-commit.c
index 745c11e..37b4e30 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -605,7 +605,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
else {
init_revisions(&rev, "");
rev.abbrev = 0;
- setup_revisions(0, NULL, &rev, parent);
+ setup_revisions(&rev, parent);
DIFF_OPT_SET(&rev.diffopt, QUIET);
DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
run_diff_index(&rev, 1 /* cached */);
@@ -850,7 +850,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
die("could not parse newly created commit");
init_revisions(&rev, prefix);
- setup_revisions(0, NULL, &rev, NULL);
+ setup_revisions(&rev, NULL);
rev.abbrev = 0;
rev.diff = 1;
diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 384d871..e7e202a 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -23,7 +23,8 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
rev.abbrev = 0;
- argc = setup_revisions(argc, argv, &rev, NULL);
+ argc = parse_revisions(argc, argv, &rev);
+ setup_revisions(&rev, NULL);
while (1 < argc && argv[1][0] == '-') {
if (!strcmp(argv[1], "--base"))
rev.max_count = 1;
diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 2f44ebf..286a9de 100644
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -20,7 +20,8 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
rev.abbrev = 0;
- argc = setup_revisions(argc, argv, &rev, NULL);
+ argc = parse_revisions(argc, argv, &rev);
+ setup_revisions(&rev, NULL);
for (i = 1; i < argc; i++) {
const char *arg = argv[i];
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 9d2a48f..0900175 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -72,7 +72,8 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
nr_sha1 = 0;
opt->abbrev = 0;
opt->diff = 1;
- argc = setup_revisions(argc, argv, opt, NULL);
+ argc = parse_revisions(argc, argv, opt);
+ setup_revisions(opt, NULL);
while (--argc > 0) {
const char *arg = *++argv;
diff --git a/builtin-diff.c b/builtin-diff.c
index 4c289e7..a92dbd4 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -283,7 +283,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
if (nongit)
die("Not a git repository");
- argc = setup_revisions(argc, argv, &rev, NULL);
+ argc = parse_revisions(argc, argv, &rev);
+ setup_revisions(&rev, NULL);
if (!rev.diffopt.output_format) {
rev.diffopt.output_format = DIFF_FORMAT_PATCH;
if (diff_setup_done(&rev.diffopt) < 0)
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 75132ba..d6726a8 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -454,7 +454,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
git_config(git_default_config, NULL);
init_revisions(&revs, prefix);
- argc = setup_revisions(argc, argv, &revs, NULL);
+ argc = parse_revisions(argc, argv, &revs);
+ setup_revisions(&revs, NULL);
argc = parse_options(argc, argv, options, fast_export_usage, 0);
if (argc > 1)
usage_with_options (fast_export_usage, options);
diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index 69a8a92..3db10c4 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -183,7 +183,7 @@ static void shortlog(const char *name, unsigned char *sha1,
if (!branch || branch->type != OBJ_COMMIT)
return;
- setup_revisions(0, NULL, rev, NULL);
+ setup_revisions(rev, NULL);
rev->ignore_merges = 1;
add_pending_object(rev, branch, name);
add_pending_object(rev, &head->object, "^HEAD");
diff --git a/builtin-log.c b/builtin-log.c
index 430d876..8be9a28 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -68,8 +68,8 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
if (default_date_mode)
rev->date_mode = parse_date_format(default_date_mode);
- argc = setup_revisions(argc, argv, rev, "HEAD");
-
+ argc = parse_revisions(argc, argv, rev);
+ setup_revisions(rev, "HEAD");
if (rev->diffopt.pickaxe || rev->diffopt.filter)
rev->always_show_header = 0;
if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
@@ -919,7 +919,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
if (numbered_files && use_stdout)
die ("--numbered-files and --stdout are mutually exclusive.");
- argc = setup_revisions(argc, argv, &rev, "HEAD");
+ argc = parse_revisions(argc, argv, &rev);
+ setup_revisions(&rev, "HEAD");
if (argc > 1)
die ("unrecognized argument: %s", argv[1]);
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 2dadec1..3d43050 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1964,7 +1964,8 @@ static void get_object_list(int ac, const char **av)
init_revisions(&revs, NULL);
save_commit_buffer = 0;
- setup_revisions(ac, av, &revs, NULL);
+ parse_revisions(ac, av, &revs);
+ setup_revisions(&revs, NULL);
while (fgets(line, sizeof(line), stdin) != NULL) {
int len = strlen(line);
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index b4a2c44..a9f5e41 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -588,7 +588,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
init_revisions(&revs, prefix);
revs.abbrev = 0;
revs.commit_format = CMIT_FMT_UNSPECIFIED;
- argc = setup_revisions(argc, argv, &revs, NULL);
+ argc = parse_revisions(argc, argv, &revs);
+ setup_revisions(&revs, NULL);
for (i = 1 ; i < argc; i++) {
const char *arg = argv[i];
diff --git a/builtin-revert.c b/builtin-revert.c
index 0270f9b..0adff14 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -253,7 +253,7 @@ static int index_is_dirty(void)
{
struct rev_info rev;
init_revisions(&rev, NULL);
- setup_revisions(0, NULL, &rev, "HEAD");
+ setup_revisions(&rev, "HEAD");
DIFF_OPT_SET(&rev.diffopt, QUIET);
DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
run_diff_index(&rev, 1);
diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index e6a2865..3428bf6 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -256,7 +256,8 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
argc--;
}
init_revisions(&rev, prefix);
- argc = setup_revisions(argc, argv, &rev, NULL);
+ argc = parse_revisions(argc, argv, &rev);
+ setup_revisions(&rev, NULL);
if (argc > 1)
die ("unrecognized argument: %s", argv[1]);
diff --git a/bundle.c b/bundle.c
index 0ba5df1..c4848d7 100644
--- a/bundle.c
+++ b/bundle.c
@@ -120,7 +120,8 @@ int verify_bundle(struct bundle_header *header, int verbose)
if (revs.pending.nr != p->nr)
return ret;
req_nr = revs.pending.nr;
- setup_revisions(2, argv, &revs, NULL);
+ parse_revisions(2, argv, &revs);
+ setup_revisions(&revs, NULL);
memset(&refs, 0, sizeof(struct object_array));
for (i = 0; i < revs.pending.nr; i++) {
@@ -226,7 +227,8 @@ int create_bundle(struct bundle_header *header, const char *path,
return error("rev-list died");
/* write references */
- argc = setup_revisions(argc, argv, &revs, NULL);
+ argc = parse_revisions(argc, argv, &revs);
+ setup_revisions(&revs, NULL);
if (argc > 1)
return error("unrecognized argument: %s'", argv[1]);
diff --git a/http-push.c b/http-push.c
index 2cd068a..aa348aa 100644
--- a/http-push.c
+++ b/http-push.c
@@ -2383,7 +2383,8 @@ int main(int argc, char **argv)
commit_argc++;
}
init_revisions(&revs, setup_git_directory());
- setup_revisions(commit_argc, commit_argv, &revs, NULL);
+ parse_revisions(commit_argc, commit_argv, &revs);
+ setup_revisions(&revs, NULL);
revs.edge_hint = 0; /* just in case */
free(new_sha1_hex);
if (old_sha1_hex) {
diff --git a/remote.c b/remote.c
index df8bd72..c95a510 100644
--- a/remote.c
+++ b/remote.c
@@ -1280,7 +1280,8 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs)
strcpy(symmetric + 43, sha1_to_hex(theirs->object.sha1));
init_revisions(&revs, NULL);
- setup_revisions(rev_argc, rev_argv, &revs, NULL);
+ parse_revisions(rev_argc, rev_argv, &revs);
+ setup_revisions(&revs, NULL);
prepare_revision_walk(&revs);
/* ... and count the commits on each side. */
diff --git a/revision.c b/revision.c
index 0191160..3a46ed4 100644
--- a/revision.c
+++ b/revision.c
@@ -981,14 +981,11 @@ static void add_ignore_packed(struct rev_info *revs, const char *name)
* Returns the number of arguments left that weren't recognized
* (which are also moved to the head of the argument list)
*/
-int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def)
+int parse_revisions(int argc, const char **argv, struct rev_info *revs)
{
- int i, flags, seen_dashdash, show_merge;
+ int i, flags, seen_dashdash;
const char **unrecognized = argv + 1;
int left = 1;
- int all_match = 0;
- int regflags = 0;
- int fixed = 0;
/* First, search for "--" */
seen_dashdash = 0;
@@ -1004,7 +1001,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
break;
}
- flags = show_merge = 0;
+ flags = 0;
for (i = 1; i < argc; i++) {
const char *arg = argv[i];
if (*arg == '-') {
@@ -1092,11 +1089,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
if (!strcmp(arg, "--default")) {
if (++i >= argc)
die("bad --default argument");
- def = argv[i];
+ revs->def = argv[i];
continue;
}
if (!strcmp(arg, "--merge")) {
- show_merge = 1;
+ revs->show_merge = 1;
continue;
}
if (!strcmp(arg, "--topo-order")) {
@@ -1302,21 +1299,25 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
}
if (!strcmp(arg, "--extended-regexp") ||
!strcmp(arg, "-E")) {
- regflags |= REG_EXTENDED;
+ if (revs->grep_filter)
+ revs->grep_filter->regflags |= REG_EXTENDED;
continue;
}
if (!strcmp(arg, "--regexp-ignore-case") ||
!strcmp(arg, "-i")) {
- regflags |= REG_ICASE;
+ if (revs->grep_filter)
+ revs->grep_filter->regflags |= REG_ICASE;
continue;
}
if (!strcmp(arg, "--fixed-strings") ||
!strcmp(arg, "-F")) {
- fixed = 1;
+ if (revs->grep_filter)
+ revs->grep_filter->fixed = 1;
continue;
}
if (!strcmp(arg, "--all-match")) {
- all_match = 1;
+ if (revs->grep_filter)
+ revs->grep_filter->all_match = 1;
continue;
}
if (!prefixcmp(arg, "--encoding=")) {
@@ -1374,22 +1375,23 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
break;
}
}
+ return left;
+}
- if (revs->grep_filter) {
- revs->grep_filter->regflags |= regflags;
- revs->grep_filter->fixed = fixed;
- }
-
- if (show_merge)
+void setup_revisions(struct rev_info *revs, const char *def)
+{
+ if (revs->def == NULL)
+ revs->def = def;
+ if (revs->show_merge)
prepare_show_merge(revs);
- if (def && !revs->pending.nr) {
+ if (revs->def && !revs->pending.nr) {
unsigned char sha1[20];
struct object *object;
unsigned mode;
- if (get_sha1_with_mode(def, sha1, &mode))
- die("bad default revision '%s'", def);
- object = get_reference(revs, def, sha1, 0);
- add_pending_object_with_mode(revs, object, def, mode);
+ if (get_sha1_with_mode(revs->def, sha1, &mode))
+ die("bad default revision '%s'", revs->def);
+ object = get_reference(revs, revs->def, sha1, 0);
+ add_pending_object_with_mode(revs, object, revs->def, mode);
}
/* Did the user ask for any diff output? Run the diff! */
@@ -1423,7 +1425,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
die("diff_setup_done failed");
if (revs->grep_filter) {
- revs->grep_filter->all_match = all_match;
compile_grep_patterns(revs->grep_filter);
}
@@ -1440,8 +1441,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
if (revs->reflog_info && revs->graph)
die("cannot combine --walk-reflogs with --graph");
-
- return left;
}
static void add_child(struct rev_info *revs, struct commit *parent, struct commit *child)
diff --git a/revision.h b/revision.h
index 5b8c56b..e45b86e 100644
--- a/revision.h
+++ b/revision.h
@@ -26,6 +26,7 @@ struct rev_info {
/* Basic information */
const char *prefix;
+ const char *def;
void *prune_data;
unsigned int early_output;
@@ -66,8 +67,9 @@ struct rev_info {
/* Format info */
unsigned int shown_one:1,
- abbrev_commit:1,
+ show_merge:1,
use_terminator:1,
+ abbrev_commit:1,
missing_newline:1;
enum date_mode date_mode;
@@ -119,7 +121,8 @@ typedef void (*show_early_output_fn_t)(struct rev_info *, struct commit_list *);
volatile show_early_output_fn_t show_early_output;
extern void init_revisions(struct rev_info *revs, const char *prefix);
-extern int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def);
+extern int parse_revisions(int argc, const char **argv, struct rev_info *revs);
+extern void setup_revisions(struct rev_info *revs, const char *def);
extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);
extern int prepare_revision_walk(struct rev_info *revs);
diff --git a/upload-pack.c b/upload-pack.c
index 9f82941..a87a9a3 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -116,7 +116,7 @@ static int do_rev_list(int fd, void *create_full_pack)
if (create_full_pack) {
const char *args[] = {"rev-list", "--all", NULL};
- setup_revisions(2, args, &revs, NULL);
+ parse_revisions(2, args, &revs);
} else {
for (i = 0; i < want_obj.nr; i++) {
struct object *o = want_obj.objects[i].item;
@@ -129,8 +129,8 @@ static int do_rev_list(int fd, void *create_full_pack)
o->flags |= UNINTERESTING;
add_pending_object(&revs, o, NULL);
}
- setup_revisions(0, NULL, &revs, NULL);
}
+ setup_revisions(&revs, NULL);
if (prepare_revision_walk(&revs))
die("revision walk setup failed");
mark_edges_uninteresting(revs.commits, &revs, show_edge);
diff --git a/wt-status.c b/wt-status.c
index 889e50f..df1130c 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -206,7 +206,7 @@ static void wt_status_print_updated(struct wt_status *s)
{
struct rev_info rev;
init_revisions(&rev, NULL);
- setup_revisions(0, NULL, &rev, s->reference);
+ setup_revisions(&rev, s->reference);
rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
rev.diffopt.format_callback = wt_status_print_updated_cb;
rev.diffopt.format_callback_data = s;
@@ -220,7 +220,7 @@ static void wt_status_print_changed(struct wt_status *s)
{
struct rev_info rev;
init_revisions(&rev, "");
- setup_revisions(0, NULL, &rev, NULL);
+ setup_revisions(&rev, NULL);
rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
rev.diffopt.format_callback = wt_status_print_changed_cb;
rev.diffopt.format_callback_data = s;
@@ -308,7 +308,7 @@ static void wt_status_print_verbose(struct wt_status *s)
struct rev_info rev;
init_revisions(&rev, NULL);
- setup_revisions(0, NULL, &rev, s->reference);
+ setup_revisions(&rev, s->reference);
rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
rev.diffopt.detect_rename = 1;
rev.diffopt.file = s->fp;
--
1.5.6.2.352.g416a6
^ permalink raw reply related
* [PATCH 3/6] revisions: parse_revisions refactor.
From: Pierre Habouzit @ 2008-07-08 9:56 UTC (permalink / raw)
To: git; +Cc: torvalds, gitster, peff, Pierre Habouzit
In-Reply-To: <1215510964-16664-3-git-send-email-madcoder@debian.org>
Reorder parse_revisions so that it works more like a parse-opt based
parser would: use two pass, the first filtering out options that the
revision parser understands, and remember if we saw a "--", and then
parse revision arguments.
Once everyone is using a parse-opt parser, it will be possible to simplify
parse_revisions further. The first pass is a NOOP for incremental
parse-opt based parsers.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
revision.c | 63 ++++++++++++++++++++++++++++++++++++-----------------------
1 files changed, 38 insertions(+), 25 deletions(-)
diff --git a/revision.c b/revision.c
index a3792c9..87f89a2 100644
--- a/revision.c
+++ b/revision.c
@@ -1250,30 +1250,44 @@ int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
*/
int parse_revisions(int argc, const char **argv, struct rev_info *revs)
{
- int i, flags, seen_dashdash;
- const char **unrecognized = argv + 1;
- int left = 1;
+ int i, left, flags;
+ int seen_dashdash = 0;
- /* First, search for "--" */
- seen_dashdash = 0;
- for (i = 1; i < argc; i++) {
+ /* First, filter out revision options and look for "--" */
+ for (left = i = 1; i < argc; i++) {
const char *arg = argv[i];
- if (strcmp(arg, "--"))
+ int opts;
+
+ if (arg[0] != '-') {
+ argv[left++] = arg;
continue;
- argv[i] = NULL;
- argc = i;
- if (argv[i + 1])
- revs->prune_data = get_pathspec(revs->prefix, argv + i + 1);
- seen_dashdash = 1;
- break;
+ }
+
+ if (!strcmp(arg, "--")) {
+ seen_dashdash = left;
+ memcpy(argv + left, argv + i, sizeof(*argv) * (argc - i));
+ left += argc - i;
+ break;
+ }
+
+ opts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);
+ if (opts > 0) {
+ i += opts - 1;
+ continue;
+ }
+ if (opts < 0)
+ exit(128);
+ argv[left++] = arg;
}
+ argv[left] = NULL;
+ argc = left;
+ /* Second, parse revisions arguments */
flags = 0;
- for (i = 1; i < argc; i++) {
+ for (left = i = 1; i < argc; i++) {
const char *arg = argv[i];
- if (*arg == '-') {
- int opts;
+ if (*arg == '-') {
if (!strcmp(arg, "--all")) {
handle_refs(revs, flags, for_each_ref);
continue;
@@ -1306,16 +1320,15 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)
revs->no_walk = 0;
continue;
}
-
- opts = handle_revision_opt(revs, argc - i, argv + i, NULL, NULL);
- if (opts > 0) {
- i += opts - 1;
- continue;
+ if (i == seen_dashdash) {
+ argv[i] = NULL;
+ argc = i;
+ if (argv[i + 1])
+ revs->prune_data = get_pathspec(revs->prefix, argv + i + 1);
+ break;
}
- if (opts < 0)
- exit(128);
- *unrecognized++ = arg;
- left++;
+
+ argv[left++] = arg;
continue;
}
--
1.5.6.2.352.g416a6
^ permalink raw reply related
* [PATCH 4/6] revisions: split handle_revision_args from parse_revisions.
From: Pierre Habouzit @ 2008-07-08 9:56 UTC (permalink / raw)
To: git; +Cc: torvalds, gitster, peff, Pierre Habouzit
In-Reply-To: <1215510964-16664-4-git-send-email-madcoder@debian.org>
This new function is meant to only parse non option revision arguments in
a row. IOW it's meant to parse what remains from a parse-opt filtering of
the argument list, with the knowledge of the "--" position (0 means none).
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
revision.c | 97 +++++++++++++++++++++++++++++++----------------------------
revision.h | 1 +
2 files changed, 52 insertions(+), 46 deletions(-)
diff --git a/revision.c b/revision.c
index 87f89a2..c7de30f 100644
--- a/revision.c
+++ b/revision.c
@@ -1241,49 +1241,11 @@ int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
return 1;
}
-/*
- * Parse revision information, filling in the "rev_info" structure,
- * and removing the used arguments from the argument list.
- *
- * Returns the number of arguments left that weren't recognized
- * (which are also moved to the head of the argument list)
- */
-int parse_revisions(int argc, const char **argv, struct rev_info *revs)
+int handle_revision_args(struct rev_info *revs,
+ int argc, const char **argv, int dashdash_pos)
{
- int i, left, flags;
- int seen_dashdash = 0;
-
- /* First, filter out revision options and look for "--" */
- for (left = i = 1; i < argc; i++) {
- const char *arg = argv[i];
- int opts;
-
- if (arg[0] != '-') {
- argv[left++] = arg;
- continue;
- }
-
- if (!strcmp(arg, "--")) {
- seen_dashdash = left;
- memcpy(argv + left, argv + i, sizeof(*argv) * (argc - i));
- left += argc - i;
- break;
- }
+ int i, left, flags = 0;
- opts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);
- if (opts > 0) {
- i += opts - 1;
- continue;
- }
- if (opts < 0)
- exit(128);
- argv[left++] = arg;
- }
- argv[left] = NULL;
- argc = left;
-
- /* Second, parse revisions arguments */
- flags = 0;
for (left = i = 1; i < argc; i++) {
const char *arg = argv[i];
@@ -1320,7 +1282,7 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)
revs->no_walk = 0;
continue;
}
- if (i == seen_dashdash) {
+ if (i == dashdash_pos) {
argv[i] = NULL;
argc = i;
if (argv[i + 1])
@@ -1332,10 +1294,10 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)
continue;
}
- if (handle_revision_arg(arg, revs, flags, seen_dashdash)) {
+ if (handle_revision_arg(arg, revs, flags, dashdash_pos)) {
int j;
- if (seen_dashdash || *arg == '^')
- die("bad revision '%s'", arg);
+ if (dashdash_pos || *arg == '^')
+ return error("bad revision '%s'", arg);
/* If we didn't have a "--":
* (1) all filenames must exist;
@@ -1351,7 +1313,50 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)
break;
}
}
- return left;
+
+ return left;
+}
+
+/*
+ * Parse revision information, filling in the "rev_info" structure,
+ * and removing the used arguments from the argument list.
+ *
+ * Returns the number of arguments left that weren't recognized
+ * (which are also moved to the head of the argument list)
+ */
+int parse_revisions(int argc, const char **argv, struct rev_info *revs)
+{
+ int i, left, dashdash_pos = 0;
+
+ /* First, filter out revision options and look for "--" */
+ for (left = i = 1; i < argc; i++) {
+ const char *arg = argv[i];
+ int opts;
+
+ if (arg[0] != '-') {
+ argv[left++] = arg;
+ continue;
+ }
+
+ if (!strcmp(arg, "--")) {
+ dashdash_pos = left;
+ memcpy(argv + left, argv + i, sizeof(*argv) * (argc - i));
+ left += argc - i;
+ break;
+ }
+
+ opts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);
+ if (opts > 0) {
+ i += opts - 1;
+ continue;
+ }
+ if (opts < 0)
+ exit(128);
+ argv[left++] = arg;
+ }
+ argv[left] = NULL;
+
+ return handle_revision_args(revs, left, argv, dashdash_pos);
}
void setup_revisions(struct rev_info *revs, const char *def)
diff --git a/revision.h b/revision.h
index 46ab713..c0f5df0 100644
--- a/revision.h
+++ b/revision.h
@@ -125,6 +125,7 @@ extern int parse_revisions(int argc, const char **argv, struct rev_info *revs);
extern void setup_revisions(struct rev_info *revs, const char *def);
extern int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
int *unkc, const char **unkv);
+extern int handle_revision_args(struct rev_info *revs, int argc, const char **argv, int dashdash_pos);
extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);
extern int prepare_revision_walk(struct rev_info *revs);
--
1.5.6.2.352.g416a6
^ permalink raw reply related
* Migration of builtin-blame to parse-option
From: Pierre Habouzit @ 2008-07-08 9:55 UTC (permalink / raw)
To: git; +Cc: torvalds, gitster, peff
This series is twofold. The first 4 patches refactor the revision
parsing machine so that it's easier to deal with from a parse-opt based
parser:
[PATCH 1/6] revisions: refactor init_revisions and setup_revisions.
21 files changed, 70 insertions(+), 54 deletions(-)
This is the patch I sent before, revisited.
[PATCH 2/6] revisions: split the pure option parsing out from parse_revisions.
2 files changed, 273 insertions(+), 308 deletions(-)
This patch splits out the revisions _option_ parsing (as opposed to
revisions arguments or pseudo arguments like --all, --not, ...) from
the revision parser. The patch is huge, but quite straightforward.
[PATCH 3/6] revisions: parse_revisions refactor.
1 files changed, 38 insertions(+), 25 deletions(-)
This patch reworks parse_revisions so that it works internally as if
it was a parse-opt parser. It's equivalent to the previous code, but
changes are tricky, and a separate commit is really worth it.
[PATCH 4/6] revisions: split handle_revision_args from parse_revisions.
2 files changed, 52 insertions(+), 46 deletions(-)
This patch splits out the last bit of parse-revisions (from the
previous commit) so that what parses revisions arguments (refs and
pseudo arguments like --all/--not/...) can be called independently.
The commit is straightforward but moves code around, hence the
separate commit from patch 3 so that one isn't lost in the code moves
while reviewing the tricky bits from patch 3.
The second part is the git-blame migration for real:
[PATCH 5/6] git-blame: migrate to incremental parse-option [1/2]
1 files changed, 118 insertions(+), 102 deletions(-)
This bit is really alike the proof of concept I saw. Most of the code
is stolen from Linus initial patch. It merely removes the git-blame
own options and deal with them with a parse-opt parser, in an
incremental way.
All in all, it's a pretty straighforward patch.
[PATCH 6/6] git-blame: migrate to incremental parse-option [2/2]
1 files changed, 40 insertions(+), 92 deletions(-)
This is by far the trickiest patch of the series, though it passes the
testsuite fine. This patch uses the function from patch 4 to process
the revision arguments. We know the dashdash position from the
incremental parse-opt, but we have to deal with the "old way" of
passing arguments to git-blame.
The new code is really less involved than before, because we deal with
a filtered argv array where we only have the revisions arguments (no
options anymore) and the <path>.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
^ permalink raw reply
* Re: Test breakage [Re: [ANNOUNCE] GIT 1.5.6.2]
From: Michael J Gruber @ 2008-07-08 9:47 UTC (permalink / raw)
To: git
In-Reply-To: <20080708051622.GC2037@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 08.07.2008 07:16:
> On Mon, Jul 07, 2008 at 02:44:49PM +0200, Michael J Gruber wrote:
>
>> On Kubuntu 7.10 (not my choice, this is at work...), "make test" gives:
>>
>> *** t9600-cvsimport.sh ***
>> [...]
>> * FAIL 6: update git module
>> cd module-git &&
>> git cvsimport -a -z 0 module &&
>> git merge origin &&
>> cd .. &&
>> test_cmp module-cvs/o_fortuna module-git/o_fortuna
>>
>> On the second (and subsequent) run, "make test" succeeds. I tried again
>> with a fresh copy from the tarball, same effect: failure on 1, success on
>> 2 and following.
>
> I tried reproducing on my Debian box with various weaks of $LANG, but I
> couldn't. Can you please try running "./t9600-cvsimport.sh -v -i" and
> send us the output of a failing instance. It's not clear which step in
> failing test is causing the problem, or if cvsps is giving off any
> useful errors.
>
>> This is with LANG=de_DE.UTF-8, whereas with LANG=C the tests succeed the
>> first time already. Are tests supposed to be done in C locale only? Do
>> they reuse data from a previous test run? I didn't notice this with
>> 1.5.6.1.
>
> They should be perfectly repeatable, but I have run into problems in the
> past with cruft in my ~/.cvsps directory. However, we should be setting
> $HOME properly in t9600 to avoid this, so I'm not sure what the culprit
> is.
Sure enough, I can't reproduce it today. I reproduced that effect
several times yesterday, or else I would not have reported anything.
Trying the very same steps today the tests succeed on the first run.
Only differences between yesterday and today are be:
- I installed git 1.5.6.2 now (had 1.5.6.1 before) under $HOME, with
PATH, PERL5LIB, LD_LIBRARY_PATH set accordingly; but tests should pick
up git stuff from the local build dir, right?
- There may have been an automated update to kubuntu meanwhile, but I
don't think so; cvsps is under $HOME anyways, and what else would
matter? Perl version maybe?
So it's WORKSFORME now. Sorry for the noise. Next time I'll not only try
to reproduce (as I did) but wait a day also!
Best
Michael
^ permalink raw reply
* Re: [PATCH] fix "git-submodule add a/b/c/repository"
From: Sylvain Joyeux @ 2008-07-08 8:08 UTC (permalink / raw)
To: Mark Levedahl; +Cc: Junio C Hamano, Lars Hjemli, Ping Yin, git
In-Reply-To: <4872DE54.5010804@gmail.com>
On Mon, Jul 07, 2008 at 11:26:12PM -0400, Mark Levedahl wrote:
> Junio C Hamano wrote:
>> I'd like to hear clarifications on two counts, please?
>> (1) If Sylvain wanted to have that appear at dir0/dir1/init not init,
>> would it have been sufficient to give that path twice (once for
>> <repository> and another for <path> parameter) to make things work as
>> expected?
>>
> git-submodule really requires two arguments:
Then it should raise an error when the following is given
git submodule add ./relative-path-in-repo
which, for now, is accepted as
git submodule add ./relative-path-in-repo ./relative-path-in-repo
(and confused me into thinking it was a normal behaviour)
Sylvain
^ permalink raw reply
* Re: [PATCH] better git-submodule status output
From: Sylvain Joyeux @ 2008-07-08 8:00 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Avery Pennarun, Lars Hjemli, Ping Yin,
Mark Levedahl, git
In-Reply-To: <7v8wwd4kvb.fsf@gitster.siamese.dyndns.org>
On Mon, Jul 07, 2008 at 11:20:08AM -0700, Junio C Hamano wrote:
> > Is it possible to make 'fetch' only .. well .. fetch objects, without
> > updating any refs?
>
> By honestly saying "The superproject binds a commit you do not have
> locally at these paths", the user *can* choose *when* to go there and
> update, or have "git submodule update" command to that for him.
Agreed. The problem is that git submodule update does not only fetches
the new refs but *also* checks out the commit currently registered in
the superproject. Therefore git-submodule status *and* git-submodule
summary cannot tell you anything after a git-submodule update.
What I was missing is, given the current discussion, a git-submodule
fetch alike to what Avery was proposing, which fetches all submodules in
one command. Having to do the fetch/update manually for each submodule
is really cumbersome -- I have around 30 modules in the superproject.
Sylvain
^ permalink raw reply
* Re* [PATCH] builtin-rerere: fix conflict markers parsing
From: Junio C Hamano @ 2008-07-08 7:52 UTC (permalink / raw)
To: Olivier Marin; +Cc: Johannes Schindelin, git
In-Reply-To: <7vy74d4mr8.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> So what about
>>
>> <<<<<<< This hunk contains =====
>> anythin
>> =======
>>
>> Hello
>> =======
>> somethin else
>> >>>>>>> problem!
>> ...
> I however think detecting that we have this ambiguous hunk is easy, and
> punting gracefully and not re-resolving in such a case is million times
> better than producing random results that the users need to be worried
> about.
I am wondering if a patch like this on top of your patch may make things
even safer. The idea is the same as the earlier a1b32fd (git-rerere:
detect unparsable conflicts, 2008-06-22) to fail rerere unless the markers
are unambiguous.
Thanks to your isspace(buf[7]), it is slightly less likely that this
safety triggers on false positives.
Thoughts?
-- >8 --
rerere: punt and do not resolve if conflict markers are ambiguous
Especially because we are introducing rerere.autoupdate configuration
(which is off by default for safety) that automatically stages the
resolution made by rerere, it is necessary to make sure that we do not
autoresolve when there is any ambiguity.
builtin-rerere.c | 17 +++++++++++++----
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/builtin-rerere.c b/builtin-rerere.c
index e618862..69c3a52 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -112,12 +112,17 @@ static int handle_file(const char *path,
strbuf_init(&one, 0);
strbuf_init(&two, 0);
while (fgets(buf, sizeof(buf), f)) {
- if (hunk == 0 && !prefixcmp(buf, "<<<<<<< "))
+ if (!prefixcmp(buf, "<<<<<<< ")) {
+ if (hunk)
+ goto bad;
hunk = 1;
- else if (hunk == 1 && !prefixcmp(buf, "=======") &&
- isspace(buf[7]))
+ } else if (!prefixcmp(buf, "=======") && isspace(buf[7])) {
+ if (hunk != 1)
+ goto bad;
hunk = 2;
- else if (hunk == 2 && !prefixcmp(buf, ">>>>>>> ")) {
+ } else if (!prefixcmp(buf, ">>>>>>> ")) {
+ if (hunk != 2)
+ goto bad;
if (strbuf_cmp(&one, &two) > 0)
strbuf_swap(&one, &two);
hunk_no++;
@@ -143,6 +148,10 @@ static int handle_file(const char *path,
strbuf_addstr(&two, buf);
else if (out)
fputs(buf, out);
+ continue;
+ bad:
+ hunk = 99; /* force error exit */
+ break;
}
strbuf_release(&one);
strbuf_release(&two);
^ permalink raw reply related
* Re: PATCH] Documentation: Tweak use case in "git stash save --keep-index"
From: Eric Raible @ 2008-07-08 7:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, szeder
In-Reply-To: <7vlk0dvsr5.fsf@gitster.siamese.dyndns.org>
The documentation suggests using "git stash apply" in the
--keep-index workflow even though doing so will lead to clutter
in the stash. And given that the changes are about to be
committed anyway "git stash pop" is more sensible.
Additionally the text preceeding the example claims that it
works for "two or more commits", but the example itself is
really tailored for just two. Expanding it just a little
makes it clear how the procedure generalizes to N commits.
Finally the example is annotated with some commentary to
explain things on a line-by-line basis.
---
[Note: If we can't rely on fixed-width fonts then the resulting
example will look stupid]
On Mon, Jul 7, 2008 at 10:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Also this may be a good way to split an existing commit into five during a
> "rebase -i" session, and the example in the documentation might want to
> talk about it in that larger picture.
If you're talking about 'edit' mode, doing a "reset HEAD^", and then
using the same "add --patch"/"stash save -k-i"/commit/"stash pop"
cycle in question then I understand and agree. But only if the
user wants to test each part separately.
Otherwise why bother with stash-k-i when you can just loop
on add-patch/commit or use git-gui?
--
Documentation/git-stash.txt | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index df26901..7d50d74 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -197,13 +197,14 @@ each change before committing:
+
----------------------------------------------------------------
... hack hack hack ...
-$ git add --patch foo
-$ git stash save --keep-index
-$ build && run tests
-$ git commit -m 'First part'
-$ git stash apply
-$ build && run tests
-$ git commit -a -m 'Second part'
+$ git add --patch foo # add just first part to the index
+$ git stash save --keep-index # save all other changes to the stash
+$ edit/build/test first part
+$ git commit foo -m 'First part' # commit fully tested change
+$ git stash pop # prepare to work on all other changes
+... repeat above five steps until one commit remains ...
+$ edit/build/test remaining parts
+$ git commit foo -m 'Remaining parts'
----------------------------------------------------------------
SEE ALSO
--
1.5.6.1.1073.g489ff.dirty
^ permalink raw reply related
* Re: [GSoC] What is status of Git's Google Summer of Code 2008 projects?
From: Jakub Narebski @ 2008-07-08 7:39 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Junio C Hamano, git, Christian Couder, Daniel Barkalow
In-Reply-To: <20080708014734.GE6726@leksak.fem-net>
On Tue, 8 July 2008, Stephan Beyer wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
>>
>> I _really_ wanted to merge the basic bits and rewrite of "am" at least to
>> pu tonight, but I had to drop them after noticing that it does not seem to
>> handle --rebasing at all (it parses to set $rebasing but after that where
>> does that bit go?
>
> Yes, you are right that am --rebasing is a no-op.
> That option was a little mystery to me, because it seemed to do nothing
> special, but I'll check again (bash-completion etc) and do appropriate
> changes.
Undocumented option '--rebasing' to git-am is internal option changing
git-am behavior to be better used by git-rebase, namely it does not
change commit message even if it doesn't follow git commit message
convention, for example if it begins not with single line summary
of commit, separated by empty line, but by multi-line paragraph.
See also t/t3405-rebase-malformed.sh
Although I am not sure if when rebase is rewritten using git-sequencer
implementing "git am --rebasing" would be truly needed. On the other
hand side it would be nice to have some _documented_ option which would
allow to git-am mail messages with commits not following git commit
messages convention...
--
Jakub Narebski
Poland
^ permalink raw reply
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