Git development
 help / color / mirror / Atom feed
* Re: Failed to clone http://repo.or.cz/w/msysgit.git
From: Abdelrazak Younes @ 2007-12-17 17:40 UTC (permalink / raw)
  To: msysgit-/JYPxA39Uh5TLH3MbocFFw; +Cc: git-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <Pine.LNX.4.64.0712171702360.9446-OGWIkrnhIhzN0uC3ymp8PA@public.gmane.org>


Johannes Schindelin wrote:
> Hi,
> 
> [please do not cull me from the Cc: list when you reply to me.  This is 
>  like replying to me, but talking away from me (to the public).]

Sorry, I am afraid I cannot do that right now because I use the gmane 
news interface and I don't have access to my smtp from here. If it is 
too irritating I'll wait up until I go back home before I answer. In any 
case, sorry for the inconvenience.

> 
> On Mon, 17 Dec 2007, Abdelrazak Younes wrote:
> 
>> Johannes Schindelin wrote:
>>> On Mon, 17 Dec 2007, Abdelrazak Younes wrote:
>>>
>>>> I am trying to clone the msysgit repository but I get this error:
>>>>
>>>> $ git clone http://repo.or.cz/w/msysgit.git
>>>> Initialized empty Git repository in d:/devel/git/msysgit/.git/
>>>> D:/program/Git/bin/git-clone: line 144: /bin/git-http-fetch: Bad file
>>>> number
> 
> Okay, I get the same error here.  Fishy.  But then, this is not the 
> correct URL either (so I think Peff's patch probably fixes it).
> 
> What you tried is the gitweb URL, which explicitely says
> 
> 	Mirror URL	git://repo.or.cz/msysgit.git
> 
> 			http://repo.or.cz/r/msysgit.git
> 
> IOW replace your "/w/" by "/r/" and have fun,

Yes, it works! Thanks and sorry for the mixup, I just copied and pasted 
the link without thinking.

Abdel.

^ permalink raw reply

* Re: [PATCH] Re-re-re-fix common tail optimization
From: Junio C Hamano @ 2007-12-17 17:58 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Johannes Sixt, Wincent Colaiuta, Jeff King, Linus Torvalds, git
In-Reply-To: <Pine.LNX.4.64.0712171250550.9446@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> >> But as at least one released version of GNU diff has a pretty serious
>> >> bug,
>> >> I would rather not rely too much on diff.  (BTW this was the reason I
>> >> wanted --no-index so badly.)
>> >>
>> >> So yeah, the second "diff" cannot be "git diff".  Maybe "cmp", but not
>> >> "git diff".
>> > 
>> > Well cmp would be fine as well, seeing all we want is a boolean "is 
>> > this the same or not" answer. (I'm not familiar with the GNU diff bug 
>> > you speak of, but was it so bad that it couldn't even get *that* 
>> > answer right?)
>> 
>> Heh, there's at least one distribution out there (Suse 10.1) that comes 
>> with a *cmp* that doesn't get that answer right if its output is 
>> connected to /dev/null, which is the case when you simply 'make test'.
>
> Yeah.  That's what it was.  I even posted a patch to GNU diff, only to 
> find out that it was already fixed in CVS.  Sigh.

Wait.  Are you still talking about diff or cmp, or are you saying that
your earlier statement about avoiding GNU diff due to its bugs is
unfounded?

^ permalink raw reply

* Re: The Reposithon!  Take 2 [URL FIX]
From: Junio C Hamano @ 2007-12-17 18:00 UTC (permalink / raw)
  To: Sam Vilain; +Cc: Nicholas Clark, Perl 5 Porters, Git Mailing List
In-Reply-To: <476664EB.5030500@vilain.net>

Sam Vilain <sam@vilain.net> writes:

> The error message isn't the best, no.
>
> Junio, any chance of this going in?

Looks innocuous and an obvious improvement to me.

^ permalink raw reply

* Re: The Reposithon!  Take 2 [URL FIX]
From: Nicholas Clark @ 2007-12-17 18:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Sam Vilain, Perl 5 Porters, Git Mailing List
In-Reply-To: <7vmys98ali.fsf@gitster.siamese.dyndns.org>

On Mon, Dec 17, 2007 at 10:00:57AM -0800, Junio C Hamano wrote:
> Sam Vilain <sam@vilain.net> writes:
> 
> > The error message isn't the best, no.
> >
> > Junio, any chance of this going in?
> 
> Looks innocuous and an obvious improvement to me.

Cool. Thanks to you both for addressing this concern of mine so rapidly.

Nicholas Clark

^ permalink raw reply

* Re: [PATCH] Re-re-re-fix common tail optimization
From: Johannes Schindelin @ 2007-12-17 18:05 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Sixt, Wincent Colaiuta, Jeff King, Linus Torvalds, git
In-Reply-To: <7vsl218aqd.fsf@gitster.siamese.dyndns.org>

Hi,

On Mon, 17 Dec 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> >> But as at least one released version of GNU diff has a pretty 
> >> >> serious bug, I would rather not rely too much on diff.  (BTW this 
> >> >> was the reason I wanted --no-index so badly.)
> >> >>
> >> >> So yeah, the second "diff" cannot be "git diff".  Maybe "cmp", but 
> >> >> not "git diff".
> >> > 
> >> > Well cmp would be fine as well, seeing all we want is a boolean "is 
> >> > this the same or not" answer. (I'm not familiar with the GNU diff 
> >> > bug you speak of, but was it so bad that it couldn't even get 
> >> > *that* answer right?)
> >> 
> >> Heh, there's at least one distribution out there (Suse 10.1) that 
> >> comes with a *cmp* that doesn't get that answer right if its output 
> >> is connected to /dev/null, which is the case when you simply 'make 
> >> test'.
> >
> > Yeah.  That's what it was.  I even posted a patch to GNU diff, only to 
> > find out that it was already fixed in CVS.  Sigh.
> 
> Wait.  Are you still talking about diff or cmp, or are you saying that 
> your earlier statement about avoiding GNU diff due to its bugs is 
> unfounded?

I do not remember offhand.  I only remembered that it was the GNU diff 
package.  But maybe it was only the "cmp" tool.  Symptoms were that tests 
were failing without "-i", but succeeding with "-i".

Okay, I found the mail:

http://article.gmane.org/gmane.comp.version-control.git/25107/match=cmp

Seems it was only "cmp".

Sorry for the noise,
Dscho

^ permalink raw reply

* [PATCH] the use of 'tr' in the test suite isn't really portable
From: H.Merijn Brand @ 2007-12-17 18:15 UTC (permalink / raw)
  To: git

Some versions of 'tr' only accept octal codes if entered with three digits,
and therefor misinterpret the '\0' in the test suite.

Some versions of 'tr' reject the (needless) use of character classes.

Signed-off-by: H.Merijn Brand <h.m.brand@xs4all.nl>
---

diff -pur git-2007-12-10_01/git-filter-branch.sh git-2007-12-10/git-filter-branch.sh
--- git-2007-12-10_01/git-filter-branch.sh      2007-12-09 10:23:48 +0100
+++ git-2007-12-10/git-filter-branch.sh 2007-12-11 13:39:02 +0100
@@ -290,7 +290,7 @@ while read commit parents; do
                eval "$filter_tree" < /dev/null ||
                        die "tree filter failed: $filter_tree"

-               git diff-index -r $commit | cut -f 2- | tr '\n' '\0' | \
+               git diff-index -r $commit | cut -f 2- | tr '\n' '\000' | \
                        xargs -0 git update-index --add --replace --remove
                git ls-files -z --others | \
                        xargs -0 git update-index --add --replace --remove
diff -pur git-2007-12-10_01/t/diff-lib.sh git-2007-12-10/t/diff-lib.sh
--- git-2007-12-10_01/t/diff-lib.sh     2007-12-09 10:23:48 +0100
+++ git-2007-12-10/t/diff-lib.sh        2007-12-11 13:39:56 +0100
@@ -21,8 +21,8 @@ compare_diff_raw_z () {
     # Also we do not check SHA1 hash generation in this test, which
     # is a job for t0000-basic.sh

-    tr '\0' '\012' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
-    tr '\0' '\012' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
+    tr '\000' '\012' <"$1" | sed -e "$sanitize_diff_raw_z" >.tmp-1
+    tr '\000' '\012' <"$2" | sed -e "$sanitize_diff_raw_z" >.tmp-2
     git diff .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2
 }

diff -pur git-2007-12-10_01/t/t0020-crlf.sh git-2007-12-10/t/t0020-crlf.sh
--- git-2007-12-10_01/t/t0020-crlf.sh   2007-12-09 10:23:48 +0100
+++ git-2007-12-10/t/t0020-crlf.sh      2007-12-10 14:25:58 +0100
@@ -5,7 +5,7 @@ test_description='CRLF conversion'
 . ./test-lib.sh

 q_to_nul () {
-       tr Q '\0'
+       tr Q '\000'
 }

 append_cr () {
diff -pur git-2007-12-10_01/t/t1300-repo-config.sh git-2007-12-10/t/t1300-repo-config.sh
--- git-2007-12-10_01/t/t1300-repo-config.sh    2007-12-09 10:23:48 +0100
+++ git-2007-12-10/t/t1300-repo-config.sh       2007-12-10 09:49:48 +0100
@@ -591,12 +591,12 @@ Qsection.sub=section.val4
 Qsection.sub=section.val5Q
 EOF

-git config --null --list | tr '[\000]' 'Q' > result
+git config --null --list | tr '\000' 'Q' > result
 echo >>result

 test_expect_success '--null --list' 'cmp result expect'

-git config --null --get-regexp 'val[0-9]' | tr '[\000]' 'Q' > result
+git config --null --get-regexp 'val[0-9]' | tr '\000' 'Q' > result
 echo >>result

 test_expect_success '--null --get-regexp' 'cmp result expect'
diff -pur git-2007-12-10_01/t/t3300-funny-names.sh git-2007-12-10/t/t3300-funny-names.sh
--- git-2007-12-10_01/t/t3300-funny-names.sh    2007-12-09 10:23:48 +0100
+++ git-2007-12-10/t/t3300-funny-names.sh       2007-12-11 13:40:32 +0100
@@ -54,7 +54,7 @@ echo 'just space
 no-funny
 tabs   ," (dq) and spaces' >expected
 test_expect_success 'git ls-files -z with-funny' \
-       'git ls-files -z | tr \\0 \\012 >current &&
+       'git ls-files -z | tr \\000 \\012 >current &&
        git diff expected current'

 t1=`git write-tree`
@@ -83,11 +83,11 @@ test_expect_success 'git diff-tree with-
 echo 'A
 tabs   ," (dq) and spaces' >expected
 test_expect_success 'git diff-index -z with-funny' \
-       'git diff-index -z --name-status $t0 | tr \\0 \\012 >current &&
+       'git diff-index -z --name-status $t0 | tr \\000 \\012 >current &&
        git diff expected current'

 test_expect_success 'git diff-tree -z with-funny' \
-       'git diff-tree -z --name-status $t0 $t1 | tr \\0 \\012 >current &&
+       'git diff-tree -z --name-status $t0 $t1 | tr \\000 \\012 >current &&
        git diff expected current'

 cat > expected <<\EOF
diff -pur git-2007-12-10_01/t/t4020-diff-external.sh git-2007-12-10/t/t4020-diff-external.sh
--- git-2007-12-10_01/t/t4020-diff-external.sh  2007-12-09 10:23:48 +0100
+++ git-2007-12-10/t/t4020-diff-external.sh     2007-12-11 13:40:44 +0100
@@ -99,7 +99,7 @@ test_expect_success 'no diff with -diff'
        git diff | grep Binary
 '

-echo NULZbetweenZwords | tr Z '\0' > file
+echo NULZbetweenZwords | tr Z '\000' > file

 test_expect_success 'force diff with "diff"' '
        echo >.gitattributes "file diff" &&
diff -pur git-2007-12-10_01/t/t4103-apply-binary.sh git-2007-12-10/t/t4103-apply-binary.sh
--- git-2007-12-10_01/t/t4103-apply-binary.sh   2007-12-09 10:23:48 +0100
+++ git-2007-12-10/t/t4103-apply-binary.sh      2007-12-11 13:40:57 +0100
@@ -24,10 +24,10 @@ git update-index --add --remove file1 fi
 git-commit -m 'Initial Version' 2>/dev/null

 git-checkout -b binary
-tr 'x' '\0' <file1 >file3
+tr 'x' '\000' <file1 >file3
 cat file3 >file4
 git add file2
-tr '\0' 'v' <file3 >file1
+tr '\000' 'v' <file3 >file1
 rm -f file2
 git update-index --add --remove file1 file2 file3 file4
 git-commit -m 'Second Version'
diff -pur git-2007-12-10_01/t/t4116-apply-reverse.sh git-2007-12-10/t/t4116-apply-reverse.sh
--- git-2007-12-10_01/t/t4116-apply-reverse.sh  2007-12-09 10:23:48 +0100
+++ git-2007-12-10/t/t4116-apply-reverse.sh     2007-12-11 13:42:13 +0100
@@ -12,14 +12,14 @@ test_description='git apply in reverse
 test_expect_success setup '

        for i in a b c d e f g h i j k l m n; do echo $i; done >file1 &&
-       tr "[ijk]" '\''[\0\1\2]'\'' <file1 >file2 &&
+       tr "ijk" '\''\000\001\002'\'' <file1 >file2 &&

        git add file1 file2 &&
        git commit -m initial &&
        git tag initial &&

        for i in a b c g h i J K L m o n p q; do echo $i; done >file1 &&
-       tr "[mon]" '\''[\0\1\2]'\'' <file1 >file2 &&
+       tr "mon" '\''\000\001\002'\'' <file1 >file2 &&

        git commit -a -m second &&
        git tag second &&
diff -pur git-2007-12-10_01/t/t4200-rerere.sh git-2007-12-10/t/t4200-rerere.sh
--- git-2007-12-10_01/t/t4200-rerere.sh 2007-12-09 10:23:48 +0100
+++ git-2007-12-10/t/t4200-rerere.sh    2007-12-11 13:42:28 +0100
@@ -129,7 +129,7 @@ test_expect_success 'rerere kicked in' "
 test_expect_success 'rerere prefers first change' 'git diff a1 expect'

 rm $rr/postimage
-echo "$sha1    a1" | tr '\012' '\0' > .git/rr-cache/MERGE_RR
+echo "$sha1    a1" | tr '\012' '\000' > .git/rr-cache/MERGE_RR

 test_expect_success 'rerere clear' 'git rerere clear'

diff -pur git-2007-12-10_01/t/t5300-pack-object.sh git-2007-12-10/t/t5300-pack-object.sh
--- git-2007-12-10_01/t/t5300-pack-object.sh    2007-12-09 10:23:48 +0100
+++ git-2007-12-10/t/t5300-pack-object.sh       2007-12-11 13:42:46 +0100
@@ -15,7 +15,7 @@ test_expect_success \
     'rm -f .git/index*
      for i in a b c
      do
-            dd if=/dev/zero bs=4k count=1 | tr "\\0" $i >$i &&
+            dd if=/dev/zero bs=4k count=1 | tr "\\000" $i >$i &&
             git update-index --add $i || return 1
      done &&
      cat c >d && echo foo >>d && git update-index --add d &&
diff -pur git-2007-12-10_01/test-sha1.sh git-2007-12-10/test-sha1.sh
--- git-2007-12-10_01/test-sha1.sh      2007-12-09 10:23:48 +0100
+++ git-2007-12-10/test-sha1.sh 2007-12-11 13:39:29 +0100
@@ -10,7 +10,7 @@ do
                {
                        test -z "$pfx" || echo "$pfx"
                        dd if=/dev/zero bs=1048576 count=$cnt 2>/dev/null |
-                       tr '[\0]' '[g]'
+                       tr '\000' 'g'
                } | ./test-sha1 $cnt
        `
        if test "$expect" = "$actual"
@@ -55,7 +55,7 @@ do
                {
                        test -z "$pfx" || echo "$pfx"
                        dd if=/dev/zero bs=1048576 count=$cnt 2>/dev/null |
-                       tr '[\0]' '[g]'
+                       tr '\000' 'g'
                } | sha1sum |
                sed -e 's/ .*//'
        `
-- 
git-2007-12-17

-- 
H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)
using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,
& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org
http://mirrors.develooper.com/hpux/            http://www.test-smoke.org
                        http://www.goldmark.org/jeff/stupid-disclaimers/

^ permalink raw reply

* [PATCH] HP-UX does not have select.h
From: H.Merijn Brand @ 2007-12-17 18:23 UTC (permalink / raw)
  To: git

HP-UX does not have select.h, but it offers all select () functionality.
The defines are in <sys/types.h> and <X11/fd.h>

Signed-off-by: H.Merijn Brand <h.m.brand@xs4all.nl>
---

diff -pur git-2007-12-10_01/git-compat-util.h git-2007-12-10/git-compat-util.h
--- git-2007-12-10_01/git-compat-util.h	2007-12-09 10:23:48 +0100
+++ git-2007-12-10/git-compat-util.h	2007-12-10 14:25:26 +0100
@@ -68,7 +68,9 @@
 #include <sys/poll.h>
 #include <sys/socket.h>
 #include <sys/ioctl.h>
+#ifndef _HPUX_SOURCE
 #include <sys/select.h>
+#endif
 #include <assert.h>
 #include <regex.h>
 #include <netinet/in.h>

-- 
git-2007-12-17

-- 
H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)
using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,
& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org
http://mirrors.develooper.com/hpux/            http://www.test-smoke.org
                        http://www.goldmark.org/jeff/stupid-disclaimers/

^ permalink raw reply

* [PATCH 2/7] parse-options: allow callbacks to ignore arguments they don't need to use.
From: Pierre Habouzit @ 2007-12-17 18:23 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1197915797-30679-2-git-send-email-madcoder@debian.org>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 parse-options.c |   22 +++++++++++++++-------
 parse-options.h |    7 +++++++
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 8f70e5d..d716ccc 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -38,7 +38,10 @@ static int get_value(struct optparse_t *p,
 {
 	const char *s, *arg;
 	const int unset = flags & PARSE_OPT_UNSET;
+	int may_ign = 0, res;
 
+	if (!unset && !p->opt && (opt->flags & PARSE_OPT_OPTARG))
+		may_ign = PARSE_OPT_MAY_IGN;
 	if (unset && p->opt)
 		return opterror(opt, "takes no value", flags);
 	if (unset && (opt->flags & PARSE_OPT_NONEG))
@@ -86,7 +89,7 @@ static int get_value(struct optparse_t *p,
 			*(const char **)opt->value = NULL;
 			return 0;
 		}
-		if (opt->flags & PARSE_OPT_OPTARG && (!arg || *arg == '-')) {
+		if (may_ign && (!arg || *arg == '-')) {
 			*(const char **)opt->value = (const char *)opt->defval;
 			return 0;
 		}
@@ -98,18 +101,23 @@ static int get_value(struct optparse_t *p,
 	case OPTION_CALLBACK:
 		if (unset || (opt->flags & PARSE_OPT_NOARG))
 			return (*opt->callback)(opt, NULL, flags);
-		if (opt->flags & PARSE_OPT_OPTARG && (!arg || *arg == '-'))
-			return (*opt->callback)(opt, NULL, flags);
-		if (!arg)
+		if (!may_ign && !arg)
 			return opterror(opt, "requires a value", flags);
-		return (*opt->callback)(opt, get_arg(p), flags);
+		if (may_ign && arg && arg[0] == '-' && arg[1])
+			return (*opt->callback)(opt, NULL, flags);
+		res = (*opt->callback)(opt, arg, flags);
+		if (!may_ign && res == PARSE_OPT_IGNORE)
+			die("should not happen: MAY_IGN unset, but arg was IGNOREd");
+		if (res == PARSE_OPT_IGNORE)
+			get_arg(p);
+		return res;
 
 	case OPTION_INTEGER:
 		if (unset) {
 			*(int *)opt->value = 0;
 			return 0;
 		}
-		if (opt->flags & PARSE_OPT_OPTARG && (!arg || !isdigit(*arg))) {
+		if (may_ign && (!arg || !isdigit(*arg))) {
 			*(int *)opt->value = opt->defval;
 			return 0;
 		}
@@ -251,7 +259,7 @@ 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(&args, arg + 2, options) < 0)
 			usage_with_options(usagestr, options);
 	}
 
diff --git a/parse-options.h b/parse-options.h
index ae6b3ca..eeb40a4 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -27,9 +27,16 @@ enum parse_opt_option_flags {
 	PARSE_OPT_HIDDEN  = 8,
 };
 
+enum parse_opt_cbres {
+	PARSE_OPT_ERR     = -1,
+	PARSE_OPT_OK      =  0,
+	PARSE_OPT_IGNORE  =  1,
+};
+
 enum parse_opt_cbflags {
 	PARSE_OPT_SHORT   = 1,
 	PARSE_OPT_UNSET   = 2,
+	PARSE_OPT_MAY_IGN = 4,
 };
 
 struct option;
-- 
1.5.4.rc0.1148.ga3ab1-dirty

^ permalink raw reply related

* [proposal] make parse-options nicer wrt optional arguments (supersedes all my recent posts on the matter)
From: Pierre Habouzit @ 2007-12-17 18:23 UTC (permalink / raw)
  To: gitster; +Cc: git

Here is a series that aims at fixing the various issues with
parse-options that were raised recently.

* preliminary patch:
  [PATCH 1/7] parse-options: Make callbacks take flags instead of boolean `unset`

* teach git parse-options to allow callbacks to ignore arguments that
  don't seem to be theirs, refactors:
  [PATCH 2/7] parse-options: allow callbacks to ignore arguments they don't need to use.
  [PATCH 3/7] parse-options: Let the integer/string cases be callbacks as well.
  [PATCH 4/7] parse-options: let OPT__ABBREV ignore arguments.

* Document this (my previous proposal + Junio's squashed):
  [PATCH 5/7] parse-options: Add a gitcli(5) man page.

* Implement my `{}` proposal, a sed -e s/{}/_/ will replace {} with _
  as a wildcard. Contains documentation for this placeholder.
  [PATCH 6/7] parse-options: have a `use default value` wildcard.

* Somehow unrelated patch, but still parse-option related (resend):
  [PATCH 7/7] git-tag: fix -l switch handling regression.



This has been pushed as my ph/parseopt branch on
git://git.madism.org/git.git.

^ permalink raw reply

* [PATCH 4/7] parse-options: let OPT__ABBREV ignore arguments.
From: Pierre Habouzit @ 2007-12-17 18:23 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1197915797-30679-4-git-send-email-madcoder@debian.org>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 parse-options.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index f3f0f2a..679a963 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -359,19 +359,21 @@ void usage_with_options(const char * const *usagestr,
 
 int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int flags)
 {
-	int v;
-
-	if (!arg) {
-		v = flags & PARSE_OPT_UNSET ? 0 : DEFAULT_ABBREV;
-	} else {
+	int v = flags & PARSE_OPT_UNSET ? 0 : DEFAULT_ABBREV;
+	if (arg) {
 		v = strtol(arg, (char **)&arg, 10);
-		if (*arg)
+		if (*arg) {
+			if (flags & PARSE_OPT_MAY_IGN) {
+				*(int *)opt->value = DEFAULT_ABBREV;
+				return PARSE_OPT_IGNORE;
+			}
 			return opterror(opt, "expects a numerical value", 0);
+		}
 		if (v && v < MINIMUM_ABBREV)
 			v = MINIMUM_ABBREV;
 		else if (v > 40)
 			v = 40;
 	}
-	*(int *)(opt->value) = v;
+	*(int *)opt->value = v;
 	return 0;
 }
-- 
1.5.4.rc0.1148.ga3ab1-dirty

^ permalink raw reply related

* [PATCH 3/7] parse-options: Let the integer/string cases be callbacks as well.
From: Pierre Habouzit @ 2007-12-17 18:23 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1197915797-30679-3-git-send-email-madcoder@debian.org>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 parse-options.c |  115 ++++++++++++++++++++++++++++---------------------------
 1 files changed, 59 insertions(+), 56 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index d716ccc..f3f0f2a 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -7,15 +7,14 @@ struct optparse_t {
 	const char *opt;
 };
 
-static inline const char *get_arg(struct optparse_t *p)
+static inline void use_arg(struct optparse_t *p)
 {
 	if (p->opt) {
-		const char *res = p->opt;
 		p->opt = NULL;
-		return res;
+	} else {
+		p->argc--;
+		++p->argv;
 	}
-	p->argc--;
-	return *++p->argv;
 }
 
 static inline const char *skip_prefix(const char *str, const char *prefix)
@@ -33,15 +32,62 @@ static int opterror(const struct option *opt, const char *reason, int flags)
 	return error("option `%s' %s", opt->long_name, reason);
 }
 
+static int parse_opt_string(const struct option *opt,
+                            const char *arg, int flags)
+{
+	*(const char **)opt->value = flags & PARSE_OPT_UNSET ? NULL : arg;
+	return 0;
+}
+
+static int parse_opt_integer(const struct option *opt,
+                             const char *arg, int flags)
+{
+	int v = flags & PARSE_OPT_UNSET ? 0 : opt->defval;
+	if (arg) {
+		v = strtol(arg, (char **)&arg, 10);
+		if (*arg) {
+			if (flags & PARSE_OPT_MAY_IGN) {
+				*(int *)opt->value = opt->defval;
+				return PARSE_OPT_IGNORE;
+			}
+			return opterror(opt, "expects a numerical value", 0);
+		}
+	}
+	*(int *)opt->value = v;
+	return 0;
+}
+
+static int run_callback(struct optparse_t *p, parse_opt_cb *cb,
+						const struct option *opt, int flags)
+{
+	const char *arg = p->opt ? p->opt : (p->argc > 1 ? p->argv[1] : NULL);
+	int may_ign = 0;
+
+	if (!p->opt && (opt->flags & PARSE_OPT_OPTARG))
+		may_ign = PARSE_OPT_MAY_IGN;
+	if ((flags & PARSE_OPT_UNSET) || (opt->flags & PARSE_OPT_NOARG))
+		return (*cb)(opt, NULL, flags);
+	if (!may_ign && !arg)
+		return opterror(opt, "requires a value", flags);
+	if (may_ign && arg && arg[0] == '-' && arg[1])
+		return (*cb)(opt, NULL, flags);
+	switch ((*cb)(opt, arg, flags | may_ign)) {
+	case PARSE_OPT_OK:
+		use_arg(p);
+		return PARSE_OPT_OK;
+	case PARSE_OPT_IGNORE:
+		if (!may_ign)
+			die("should not happen: MAY_IGN unset, but arg was IGNOREd");
+		return PARSE_OPT_IGNORE;
+	default:
+		return PARSE_OPT_ERR;
+	}
+}
+
 static int get_value(struct optparse_t *p,
                      const struct option *opt, int flags)
 {
-	const char *s, *arg;
 	const int unset = flags & PARSE_OPT_UNSET;
-	int may_ign = 0, res;
-
-	if (!unset && !p->opt && (opt->flags & PARSE_OPT_OPTARG))
-		may_ign = PARSE_OPT_MAY_IGN;
 	if (unset && p->opt)
 		return opterror(opt, "takes no value", flags);
 	if (unset && (opt->flags & PARSE_OPT_NONEG))
@@ -63,7 +109,6 @@ static int get_value(struct optparse_t *p,
 		}
 	}
 
-	arg = p->opt ? p->opt : (p->argc > 1 ? p->argv[1] : NULL);
 	switch (opt->type) {
 	case OPTION_BIT:
 		if (unset)
@@ -71,63 +116,21 @@ static int get_value(struct optparse_t *p,
 		else
 			*(int *)opt->value |= opt->defval;
 		return 0;
-
 	case OPTION_BOOLEAN:
 		*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
 		return 0;
-
 	case OPTION_SET_INT:
 		*(int *)opt->value = unset ? 0 : opt->defval;
 		return 0;
-
 	case OPTION_SET_PTR:
 		*(void **)opt->value = unset ? NULL : (void *)opt->defval;
 		return 0;
-
 	case OPTION_STRING:
-		if (unset) {
-			*(const char **)opt->value = NULL;
-			return 0;
-		}
-		if (may_ign && (!arg || *arg == '-')) {
-			*(const char **)opt->value = (const char *)opt->defval;
-			return 0;
-		}
-		if (!arg)
-			return opterror(opt, "requires a value", flags);
-		*(const char **)opt->value = get_arg(p);
-		return 0;
-
+		return run_callback(p, &parse_opt_string, opt, flags);
 	case OPTION_CALLBACK:
-		if (unset || (opt->flags & PARSE_OPT_NOARG))
-			return (*opt->callback)(opt, NULL, flags);
-		if (!may_ign && !arg)
-			return opterror(opt, "requires a value", flags);
-		if (may_ign && arg && arg[0] == '-' && arg[1])
-			return (*opt->callback)(opt, NULL, flags);
-		res = (*opt->callback)(opt, arg, flags);
-		if (!may_ign && res == PARSE_OPT_IGNORE)
-			die("should not happen: MAY_IGN unset, but arg was IGNOREd");
-		if (res == PARSE_OPT_IGNORE)
-			get_arg(p);
-		return res;
-
+		return run_callback(p, opt->callback, opt, flags);
 	case OPTION_INTEGER:
-		if (unset) {
-			*(int *)opt->value = 0;
-			return 0;
-		}
-		if (may_ign && (!arg || !isdigit(*arg))) {
-			*(int *)opt->value = opt->defval;
-			return 0;
-		}
-		if (!arg)
-			return opterror(opt, "requires a value", flags);
-		*(int *)opt->value = strtol(get_arg(p), (char **)&s, 10);
-		if (*s)
-			return opterror(opt, "expects a numerical value", flags);
-		return 0;
-
+		return run_callback(p, &parse_opt_integer, opt, flags);
 	default:
 		die("should not happen, someone must be hit on the forehead");
 	}
-- 
1.5.4.rc0.1148.ga3ab1-dirty

^ permalink raw reply related

* [PATCH 1/7] parse-options: Make callbacks take flags instead of boolean `unset`
From: Pierre Habouzit @ 2007-12-17 18:23 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1197915797-30679-1-git-send-email-madcoder@debian.org>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-branch.c       |    2 +-
 builtin-commit.c       |    4 ++--
 builtin-fast-export.c  |    4 ++--
 builtin-for-each-ref.c |    2 +-
 builtin-tag.c          |    2 +-
 parse-options.c        |   37 ++++++++++++++++---------------------
 parse-options.h        |    7 ++++++-
 7 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index 089cae5..677eee5 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -531,7 +531,7 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 		die("Branch is renamed, but update of config-file failed");
 }
 
-static int opt_parse_with_commit(const struct option *opt, const char *arg, int unset)
+static int opt_parse_with_commit(const struct option *opt, const char *arg, int flags)
 {
 	unsigned char sha1[20];
 	struct commit *commit;
diff --git a/builtin-commit.c b/builtin-commit.c
index 0a91013..ca18a5c 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -52,10 +52,10 @@ static int no_edit, initial_commit, in_merge;
 const char *only_include_assumed;
 struct strbuf message;
 
-static int opt_parse_m(const struct option *opt, const char *arg, int unset)
+static int opt_parse_m(const struct option *opt, const char *arg, int flags)
 {
 	struct strbuf *buf = opt->value;
-	if (unset)
+	if (flags & PARSE_OPT_UNSET)
 		strbuf_setlen(buf, 0);
 	else {
 		strbuf_addstr(buf, arg);
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index ef27eee..9f914b9 100755
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -26,9 +26,9 @@ static int progress;
 static enum { VERBATIM, WARN, STRIP, ABORT } signed_tag_mode = ABORT;
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
-				     const char *arg, int unset)
+				     const char *arg, int flags)
 {
-	if (unset || !strcmp(arg, "abort"))
+	if (flags & PARSE_OPT_UNSET || !strcmp(arg, "abort"))
 		signed_tag_mode = ABORT;
 	else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
 		signed_tag_mode = VERBATIM;
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index f36a43c..3eecfe9 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -802,7 +802,7 @@ static struct ref_sort *default_sort(void)
 	return sort;
 }
 
-int opt_parse_sort(const struct option *opt, const char *arg, int unset)
+int opt_parse_sort(const struct option *opt, const char *arg, int flags)
 {
 	struct ref_sort **sort_tail = opt->value;
 	struct ref_sort *s;
diff --git a/builtin-tag.c b/builtin-tag.c
index 274901a..fd44b2e 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -348,7 +348,7 @@ struct msg_arg {
 	struct strbuf buf;
 };
 
-static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
+static int parse_msg_arg(const struct option *opt, const char *arg, int flags)
 {
 	struct msg_arg *msg = opt->value;
 
diff --git a/parse-options.c b/parse-options.c
index e12b428..8f70e5d 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1,9 +1,6 @@
 #include "git-compat-util.h"
 #include "parse-options.h"
 
-#define OPT_SHORT 1
-#define OPT_UNSET 2
-
 struct optparse_t {
 	const char **argv;
 	int argc;
@@ -29,9 +26,9 @@ static inline const char *skip_prefix(const char *str, const char *prefix)
 
 static int opterror(const struct option *opt, const char *reason, int flags)
 {
-	if (flags & OPT_SHORT)
+	if (flags & PARSE_OPT_SHORT)
 		return error("switch `%c' %s", opt->short_name, reason);
-	if (flags & OPT_UNSET)
+	if (flags & PARSE_OPT_UNSET)
 		return error("option `no-%s' %s", opt->long_name, reason);
 	return error("option `%s' %s", opt->long_name, reason);
 }
@@ -40,14 +37,14 @@ static int get_value(struct optparse_t *p,
                      const struct option *opt, int flags)
 {
 	const char *s, *arg;
-	const int unset = flags & OPT_UNSET;
+	const int unset = flags & PARSE_OPT_UNSET;
 
 	if (unset && p->opt)
 		return opterror(opt, "takes no value", flags);
 	if (unset && (opt->flags & PARSE_OPT_NONEG))
 		return opterror(opt, "isn't available", flags);
 
-	if (!(flags & OPT_SHORT) && p->opt) {
+	if (!(flags & PARSE_OPT_SHORT) && p->opt) {
 		switch (opt->type) {
 		case OPTION_CALLBACK:
 			if (!(opt->flags & PARSE_OPT_NOARG))
@@ -99,15 +96,13 @@ static int get_value(struct optparse_t *p,
 		return 0;
 
 	case OPTION_CALLBACK:
-		if (unset)
-			return (*opt->callback)(opt, NULL, 1);
-		if (opt->flags & PARSE_OPT_NOARG)
-			return (*opt->callback)(opt, NULL, 0);
+		if (unset || (opt->flags & PARSE_OPT_NOARG))
+			return (*opt->callback)(opt, NULL, flags);
 		if (opt->flags & PARSE_OPT_OPTARG && (!arg || *arg == '-'))
-			return (*opt->callback)(opt, NULL, 0);
+			return (*opt->callback)(opt, NULL, flags);
 		if (!arg)
 			return opterror(opt, "requires a value", flags);
-		return (*opt->callback)(opt, get_arg(p), 0);
+		return (*opt->callback)(opt, get_arg(p), flags);
 
 	case OPTION_INTEGER:
 		if (unset) {
@@ -135,7 +130,7 @@ static int parse_short_opt(struct optparse_t *p, const struct option *options)
 	for (; options->type != OPTION_END; options++) {
 		if (options->short_name == *p->opt) {
 			p->opt = p->opt[1] ? p->opt + 1 : NULL;
-			return get_value(p, options, OPT_SHORT);
+			return get_value(p, options, PARSE_OPT_SHORT);
 		}
 	}
 	return error("unknown switch `%c'", *p->opt);
@@ -173,7 +168,7 @@ is_abbreviated:
 					ambiguous_option = abbrev_option;
 					ambiguous_flags = abbrev_flags;
 				}
-				if (!(flags & OPT_UNSET) && *arg_end)
+				if (!(flags & PARSE_OPT_UNSET) && *arg_end)
 					p->opt = arg_end + 1;
 				abbrev_option = options;
 				abbrev_flags = flags;
@@ -181,13 +176,13 @@ is_abbreviated:
 			}
 			/* negated and abbreviated very much? */
 			if (!prefixcmp("no-", arg)) {
-				flags |= OPT_UNSET;
+				flags |= PARSE_OPT_UNSET;
 				goto is_abbreviated;
 			}
 			/* negated? */
 			if (strncmp(arg, "no-", 3))
 				continue;
-			flags |= OPT_UNSET;
+			flags |= PARSE_OPT_UNSET;
 			rest = skip_prefix(arg + 3, options->long_name);
 			/* abbreviated and negated? */
 			if (!rest && !prefixcmp(options->long_name, arg + 3))
@@ -207,9 +202,9 @@ is_abbreviated:
 		return error("Ambiguous option: %s "
 			"(could be --%s%s or --%s%s)",
 			arg,
-			(ambiguous_flags & OPT_UNSET) ?  "no-" : "",
+			(ambiguous_flags & PARSE_OPT_UNSET) ?  "no-" : "",
 			ambiguous_option->long_name,
-			(abbrev_flags & OPT_UNSET) ?  "no-" : "",
+			(abbrev_flags & PARSE_OPT_UNSET) ?  "no-" : "",
 			abbrev_option->long_name);
 	if (abbrev_option)
 		return get_value(p, abbrev_option, abbrev_flags);
@@ -351,12 +346,12 @@ void usage_with_options(const char * const *usagestr,
 /*----- some often used options -----*/
 #include "cache.h"
 
-int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset)
+int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int flags)
 {
 	int v;
 
 	if (!arg) {
-		v = unset ? 0 : DEFAULT_ABBREV;
+		v = flags & PARSE_OPT_UNSET ? 0 : DEFAULT_ABBREV;
 	} else {
 		v = strtol(arg, (char **)&arg, 10);
 		if (*arg)
diff --git a/parse-options.h b/parse-options.h
index 102ac31..ae6b3ca 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -27,8 +27,13 @@ enum parse_opt_option_flags {
 	PARSE_OPT_HIDDEN  = 8,
 };
 
+enum parse_opt_cbflags {
+	PARSE_OPT_SHORT   = 1,
+	PARSE_OPT_UNSET   = 2,
+};
+
 struct option;
-typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
+typedef int parse_opt_cb(const struct option *, const char *arg, int flags);
 
 /*
  * `type`::
-- 
1.5.4.rc0.1148.ga3ab1-dirty

^ permalink raw reply related

* [PATCH 7/7] git-tag: fix -l switch handling regression.
From: Pierre Habouzit @ 2007-12-17 18:23 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1197915797-30679-7-git-send-email-madcoder@debian.org>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-tag.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/builtin-tag.c b/builtin-tag.c
index fd44b2e..c7a1563 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -16,7 +16,7 @@
 static const char * const git_tag_usage[] = {
 	"git-tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
 	"git-tag -d <tagname>...",
-	"git-tag [-n [<num>]] -l [<pattern>]",
+	"git-tag -l [-n [<num>]] [<pattern>]",
 	"git-tag -v <tagname>...",
 	NULL
 };
@@ -370,13 +370,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	struct ref_lock *lock;
 
 	int annotate = 0, sign = 0, force = 0, lines = 0,
-					delete = 0, verify = 0;
-	char *list = NULL, *msgfile = NULL, *keyid = NULL;
-	const char *no_pattern = "NO_PATTERN";
+		list = 0, delete = 0, verify = 0;
+	char *msgfile = NULL, *keyid = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct option options[] = {
-		{ OPTION_STRING, 'l', NULL, &list, "pattern", "list tag names",
-			PARSE_OPT_OPTARG, NULL, (intptr_t) no_pattern },
+		OPT_INTEGER('l', NULL, &list, "list tag names"),
 		{ OPTION_INTEGER, 'n', NULL, &lines, NULL,
 				"print n lines of each tag message",
 				PARSE_OPT_OPTARG, NULL, 1 },
@@ -408,7 +406,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		annotate = 1;
 
 	if (list)
-		return list_tags(list == no_pattern ? NULL : list, lines);
+		return list_tags(argv[0], lines);
 	if (delete)
 		return for_each_tag_name(argv, delete_tag);
 	if (verify)
-- 
1.5.4.rc0.1148.ga3ab1-dirty

^ permalink raw reply related

* [PATCH 5/7] parse-options: Add a gitcli(5) man page.
From: Pierre Habouzit @ 2007-12-17 18:23 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1197915797-30679-5-git-send-email-madcoder@debian.org>

This page should hold every information about the git ways to parse command
lines, and best practices to be used for scripting.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 Documentation/Makefile   |    2 +-
 Documentation/gitcli.txt |  113 ++++++++++++++++++++++++++++++++++++++++++++++
 Makefile                 |    1 +
 3 files changed, 115 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/gitcli.txt

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 76df06c..c4486d3 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -2,7 +2,7 @@ MAN1_TXT= \
 	$(filter-out $(addsuffix .txt, $(ARTICLES) $(SP_ARTICLES)), \
 		$(wildcard git-*.txt)) \
 	gitk.txt
-MAN5_TXT=gitattributes.txt gitignore.txt gitmodules.txt
+MAN5_TXT=gitattributes.txt gitignore.txt gitcli.txt gitmodules.txt
 MAN7_TXT=git.txt
 
 MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)
diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt
new file mode 100644
index 0000000..b7dcf9c
--- /dev/null
+++ b/Documentation/gitcli.txt
@@ -0,0 +1,113 @@
+gitcli(5)
+=========
+
+NAME
+----
+gitcli - git command line interface and conventions
+
+SYNOPSIS
+--------
+gitcli
+
+
+DESCRIPTION
+-----------
+
+This manual describes best practice in how to use git CLI.  Here are
+the rules that you should follow when you are scripting git:
+
+ * it's preferred to use the non dashed form of git commands, which means that
+   you should prefer `"git foo"` to `"git-foo"`.
+
+ * splitting short options to separate words (prefer `"git foo -a -b"`
+   to `"git foo -ab"`, the latter may not even work).
+
+ * when a command line option takes an argument, use the 'sticked' form.  In
+   other words, write `"git foo -oArg"` instead of `"git foo -o Arg"` for short
+   options, and `"git foo --long-opt=Arg"` instead of `"git foo --long-opt Arg"`
+   for long options.  An option that takes optional option-argument must be
+   written in the 'sticked' form.
+
+ * when you give a revision parameter to a command, make sure the parameter is
+   not ambiguous with a name of a file in the work tree.  E.g. do not write
+   `"git log -1 HEAD"` but write `"git log -1 HEAD --"`; the former will not work
+   if you happen to have a file called `HEAD` in the work tree.
+
+
+ENHANCED CLI
+------------
+From the git 1.5.4 series and further, many git commands (not all of them at the
+time of the writing though) come with an enhanced option parser.
+
+Here is an exhaustive list of the facilities provided by this option parser.
+
+
+Magic Options
+~~~~~~~~~~~~~
+Commands which have the enhanced option parser activated all understand a
+couple of magic command line options:
+
+-h::
+	gives a pretty printed usage of the command.
++
+---------------------------------------------
+$ git describe -h
+usage: git-describe [options] <committish>*
+
+    --contains            find the tag that comes after the commit
+    --debug               debug search strategy on stderr
+    --all                 use any ref in .git/refs
+    --tags                use any tag in .git/refs/tags
+    --abbrev [<n>]        use <n> digits to display SHA-1s
+    --candidates <n>      consider <n> most recent tags (default: 10)
+---------------------------------------------
+
+--help-all::
+	Some git commands take options that are only used for plumbing or that
+	are deprecated, and such options are hidden from the default usage. This
+	option gives the full list of options.
+
+
+Negating options
+~~~~~~~~~~~~~~~~
+Options with long option names can be negated by prefixing `"--no-"`. For
+example, `"git branch"` has the option `"--track"` which is 'on' by default. You
+can use `"--no-track"` to override that behaviour. The same goes for `"--color"`
+and `"--no-color"`.
+
+
+Aggregating short options
+~~~~~~~~~~~~~~~~~~~~~~~~~
+Commands that support the enhanced option parser allow you to aggregate short
+options. This means that you can for example use `"git rm -rf"` or
+`"git clean -fdx"`.
+
+
+Separating argument from the option
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+You can write the mandatory option parameter to an option as a separate
+word on the command line.  That means that all the following uses work:
+
+----------------------------
+$ git foo --long-opt=Arg
+$ git foo --long-opt Arg
+$ git foo -oArg
+$ git foo -o Arg
+----------------------------
+
+However, this is *NOT* allowed for switches with an optionnal value, where the
+'sticked' form must be used:
+----------------------------
+$ git describe --abbrev HEAD     # correct
+$ git describe --abbrev=10 HEAD  # correct
+$ git describe --abbrev 10 HEAD  # NOT WHAT YOU MEANT
+----------------------------
+
+
+Documentation
+-------------
+Documentation by Pierre Habouzit.
+
+GIT
+---
+Part of the gitlink:git[7] suite
diff --git a/Makefile b/Makefile
index 7776077..eda7b1a 100644
--- a/Makefile
+++ b/Makefile
@@ -1172,6 +1172,7 @@ check-docs::
 		documented,gitattributes | \
 		documented,gitignore | \
 		documented,gitmodules | \
+		documented,gitcli | \
 		documented,git-tools | \
 		sentinel,not,matching,is,ok ) continue ;; \
 		esac; \
-- 
1.5.4.rc0.1148.ga3ab1-dirty

^ permalink raw reply related

* [PATCH 6/7] parse-options: have a `use default value` wildcard.
From: Pierre Habouzit @ 2007-12-17 18:23 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1197915797-30679-6-git-send-email-madcoder@debian.org>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 Documentation/gitcli.txt |   20 +++++++++++++++-----
 parse-options.c          |   10 ++++++++--
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt
index b7dcf9c..a304072 100644
--- a/Documentation/gitcli.txt
+++ b/Documentation/gitcli.txt
@@ -95,14 +95,24 @@ $ git foo -oArg
 $ git foo -o Arg
 ----------------------------
 
-However, this is *NOT* allowed for switches with an optionnal value, where the
-'sticked' form must be used:
+However, this may become ambiguous for switches with an optional value. The
+enhanced option parser provides a placeholder `{}` that tells to the option
+parser that it should not try to find an argument to this switch.  Though if
+you use '{}' sticked to the option, `{}` is passed as the value.
 ----------------------------
-$ git describe --abbrev HEAD     # correct
-$ git describe --abbrev=10 HEAD  # correct
-$ git describe --abbrev 10 HEAD  # NOT WHAT YOU MEANT
+# all the following uses work
+$ git describe --abbrev HEAD
+$ git describe --abbrev {} HEAD
+$ git describe --abbrev=10 HEAD
+$ git describe --abbrev 10 HEAD
+
+# doesn't work
+$ git describe --abbrev={} HEAD
 ----------------------------
 
+Note that an optional switch will never try to use the next token as an
+argument if it starts with a dash and is not `-`.
+
 
 Documentation
 -------------
diff --git a/parse-options.c b/parse-options.c
index 679a963..8734bb1 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -69,8 +69,14 @@ static int run_callback(struct optparse_t *p, parse_opt_cb *cb,
 		return (*cb)(opt, NULL, flags);
 	if (!may_ign && !arg)
 		return opterror(opt, "requires a value", flags);
-	if (may_ign && arg && arg[0] == '-' && arg[1])
-		return (*cb)(opt, NULL, flags);
+	if (may_ign && arg) {
+		if (arg[0] == '-' && arg[1])
+			return (*cb)(opt, NULL, flags);
+		if (!strcmp(arg, "{}")) {
+			use_arg(p);
+			return (*cb)(opt, NULL, flags);
+		}
+	}
 	switch ((*cb)(opt, arg, flags | may_ign)) {
 	case PARSE_OPT_OK:
 		use_arg(p);
-- 
1.5.4.rc0.1148.ga3ab1-dirty

^ permalink raw reply related

* Re: [PATCH 1/7] parse-options: Make callbacks take flags instead of boolean `unset`
From: Pierre Habouzit @ 2007-12-17 18:54 UTC (permalink / raw)
  To: gitster; +Cc: git
In-Reply-To: <1197915797-30679-2-git-send-email-madcoder@debian.org>

[-- Attachment #1: Type: text/plain, Size: 774 bytes --]

And of course here is the MadBug #1, to be squashed:
---
 builtin-rev-parse.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 20d1789..3e8ee62 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -210,10 +210,10 @@ static int try_difference(const char *arg)
 	return 0;
 }
 
-static int parseopt_dump(const struct option *o, const char *arg, int unset)
+static int parseopt_dump(const struct option *o, const char *arg, int flags)
 {
 	struct strbuf *parsed = o->value;
-	if (unset)
+	if (flags & PARSE_OPT_UNSET)
 		strbuf_addf(parsed, " --no-%s", o->long_name);
 	else if (o->short_name)
 		strbuf_addf(parsed, " -%c", o->short_name);
-- 
1.5.4.rc0.1151.g102b0


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply related

* Re: [PATCH 7/7] git-tag: fix -l switch handling regression.
From: Pierre Habouzit @ 2007-12-17 18:56 UTC (permalink / raw)
  To: gitster; +Cc: git
In-Reply-To: <1197915797-30679-8-git-send-email-madcoder@debian.org>

[-- Attachment #1: Type: text/plain, Size: 530 bytes --]

  And I managed to resend the broken version, hurray myself.

> +		OPT_INTEGER('l', NULL, &list, "list tag names"),
                OPT_BOOLEAN



Both these last minute fixes are applied to my public git.git.

Let's now write 1000 times: I will run the test-suite before I send
patches, I will rune the test-suite before I send patches, …
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH 7/7] git-tag: fix -l switch handling regression.
From: Pierre Habouzit @ 2007-12-17 19:03 UTC (permalink / raw)
  To: gitster, git
In-Reply-To: <20071217185652.GE22554@artemis.madism.org>

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

On Mon, Dec 17, 2007 at 06:56:52PM +0000, Pierre Habouzit wrote:
>   And I managed to resend the broken version, hurray myself.
> 
> > +		OPT_INTEGER('l', NULL, &list, "list tag names"),
>                 OPT_BOOLEAN
> 
> 
> 
> Both these last minute fixes are applied to my public git.git.
> 
> Let's now write 1000 times: I will run the test-suite before I send
> patches, I will rune the test-suite before I send patches, …

  oh and t7004 doesn't pass anymore because of the:

  git -n xxx -l or git -n "" -l tests. If we really want to allow that
(but it _REALLY_ feels wrong to me) we have to make '-l' a callback that
groks non integers as 0. Else the test also has to be fixed, I'm not
sure what to do here.


-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: Windows binaries for qgit 2.0
From: Marco Costalba @ 2007-12-17 19:05 UTC (permalink / raw)
  To: Abdelrazak Younes
  Cc: git-u79uwXL29TY76Z2rM5mHXA, msysgit-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <fk5grp$7il$2@ger.gmane.org>


On Dec 17, 2007 10:51 AM, Abdelrazak Younes <younes.a-GANU6spQydw@public.gmane.org> wrote:
>
> I would like to help you with that but I can't retrieve the repository:
>
> $ git clone git://git.kernel.org/pub/scm/qgit/qgit4.git qgit4.git
> Initialized empty Git repository in d:/devel/git/qgit4/qgit4.git/.git/
> git.kernel.org[0: 130.239.17.7]: errno=Invalid argument
> git.kernel.org[1: 199.6.1.166]: errno=Bad file descriptor
> git.kernel.org[2: 204.152.191.8]: errno=Bad file descriptor
> git.kernel.org[3: 204.152.191.40]: errno=Bad file descriptor
> fatal: unable to connect a socket (Bad file descriptor)
> fetch-pack from 'git://git.kernel.org/pub/scm/qgit/qgit4.git' failed.
>

This is very strange, I can clone without problems...someone has ideas?


> $ git clone http://git.kernel.org/pub/scm/qgit/qgit4.git qgit4.git
> Initialized empty Git repository in d:/devel/git/qgit4/qgit4.git/.git/
> Cannot get remote repository information.
> Perhaps git-update-server-info needs to be run there?
>

Well, perhaps, but to clone with git protocol you don't need that.


Marco

^ permalink raw reply

* [PATCH] Plug a resource leak in threaded pack-objects code.
From: Johannes Sixt @ 2007-12-17 19:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Peter Baumann, Nicolas Pitre
In-Reply-To: <476628D1.4020300@viscovery.net>

A mutex and a condition variable is allocated for each thread and torn
down when the thread terminates. However, for certain workloads it can
happen that some threads are actually not started at all. In this case
we would leak the mutex and condition variable. Now we allocate them only
for those threads that are actually started.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
> I just discovered a theoretical resource leakage:
>
> Will send a patch this evening.

Here it is.

-- Hannes

 builtin-pack-objects.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 5765d02..e0ce114 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1670,8 +1670,6 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,
 		p[i].processed = processed;
 		p[i].working = 1;
 		p[i].data_ready = 0;
-		pthread_mutex_init(&p[i].mutex, NULL);
-		pthread_cond_init(&p[i].cond, NULL);
 
 		/* try to split chunks on "path" boundaries */
 		while (sub_size < list_size && list[sub_size]->hash &&
@@ -1690,6 +1688,8 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,
 	for (i = 0; i < delta_search_threads; i++) {
 		if (!p[i].list_size)
 			continue;
+		pthread_mutex_init(&p[i].mutex, NULL);
+		pthread_cond_init(&p[i].cond, NULL);
 		ret = pthread_create(&p[i].thread, NULL,
 				     threaded_find_deltas, &p[i]);
 		if (ret)
-- 
1.5.4.rc0.37.g78e7

^ permalink raw reply related

* Re: Windows binaries for qgit 2.0
From: Johannes Schindelin @ 2007-12-17 19:13 UTC (permalink / raw)
  To: Marco Costalba; +Cc: Abdelrazak Younes, git, msysgit
In-Reply-To: <e5bfff550712171105k62b90853w1c5eed64bd11fb23@mail.gmail.com>

Hi,

On Mon, 17 Dec 2007, Marco Costalba wrote:

> On Dec 17, 2007 10:51 AM, Abdelrazak Younes <younes.a@free.fr> wrote:
> >
> > I would like to help you with that but I can't retrieve the repository:
> >
> > $ git clone git://git.kernel.org/pub/scm/qgit/qgit4.git qgit4.git
> > Initialized empty Git repository in d:/devel/git/qgit4/qgit4.git/.git/
> > git.kernel.org[0: 130.239.17.7]: errno=Invalid argument
> > git.kernel.org[1: 199.6.1.166]: errno=Bad file descriptor
> > git.kernel.org[2: 204.152.191.8]: errno=Bad file descriptor
> > git.kernel.org[3: 204.152.191.40]: errno=Bad file descriptor
> > fatal: unable to connect a socket (Bad file descriptor)
> > fetch-pack from 'git://git.kernel.org/pub/scm/qgit/qgit4.git' failed.

This looks familiar.  It happens when there was no response to the 4 IPs 
of git.kernel.org.  This might be due to a firewall which blocks git://

> > $ git clone http://git.kernel.org/pub/scm/qgit/qgit4.git qgit4.git 
> > Initialized empty Git repository in d:/devel/git/qgit4/qgit4.git/.git/ 
> > Cannot get remote repository information. Perhaps 
> > git-update-server-info needs to be run there?
> 
> Well, perhaps, but to clone with git protocol you don't need that.

It is generally a good idea to provide the server-info for dumb protocols, 
because not everybody is fortunate enough (like you, evidently, because 
you do not seem to care all that much...) to control her outbound firewall 
restrictions.

Ciao,
Dscho

^ permalink raw reply

* Re: [msysGit] Re: Windows binaries for qgit 2.0
From: Marco Costalba @ 2007-12-17 19:14 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Abdelrazak Younes, msysgit, git
In-Reply-To: <Pine.LNX.4.64.0712171042520.9446@racer.site>

On Dec 17, 2007 11:44 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Why would anything that has to do with MSVC2005 be interesting to msysGit?
> Note the "msys" part of "msysGit".
>
> FWIW a member of our team works on compiling/including qgit into msysGit.
> But definitely not using closed-source compilers.  I would violently
> oppose that.
>

I would (violently) agree with you, but I also violently oppose to
waste a week end fighting with Qt4 + mingw compilation. MSVC2005 is
needed as a kind of "debug tool" to better understand if the problem
is with Qt4 or with mingw (as I suspect).

Abdel is very kind to try to help in caming out with a qgit.exe more
or less ready to be packaged. I'm not opposed, in this phase, to
follow different _technically_ sound paths. Then when the dust settles
down we could do our consideration regarding open source, in which I
belive very firmly, so firmly that I'm not scared to _test_ different
ways if this can be useful to shed some light on this issue.

Marco

^ permalink raw reply

* Re: Windows binaries for qgit 2.0
From: Marco Costalba @ 2007-12-17 19:17 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Abdelrazak Younes, git-u79uwXL29TY76Z2rM5mHXA,
	msysgit-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <Pine.LNX.4.64.0712171911070.9446-OGWIkrnhIhzN0uC3ymp8PA@public.gmane.org>


On Dec 17, 2007 8:13 PM, Johannes Schindelin <Johannes.Schindelin-Mmb7MZpHnFY@public.gmane.org> wrote:
>
> It is generally a good idea to provide the server-info for dumb protocols,
> because not everybody is fortunate enough (like you, evidently, because
> you do not seem to care all that much...) to control her outbound firewall
> restrictions.
>

I have taken note of Peter suggestion's to do a

chmod +x foo.git/hooks/post-update

on kernel repository, I don't remember if I have already done it many
months ago, but I'm planning to redo for safety as soon as I reach my
development box.

Thanks
Marco

^ permalink raw reply

* Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.
From: Junio C Hamano @ 2007-12-17 19:52 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Jeff King, git
In-Reply-To: <20071217123307.GK7453@artemis.madism.org>

Pierre Habouzit <madcoder@debian.org> writes:

> After having written this mail 4 time already, I came up with an idea I
> kind of like: like find, we could make {} be a placeholder for the
> "default" argument. For example:
>
>   $ git foo --abbrev {} 10
>   $ git log -M {} 1
>   ...
>
> {} would have the same semantics as your --long-opt-default. It tells the
> option parser that "no there isn't anything to grok for that command thank you
> very much". Of course if for some reason you really want to pass "{}" to the
> command, the stuck form holds:
>
>   $ git foo --long-opt={}
>   $ git foo -o{}
>
> What do you think ?

1. {} means a completely different thing to find ("place the real value
   here"); there is no similarity.  I would strongly oppose to it.  If
   you want to invoke opt with default but still want to pass "{}" as an
   argument unrelated to that opt, you would do "--opt={} {}".  That's
   double ugly.

2. For a long option with optional option-argument, --abbrev-default (or
   in the other order, --default-abbrev) to mark "there is no option
   argument, do not do your context sensitive parsing" and using an
   explicit '=' (e.g. --abbrev=<value>) to mark "this is the argument,
   do not do your context sensitive parsing" is much more readable.

3. There are only handful options with optional option-argument that
   does not have long format.  I think it is reasonable to require
   "stuck argument" to them.  For most of the short options that take
   optional option-argument, traditionally we did not allow them to be
   spelled as separate words, so there is no regression to introduce
   such a behaviour.  -B/-M/-C options to diff family would be handled
   sanely this way.

   Another possibility, which I do not like very much, is to add long
   format to them, if only for paranoid scripters who want rename
   detection with the default threshold and cannot say "diff -M $foo".
   They can say "diff --detect-rename-default $foo" instead ("-M" is a
   bad example here, as giving a single path never makes sense for -M so
   $foo cannot be a file whose name is e.g. "20", and default number of
   abbreviated commit object name is longer than 2 which means it would
   make it longer than "percentage" form of threshold).

So in short, for an option that takes optional option-argument:

   - if it is given as "--<long-name>-default", there is no optional
     argument, period.

   - if it is given as "--long-name" but there is no next word, there is
     no optional argument, either.

   - if it is given as "--long-name=value", that "value" is the
     argument.  Barf if it does not validate.

   - if it is given as "--long-name", and there is a next word, see if
     that is plausible as its argument.  Get it and signal the caller
     you consumed it, if it is.  Ignore it and signal the caller you
     didn't, if it isn't.

   - if it is given as "-svalue", that "value" is the argument.  Barf if
     it does not validate.

   - if it is given as "-s", and there is a next word, and if the option
     has long format counterpart as well, then see if the next word is
     plausible as its argument.  Get it and signal the caller you
     consumed it, if it is.  Ignore it and signal the caller you didn't,
     if it isn't.

   - if it is given as "-s" but the previous rule did not trigger, there
     is no optional argument.

^ permalink raw reply

* Re: [PATCH] provide advance warning of some future pack default changes
From: Joel Becker @ 2007-12-17 20:09 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Jakub Narebski, Junio C Hamano, git
In-Reply-To: <alpine.LFD.0.999999.0712142114400.8467@xanadu.home>

On Fri, Dec 14, 2007 at 09:23:38PM -0500, Nicolas Pitre wrote:
> On Fri, 14 Dec 2007, Joel Becker wrote:
> > 	The relevant message is:
> > 
> > Message-ID: <7vveaindgp.fsf@gitster.siamese.dyndns.org>
> > 
> > See the paragraphs at the bottom.  The thread, started by me, begins
> > with:
> > 
> > Message-ID: <20070910205429.GE27837@tasint.org>
> 
> OK.  From those emails Junio forwarded to me, I don't see any case for 
> actual _corruptions_.  Git does indeed refuse to work with unknown pack 
> index or unknown objects in a pack.  Really old versions were not overly 
> clueful as to why they refused to work, but they should never corrupt a 
> pack which, for all purposes, is always read-only anyway.

	You may not see a case for actual corruptions, but my coworker
updated his tree on a box with 1.5.x, then tried to work on a box with
1.4.x (I think 1.4.2 back then), and ended up with a tree that was
unusable.  He had to re-clone, and I think he got lucky recovering
pending changes (probably using 1.5.x on the branches with the changes,
as master was what got broken).
	My point is not that change is always bad, but that we should
really look forward to what we're doing, and that "it's in the release
notes" is not sufficient if an older git gives an incomprehensible error
or a silent problem.  I was responding to the cavalier statement "well,
it's in the release notes, so don't worry about older versions".

Joel

-- 

"Vote early and vote often." 
        - Al Capone

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox