* Re: [PATCH] include/asm-arm/: Spelling fixes
From: Jeff King @ 2007-12-17 20:51 UTC (permalink / raw)
To: Joe Perches; +Cc: Junio C Hamano, J. Bruce Fields, git
In-Reply-To: <1197922971.27386.32.camel@localhost>
On Mon, Dec 17, 2007 at 12:22:51PM -0800, Joe Perches wrote:
> $ let count=0 ; for line in $(cat patch_order) ; do ((count++)); \
> printf -v tmp "%04u-Spelling-%s" $count $line ; \
> tmp=${tmp//\//-} ; tmp=${tmp// /}; \
> git-format-patch -N --thread --start-number $count -s \
> -o patch3 \
> --subject-prefix="[PATCH] Spelling: $line" master $line ; done
> [converted the subjects appropriately]
> $ git-send-email --smtp-server <foo> --smtp-user <bar> \
> --from "Joe Perches <joe@perches.com>" \
> --to "linux-kernel@vger.kernel.org" \
> --cc "Andrew Morton <akpm@linux-foundation.org>" \
> --cc-cmd "../get_maintainer.pl --nogit" \
> --no-thread --suppress-from patch3
Ah. The problem is that git-send-email unconditionally adds a
message-id. Usually git-format-patch doesn't add one, but for obvious
reasons, it must when doing --thread. Here is a fix.
-- >8 --
git-send-email: avoid duplicate message-ids
We used to unconditionally add a message-id to the outgoing
email without bothering to check if it already had one.
Instead, let's use the existing one.
Signed-off-by: Jeff King <peff@peff.net>
---
It will also happily add duplicate --in-reply-to and --references
headers, but those can be suppressed with --no-thread. Arguably, it
should detect this case and turn on --no-thread, but looking at
git-send-email makes me want to claw my eyes out.
git-send-email.perl | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 1d6f466..083466a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -580,7 +580,7 @@ sub send_message
$ccline = "\nCc: $cc";
}
my $sanitized_sender = sanitize_address($sender);
- make_message_id();
+ make_message_id() unless defined($message_id);
my $header = "From: $sanitized_sender
To: $to${ccline}
@@ -718,6 +718,9 @@ foreach my $t (@files) {
}
push @xh, $_;
}
+ elsif (/^Message-Id: (.*)/i) {
+ $message_id = $1;
+ }
elsif (!/^Date:\s/ && /^[-A-Za-z]+:\s+\S/) {
push @xh, $_;
}
--
1.5.4.rc0.1146.gc97f-dirty
^ permalink raw reply related
* Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.
From: Junio C Hamano @ 2007-12-17 20:42 UTC (permalink / raw)
To: Jeff King; +Cc: Pierre Habouzit, git
In-Reply-To: <20071217203143.GA2105@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Dec 17, 2007 at 11:52:29AM -0800, Junio C Hamano wrote:
>
>> So in short, for an option that takes optional option-argument:
>
> I agree with everything you said, except...
>
>> - 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.
>
> This "plausible" makes me a little nervous, and I wonder why we want to
> support this at all. Is it
>
> 1. We have traditionally supported "--abbrev 10"? I don't think this
> is the case.
> 2. Consistency with "--non-optional-arg foo"? Do we have any such
> non-optional long arguments? I didn't see any; I think we stick
> with --non-optional-arg=foo everywhere.
> 3. More convenience to the user? I don't see how " " is easier than
> "=".
You forgot one case.
4. Everybody who does _not_ know that we traditionally did not
support the form would expect "--abbrev 10" and "-n 4" to work.
^ permalink raw reply
* Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.
From: Pierre Habouzit @ 2007-12-17 20:42 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20071217203143.GA2105@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 2324 bytes --]
On Mon, Dec 17, 2007 at 08:31:43PM +0000, Jeff King wrote:
> On Mon, Dec 17, 2007 at 11:52:29AM -0800, Junio C Hamano wrote:
>
> > So in short, for an option that takes optional option-argument:
>
> I agree with everything you said, except...
>
> > - 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.
>
> This "plausible" makes me a little nervous, and I wonder why we want to
> support this at all. Is it
>
> 1. We have traditionally supported "--abbrev 10"? I don't think this
> is the case.
Yes, that's why the restriction bugs me a bit too.
> 2. Consistency with "--non-optional-arg foo"? Do we have any such
> non-optional long arguments? I didn't see any; I think we stick
> with --non-optional-arg=foo everywhere.
there are some, I don't recall the exact commands, but option parsing
was quite inconsistent in git (well still is), there are the very simple
loops that just do strcmp and look for the '=', there are the loops that
allow interleaving of options and arguments (and that rewrite argc/argv
a bit like parseopt does) and also the ones that allow the separate
mode, and the one that do both.
The force-stick-mode is a regression for them.
> > - 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.
>
> Similarly, what is the goal here?
>
> 1. Have we ever supported "-s foo"? Not for -B/-M/-C, nor for
> shortlog's -w.
Yes for git tag -n for example, and there are some other
examples, look at maint for commands that have been migrated to
parse_options, some behave like that, and more than one for sure.
> 3. It's longer to type.
It's way more readable, but YMMV.
--
·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] provide advance warning of some future pack default changes
From: Nicolas Pitre @ 2007-12-17 20:41 UTC (permalink / raw)
To: Joel Becker; +Cc: Jakub Narebski, Junio C Hamano, git
In-Reply-To: <20071217200920.GB19816@mail.oracle.com>
On Mon, 17 Dec 2007, Joel Becker wrote:
> 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).
I still claim that there wasn't any corruptions.
Just for fun, just edit some document with Microsoft Office 95, then
open the same document with Office 2007 and save it with default
settings. Now try to open it back with Office 95. It won't work.
Does that mean that the document got corrupted?
> 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".
Your allegation of corruptions is cavalier just as well.
I'm telling you that there won't be any such corruption. Just like in
the M$ Office case, it is expected that newer versions make data
unusable by older versions at some point -- that's the inevitable side
effect of progress.
And we cannot always anticipate what kind of incompatibility will be
worth making in the future, so it is hard to come with proper error
messages in all cases today.
Recent enough Git versions do suggest upgrading when they refuse to
access repositories though, and later Git versions can be configured to
remain compatible with old Git versions. And we also document it.
And when you still cannot figure it out on your own, then there is that
free support available on a public mailing list, and even an IRC
channel.
So I don't see how we could do better in that regard. Carving the
repository format in stone to keep ancient versions working forever is
_not_ a solution.
Nicolas
^ permalink raw reply
* Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.
From: Jeff King @ 2007-12-17 20:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pierre Habouzit, git
In-Reply-To: <7vy7bt6qv6.fsf@gitster.siamese.dyndns.org>
On Mon, Dec 17, 2007 at 11:52:29AM -0800, Junio C Hamano wrote:
> So in short, for an option that takes optional option-argument:
I agree with everything you said, except...
> - 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.
This "plausible" makes me a little nervous, and I wonder why we want to
support this at all. Is it
1. We have traditionally supported "--abbrev 10"? I don't think this
is the case.
2. Consistency with "--non-optional-arg foo"? Do we have any such
non-optional long arguments? I didn't see any; I think we stick
with --non-optional-arg=foo everywhere.
3. More convenience to the user? I don't see how " " is easier than
"=".
> - 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.
Similarly, what is the goal here?
1. Have we ever supported "-s foo"? Not for -B/-M/-C, nor for
shortlog's -w.
2. This would add consistency to non-optional arguments.
3. It's longer to type.
So I see a slight case for "-s foo", but none at all for "--long foo".
-Peff
^ permalink raw reply
* Re: [PATCH] the use of 'tr' in the test suite isn't really portable
From: Junio C Hamano @ 2007-12-17 20:25 UTC (permalink / raw)
To: H.Merijn Brand; +Cc: git
In-Reply-To: <20071217191508.79cc05e2@pc09.procura.nl>
The contents looked correct but the patch was garbled.
--
Applying the use of 'tr' in the test suite isn't really portable
.dotest/patch:11: indent with spaces.
git diff-index -r $commit | cut -f 2- | tr '\n' '\000' | \
.dotest/patch:153: indent with spaces.
dd if=/dev/zero bs=4k count=1 | tr "\\000" $i >$i &&
.dotest/patch:165: indent with spaces.
tr '\000' 'g'
.dotest/patch:174: indent with spaces.
tr '\000' 'g'
error: patch failed: git-filter-branch.sh:290
error: git-filter-branch.sh: patch does not apply
error: patch failed: t/t0020-crlf.sh:5
error: t/t0020-crlf.sh: patch does not apply
error: patch failed: t/t3300-funny-names.sh:54
error: t/t3300-funny-names.sh: patch does not apply
error: patch failed: t/t4020-diff-external.sh:99
error: t/t4020-diff-external.sh: patch does not apply
error: patch failed: t/t4116-apply-reverse.sh:12
error: t/t4116-apply-reverse.sh: patch does not apply
error: patch failed: t/t4200-rerere.sh:129
error: t/t4200-rerere.sh: patch does not apply
error: patch failed: t/t5300-pack-object.sh:15
error: t/t5300-pack-object.sh: patch does not apply
error: patch failed: test-sha1.sh:10
error: test-sha1.sh: patch does not apply
.dotest/patch:11: indent with spaces.
git diff-index -r $commit | cut -f 2- | tr '\n' '\000' | \
.dotest/patch:153: indent with spaces.
dd if=/dev/zero bs=4k count=1 | tr "\\000" $i >$i &&
.dotest/patch:165: indent with spaces.
tr '\000' 'g'
.dotest/patch:174: indent with spaces.
tr '\000' 'g'
^ permalink raw reply
* Re: [PATCH] rebase -p -i: handle "no changes" gracefully
From: Junio C Hamano @ 2007-12-17 20:23 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Pieter de Bie, Johannes Sixt, git, gitster
In-Reply-To: <Pine.LNX.4.64.0712171649420.9446@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Since commit 376ccb8cbb453343998e734d8a1ce79f57a4e092, unchanged
> SHA-1s are no longer mapped via $REWRITTEN. But the updating
> phase was not prepared for the old head not being rewritten.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> On Mon, 17 Dec 2007, Pieter de Bie wrote:
>
> > Ok, but what about the error in the rebase?
> >
> > On Dec 14, 2007, at 2:21 AM, Pieter de Bie wrote:
> > > Tirana:~/git pieter$ time git rebase -p -i HEAD~100
> > > cat:
> > > /Users/pieter/git/.git/.dotest-merge/rewritten/1e8df762b38e01685f3aa3613e2d61f73346fcbe:
> > > No such file or directory
>
> This buglet was not caught earlier, probably because a
> non-rewriting rebase is not really interesting ;-)
Hmph, care to add a test to t3404?
^ permalink raw reply
* Re: [PATCH] include/asm-arm/: Spelling fixes
From: Joe Perches @ 2007-12-17 20:22 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: git
In-Reply-To: <20071217201219.GC13515@fieldses.org>
On Mon, 2007-12-17 at 15:12 -0500, J. Bruce Fields wrote:
> Hope you don't mind my cc'ing the git list:
Nope. Not a bit.
I had patches that were committed in word order that
I wanted to aggregate by subsystem.
I took all the patches, created a branch, committed
all the patches at once, then used these commands:
(patch_order is a directory list)
$ let count=0 ; for line in $(cat patch_order) ; do ((count++)); \
printf -v tmp "%04u-Spelling-%s" $count $line ; \
tmp=${tmp//\//-} ; tmp=${tmp// /}; \
git-format-patch -N --thread --start-number $count -s \
-o patch3 \
--subject-prefix="[PATCH] Spelling: $line" master $line ; done
[converted the subjects appropriately]
$ git-send-email --smtp-server <foo> --smtp-user <bar> \
--from "Joe Perches <joe@perches.com>" \
--to "linux-kernel@vger.kernel.org" \
--cc "Andrew Morton <akpm@linux-foundation.org>" \
--cc-cmd "../get_maintainer.pl --nogit" \
--no-thread --suppress-from patch3
I probably just reversed the "--thread" on
git-format-patch and send-email.
cheers, Joe
^ permalink raw reply
* Re: [PATCH 7/7] git-tag: fix -l switch handling regression.
From: Junio C Hamano @ 2007-12-17 20:13 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <20071217190328.GF22554@artemis.madism.org>
Pierre Habouzit <madcoder@debian.org> writes:
> 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.
I did not understand what "git tag -n xxx" was meant to do, either.
Time to run blame and ask the responsible party?
I suspect "-n ''" there might be meant as a way to spell the "no
argument here -- use our default" instruction. It looks slightly nicer
than that '{}' but not quite.
^ permalink raw reply
* Re: [PATCH] include/asm-arm/: Spelling fixes
From: J. Bruce Fields @ 2007-12-17 20:12 UTC (permalink / raw)
To: Joe Perches; +Cc: git
In-Reply-To: <1197921847.27386.16.camel@localhost>
Hope you don't mind my cc'ing the git list:
On Mon, Dec 17, 2007 at 12:04:07PM -0800, Joe Perches wrote:
> On Mon, 2007-12-17 at 14:56 -0500, J. Bruce Fields wrote:
> > My mail client seems to be flagging all your messages as duplicates of each
> > other.
> >
> > Hm. It may be that your headers have two Message-Id's...
> > Message-Id: <1197919875-5288-3-git-send-email-joe@perches.com>
> > X-Mailer: git-send-email 1.5.3.7.949.g2221a6
> > Message-Id:
> > <5703e57f925f31fc0eb38873bd7f10fc44f99cb4.1197918889.git.joe@perches.com
> > >
> >
> > ... the second of which is pretty weird (and is the same for every message. Is
> > that a git-send-email bug or something else?
>
> Dunno. I did a git-format-patch, then git-send-email.
Weird. Anyone know how that could happen?
--b.
^ 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
* 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: 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: [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: 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
* [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: 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
* 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: [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 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
* [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
* [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 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 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 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox