Following Kristian momentum, I've reworked his parse_option module

quite a lot, and now have some quite interesting features. The series is

available from git://git.madism.org/git.git (branch ph/strbuf).
  The following series is open for comments, it's not 100% ready for

inclusion IMHO, as some details may need to be sorted out first, and

that I've not re-read the patches thoroughly yet. Though I uses the tip

of that branch as my everyday git for 2 weeks or so without any

noticeable issues.
  And as examples are always easier to grok:
$ git fetch -h

usage: git-fetch [options] [<repository> <refspec>...]
  -q, --quiet           be quiet

  -v, --verbose         be verbose
  -a, --append          append in .git/FETCH_HEAD

  -f, --force           force non fast-forwards updates

  --no-tags             don't follow tags at all

  -t, --tags            fetch all tags

  --depth <depth>       deepen history of a shallow clone
Advanced Options

  -k, --keep            keep downloaded pack

  -u, --update-head-ok  allow to update the head in the current branch

  --upload-pack <path>  path to git-upload-pack on the remote
$ git rm -rf xdiff # yeah -rf now works !

rm 'xdiff/xdiff.h'

rm 'xdiff/xdiffi.c'

rm 'xdiff/xdiffi.h'

rm 'xdiff/xemit.c'

rm 'xdiff/xemit.h'

rm 'xdiff/xinclude.h'

rm 'xdiff/xmacros.h'

rm 'xdiff/xmerge.c'

rm 'xdiff/xprepare.c'

rm 'xdiff/xprepare.h'

rm 'xdiff/xtypes.h'

rm 'xdiff/xutils.c'

rm 'xdiff/xutils.h'
-

Very nice.  I worked on gitopt around summer of 2006 but never had the

time to test it thoroughly.  It was a _lot_ more intrusive than yours

currently is (it touched the diff + revision family of commands).
One feature I really like is automatically handling of long option

abbreviations.  gitopt supported this at the expense of complexity

and the aforementioned intrusivenes.  This allows automatic handling

of the abbreviation style seen commonly in git shell scripts:
   --a|--am|--ame|--amen|--amend)  (from git-commit.sh)
--

Eric Wong

-

Yes, but if you do that, you can't order options in the order you

want (because of first match issues), making the help dumps hopelessly

random. I prefer exact match, especially since your shell can help you

autocomplete the proper command.
  I intend to have some magic in the parse_options module to dump the

options in a machine parseable way, so that zsh/bash completion for the

parseopt aware commands is almost trivial. (this was requested from one

of the zsh upstream developpers, and it definitely make sense).
  Note that I didn't migrated all the commands yet especially not

diff.c, We'll need a new construct for that: embedding a struct options

array into another to inherit its flags, though I'm not sure it's

enough, as a struct options right now embeds pointers to the variables

it fills, which doesn't work with the "pure" `diff_opt_parse` approach

right now. But I'm sure I'll come up with something :)
--=20

=C2=B7O=C2=B7  Pierre Habouzit

=C2=B7=C2=B7O                                                madcoder@debia=

n.org

OOO                                                http://www.madism.org

When there is an option "--amend", the option parser now recognizes

"--am" for that option, provided that there is no other option beginning

with "--am".
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

---
	On Sun, 14 Oct 2007, Pierre Habouzit wrote:
	> On Sun, Oct 14, 2007 at 09:18:55AM +0000, Eric Wong wrote:

	> >

	> > One feature I really like is automatically handling of long

	> > option abbreviations.  gitopt supported this at the expense of

	> > complexity and the aforementioned intrusivenes.  This allows

	> > automatic handling of the abbreviation style seen commonly in

	> > git shell scripts:

	> >

	> >    --a|--am|--ame|--amen|--amend)  (from git-commit.sh)

	>

	> Yes, but if you do that, you can't order options in the order

	> you want (because of first match issues), making the help dumps

	> hopelessly random.
I think this patch proves that you do not need to order the options...
;-)
 parse-options.c          |   32 ++++++++++++++++++++++++++++++++

 t/t0040-parse-options.sh |   22 ++++++++++++++++++++++

 2 files changed, 54 insertions(+), 0 deletions(-)
diff --git a/parse-options.c b/parse-options.c

index 72656a8..afc6c89 100644

--- a/parse-options.c

+++ b/parse-options.c

@@ -102,6 +102,13 @@ static int parse_short_opt(struct optparse_t *p, const struct option *options)

 static int parse_long_opt(struct optparse_t *p, const char *arg,

                           const struct option *options)

 {

+	const char *arg_end = strchr(arg, '=');

+	const struct option *abbrev_option = NULL;

+	int abbrev_flags = 0;

+

+	if (!arg_end)

+		arg_end = arg + strlen(arg);

+

 	for (; options->type != OPTION_END; options++) {

 		const char *rest;

 		int flags = 0;

@@ -111,10 +118,33 @@ static int parse_long_opt(struct optparse_t *p, const char *arg,
 		rest = skip_prefix(arg, options->long_name);

 		if (!rest) {

+			/* abbreviated? */

+			if (!strncmp(options->l...
[ message continues ]

Hi,
And an amend for ultra-abbreviated options (as you noticed on IRC):
diff --git a/parse-options.c b/parse-options.c

index afc6c89..acabb98 100644

--- a/parse-options.c

+++ b/parse-options.c

@@ -137,6 +137,11 @@ is_abbreviated:

 				abbrev_flags = flags;

 				continue;

 			}

+			/* negated and abbreviated very much? */

+			if (!prefixcmp("no-", arg)) {

+				flags |= OPT_UNSET;

+				goto is_abbreviated;

+			}

 			/* negated? */

 			if (strncmp(arg, "no-", 3))

 				continue;

-

squashed on top on the previous, and pushed to my ph/parseopt branch.
--=20

=C2=B7O=C2=B7  Pierre Habouzit

=C2=B7=C2=B7O                                                madcoder@debia=

n.org

OOO                                                http://www.madism.org

Awesome.  Thanks to both of you.
--

Eric Wong

-

Hi,
Hehe, you're welcome.  Pierre even realised that my patch was not complete

(it did not catch overly short abbreviations "--n" and "--no"), and that

has been fixed, too.
While I have your attention: last weekend, I spoke to a guy from the

ffmpeg project, and he said that the only thing preventing them from

switching to git was the lack of svn:external support...
(Of course I know that it is more difficult than that: ffmpeg itself is an

svn:external of MPlayer, but maybe we can get both of them to switch ;-)
Do you have any idea when/if you're coming around to add that to git-svn?
Ciao,

Dscho
-

Soonish, possibly within a next week, even.  I have actually have

started a project (using git) that wants to use SVN-hosted repositories

directly submodules; so the fact that I'll actually need something like

it bodes well for getting it implemented :)
--

Eric Wong

-

Hi,
Hehe.  Thanks!
Ciao,

Dscho
-

Thanks for making this another thread because I didn't read the

answers to that patch and I was going to try and implement this

(svn:externals via submodules) sooner or later.  Hadn't I seen this,

we'd probably end up duplicating effort.  Maybe I can help with the

implementation?
This week I'm probably going to start to dive in git-svn by

implementing simpler things first:

   - git svn create-ignore (to create one .gitignore per directory

from the svn:ignore properties.  This has the disadvantage of

committing the .gitignore during the next dcommit, but when you

import a repo with tons of ignores (>1000), using git svn show-ignore

to build .git/info/exclude is *not* a good idea, because things like

git-status will end up doing >1000 fnmatch *per file* in the repo,

which leads to git-status taking more than 4s on my Core2Duo 2Ghz 2G

RAM)

   - git svn propget (to easily retrieve svn properties from withing

git-svn).  git svn propset would be nice too, but I guess it's harder

to implement.
Cheers,
--

Benoit Sigoure aka Tsuna

EPITA Research and Development Laboratory

Ouch.
That sounds largely unavoidable.. *But*.
Maybe we have a bug here. In particular, we generally shouldn't care about

the exclude/.gitignore file for ay paths that we know about, which means

that during an import, we really shouldn't ever even care about

.gitignore, since all the files are files we are expected to know about.
So yes, in general, "git status" is going to be slow in a tree that has

been built (since things like object files etc will have to be checked

against the exclude list! (*)), but if it's a clean import with no

generated files and only files we already know about, that should not be

the case.
So maybe we have a totally unnecessary performance issue, and do all the

fnmatch() on every path, whether we know about it or not?
		Linus
(*) It might be that we could also re-order the exclude list so that

entries that trigger are moved to the head of the list, because it's

likely that if you have tons of exclude entries, some of them trigger a

lot more than others (ie "*.o"), and trying those first is likely a good

idea.

-

I re-used the test that was posted some time ago:
------------------------------------------------------------------------

---

#

# first create a tree of roughly 100k files

#

mkdir bummer

cd bummer

for ((i=0;i<100;i++)); do

mkdir $i && pushd $i;

for ((j=0;j<1000;j++)); do

echo "$j" >$j; done; popd;

done
#

# init and add this to git

#

time git init

git config user.email "no@thx"

git config user.name "nothx"

time git add .

time git commit -m 'buurrrrn' -a
for ((j=0;j<1000;j++)); do

   echo "/pattern$j" >.git/info/exclude

done
#

# git-status, tunes in at around ~8s for me

#

time git-status

time git-status

time git-status

------------------------------------------------------------------------

---
[...]

git commit -m 'buurrrrn' -a  5.62s user 16.84s system 87% cpu 25.634

total

# On branch master

nothing to commit (working directory clean)

git-status  2.48s user 5.97s system 96% cpu 8.718 total

# On branch master

nothing to commit (working directory clean)

git-status  2.48s user 5.94s system 97% cpu 8.646 total

# On branch master

nothing to commit (working directory clean)

git-status  2.48s user 5.95s system 96% cpu 8.720 total
--

Benoit Sigoure aka Tsuna

EPITA Research and Development Laboratory

I think your test is scrogged. You should add the ".gitignore" file

*before* you do the "git add .". That's when it's going to hurt (since

that's when you have new files you don't yet know about).
But then it should hurt only for the "git add ." phase, not for anything

else (unless we have the performance bug of doing the ignore matching even

on files we know about). And more importantly, it should hurt only once

(since afterwards, we'll know about the files and know not to ignore

them).
		Linus

-

There is no .gitignore, only .git/info/exclude.
--

Benoit Sigoure aka Tsuna

EPITA Research and Development Laboratory

They do exactly the same thing (apart from the nesting nature of

.gitignore wrt subdirectories), so that doesn't change anything.
		Linus

-

I fail to see how the mechanism work then.  You said that I needed to

add the .gitignore before adding all the other bummer stuff, fair

enough.  AFAIK .git/info/exclude doesn't need to be added, it's just

there.  But you can try to change the test, add the .git/info/exclude

*first* and then make a commit and then add all the bummer stuff and

then commit, and finally, do a git-status, for me it still takes 9s.
--

Benoit Sigoure aka Tsuna

EPITA Research and Development Laboratory

I built ignore support for git-svnignore a long time ago. It converts

the per-directory svn:ignore to per-directory .gitignore at commit

import time, which is very handy:
-I <ignorefile_name>::

        Import the svn:ignore directory property to files with this

        name in each directory. (The Subversion and GIT ignore

        syntaxes are similar enough that using the Subversion patterns

        directly with "-I .gitignore" will almost always just work.)
The only downside with that is that svn ignore patterns are

non-recursive, while git ignore patterns are recursive. This could be

solved by prefixing them with a "/".
--

Karl Hasselström, kha@treskal.com

      www.treskal.com/kalle

-

Has anyone put any thought into mapping the other direction?

i.e. .gitignore  ->  svn:ignore
-chris

-

If we support .gitignore <-> svn:ignore in git-svn; bidirectional,

transparent mapping is the only way I want to go.
This means that *all* .gitignore files will be translated to svn:ignore

files and vice versa; and the .gitignore files will be NOT be committed

to SVN itself, but present in the git-svn created mirrors.  Recursive

.gitignore definitions will be mapped to svn:ignore recursively on the

client side; and non-recursive ones will only map to one directory.
Sound good?
I may be sleepy at the moment, but the thought of implementing this is

sounding complicated now...
One goal of git-svn is that other users shouldn't be able to tell if a

user is using git-svn or plain svn; even.
But back to submodules, I plan on mapping svn:externals <=> .gitmodules

files in a similar fashion.  .gitmodule files will never be seen by SVN

users, period.
That being said, the first step to submodule/externals support in

git-svn will be to allow /any/ git repository to use a submodule that

points to SVN; and then git-submodule will invoke git-svn if it

sees such a submodule.
Yes, I have a plan, sort of...
Since externals/submodules don't operate recursively in either

system like .gitignore; supporting svn:externals <=> submodules

will be much easier and done first[1] :)
[1] - I've personally rarely bothered with putting svn:ignores in the

repository and have been very much spoiled by .git/info/exclude;

whereas externals support I have semi-immediate use for.
--

Eric Wong

-

OTOH, a general propset solution would probably be good enough that I

wouldn't even miss any transparent .gitignore -> svn:ignore mapping.
I would just accept that I'd have to explicitly specify the
That's great.   I'm eager to see/test the svn:externals support.  Thanks.
-chris

-

I think this is a mistake. If a user adds *.foo to the top-level

.gitignore, this will add *.foo to svn:ignore of _every_ directory in

the whole tree. And coming up with semantics that are sane for e.g.

git -> svn -> git roundtrips seems difficult.
It would be better and far simpler to either
  1. Move the contents of svn:ignore and .gitignore back and forth

     untouched, disregarding the slight semantic mismatch.

     git-svnignore does this (albeit only in one direction), and it

     works surprisingly well in my experience.
  2. Do as in (1), but call the file .svnignore instead of .gitignore.

     And have a git-svn command that translates all the .svnignore
Agreed.
--

Karl Hasselström, kha@treskal.com

      www.treskal.com/kalle

-

How spoiled we are. I just ran cvs status on a checkout of a repo located

on a server in the local network. It took 6 seconds to complete :P
--

Andreas Ericsson                   andreas.ericsson@op5.se

OP5 AB                             www.op5.se

Tel: +46 8-230225                  Fax: +46 8-230231

-

Hehe, true, once you get used to the taste of Git, you'll never want

to switch back to these disgusting SCMs.
--

Benoit Sigoure aka Tsuna

EPITA Research and Development Laboratory

Great to see two things:
- the simplification in the commands switched over to use the options

parser
- the improved readability and usefulness of the options help
Great work, Pierre! I'll take a closer look at this and trial it in

my local Git install for a while to see if any issues come up.
Wincent
-

The option parser takes argc, argv, an array of struct option

and a usage string.  Each of the struct option elements in the array

describes a valid option, its type and a pointer to the location where the

value is written.  The entry point is parse_options(), which scans through

the given argv, and matches each option there against the list of valid

options.  During the scan, argv is rewritten to only contain the

non-option command line arguments and the number of these is returned.
Aggregation of single switches is allowed:

  -rC0 is the same as -r -C 0 (supposing that -C wants an arg).
Every long option automatically support the option with the same name,

prefixed with 'no-' to unset the switch. It assumes that initial value for

strings are "NULL" and for integers is "0".
Long options are supported either with '=' or without:

  --some-option=foo is the same as --some-option foo
Also be able to generate the usage automatically.
Acked-by: Kristian H

Signed-off-by: Jonas Fonseca <fonseca@diku.dk>

---

 Documentation/git-add.txt          |    4 ++--

 Documentation/git-branch.txt       |    2 +-

 Documentation/git-mv.txt           |    2 +-

 Documentation/git-rm.txt           |    4 ++--

 Documentation/git-symbolic-ref.txt |    2 +-

 5 files changed, 7 insertions(+), 7 deletions(-)
 Maybe this should wait, but it is just to document that this series (as

 of the version in your git tree) also adds new option aliases.
 BTW, I didn't bother to change the synopsis lines but maybe I should.
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt

index 2fe7355..963e1ab 100644

--- a/Documentation/git-add.txt

+++ b/Documentation/git-add.txt

@@ -50,10 +50,10 @@ OPTIONS

 	and `dir/file2`) can be given to add all files in the

 	directory, recursively.
--n::

+-n, \--dry-run::

         Don't actually add the file(s), just show if they exist.
--v::

+-v, \--verbose::

         Be verbose.
 -f::

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt

index b7285bc..5e81aa4 100644

--- a/Documentation/git-branch.txt

+++ b/Documentation/git-branch.txt

@@ -85,7 +85,7 @@ OPTIONS

 -a::

 	List both remote-tracking branches and local branches.
--v::

+-v, --verbose::

 	Show sha1 and commit subject line for each head.
 --abbrev=<length>::

diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt

index 2c9cf74..3b8ca76 100644

--- a/Documentation/git-mv.txt

+++ b/Documentation/git-mv.txt

@@ -34,7 +34,7 @@ OPTIONS

 	condition. An error happens when a source is neither existing nor

         controlled by GIT, or when it would overwrite an existing

         file unless '-f' is given.

--n::

+-n, \--dry-run::

 	Do nothing; only show what would happen
diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt

index be61a82..48c1d97 100644

--- a/Documentation/git-rm.txt

+++ b/Documentation/git-rm.txt

@@ -30,7 +30,7 @@ OPTIONS

 -f::

 	Override the up-to-date check.

...
[ message continues ]

added and pushed.

--=20

=C2=B7O=C2=B7  Pierre Habouzit

=C2=B7=C2=B7O                                                madcoder@debia=

n.org

OOO                                                http://www.madism.org

"const struct option *opt"? You never modify the struct option itself,

only the values under the pointers it contains. Using const here will

allow the compiler to reuse string constants (not that there will be
"const struct option *opts"?
Why not "const char *const *usagestr"? Especially if you change

"usagestr" (the pointer itself) later. "[]" is sometimes a hint that

the pointer itself should not be changed, being an array.
And you want make opts const.
BTW, it does not "make" usage. It calls the usage() or prints a usage

description. "make" implies it creates the "usage", which according to
This will crash for empty usagestr, like  "{ NULL }". Was it

deliberately? (I'd make it deliberately, if I were you. I'd even used
BTW, if you just printed the usage message out (it is about usage of a

program, isn't it?) and called exit() everyone would be just as happy.

And you wouldn't have to include strbuf (it is the only use of it),

less code, too. It'd make simplier to stea^Wcopy your implementation,

which I like :)
-

Yes this is intentional, there should be at least on string in the
  the reason is that usage() is a wrapper around a callback, and I

suppose it's used by some GUI's or anything like that.
  FWIW you can rework the .c like this:
  pos =3D 0; /* and not pos =3D sb.len */
  replace the strbuf_add* by the equivalents:

  pos +=3D printf("....");
  and tada, you're done.
  Note that in the most recent version, I also deal with a

OPTION_CALLBACK that passes the value to a callback.
Cheers,

--=20

=C2=B7O=C2=B7  Pierre Habouzit

=C2=B7=C2=B7O                                                madcoder@debia=

n.org

OOO                                                http://www.madism.org

It is not. Not yet. What could they use a usage text for?

Besides, you could just export the callback (call_usage_callback or
on top of yours:
From: Alex Riesen <raa.lkml@gmail.com>

Date: Sun, 14 Oct 2007 00:10:51 +0200

Subject: [PATCH] Rework make_usage to print the usage message immediately
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>

---

 parse-options.c |   60 ++++++++++++++++++++++++------------------------------

 1 files changed, 27 insertions(+), 33 deletions(-)
diff --git a/parse-options.c b/parse-options.c

index 07abb50..1e3940f 100644

--- a/parse-options.c

+++ b/parse-options.c

@@ -1,6 +1,5 @@

 #include "git-compat-util.h"

 #include "parse-options.h"

-#include "strbuf.h"
 #define OPT_SHORT 1

 #define OPT_UNSET 2

@@ -171,57 +170,52 @@ int parse_options(int argc, const char **argv,
 void make_usage(const char * const usagestr[], struct option *opts, int cnt)

 {

-	struct strbuf sb;

-

-	strbuf_init(&sb, 4096);

-	do {

-		strbuf_addstr(&sb, *usagestr++);

-		strbuf_addch(&sb, '\n');

-	} while (*usagestr);

+	fprintf(stderr, "usage: ");

+	while (*usagestr)

+		fprintf(stderr, "%s\n", *usagestr++);
 	if (cnt && opts->type != OPTION_GROUP)

-		strbuf_addch(&sb, '\n');

+		fputc('\n', stderr);
 	for (; cnt-- > 0; opts++) {

 		size_t pos;
 		if (opts->type == OPTION_GROUP) {

-			strbuf_addch(&sb, '\n');

+			fputc('\n', stderr);

 			if (*opts->help)

-				strbuf_addf(&sb, "%s\n", opts->help);

+				fprintf(stderr, "%s\n", opts->help);

 			continue;

 		}
-		pos = sb.len;

-		strbuf_addstr(&sb, "    ");

-		if (opts->short_name) {

-			strbuf_addf(&sb, "-%c", opts->short_name);

-		}

-		if (opts->long_name) {

-			strbuf_addf(&sb, opts->short_name ? ", --%s" : "--%s",

-						opts->long_name);

-		}

+		pos = fprintf(stderr, "    ");

+		if (opts->short_name)

+			pos += fprintf(stderr, "-%c", opts->short_name);

+		if (opts->long_name)

+			pos += fp...
[ message continues ]

Added (reworked a bit for the current state of parse_options), and pushed.
--=20

=C2=B7O=C2=B7  Pierre Habouzit

=C2=B7=C2=B7O                                                madcoder@debia=

n.org

OOO                                                http://www.madism.org

Hi,
This is only used once; I wonder if it is really that more readable having 
Personally, I do not like abbreviations like that.  They do not save that

much screen estate (skip_prefix is only 4 characters longer, but much more 
Would this not be more intuitive as
		if (!prefixcmp(arg, "no-")) {

			arg += 3;

			flags |= OPT_UNSET;

		}

		rest = skip_prefix(arg, options[i].long_name);
Is this really no error?  For example, "git log 
How about calling this "usage_with_options()"?  With that name I expected 
I would prefer this as 
			if (!(flags & OPT_COPY_DASHDASH)) {

				optp.argc--;

				optp.argv++;

			}
While I'm at it: could you use "args" instead of "optp"?  It is misleading

both in that it not only contains options (but other arguments, too) as in

that it is not a pointer (the trailing "p" is used as an indicator of that

very often, including git's source code).
In the same vein, OPT_COPY_DASHDASH should be named 
I know that I proposed "BOOLEAN", but actually, you use it more like an

"INCREMENTAL", right?
Other than that: I like it very much.
Ciao,

Dscho
-

Yes, that can be done indeed, but the point is, we have sometimes

option whose long-name is "no-foo" (because it's what makes sense) but I
  yes, I don't like _BOOLEAN either, I would have prefered _FLAG or sth
--=20

=C2=B7O=C2=B7  Pierre Habouzit

=C2=B7=C2=B7O                                                madcoder@debia=

n.org

OOO                                                http://www.madism.org

From: Kristian H

Hi,
I see you terminate the list by a ",".  How does this play with the option

parser?
Thinking about this more, I am reverting my stance on the ARRAY_SIZE()

issue.  I think if you introduce a "OPTION_NONE = 0" in the enum, then

this single last comma should be enough.
In the same vein, you would not need the NULL in builtin_add_usage[],

right?
Ciao,

Dscho
-

adding a trailing comma does not add a NULL after that, it's ignored,

you're confused.
    =E2=94=8C=E2=94=80(17:00)=E2=94=80=E2=94=80=E2=94=80=E2=94=80

    =E2=94=94[artemis] cat a.c

    #include <stdio.h>
      int main(void) {

	const char * const arr[] =3D { "1", "2", };

	printf("%d\n", sizeof(arr) / sizeof(arr[0]));

	return 0;

    };

    =E2=94=8C=E2=94=80(17:00)=E2=94=80=E2=94=80=E2=94=80=E2=94=80

    =E2=94=94[artemis] ./a

    2
  Very few compilers do not grok trailing commas, I always put them

because it avoids spurious diffs for nothing, and that you can reorder

lines easily too.
  Note that I don't really like using ARRAY_SIZE either, I kept it that

way, but my taste would rather be to have an "empty" option, and

explicitely mark the end of the array.
--=20

=C2=B7O=C2=B7  Pierre Habouzit

=C2=B7=C2=B7O                                                madcoder@debia=

n.org

OOO                                                http://www.madism.org

You can have both. Just stop at NULL-entry or when the 'size' elements

passed, whatever happens first.
-

Well I dislike the "count" thing, and Dscho agreed that it somehow

sucked too. If you go see the current state of the ph/parseopt series

you'll see it's not here anymore.
--=20

=C2=B7O=C2=B7  Pierre Habouzit

=C2=B7=C2=B7O                                                madcoder@debia=

n.org

OOO                                                http://www.madism.org

Signed-off-by: Pierre Habouzit <madcoder@debian.org>

---

 builtin-rm.c |   56 +++++++++++++++++++++++++-------------------------------

 1 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/builtin-rm.c b/builtin-rm.c

index 3b0677e..74eb23c 100644

--- a/builtin-rm.c

+++ b/builtin-rm.c

@@ -8,9 +8,12 @@

 #include "dir.h"

 #include "cache-tree.h"

 #include "tree-walk.h"

+#include "parse-options.h"
-static const char builtin_rm_usage[] =

-"git-rm [-f] [-n] [-r] [--cached] [--ignore-unmatch] [--quiet] [--] <file>...";

+static const char * const builtin_rm_usage[] = {

+	"git-rm [options] [--] <file>...",

+	NULL

+};
 static struct {

 	int nr, alloc;

@@ -121,11 +124,22 @@ static int check_local_mod(unsigned char *head, int index_only)
 static struct lock_file lock_file;
+static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;

+static int ignore_unmatch = 0;

+

+static struct option builtin_rm_options[] = {

+	OPT_BOOLEAN('n', NULL,             &show_only,  "dry-run"),

+	OPT_BOOLEAN( 0 , "cached",         &index_only, "only remove from the index"),

+	OPT_BOOLEAN('f', NULL,             &force,      "override the up-to-date check"),

+	OPT_BOOLEAN('r', NULL,             &recursive,  "allow recursive removal"),

+	OPT_BOOLEAN( 0 , "quiet",          &quiet,      "be quiet"),

+	OPT_BOOLEAN( 0 , "ignore-unmatch", &ignore_unmatch,

+				"exit with a zero status even if nothing matched")

+};

+

 int cmd_rm(int argc, const char **argv, const char *prefix)

 {

 	int i, newfd;

-	int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;

-	int ignore_unmatch = 0;

 	const char **pathspec;

 	char *seen;
@@ -136,34 +150,14 @@ int cmd_rm(int argc, const char **argv, const char *prefix)

 	if (read_cache() < 0)

 		die("index file corrupt");
-	for (i = 1 ; i < argc ; i++) {

-		const char *arg = argv[i];

-

-		if (*arg != '-')

-			break;

-		else if (!strcmp(arg, "--")) {

-			i++;

-			b...
[ message continues ]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>

---

 builtin-mv.c |   55 ++++++++++++++++++++++---------------------------------

 1 files changed, 22 insertions(+), 33 deletions(-)
diff --git a/builtin-mv.c b/builtin-mv.c

index b944651..17c3655 100644

--- a/builtin-mv.c

+++ b/builtin-mv.c

@@ -8,9 +8,12 @@

 #include "dir.h"

 #include "cache-tree.h"

 #include "path-list.h"

+#include "parse-options.h"
-static const char builtin_mv_usage[] =

-"git-mv [-n] [-f] (<source> <destination> | [-k] <source>... <destination>)";

+static const char * const builtin_mv_usage[] = {

+	"git-mv [options] <source>... <destination>",

+	NULL

+};
 static const char **copy_pathspec(const char *prefix, const char **pathspec,

 				  int count, int base_name)

@@ -63,6 +66,11 @@ int cmd_mv(int argc, const char **argv, const char *prefix)

 {

 	int i, newfd, count;

 	int verbose = 0, show_only = 0, force = 0, ignore_errors = 0;

+	struct option builtin_mv_options[] = {

+		OPT_BOOLEAN('n', NULL, &show_only,"dry-run"),

+		OPT_BOOLEAN('f', NULL, &force, "force move/rename even if target exists"),

+		OPT_BOOLEAN('k', NULL, &ignore_errors, "skip move/rename errors"),

+	};

 	const char **source, **destination, **dest_path;

 	enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;

 	struct stat st;

@@ -78,47 +86,28 @@ int cmd_mv(int argc, const char **argv, const char *prefix)

 	if (read_cache() < 0)

 		die("index file corrupt");
-	for (i = 1; i < argc; i++) {

-		const char *arg = argv[i];

+	count = parse_options(argc, argv, builtin_mv_options,

+	                      ARRAY_SIZE(builtin_mv_options),

+	                      builtin_mv_usage, 0);

+	if (--count < 1)

+		make_usage(builtin_mv_usage, builtin_mv_options,

+				   ARRAY_SIZE(builtin_mv_options));
-		if (arg[0] != '-')

-			break;

-		if (!strcmp(arg, "--")) {

-			i++;

-			break;

-		}

-		if (!strcmp(arg, "-n")) {

-			show_only = 1;

-			continue;

-		}

-		if...
[ message continues ]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>

---

 builtin-branch.c |  151 ++++++++++++++++++++---------------------------------

 1 files changed, 57 insertions(+), 94 deletions(-)
diff --git a/builtin-branch.c b/builtin-branch.c

index 3da8b55..3ca225e 100644

--- a/builtin-branch.c

+++ b/builtin-branch.c

@@ -11,9 +11,16 @@

 #include "commit.h"

 #include "builtin.h"

 #include "remote.h"

-

-static const char builtin_branch_usage[] =

-  "git-branch [-r] (-d | -D) <branchname> | [--track | --no-track] [-l] [-f] <branchname> [<start-point>] | (-m | -M) [<oldbranch>] <newbranch> | [--color | --no-color] [-r | -a] [-v [--abbrev=<length> | --no-abbrev]]";

+#include "parse-options.h"

+

+static const char * const builtin_branch_usage[] = {

+	"",

+	"git-branch [options] [-r | -a]",

+	"git-branch [options] [-l] [-f] <branchname> [<start-point>]",

+	"git-branch [options] [-r] (-d | -D) <branchname>",

+	"git-branch [options] (-m | -M) [<oldbranch>] <newbranch>",

+	NULL

+};
 #define REF_UNKNOWN_TYPE    0x00

 #define REF_LOCAL_BRANCH    0x01

@@ -505,93 +512,49 @@ int cmd_branch(int argc, const char **argv, const char *prefix)

 	int rename = 0, force_rename = 0;

 	int verbose = 0, abbrev = DEFAULT_ABBREV, detached = 0;

 	int reflog = 0, track;

-	int kinds = REF_LOCAL_BRANCH;

-	int i;

+	int kinds = REF_LOCAL_BRANCH, kind_remote = 0, kind_local = 0;

+	int count;

+

+	struct option builtin_branch_options[] = {

+		OPT_GROUP("Generic options"),

+		OPT_BOOLEAN('v', NULL,     &verbose, "be verbose"),

+		OPT_BOOLEAN( 0 , "track",  &track, "set up tracking mode (see git-pull(1))"),

+		OPT_BOOLEAN( 0 , "color",  &branch_use_color, "use colored output"),

+		OPT_BOOLEAN('r', NULL,     &kind_remote, "act on remote-tracking branches"),

+		OPT_INTEGER( 0 , "abbrev", &abbrev, "minimum display length for sha1"),

+

+		OPT_GROUP("Specific git-branch actions:"),

+		OPT_BOOLEAN('a', NULL,     &kind_lo...
[ message continues ]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>

---

 builtin-describe.c |   76 ++++++++++++++++++++++++----------------------------

 1 files changed, 35 insertions(+), 41 deletions(-)
diff --git a/builtin-describe.c b/builtin-describe.c

index 669110c..d4e4b01 100644

--- a/builtin-describe.c

+++ b/builtin-describe.c

@@ -4,12 +4,15 @@

 #include "refs.h"

 #include "builtin.h"

 #include "exec_cmd.h"

+#include "parse-options.h"
 #define SEEN		(1u<<0)

 #define MAX_TAGS	(FLAG_BITS - 1)
-static const char describe_usage[] =

-"git-describe [--all] [--tags] [--abbrev=<n>] <committish>*";

+static const char * const describe_usage[] = {

+	"git-describe [options] <committish>*",

+	NULL

+};
 static int debug;	/* Display lots of verbose info */

 static int all;	/* Default to annotated tags only */

@@ -242,57 +245,48 @@ static void describe(const char *arg, int last_one)
 int cmd_describe(int argc, const char **argv, const char *prefix)

 {

-	int i;

+	int count;

 	int contains = 0;

+	struct option builtin_describe_options[] = {

+		OPT_BOOLEAN(0, "contains",   &contains, "find the tag that comes after the commit"),

+		OPT_BOOLEAN(0, "debug",      &debug, "debug search strategy on stderr"),

+		OPT_BOOLEAN(0, "all",        &all, "use any ref in .git/refs"),

+		OPT_BOOLEAN(0, "tags",       &tags, "use any tag in .git/refs/tags"),

+		OPT_INTEGER(0, "abbrev",     &abbrev, "use <n> digits to display sha1"),

+		OPT_INTEGER(0, "candidates", &max_candidates,

+					"consider <n> most recent tags (default: 10)"),

+	};
-	for (i = 1; i < argc; i++) {

-		const char *arg = argv[i];

-

-		if (*arg != '-')

-			break;

-		else if (!strcmp(arg, "--contains"))

-			contains = 1;

-		else if (!strcmp(arg, "--debug"))

-			debug = 1;

-		else if (!strcmp(arg, "--all"))

-			all = 1;

-		else if (!strcmp(arg, "--tags"))

-			tags = 1;

-		else if (!prefixcmp(arg, "--abbrev=")) {

-			abbrev = strtoul(arg + 9, NULL, 10);

-	...
[ message continues ]

Also fix a possible segfault in the refspec parser.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>

---

 builtin-fetch.c |  145 +++++++++++++++++++------------------------------------

 1 files changed, 50 insertions(+), 95 deletions(-)
diff --git a/builtin-fetch.c b/builtin-fetch.c

index cf7498b..4a58955 100644

--- a/builtin-fetch.c

+++ b/builtin-fetch.c

@@ -8,11 +8,15 @@

 #include "path-list.h"

 #include "remote.h"

 #include "transport.h"

+#include "parse-options.h"
-static const char fetch_usage[] = "git-fetch [-a | --append] [--upload-pack <upload-pack>] [-f | --force] [--no-tags] [-t | --tags] [-k | --keep] [-u | --update-head-ok] [--depth <depth>] [-v | --verbose] [<repository> <refspec>...]";

+static const char * const fetch_usage[] = {

+	"git-fetch [options] [<repository> <refspec>...]",

+	NULL

+};
 static int append, force, tags, no_tags, update_head_ok, verbose, quiet;

-static char *default_rla = NULL;

+static struct strbuf default_rla = STRBUF_INIT;

 static struct transport *transport;
 static void unlock_pack(void)

@@ -131,7 +135,7 @@ static int s_update_ref(const char *action,

 	static struct ref_lock *lock;
 	if (!rla)

-		rla = default_rla;

+		rla = default_rla.buf;

 	snprintf(msg, sizeof(msg), "%s: %s", rla, action);

 	lock = lock_any_ref_for_update(ref->name,

 				       check_old ? ref->old_sha1 : NULL, 0);

@@ -443,91 +447,47 @@ static void set_option(const char *name, const char *value)

 int cmd_fetch(int argc, const char **argv, const char *prefix)

 {

 	struct remote *remote;

-	int i, j, rla_offset;

+	int count, i;

 	static const char **refs = NULL;

 	int ref_nr = 0;

-	int cmd_len = 0;

 	const char *depth = NULL, *upload_pack = NULL;

 	int keep = 0;

-

+	struct option builtin_fetch_options[] = {

+		OPT_BOOLEAN('q', "quiet",   &quiet,   "be quiet"),

+		OPT_BOOLEAN('v', "verbose", &verbose, "be verbose"),

+

+		OPT_GROUP(""),

+		OPT_BOOLEAN('a', "append",  &append,  "appe...
[ message continues ]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>

---

 builtin-revert.c |   67 ++++++++++++++++++++++++-----------------------------

 1 files changed, 30 insertions(+), 37 deletions(-)
diff --git a/builtin-revert.c b/builtin-revert.c

index a655c8e..60570db 100644

--- a/builtin-revert.c

+++ b/builtin-revert.c

@@ -7,6 +7,7 @@

 #include "run-command.h"

 #include "exec_cmd.h"

 #include "utf8.h"

+#include "parse-options.h"
 /*

  * This implements the builtins revert and cherry-pick.

@@ -19,51 +20,42 @@

  * Copyright (c) 2005 Junio C Hamano

  */
-static const char *revert_usage = "git-revert [--edit | --no-edit] [-n] <commit-ish>";

+static const char * const revert_usage[] = {

+	"git-revert [options] <commit-ish>",

+	NULL

+};
-static const char *cherry_pick_usage = "git-cherry-pick [--edit] [-n] [-r] [-x] <commit-ish>";

+static const char * const cherry_pick_usage[] = {

+	"git-cherry-pick [options] <commit-ish>",

+	NULL

+};
-static int edit;

-static int replay;

+static int edit, no_replay, no_commit, needed_deref;

 static enum { REVERT, CHERRY_PICK } action;

-static int no_commit;

 static struct commit *commit;

-static int needed_deref;
 static const char *me;
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
-static void parse_options(int argc, const char **argv)

+static void parse_args(int argc, const char **argv)

 {

-	const char *usage_str = action == REVERT ?

-		revert_usage : cherry_pick_usage;

+	const char * const * usage_str =

+		action == REVERT ?  revert_usage : cherry_pick_usage;

 	unsigned char sha1[20];

 	const char *arg;

-	int i;

-

-	if (argc < 2)

-		usage(usage_str);

-

-	for (i = 1; i < argc; i++) {

-		arg = argv[i];

-		if (arg[0] != '-')

-			break;

-		if (!strcmp(arg, "-n") || !strcmp(arg, "--no-commit"))

-			no_commit = 1;

-		else if (!strcmp(arg, "-e") || !strcmp(arg, "--edit"))

-			edit = 1;

-		else if (!strcmp(arg, "--no-edit"))

-			edit = 0;

-		else if (!strcmp(arg, "-x") || !strcm...
[ message continues ]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>

---

 builtin-update-ref.c |   71 +++++++++++++++++++++-----------------------------

 1 files changed, 30 insertions(+), 41 deletions(-)
diff --git a/builtin-update-ref.c b/builtin-update-ref.c

index fe1f74c..eafb642 100644

--- a/builtin-update-ref.c

+++ b/builtin-update-ref.c

@@ -1,59 +1,48 @@

 #include "cache.h"

 #include "refs.h"

 #include "builtin.h"

+#include "parse-options.h"
-static const char git_update_ref_usage[] =

-"git-update-ref [-m <reason>] (-d <refname> <value> | [--no-deref] <refname> <value> [<oldval>])";

+static const char * const git_update_ref_usage[] = {

+	"",

+	"git-update-ref [options] -d <refname> <oldval>",

+	"git-update-ref [options]    <refname> <newval> [<oldval>]",

+	NULL

+};
 int cmd_update_ref(int argc, const char **argv, const char *prefix)

 {

-	const char *refname=NULL, *value=NULL, *oldval=NULL, *msg=NULL;

+	const char *refname, *value, *oldval, *msg=NULL;

 	unsigned char sha1[20], oldsha1[20];

-	int i, delete, ref_flags;

+	int count, delete = 0, no_deref = 0;

+	struct option git_update_ref_options[] = {

+		OPT_STRING( 'm', NULL, &msg, "reason", "reason of the update"),

+		OPT_BOOLEAN('d', NULL, &delete, "deletes the reference"),

+		OPT_BOOLEAN( 0 , "no-deref", &no_deref,

+					"update <refname> not the one it points to"),

+	};
-	delete = 0;

-	ref_flags = 0;

 	git_config(git_default_config);

-

-	for (i = 1; i < argc; i++) {

-		if (!strcmp("-m", argv[i])) {

-			if (i+1 >= argc)

-				usage(git_update_ref_usage);

-			msg = argv[++i];

-			if (!*msg)

-				die("Refusing to perform update with empty message.");

-			continue;

-		}

-		if (!strcmp("-d", argv[i])) {

-			delete = 1;

-			continue;

-		}

-		if (!strcmp("--no-deref", argv[i])) {

-			ref_flags |= REF_NODEREF;

-			continue;

-		}

-		if (!refname) {

-			refname = argv[i];

-			continue;

-		}

-		if (!value) {

-			value...
[ message continues ]

Signed-off-by: Jonas Fonseca <fonseca@diku.dk>

---

 builtin-branch.c     |    1 -

 builtin-update-ref.c |    1 -

 parse-options.c      |    2 +-

 3 files changed, 1 insertions(+), 3 deletions(-)
 Pierre Habouzit <madcoder@debian.org> wrote Sat, Oct 13, 2007:

 > Signed-off-by: Pierre Habouzit <madcoder@debian.org>

 > ---

 >  builtin-update-ref.c |   71 +++++++++++++++++++++-----------------------------

 >  1 files changed, 30 insertions(+), 41 deletions(-)

 >

 > diff --git a/builtin-update-ref.c b/builtin-update-ref.c

 > index fe1f74c..eafb642 100644

 > --- a/builtin-update-ref.c

 > +++ b/builtin-update-ref.c

 > @@ -1,59 +1,48 @@

 >  #include "cache.h"

 >  #include "refs.h"

 >  #include "builtin.h"

 > +#include "parse-options.h"

 >

 > -static const char git_update_ref_usage[] =

 > -"git-update-ref [-m <reason>] (-d <refname> <value> | [--no-deref] <refname> <value> [<oldval>])";

 > +static const char * const git_update_ref_usage[] = {

 > +	"",

 > +	"git-update-ref [options] -d <refname> <oldval>",

 > +	"git-update-ref [options]    <refname> <newval> [<oldval>]",

 > +	NULL

 > +};
 How about something like this to get rid of these empty strings

 that look strange?
	> ./git update-ref -h

	usage: git-update-ref [options] -d <refname> <oldval>

	   or: git-update-ref [options]    <refname> <newval> [<oldval>]
	    -m <reason>           reason of the update

	    -d                    deletes the reference

	    --no-deref            update <refname> not the one it points to
diff --git a/builtin-branch.c b/builtin-branch.c

index fbf983e..d7c4657 100644

--- a/builtin-branch.c

+++ b/builtin-branch.c

@@ -14,7 +14,6 @@

 #include "parse-options.h"
 static const char * const builtin_branch_usage[] = {

-	"",

 	"git-branch [options] [-r | -a]",

 	"git-branch [options] [-l] [...
[ message continues ]

I like the idea, though we may want to have more text to explain some

things about the command, so I'll do something in between that uses or:

until an empty line is met, and just prefix the result with four spaces

else, this way we can have:
usage: git-foo ...

   or: git-foo ...
    Did you know that you can do bar with git-foo ?

    but beware that it cannot do quux.
    -m <reason>           reason of the update

    -d                    deletes the reference

    --no-deref            update <refname> not the one it points to
--=20

=C2=B7O=C2=B7  Pierre Habouzit

=C2=B7=C2=B7O                                                madcoder@debia=

n.org

OOO                                                http://www.madism.org

Signed-off-by: Pierre Habouzit <madcoder@debian.org>

---

 builtin-symbolic-ref.c |   54 +++++++++++++++++------------------------------

 1 files changed, 20 insertions(+), 34 deletions(-)
diff --git a/builtin-symbolic-ref.c b/builtin-symbolic-ref.c

index 9eb95e5..c5e5173 100644

--- a/builtin-symbolic-ref.c

+++ b/builtin-symbolic-ref.c

@@ -1,9 +1,12 @@

 #include "builtin.h"

 #include "cache.h"

 #include "refs.h"

+#include "parse-options.h"
-static const char git_symbolic_ref_usage[] =

-"git-symbolic-ref [-q] [-m <reason>] name [ref]";

+static const char * const git_symbolic_ref_usage[] = {

+	"git-symbolic-ref [options] name [ref]",

+	NULL

+};
 static void check_symref(const char *HEAD, int quiet)

 {

@@ -26,44 +29,27 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)

 {

 	int quiet = 0;

 	const char *msg = NULL;

+	struct option git_symbolic_ref_options[] = {

+		OPT_BOOLEAN('q', "quiet", &quiet, "do not issue errors"),

+		OPT_STRING( 'm', NULL, &msg, "reason", "reason of the update"),

+	};
 	git_config(git_default_config);

-

-	while (1 < argc) {

-		const char *arg = argv[1];

-		if (arg[0] != '-')

-			break;

-		else if (!strcmp("-q", arg))

-			quiet = 1;

-		else if (!strcmp("-m", arg)) {

-			argc--;

-			argv++;

-			if (argc <= 1)

-				break;

-			msg = argv[1];

-			if (!*msg)

-				die("Refusing to perform update with empty message");

-		}

-		else if (!strcmp("--", arg)) {

-			argc--;

-			argv++;

-			break;

-		}

-		else

-			die("unknown option %s", arg);

-		argc--;

-		argv++;

-	}

-

+	argc = parse_options(argc, argv, git_symbolic_ref_options,

+	                     ARRAY_SIZE(git_symbolic_ref_options),

+	                     git_symbolic_ref_usage, 0);

+	if (msg &&!*msg)

+		die("Refusing to perform update with empty message");

 	switch (argc) {

-	case 2:

-		check_symref(argv[1], quiet);

+	case 1:

+		check_symref(argv[0], quiet);

 		break;

-	case 3:

-		create_symref(argv[1], argv[2], msg...
[ message continues ]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>

---

 builtin-http-fetch.c |   67 ++++++++++++++++++++++---------------------------

 1 files changed, 30 insertions(+), 37 deletions(-)
diff --git a/builtin-http-fetch.c b/builtin-http-fetch.c

index 4a50dbd..5e1003e 100644

--- a/builtin-http-fetch.c

+++ b/builtin-http-fetch.c

@@ -1,65 +1,59 @@

 #include "cache.h"

 #include "walker.h"

+#include "parse-options.h"

+

+static char const * const http_fetch_usage[] = {

+	"git-http-fetch [option] (--stdin | <commit-id>) url",

+	NULL

+};
 int cmd_http_fetch(int argc, const char **argv, const char *prefix)

 {

 	struct walker *walker;

 	int commits_on_stdin = 0;

 	int commits;

-	const char **write_ref = NULL;

+	const char *wref = NULL, **write_ref = NULL;

 	char **commit_id;

-	const char *url;

-	int arg = 1;

 	int rc = 0;

-	int get_tree = 0;

-	int get_history = 0;

-	int get_all = 0;

+	int get_tree = 0, get_history = 0, get_all = 0;

 	int get_verbosely = 0;

 	int get_recover = 0;

+	struct option http_fetch_options[] = {

+		OPT_BOOLEAN('c', NULL, &get_history, "get the commit objects"),

+		OPT_BOOLEAN('t', NULL, &get_tree, "get the trees"),

+		OPT_BOOLEAN('a', NULL, &get_all, "get all the objects"),

+		OPT_BOOLEAN('v', "verbose", &get_verbosely, "be verbose"),

+		OPT_STRING( 'w', NULL, &wref, "file",

+					"write the commit-id into a $GIT/refs/<file>"),

+		OPT_BOOLEAN( 0 , "recover", &get_recover, "recover from an interrupted fetch"),

+		OPT_BOOLEAN( 0 , "stdin", &commits_on_stdin, "read the commit id's from stdin"),

+	};
 	git_config(git_default_config);

-

-	while (arg < argc && argv[arg][0] == '-') {

-		if (argv[arg][1] == 't') {

-			get_tree = 1;

-		} else if (argv[arg][1] == 'c') {

-			get_history = 1;

-		} else if (argv[arg][1] == 'a') {

-			get_all = 1;

-			get_tree = 1;

-			get_history = 1;

-		} else if (argv[arg][1] == 'v') {

-			get_verbosely = 1;

-		} else if (argv[arg][1] == 'w') {

-			write_r...
[ message continues ]