* Re: [RFC] Re: Convert 'git blame' to parse_options()
From: Jeff King @ 2008-06-23 19:53 UTC (permalink / raw)
To: Linus Torvalds
Cc: Johannes Schindelin, Pierre Habouzit, Git Mailing List,
Junio C Hamano
In-Reply-To: <alpine.LFD.1.10.0806231137070.2926@woody.linux-foundation.org>
On Mon, Jun 23, 2008 at 11:47:49AM -0700, Linus Torvalds wrote:
> > $ git blame --default HEAD git.c
> > fatal: cannot stat path HEAD: No such file or directory
> >
> > Oops.
>
> Oops. And then, how would you fix this most easily?
>
> Be honest now.
Your statement is obviously loaded. I haven't seen _anything_ that fixes
it except what I have already mentioned. But I'm sure you are going to
complain that it isn't easy enough.
> Example: many arguments cause multiple option variables to change.
> parse_options() simply can't handle that well - you can do it with a
> callback, but then you need to make the option variables global or make
> them a structure or something. All of which just makes it nasty to do
> partial conversions for the simple cases.
I'm not so sure. I assumed that most of the callbacks would simply take
a "struct rev_list". So you would end up in builtin-blame.c with:
...
OPT__REVISION(&my_rev_list),
...
in your options table. And if setup_revisions takes options that affect
things that _aren't_ in that struct, then they probably ought to be.
> And I guarantee that just adding PARSE_OPT_{CONTINUE|STOP}_ON_UNKNOWN is
> going to be the smallest patch, and make for the easiest usage case. It
> may not be "pretty", but I can whip up a patch in five minutes.
I don't have a problem with STOP_ON_UNKNOWN, as I think it is a building
block upon which sane things can be done (like linearly going through
each parser and saying "did you want this?"). I think IGNORE/CONTINUE
has a fundamental flaw.
> Or are we going to sit around discussing this for another five months?
Please! :)
Pierre was working on the approach I mentioned, but I think he is short
on time. I will take a look at the conversion, but I have a few other
fixes on my plate first.
In the meantime, I don't think your patch makes anything _worse_, since
we already have these sorts of bugs in the current parsing code.
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] parse_options: Add flag to prevent errors for further processing
From: Junio C Hamano @ 2008-06-23 19:55 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Shawn Bohrer, Jeff King, git, madcoder
In-Reply-To: <alpine.DEB.1.00.0806221953470.6439@racer>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Thu, 19 Jun 2008, Shawn Bohrer wrote:
>
>> On Wed, Jun 18, 2008 at 11:52:42AM -0700, Junio C Hamano wrote:
> I believe not. I think that Junio prefers some callback that can handle a
> whole bunch of options (as opposed to the callback we can have now, to
> handle arguments for a specific option).
Sorry, no. I do not want callbacks. I've been saying that parser
cascading is easier if you use an incremental interface like diff option
parser does.
^ permalink raw reply
* Re: Git clone behaviour change in 1.5.6 (vs 1.5.5.1)
From: Daniel Barkalow @ 2008-06-23 19:56 UTC (permalink / raw)
To: zuh; +Cc: git
In-Reply-To: <177e83dd0806231251u223717e4s2149f69313787ead@mail.gmail.com>
On Mon, 23 Jun 2008, Kalle Vahlman wrote:
> Hi!
>
> Switching to the 1.5.6 release from 1.5.5.1, I found out that the
> rewritten git-clone command changed its behaviour wrt cloning to a
> non-existing destination directory structure. In the shell version the
> destination (work tree) is created with 'mkdir -p' but in the C
> version with just the mkdir() call which doesn't create the parent
> directories.
>
> I can't find any indication that this would be intended in the repo
> history nor in the mailing list, so I'm left thinking that this is an
> unwanted regression. Could someone confirm this?
It wasn't an intentional change, anyway.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH v4] git-add--interactive: manual hunk editing mode
From: Jeff King @ 2008-06-23 19:57 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Thomas Rast, git
In-Reply-To: <alpine.DEB.1.00.0806231952360.6440@racer>
On Mon, Jun 23, 2008 at 07:54:29PM +0100, Johannes Schindelin wrote:
> I am not sure when I will have time for that (particularly given that I
> got sidetracked with the OPTION_OPTIONS patch, when I should have worked
> on something completely different).
Heh, I know the feeling. ;)
> In the meantime, feel free to submit in my name.
Do you remember off the top of your head whether more work was needed?
They looked good to me, but then I seem to recall you saying in another
thread that Junio pointed out some flaw with your approach (but I never
quite understood what the flaw was).
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] parse_options: Add flag to prevent errors for further processing
From: Jeff King @ 2008-06-23 19:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Shawn Bohrer, git, madcoder
In-Reply-To: <7v4p7khqp7.fsf@gitster.siamese.dyndns.org>
On Mon, Jun 23, 2008 at 12:55:00PM -0700, Junio C Hamano wrote:
> >> On Wed, Jun 18, 2008 at 11:52:42AM -0700, Junio C Hamano wrote:
> > I believe not. I think that Junio prefers some callback that can handle a
> > whole bunch of options (as opposed to the callback we can have now, to
> > handle arguments for a specific option).
>
> Sorry, no. I do not want callbacks. I've been saying that parser
> cascading is easier if you use an incremental interface like diff option
> parser does.
Now I'm confused: my understanding is that the diff option parser just
leaves unrecognized stuff in argv. But isn't that what a
PARSE_OPTIONS_IGNORE_UNKNOWN flag would do, and isn't that wrong?
-Peff
^ permalink raw reply
* Re: [RFC] Re: Convert 'git blame' to parse_options()
From: Pierre Habouzit @ 2008-06-23 20:04 UTC (permalink / raw)
To: Jeff King
Cc: Linus Torvalds, Johannes Schindelin, Git Mailing List,
Junio C Hamano
In-Reply-To: <20080623195314.GA29569@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]
On Mon, Jun 23, 2008 at 07:53:14PM +0000, Jeff King wrote:
> On Mon, Jun 23, 2008 at 11:47:49AM -0700, Linus Torvalds wrote:
> > Or are we going to sit around discussing this for another five months?
>
> Please! :)
>
> Pierre was working on the approach I mentioned, but I think he is short
> on time. I will take a look at the conversion, but I have a few other
> fixes on my plate first.
>
> In the meantime, I don't think your patch makes anything _worse_, since
> we already have these sorts of bugs in the current parsing code.
To be fair, I lack the time to do a complete parse option overhaul
that is like a few days of work doing that only (and my employer will
probably complain if I do so ;P). Though I'm trying to make
parse_options incremental right now, and I believe that it can work
quite well, and allow way more incremental conversions.
One can overcome many limitations by exposing some stuff from the
parse_options logic, and just parse things in one pass. The _really_
nice thing with this approach is that you can trivially merge
parse_options descriptors by chaining parse_option_step (see my other
mail).
I'll probably have a RFC series soon.
--
·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: git svn --add-author-from implies --use-log-author
From: Avery Pennarun @ 2008-06-23 20:12 UTC (permalink / raw)
To: Mircea Bardac; +Cc: git, Fredrik Skolmli
In-Reply-To: <485BB134.9080203@mircea.bardac.net>
On 6/20/08, Mircea Bardac <dev@mircea.bardac.net> wrote:
> > when running git svn dcommit --add-author-from on a git-svn repository,
> > --use-log-author is not implied by itself. This causes the rewritten history
> > to not show the author in the way most users would expect it to be, "Name
> > <email@domain.tld>", but instead "<user@uuid>".
> >
> > Instead of forcing people to write "svn dcommit --add-author-from
> > --use-log-author", is it a bad move to imply the last argument, making the
> > history look more user-friendly to start with?
>
> The documentation mentions that these 2 parameters are for init and, like
> other parameters, I had the feeling they will affect either the "init"
> process or the entire existance of the repository. Now that I look better, I
> see that there is no "config" note next to them.
They are separate options; --add-author-from affects dcommit (but not
fetch or clone), while --use-log-author affects fetch and clone (but
not dcommit). They do different things, and I can imagine wanting one
and not the other, so having one imply the other isn't very safe.
You can set config options for these, however:
git config svn.addAuthorFrom true
git config svn.useLogAuthor true
(I actually use "git config --global" to set these on my system so
they apply to all my git-svn repositories.)
> I would also like some option to stop pushing a commit if it doesn't have
> any From/Signed-off-by line in it.
[...]
> P.S. I am very curious why using From and not Signed-off-by. I had the
> feeling that Signed-off-by is being used to also mark the path of a patch.
> Also, is there a centralized list of all the ways a patch can be "marked"
Signed-off-by is a legal declaration. Please read the documentation
carefully before using it.
From: is automatically added (when you enable the option and there's
no Signed-off-by or From already) and is just informational.
Have fun,
Avery
^ permalink raw reply
* Re: [RFC] Re: Convert 'git blame' to parse_options()
From: Linus Torvalds @ 2008-06-23 20:12 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, Pierre Habouzit, Git Mailing List,
Junio C Hamano
In-Reply-To: <20080623195314.GA29569@sigill.intra.peff.net>
On Mon, 23 Jun 2008, Jeff King wrote:
>
> I'm not so sure. I assumed that most of the callbacks would simply take
> a "struct rev_list". So you would end up in builtin-blame.c with:
>
> ...
> OPT__REVISION(&my_rev_list),
> ...
>
> in your options table. And if setup_revisions takes options that affect
> things that _aren't_ in that struct, then they probably ought to be.
The world isn't like "it ought to be". It is like it is.
If you keep on dreaming about how things "ought to be", you'll never
confront the things as they are. The fact is, turning the existing very
simple argument parsers into using "parse_options()" is just _hard_.
Then you say that it all "ought to be" in those options already.
Sure. Do we have some invisible sky wizard to make it so?
The thign is, parse_options() was introduced 8 months ago or so. Have you
looked at the output of
git grep 'str[n]*cmp.*"--'
lately? Yes, they probably all "ought to be" something or other, but as it
is, the fact is that they are a damn pain to convert.
I tried to do just _one_ program. Trust me, I'm not going to even bother
trying to do another unless parse_options() is made more palatable to do
in small pieces.
Linus
^ permalink raw reply
* Re: [PATCH 1/2] parse_options: Add flag to prevent errors for further processing
From: Junio C Hamano @ 2008-06-23 20:17 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, Shawn Bohrer, git, madcoder
In-Reply-To: <20080623195906.GC29569@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Now I'm confused: my understanding is that the diff option parser just
> leaves unrecognized stuff in argv. But isn't that what a
> PARSE_OPTIONS_IGNORE_UNKNOWN flag would do, and isn't that wrong?
I was thinking more about the way how the lower level diff_opt_parse()
works by letting the caller to handle things that it itself does not know
how.
But I say this because I am not interested in "-a -b -c <=> -abc" and
haven't thought about how you would go about parsing something like that
sanely with partial knowledge.
^ permalink raw reply
* (unknown),
From: VIP Casino Club @ 2008-06-23 20:54 UTC (permalink / raw)
To: git
Enjoy all the action of Las Vegas with 24 hour customer services, great
offers and promotions
and reliable payments from your armchair with ClubVIP:
***Download and install our free software
***Register your account
***Purchase chips and get playing!
See you at the tables!
http://www.webtopjackpot.net
hewruhxycu lyyj bdihiabda
mauugema bu
siaocwenrvumip bfyxuu mueglofaeu
oereqyxlebjier k koopwanusohuc
znhiqs x
tivequwexe sju jeozenov
^ permalink raw reply
* Re: [PATCH 1/2] parse_options: Add flag to prevent errors for further processing
From: Johannes Schindelin @ 2008-06-23 20:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn Bohrer, Jeff King, git, madcoder
In-Reply-To: <7v4p7khqp7.fsf@gitster.siamese.dyndns.org>
Hi,
On Mon, 23 Jun 2008, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Thu, 19 Jun 2008, Shawn Bohrer wrote:
> >
> >> On Wed, Jun 18, 2008 at 11:52:42AM -0700, Junio C Hamano wrote:
> >
> > I believe not. I think that Junio prefers some callback that can
> > handle a whole bunch of options (as opposed to the callback we can
> > have now, to handle arguments for a specific option).
>
> Sorry, no. I do not want callbacks. I've been saying that parser
> cascading is easier if you use an incremental interface like diff option
> parser does.
Sorry, I misunderstood. At least you clarified it for me now.
Thanks,
Dscho
^ permalink raw reply
* Re: Code Formatting vs Trailing Whitespaces
From: Robin Rosenberg @ 2008-06-23 20:31 UTC (permalink / raw)
To: Florian Köberle; +Cc: git, Shawn O. Pearce, Marek Zawirski
In-Reply-To: <485FE856.1080808@web.de>
måndagen den 23 juni 2008 20.15.50 skrev Florian Köberle:
> Hi
>
> I noticed that Robin removed trailing whitespaces from my patch(thanks).
> It appears that if you enable code formatting and the removement of
> trailing whitespaces in the save actions option that the formatting
> option will win.
> It will add a space at the second line of a javadoc comment:
> /**
> ~ * <----
>
> I think this is a bug in eclipse, did any of you create a bug report for
> that? Or a feature request for an "no whitespace after javadoc" option
> in the code formatter page?
This one matches: https://bugs.eclipse.org/bugs/show_bug.cgi?id=49619
> I want to have both options active, because I don't want to commit wrong
> formatted patches.
When I have both options active, it adds the whitespace when formatting, but
removes it when saving in (version 3.3.2).
-- robin
^ permalink raw reply
* Re: Git clone behaviour change in 1.5.6 (vs 1.5.5.1)
From: Jeff King @ 2008-06-23 20:38 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: zuh, git
In-Reply-To: <alpine.LNX.1.00.0806231554380.19665@iabervon.org>
On Mon, Jun 23, 2008 at 03:56:41PM -0400, Daniel Barkalow wrote:
> > Switching to the 1.5.6 release from 1.5.5.1, I found out that the
> > rewritten git-clone command changed its behaviour wrt cloning to a
> > non-existing destination directory structure. In the shell version the
> > destination (work tree) is created with 'mkdir -p' but in the C
> > version with just the mkdir() call which doesn't create the parent
> > directories.
> >
> > I can't find any indication that this would be intended in the repo
> > history nor in the mailing list, so I'm left thinking that this is an
> > unwanted regression. Could someone confirm this?
>
> It wasn't an intentional change, anyway.
Here is a partial fix. For --bare repo creation, we follow a different
codepath, and I think the fix is likely to be a bit tricky. We actually
fail in make_absolute_path, before we try creating the directory. But I
suspect we can't just create the directory ourselves because init_db
expects it not to exist.
-- >8 --
clone: create intermediate directories of repo destination
The shell version used to use "mkdir -p" to create the repo
path, but the C version just calls "mkdir". Let's replicate
the old behavior.
---
builtin-clone.c | 40 +++++++++++++++++++++++++++++++++++++++-
t/t5601-clone.sh | 14 ++++++++++++++
2 files changed, 53 insertions(+), 1 deletions(-)
diff --git a/builtin-clone.c b/builtin-clone.c
index 5c5acb4..758d02c 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -168,6 +168,44 @@ static void setup_reference(const char *repo)
free(ref_git_copy);
}
+static int mkdirp_recurse(char *path, int mode)
+{
+ int n;
+ char tmp;
+ int r;
+
+ if (mkdir(path, mode) == 0)
+ return 0;
+ if (errno != ENOENT)
+ return -1;
+
+ for (n = strlen(path); n > 0 && path[n] != '/'; n--)
+ ;
+ if (n == 0)
+ return -1;
+ tmp = path[n];
+ path[n] = '\0';
+ r = mkdirp_recurse(path, mode);
+ path[n] = tmp;
+ if (r < 0)
+ return -1;
+ return mkdir(path, mode);
+}
+
+static int mkdirp(const char *path, int mode)
+{
+ char buf[PATH_MAX];
+ int n;
+
+ n = strlen(path);
+ if (n >= PATH_MAX) {
+ errno = ENAMETOOLONG;
+ return -1;
+ }
+ memcpy(buf, path, n+1);
+ return mkdirp_recurse(buf, mode);
+}
+
static void copy_or_link_directory(char *src, char *dest)
{
struct dirent *de;
@@ -404,7 +442,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
if (!option_bare) {
junk_work_tree = work_tree;
- if (mkdir(work_tree, 0755))
+ if (mkdirp(work_tree, 0755))
die("could not create work tree dir '%s'.", work_tree);
set_git_work_tree(work_tree);
}
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 593d1a3..c2c83f0 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -30,4 +30,18 @@ test_expect_success 'clone checks out files' '
'
+test_expect_success 'clone creates intermediate directories' '
+
+ git clone src long/path/to/dst &&
+ test -f long/path/to/dst/file
+
+'
+
+test_expect_failure 'clone creates intermediate directories for bare repo' '
+
+ git clone --bare src long/path/to/bare/dst &&
+ test -f long/path/to/bare/dst/config
+
+'
+
test_done
--
1.5.6.105.gceec6.dirty
^ permalink raw reply related
* Re: Git clone behaviour change in 1.5.6 (vs 1.5.5.1)
From: Brandon Casey @ 2008-06-23 21:03 UTC (permalink / raw)
To: Jeff King; +Cc: Daniel Barkalow, zuh, git
In-Reply-To: <20080623203835.GA8105@sigill.intra.peff.net>
Jeff King wrote:
> On Mon, Jun 23, 2008 at 03:56:41PM -0400, Daniel Barkalow wrote:
>
>>> Switching to the 1.5.6 release from 1.5.5.1, I found out that the
>>> rewritten git-clone command changed its behaviour wrt cloning to a
>>> non-existing destination directory structure. In the shell version the
>>> destination (work tree) is created with 'mkdir -p' but in the C
>>> version with just the mkdir() call which doesn't create the parent
>>> directories.
> Here is a partial fix.
There is a mkdir_p() in builtin-merge-recursive.c which calls
safe_create_leading_directories() in sha1_file.c
Can these functions be reused?
-brandon
^ permalink raw reply
* Re: [RFC] Re: Convert 'git blame' to parse_options()
From: Pierre Habouzit @ 2008-06-23 21:09 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jeff King, Johannes Schindelin, Git Mailing List, Junio C Hamano
In-Reply-To: <alpine.LFD.1.10.0806231158340.2926@woody.linux-foundation.org>
[-- Attachment #1: Type: text/plain, Size: 1575 bytes --]
On Mon, Jun 23, 2008 at 07:16:17PM +0000, Linus Torvalds wrote:
>
>
> On Mon, 23 Jun 2008, Linus Torvalds wrote:
> >
> > Or are we going to sit around discussing this for another five months?
>
> So the gauntlet is thrown.
Let's see if I can catch it elegantly.
Following is a series split into incremental patches to migrate the
parse_options API to an incremental one. Only the last step provides a
functional incremental API.
With that, you write parsers this way:
{
struct parse_opt_ctx_t ctx;
parse_options_start(&ctx, argc, argv, 0);
for (;;) {
const char *arg;
switch (parse_options_step(&ctx, options, usagestr)) {
case PARSE_OPT_HELP:
/* dump your help here, the one for options/usagestr is already dumped */
exit(129);
case PARSE_OPT_DONE:
goto done;
}
arg = *ctx->argv++;
ctx->argc--;
if (strcmp(arg, "-")) {
/* you're on baby ! */
} else if ....
} else {
error("unknown option %s", arg);
parse_options_usage(options, usagestr);
/* dump your help here */
exit(129);
}
}
done:
argc = parse_options_end(&ctx);
}
It's slightly more involved than what Linus handwaved, but I believe
it's usable this way.
--
·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] parse-opt: Export a non NORETURN usage dumper.
From: Pierre Habouzit @ 2008-06-23 21:11 UTC (permalink / raw)
To: git; +Cc: peff, Johannes.Schindelin, gitster, Pierre Habouzit
In-Reply-To: <1214255482-2086-1-git-send-email-madcoder@debian.org>
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
parse-options.c | 24 +++++++++++++++++-------
parse-options.h | 9 +++++++++
2 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 774e647..4e9e675 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -257,8 +257,8 @@ int parse_options_end(struct parse_opt_ctx_t *ctx)
return ctx->cpidx + ctx->argc;
}
-static NORETURN void usage_with_options_internal(const char * const *,
- const struct option *, int);
+static int usage_with_options_internal(const char * const *,
+ const struct option *, int, int);
int parse_options(int argc, const char **argv, const struct option *options,
const char * const usagestr[], int flags)
@@ -302,7 +302,7 @@ int parse_options(int argc, const char **argv, const struct option *options,
}
if (!strcmp(arg + 2, "help-all"))
- usage_with_options_internal(usagestr, options, 1);
+ usage_with_options_internal(usagestr, options, 1, 1);
if (!strcmp(arg + 2, "help"))
usage_with_options(usagestr, options);
if (parse_long_opt(&ctx, arg + 2, options))
@@ -315,8 +315,8 @@ int parse_options(int argc, const char **argv, const struct option *options,
#define USAGE_OPTS_WIDTH 24
#define USAGE_GAP 2
-void usage_with_options_internal(const char * const *usagestr,
- const struct option *opts, int full)
+int usage_with_options_internal(const char * const *usagestr,
+ const struct option *opts, int full, int do_exit)
{
fprintf(stderr, "usage: %s\n", *usagestr++);
while (*usagestr && **usagestr)
@@ -401,15 +401,25 @@ void usage_with_options_internal(const char * const *usagestr,
}
fputc('\n', stderr);
- exit(129);
+ if (do_exit)
+ exit(129);
+ return PARSE_OPT_HELP;
}
void usage_with_options(const char * const *usagestr,
const struct option *opts)
{
- usage_with_options_internal(usagestr, opts, 0);
+ usage_with_options_internal(usagestr, opts, 0, 1);
+ exit(129); /* make gcc happy */
+}
+
+int parse_options_usage(const char * const *usagestr,
+ const struct option *opts)
+{
+ return usage_with_options_internal(usagestr, opts, 0, 0);
}
+
/*----- some often used options -----*/
#include "cache.h"
diff --git a/parse-options.h b/parse-options.h
index db6c986..424ea46 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -113,6 +113,12 @@ extern NORETURN void usage_with_options(const char * const *usagestr,
/*----- incremantal advanced APIs -----*/
+enum {
+ PARSE_OPT_HELP = -1,
+ PARSE_OPT_DONE,
+ PARSE_OPT_UNKNOWN,
+};
+
struct parse_opt_ctx_t {
const char **argv;
const char **out;
@@ -121,6 +127,9 @@ struct parse_opt_ctx_t {
int flags;
};
+extern int parse_options_usage(const char * const *usagestr,
+ const struct option *opts);
+
extern void parse_options_start(struct parse_opt_ctx_t *ctx,
int argc, const char **argv, int flags);
--
1.5.6.117.g5fe2
^ permalink raw reply related
* [PATCH] parse-opt: have parse_options_{start,end}.
From: Pierre Habouzit @ 2008-06-23 21:11 UTC (permalink / raw)
To: git; +Cc: peff, Johannes.Schindelin, gitster, Pierre Habouzit
In-Reply-To: <20080623210935.GC13395@artemis.madism.org>
Make the struct optparse_t public under the better name parse_opt_ctx_t.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
parse-options.c | 69 +++++++++++++++++++++++++++++++------------------------
parse-options.h | 16 ++++++++++++
2 files changed, 55 insertions(+), 30 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index b8bde2b..774e647 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -4,14 +4,7 @@
#define OPT_SHORT 1
#define OPT_UNSET 2
-struct optparse_t {
- const char **argv;
- const char **out;
- int argc, cpidx;
- const char *opt;
-};
-
-static inline const char *get_arg(struct optparse_t *p)
+static inline const char *get_arg(struct parse_opt_ctx_t *p)
{
if (p->opt) {
const char *res = p->opt;
@@ -37,7 +30,7 @@ static int opterror(const struct option *opt, const char *reason, int flags)
return error("option `%s' %s", opt->long_name, reason);
}
-static int get_value(struct optparse_t *p,
+static int get_value(struct parse_opt_ctx_t *p,
const struct option *opt, int flags)
{
const char *s, *arg;
@@ -131,7 +124,7 @@ static int get_value(struct optparse_t *p,
}
}
-static int parse_short_opt(struct optparse_t *p, const struct option *options)
+static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *options)
{
for (; options->type != OPTION_END; options++) {
if (options->short_name == *p->opt) {
@@ -142,7 +135,7 @@ static int parse_short_opt(struct optparse_t *p, const struct option *options)
return error("unknown switch `%c'", *p->opt);
}
-static int parse_long_opt(struct optparse_t *p, const char *arg,
+static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
const struct option *options)
{
const char *arg_end = strchr(arg, '=');
@@ -247,45 +240,63 @@ void check_typos(const char *arg, const struct option *options)
}
}
+void parse_options_start(struct parse_opt_ctx_t *ctx,
+ int argc, const char **argv, int flags)
+{
+ memset(ctx, 0, sizeof(*ctx));
+ ctx->argc = argc - 1;
+ ctx->argv = argv + 1;
+ ctx->out = argv;
+ ctx->flags = flags;
+}
+
+int parse_options_end(struct parse_opt_ctx_t *ctx)
+{
+ memmove(ctx->out + ctx->cpidx, ctx->argv, ctx->argc * sizeof(*ctx->out));
+ ctx->out[ctx->cpidx + ctx->argc] = NULL;
+ return ctx->cpidx + ctx->argc;
+}
+
static NORETURN void usage_with_options_internal(const char * const *,
const struct option *, int);
int parse_options(int argc, const char **argv, const struct option *options,
const char * const usagestr[], int flags)
{
- struct optparse_t args = { argv + 1, argv, argc - 1, 0, NULL };
+ struct parse_opt_ctx_t ctx;
- for (; args.argc; args.argc--, args.argv++) {
- const char *arg = args.argv[0];
+ parse_options_start(&ctx, argc, argv, flags);
+ for (; ctx.argc; ctx.argc--, ctx.argv++) {
+ const char *arg = ctx.argv[0];
if (*arg != '-' || !arg[1]) {
- if (flags & PARSE_OPT_STOP_AT_NON_OPTION)
+ if (ctx.flags & PARSE_OPT_STOP_AT_NON_OPTION)
break;
- args.out[args.cpidx++] = args.argv[0];
+ ctx.out[ctx.cpidx++] = ctx.argv[0];
continue;
}
if (arg[1] != '-') {
- args.opt = arg + 1;
- if (*args.opt == 'h')
+ ctx.opt = arg + 1;
+ if (*ctx.opt == 'h')
usage_with_options(usagestr, options);
- if (parse_short_opt(&args, options) < 0)
+ if (parse_short_opt(&ctx, options) < 0)
usage_with_options(usagestr, options);
- if (args.opt)
+ if (ctx.opt)
check_typos(arg + 1, options);
- while (args.opt) {
- if (*args.opt == 'h')
+ while (ctx.opt) {
+ if (*ctx.opt == 'h')
usage_with_options(usagestr, options);
- if (parse_short_opt(&args, options) < 0)
+ if (parse_short_opt(&ctx, options) < 0)
usage_with_options(usagestr, options);
}
continue;
}
if (!arg[2]) { /* "--" */
- if (!(flags & PARSE_OPT_KEEP_DASHDASH)) {
- args.argc--;
- args.argv++;
+ if (!(ctx.flags & PARSE_OPT_KEEP_DASHDASH)) {
+ ctx.argc--;
+ ctx.argv++;
}
break;
}
@@ -294,13 +305,11 @@ int parse_options(int argc, const char **argv, const struct option *options,
usage_with_options_internal(usagestr, options, 1);
if (!strcmp(arg + 2, "help"))
usage_with_options(usagestr, options);
- if (parse_long_opt(&args, arg + 2, options))
+ if (parse_long_opt(&ctx, arg + 2, options))
usage_with_options(usagestr, options);
}
- memmove(args.out + args.cpidx, args.argv, args.argc * sizeof(*args.out));
- args.out[args.cpidx + args.argc] = NULL;
- return args.cpidx + args.argc;
+ return parse_options_end(&ctx);
}
#define USAGE_OPTS_WIDTH 24
diff --git a/parse-options.h b/parse-options.h
index 4ee443d..db6c986 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -111,6 +111,22 @@ extern int parse_options(int argc, const char **argv,
extern NORETURN void usage_with_options(const char * const *usagestr,
const struct option *options);
+/*----- incremantal advanced APIs -----*/
+
+struct parse_opt_ctx_t {
+ const char **argv;
+ const char **out;
+ int argc, cpidx;
+ const char *opt;
+ int flags;
+};
+
+extern void parse_options_start(struct parse_opt_ctx_t *ctx,
+ int argc, const char **argv, int flags);
+
+extern int parse_options_end(struct parse_opt_ctx_t *ctx);
+
+
/*----- some often used options -----*/
extern int parse_opt_abbrev_cb(const struct option *, const char *, int);
extern int parse_opt_approxidate_cb(const struct option *, const char *, int);
--
1.5.6.117.g5fe2
^ permalink raw reply related
* [PATCH] parse-opt: fake short strings for callers to believe in.
From: Pierre Habouzit @ 2008-06-23 21:11 UTC (permalink / raw)
To: git; +Cc: peff, Johannes.Schindelin, gitster, Pierre Habouzit
In-Reply-To: <1214255482-2086-4-git-send-email-madcoder@debian.org>
If we begin to parse -abc and that the parser knew about -a and -b, it
will fake a -c switch for the caller to deal with.
Of course in the case of -acb (supposing -c is not taking an argument) the
caller will have to be especially clever to do the same thing. We could
think about exposing an API to do so if it's really needed, but oh well...
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
parse-options.c | 18 ++++++++++++++++--
parse-options.h | 12 ++++++++++++
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 1a1d4dc..f3fd296 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -248,6 +248,7 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
ctx->argv = argv + 1;
ctx->out = argv;
ctx->flags = flags;
+ strbuf_init(&ctx->buf, 0);
}
static int usage_with_options_internal(const char * const *,
@@ -257,6 +258,9 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
const struct option *options,
const char * const usagestr[])
{
+ /* we must reset ->opt, unknown short option leave it dangling */
+ ctx->opt = NULL;
+
for (; ctx->argc; ctx->argc--, ctx->argv++) {
const char *arg = ctx->argv[0];
@@ -275,7 +279,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
case -1:
return parse_options_usage(usagestr, options);
case -2:
- return PARSE_OPT_UNKNOWN;
+ goto unknown_fixup;
}
if (ctx->opt)
check_typos(arg + 1, options);
@@ -286,10 +290,19 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
case -1:
return parse_options_usage(usagestr, options);
case -2:
- return PARSE_OPT_UNKNOWN;
+ goto unknown_fixup;
}
}
continue;
+unknown_fixup:
+ /* fake a short option thing to hide the fact that we may have
+ * started to parse aggregated stuff
+ */
+ strbuf_reset(&ctx->buf);
+ strbuf_addch(&ctx->buf, '-');
+ strbuf_addstr(&ctx->buf, ctx->opt);
+ *ctx->argv = ctx->buf.buf;
+ return PARSE_OPT_UNKNOWN;
}
if (!arg[2]) { /* "--" */
@@ -318,6 +331,7 @@ int parse_options_end(struct parse_opt_ctx_t *ctx)
{
memmove(ctx->out + ctx->cpidx, ctx->argv, ctx->argc * sizeof(*ctx->out));
ctx->out[ctx->cpidx + ctx->argc] = NULL;
+ strbuf_release(&ctx->buf);
return ctx->cpidx + ctx->argc;
}
diff --git a/parse-options.h b/parse-options.h
index 9da5e8c..14447d5 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -1,6 +1,8 @@
#ifndef PARSE_OPTIONS_H
#define PARSE_OPTIONS_H
+#include "strbuf.h"
+
enum parse_opt_type {
/* special types */
OPTION_END,
@@ -119,12 +121,18 @@ enum {
PARSE_OPT_UNKNOWN,
};
+/*
+ * It's okay for the caller to consume argv/argc in the usual way.
+ * Other fields of that structure are private to parse-options and should not
+ * be modified in any way.
+ */
struct parse_opt_ctx_t {
const char **argv;
const char **out;
int argc, cpidx;
const char *opt;
int flags;
+ struct strbuf buf;
};
extern int parse_options_usage(const char * const *usagestr,
@@ -133,6 +141,10 @@ extern int parse_options_usage(const char * const *usagestr,
extern void parse_options_start(struct parse_opt_ctx_t *ctx,
int argc, const char **argv, int flags);
+/* Warning: you cannot keep pointers to ctx->argv during the parse
+ * because some "option strings" are faked. It's okay to use
+ * ctx->argv after a parse_options_end obviously
+ */
extern int parse_options_step(struct parse_opt_ctx_t *ctx,
const struct option *options,
const char * const usagestr[]);
--
1.5.6.117.g5fe2
^ permalink raw reply related
* [PATCH] parse-opt: create parse_options_step.
From: Pierre Habouzit @ 2008-06-23 21:11 UTC (permalink / raw)
To: git; +Cc: peff, Johannes.Schindelin, gitster, Pierre Habouzit
In-Reply-To: <1214255482-2086-2-git-send-email-madcoder@debian.org>
For now it's unable to stop at unknown options, this commit merely
reorganize some code around.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
parse-options.c | 89 +++++++++++++++++++++++++++++++-----------------------
parse-options.h | 4 ++
2 files changed, 55 insertions(+), 38 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 4e9e675..71b3476 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -250,64 +250,79 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
ctx->flags = flags;
}
-int parse_options_end(struct parse_opt_ctx_t *ctx)
-{
- memmove(ctx->out + ctx->cpidx, ctx->argv, ctx->argc * sizeof(*ctx->out));
- ctx->out[ctx->cpidx + ctx->argc] = NULL;
- return ctx->cpidx + ctx->argc;
-}
-
static int usage_with_options_internal(const char * const *,
- const struct option *, int, int);
+ const struct option *, int);
-int parse_options(int argc, const char **argv, const struct option *options,
- const char * const usagestr[], int flags)
+int parse_options_step(struct parse_opt_ctx_t *ctx,
+ const struct option *options,
+ const char * const usagestr[])
{
- struct parse_opt_ctx_t ctx;
-
- parse_options_start(&ctx, argc, argv, flags);
- for (; ctx.argc; ctx.argc--, ctx.argv++) {
- const char *arg = ctx.argv[0];
+ for (; ctx->argc; ctx->argc--, ctx->argv++) {
+ const char *arg = ctx->argv[0];
if (*arg != '-' || !arg[1]) {
- if (ctx.flags & PARSE_OPT_STOP_AT_NON_OPTION)
+ if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
break;
- ctx.out[ctx.cpidx++] = ctx.argv[0];
+ ctx->out[ctx->cpidx++] = ctx->argv[0];
continue;
}
if (arg[1] != '-') {
- ctx.opt = arg + 1;
- if (*ctx.opt == 'h')
- usage_with_options(usagestr, options);
- if (parse_short_opt(&ctx, options) < 0)
+ ctx->opt = arg + 1;
+ if (*ctx->opt == 'h')
+ return parse_options_usage(usagestr, options);
+ if (parse_short_opt(ctx, options) < 0)
usage_with_options(usagestr, options);
- if (ctx.opt)
+ if (ctx->opt)
check_typos(arg + 1, options);
- while (ctx.opt) {
- if (*ctx.opt == 'h')
- usage_with_options(usagestr, options);
- if (parse_short_opt(&ctx, options) < 0)
+ while (ctx->opt) {
+ if (*ctx->opt == 'h')
+ return parse_options_usage(usagestr, options);
+ if (parse_short_opt(ctx, options) < 0)
usage_with_options(usagestr, options);
}
continue;
}
if (!arg[2]) { /* "--" */
- if (!(ctx.flags & PARSE_OPT_KEEP_DASHDASH)) {
- ctx.argc--;
- ctx.argv++;
+ if (!(ctx->flags & PARSE_OPT_KEEP_DASHDASH)) {
+ ctx->argc--;
+ ctx->argv++;
}
break;
}
if (!strcmp(arg + 2, "help-all"))
- usage_with_options_internal(usagestr, options, 1, 1);
+ return usage_with_options_internal(usagestr, options, 1);
if (!strcmp(arg + 2, "help"))
- usage_with_options(usagestr, options);
- if (parse_long_opt(&ctx, arg + 2, options))
+ return parse_options_usage(usagestr, options);
+ if (parse_long_opt(ctx, arg + 2, options))
usage_with_options(usagestr, options);
}
+ return PARSE_OPT_DONE;
+}
+
+int parse_options_end(struct parse_opt_ctx_t *ctx)
+{
+ memmove(ctx->out + ctx->cpidx, ctx->argv, ctx->argc * sizeof(*ctx->out));
+ ctx->out[ctx->cpidx + ctx->argc] = NULL;
+ return ctx->cpidx + ctx->argc;
+}
+
+int parse_options(int argc, const char **argv, const struct option *options,
+ const char * const usagestr[], int flags)
+{
+ struct parse_opt_ctx_t ctx;
+
+ parse_options_start(&ctx, argc, argv, flags);
+ switch (parse_options_step(&ctx, options, usagestr)) {
+ case PARSE_OPT_HELP:
+ exit(129);
+ case PARSE_OPT_DONE:
+ break;
+ default: /* PARSE_OPT_UNKNOWN */
+ abort(); /* unreached yet */
+ }
return parse_options_end(&ctx);
}
@@ -316,7 +331,7 @@ int parse_options(int argc, const char **argv, const struct option *options,
#define USAGE_GAP 2
int usage_with_options_internal(const char * const *usagestr,
- const struct option *opts, int full, int do_exit)
+ const struct option *opts, int full)
{
fprintf(stderr, "usage: %s\n", *usagestr++);
while (*usagestr && **usagestr)
@@ -401,22 +416,20 @@ int usage_with_options_internal(const char * const *usagestr,
}
fputc('\n', stderr);
- if (do_exit)
- exit(129);
return PARSE_OPT_HELP;
}
void usage_with_options(const char * const *usagestr,
const struct option *opts)
{
- usage_with_options_internal(usagestr, opts, 0, 1);
- exit(129); /* make gcc happy */
+ usage_with_options_internal(usagestr, opts, 0);
+ exit(129);
}
int parse_options_usage(const char * const *usagestr,
const struct option *opts)
{
- return usage_with_options_internal(usagestr, opts, 0, 0);
+ return usage_with_options_internal(usagestr, opts, 0);
}
diff --git a/parse-options.h b/parse-options.h
index 424ea46..9da5e8c 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -133,6 +133,10 @@ extern int parse_options_usage(const char * const *usagestr,
extern void parse_options_start(struct parse_opt_ctx_t *ctx,
int argc, const char **argv, int flags);
+extern int parse_options_step(struct parse_opt_ctx_t *ctx,
+ const struct option *options,
+ const char * const usagestr[]);
+
extern int parse_options_end(struct parse_opt_ctx_t *ctx);
--
1.5.6.117.g5fe2
^ permalink raw reply related
* [PATCH] parse-opt: do not pring errors on unknown options, return -2 intead.
From: Pierre Habouzit @ 2008-06-23 21:11 UTC (permalink / raw)
To: git; +Cc: peff, Johannes.Schindelin, gitster, Pierre Habouzit
In-Reply-To: <1214255482-2086-3-git-send-email-madcoder@debian.org>
This way we can catch "unknown" options more easily.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
parse-options.c | 35 ++++++++++++++++++++++++++---------
1 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 71b3476..1a1d4dc 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -132,7 +132,7 @@ static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *optio
return get_value(p, options, OPT_SHORT);
}
}
- return error("unknown switch `%c'", *p->opt);
+ return -2;
}
static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
@@ -217,7 +217,7 @@ is_abbreviated:
abbrev_option->long_name);
if (abbrev_option)
return get_value(p, abbrev_option, abbrev_flags);
- return error("unknown option `%s'", arg);
+ return -2;
}
void check_typos(const char *arg, const struct option *options)
@@ -271,15 +271,23 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
ctx->opt = arg + 1;
if (*ctx->opt == 'h')
return parse_options_usage(usagestr, options);
- if (parse_short_opt(ctx, options) < 0)
- usage_with_options(usagestr, options);
+ switch (parse_short_opt(ctx, options)) {
+ case -1:
+ return parse_options_usage(usagestr, options);
+ case -2:
+ return PARSE_OPT_UNKNOWN;
+ }
if (ctx->opt)
check_typos(arg + 1, options);
while (ctx->opt) {
if (*ctx->opt == 'h')
return parse_options_usage(usagestr, options);
- if (parse_short_opt(ctx, options) < 0)
- usage_with_options(usagestr, options);
+ switch (parse_short_opt(ctx, options)) {
+ case -1:
+ return parse_options_usage(usagestr, options);
+ case -2:
+ return PARSE_OPT_UNKNOWN;
+ }
}
continue;
}
@@ -296,8 +304,12 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
return usage_with_options_internal(usagestr, options, 1);
if (!strcmp(arg + 2, "help"))
return parse_options_usage(usagestr, options);
- if (parse_long_opt(ctx, arg + 2, options))
- usage_with_options(usagestr, options);
+ switch (parse_long_opt(ctx, arg + 2, options)) {
+ case -1:
+ return parse_options_usage(usagestr, options);
+ case -2:
+ return PARSE_OPT_UNKNOWN;
+ }
}
return PARSE_OPT_DONE;
}
@@ -321,7 +333,12 @@ int parse_options(int argc, const char **argv, const struct option *options,
case PARSE_OPT_DONE:
break;
default: /* PARSE_OPT_UNKNOWN */
- abort(); /* unreached yet */
+ if (ctx.argv[0][1] == '-') {
+ error("unknown option `%s'", ctx.argv[0] + 2);
+ } else {
+ error("unknown switch `%c'", *ctx.opt);
+ }
+ usage_with_options(usagestr, options);
}
return parse_options_end(&ctx);
--
1.5.6.117.g5fe2
^ permalink raw reply related
* Re: Code Formatting vs Trailing Whitespaces
From: Florian Köberle @ 2008-06-23 21:17 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git, Shawn O. Pearce, Marek Zawirski
In-Reply-To: <200806232231.38391.robin.rosenberg.lists@dewire.com>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi
| This one matches: https://bugs.eclipse.org/bugs/show_bug.cgi?id=49619
Thank you. I voted for that feature, maybe it helps to draw attention to
this feature wish.
| When I have both options active, it adds the whitespace when
formatting, but
| removes it when saving in (version 3.3.2).
Strange, I use version 3.3.2 too. I configured it in the workspace settings.
Maybe you did in the project settings?
Best regards,
Florian
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFIYBLy59ca4mzhfxMRAs7vAJ4la1ABhh5Q6pwvhGdx9bZHJvQvWwCfS8nr
2oakNrgbvGSVlGkLzwlJYbw=
=rZFh
-----END PGP SIGNATURE-----
^ permalink raw reply
* apply --recount, was Re: [PATCH v4] git-add--interactive: manual hunk editing mode
From: Johannes Schindelin @ 2008-06-23 21:16 UTC (permalink / raw)
To: Jeff King; +Cc: Thomas Rast, git
In-Reply-To: <20080623195719.GB29569@sigill.intra.peff.net>
Hi,
On Mon, 23 Jun 2008, Jeff King wrote:
> On Mon, Jun 23, 2008 at 07:54:29PM +0100, Johannes Schindelin wrote:
>
> > I am not sure when I will have time for that (particularly given that
> > I got sidetracked with the OPTION_OPTIONS patch, when I should have
> > worked on something completely different).
>
> Heh, I know the feeling. ;)
Yes, us mere mortals cannot hack on Linux and Git all the time, much as
we would like to (be able to).
> > In the meantime, feel free to submit in my name.
>
> Do you remember off the top of your head whether more work was needed?
> They looked good to me, but then I seem to recall you saying in another
> thread that Junio pointed out some flaw with your approach (but I never
> quite understood what the flaw was).
I think it was not a flaw, but something to be worried about:
http://mid.gmane.org/7v4p87zcv6.fsf@gitster.siamese.dyndns.org
To spare you following that link: Junio wanted to reuse "git apply
--recount" to apply mboxes, where a separator "^-- $" to the signature is
quite common, and could be mistaken for a "-" line of a hunk.
I am not quite sure how to deal with this; "--recount=nosig" (assume there
is no signature) and "--recount=stripsig" (assume that "^-- $" is a
lead-in to a signature) come to mind, defaulting to "nosig" with a
warning.
However, I think that this issue should not concern us _now_. As long as
--recount is only to be used in "add -i" and "add -e", I think the patch
is good as is:
http://repo.or.cz/w/git/dscho.git?a=commitdiff;h=c95d4b3da2e5f595f600720b91b8e396b43fe0ae
Ciao,
Dscho
^ permalink raw reply
* Re: git svn --add-author-from implies --use-log-author
From: Mircea Bardac @ 2008-06-23 21:22 UTC (permalink / raw)
To: Git Mailing List; +Cc: Avery Pennarun
In-Reply-To: <32541b130806231312l679aba31ra3daac2bb634cf1b@mail.gmail.com>
Avery Pennarun wrote:
> You can set config options for these, however:
>
> git config svn.addAuthorFrom true
> git config svn.useLogAuthor true
>
> (I actually use "git config --global" to set these on my system so
> they apply to all my git-svn repositories.)
Oh great. This is what I actually wanted. Makes a lot more sense to have
them as (global) variables.
My initial thought was that there are (somewhere) some variables being
set for the repository by using --add-author-from and --use-log-author
with "git svn clone ...". I find this quite intuitive (and maybe this
should be default?). I can't see a reason for using these options once
and not using them later, but it might just be me.
>> P.S. I am very curious why using From and not Signed-off-by. I had the
>> feeling that Signed-off-by is being used to also mark the path of a patch.
>> Also, is there a centralized list of all the ways a patch can be "marked"
>
> Signed-off-by is a legal declaration. Please read the documentation
> carefully before using it.
>
> From: is automatically added (when you enable the option and there's
> no Signed-off-by or From already) and is just informational.
I have found that Documentation/SubmittingPatches contains info on how
to use "Signed-off-by:"/"Acked-by:"/a little bit of "From:" but, as far
as I remember, others have been/are used. Are they all gathered
somewhere, as recommendations?
Many thanks.
--
Mircea
http://mircea.bardac.net
^ permalink raw reply
* Re: [RFC] Re: Convert 'git blame' to parse_options()
From: Pierre Habouzit @ 2008-06-23 21:23 UTC (permalink / raw)
To: Linus Torvalds, Jeff King, Johannes Schindelin, Git Mailing List,
Junio C Hamano <g
In-Reply-To: <20080623210935.GC13395@artemis.madism.org>
[-- Attachment #1: Type: text/plain, Size: 1237 bytes --]
On Mon, Jun 23, 2008 at 09:09:35PM +0000, Pierre Habouzit wrote:
> {
> struct parse_opt_ctx_t ctx;
>
> parse_options_start(&ctx, argc, argv, 0);
>
> for (;;) {
> const char *arg;
>
> switch (parse_options_step(&ctx, options, usagestr)) {
> case PARSE_OPT_HELP:
> /* dump your help here, the one for options/usagestr is already dumped */
At the expense of reworking how help is output, especially in the line
above, we also can have the nice feature that you can chain
parse_options_step with different options structure basically this way
(error handling removed):
if (parse_options_step(&ctx, options1) == PARSE_OPT_UNKNOWN &&
parse_options_step(&ctx, options2) == PARSE_OPT_UNKNOWN &&
...
parse_options_step(&ctx, options_n) == PARSE_OPT_UNKNOWN)
{
/* you're on ! */
}
But with the proposed series, it's not possible (well in the sense
that for valid command lines it will work, but you'll end up with funny
usage dumps for invalid ones).
--
·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: [RFC] Re: Convert 'git blame' to parse_options()
From: Junio C Hamano @ 2008-06-23 21:23 UTC (permalink / raw)
To: Pierre Habouzit
Cc: Linus Torvalds, Jeff King, Johannes Schindelin, Git Mailing List
In-Reply-To: <20080623210935.GC13395@artemis.madism.org>
Pierre Habouzit <madcoder@debian.org> writes:
> With that, you write parsers this way:
>
> {
> struct parse_opt_ctx_t ctx;
>
> parse_options_start(&ctx, argc, argv, 0);
>
> for (;;) {
> const char *arg;
>
> switch (parse_options_step(&ctx, options, usagestr)) {
> case PARSE_OPT_HELP:
> /* dump your help here, the one for options/usagestr is already dumped */
> exit(129);
> case PARSE_OPT_DONE:
> goto done;
> }
>
> arg = *ctx->argv++;
> ctx->argc--;
>
> if (strcmp(arg, "-")) {
> /* you're on baby ! */
> } else if ....
> } else {
> error("unknown option %s", arg);
> parse_options_usage(options, usagestr);
> /* dump your help here */
> exit(129);
> }
> }
>
> done:
> argc = parse_options_end(&ctx);
> }
Nice. I have started doing the same (insignificant details are different;
e.g. I used "positive is unknown" convention instead ) and then the
solution is sitting in my mbox ;-)
^ 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