Git development
 help / color / mirror / Atom feed
* Re: Tracking branch history
From: Junio C Hamano @ 2006-05-14 23:16 UTC (permalink / raw)
  To: git
In-Reply-To: <loom.20060513T140528-554@post.gmane.org>

Elrond <elrond+kernel.org@samba-tng.org> writes:

> Daniel Barkalow <barkalow <at> iabervon.org> writes:
>
>> 
>> One feature that might make git more intuitive to people is if we were to 
>> additionally track the history of what commit was the head of each branch 
>> over time. This is only vaguely related to the history of the content, but 
>> it's well-defined and sometimes significant.
>> 
>> E.g., if you know that two weeks ago, what you had worked, but it doesn't 
>> work now, you can use git-bisect to figure out what happened, but first 
>> you have to figure out what commit it was that you were using two weeks 
>> ago. Two weeks ago, we had that information, but we didn't keep it.
>
> On a related issue:
>
> Looking at a commit:
>    commit id-commit
>    parent id-1
>    parent id-2
>    parent id-3
>
>        Merge branch 'branch-2', 'branch-3'
>
> One can tell the name of the branches for id-2 and id-3 (branch-2, 3),
> but one can't tell the name of id-1.

That's deliberate.  If you are merging into a branch other than
"master", the message would say:

        commit ea892b27b15fbc46a3bb3ad2ddce737dc6590ae5
        Merge: 7278a29... 8d48ad6...
        Author: Junio C Hamano <junkio@cox.net>
        Date:   Sat May 13 18:49:54 2006 -0700

            Merge branch 'lt/config' into next

            * lt/config:
              git config syntax updates
              Another config file parsing fix.
              checkout: use --aggressive when running a 3-way merge (-m).
              Fix git-pack-objects for 64-bit platforms
              fix diff-delta bad memory access

The point is to keep the punch line as short and meaningful for
the most common case.

^ permalink raw reply

* Re: Tracking branch history
From: Junio C Hamano @ 2006-05-14 23:14 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <20060513181816.GA12475@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

> Log ref updates to logs/refs/<ref>
>
> If config parameter core.logRefUpdates is true then append a line
> to .git/logs/refs/<ref> whenever git-update-ref <ref> is executed.

I cannot decide if a parameter makes more sense, or just making
the existence of such a file a cue is better.  For example, I do
not much care about when I updated each of my topic branch head,
while I do care about master, next, and pu branches.  A global
parameter would make this black-or-white choice, while opening
the log without O_CREAT and write things out only when the log
file exists might make things as easy and controllable.

I could "touch" the ones I care about to prime the process.

Hmm?

^ permalink raw reply

* Re: Branch relationships
From: Josef Weidendorfer @ 2006-05-14 23:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8xp4ntbf.fsf@assigned-by-dhcp.cox.net>

On Monday 15 May 2006 00:19, you wrote:
> > I suppose "branch.<branch name>.origin" is still the way to go for
> > specifying the upstream?
> 
> Probably "origin" is a better name for it; I was assuming
> "branch.<branch name>.remote = foo" refers to a [remote "foo"]
> section and means "when on this branch, pull from foo and merge
> from it".

Maybe.

But there is a misunderstanding. I wanted the branch attribute "origin"
to specify the upstream _branch_, not a remote.
After a "git clone", we would have

 [remote "origin"]
   url = ...
   fetch = master:origin

 [branch "master"]
   origin = "origin" ; upstream of master is local branch "origin"

 [branch "origin"]
   tracksremote ; bool

Now adding a further development branch for remote branch "topic", we would add

 [remote "origin"]
   ...
   fetch = topic:tracking-topic

 [branch "local-topic"]
   origin = "tracking-topic"

 [branch "tracking-topic"]
   tracksremote

Now, a "git pull" on branch "local-topic" does the right thing: it fetches
from remote "origin", as "tracking-topic" is given in a refspec there, and merges
"tracking-topic" to the current branch "local-topic", as given by the origin
attribute.

This also extends to local upstreams: a "git checkout -b topic2 master" would
append

 [branch "topic2"]
   origin = "master"

and a "git pull" on topic2 would merge the upstream "master".

Josef

^ permalink raw reply

* Re: Branch relationships
From: Junio C Hamano @ 2006-05-14 22:19 UTC (permalink / raw)
  To: Josef Weidendorfer; +Cc: git
In-Reply-To: <200605150001.48548.Josef.Weidendorfer@gmx.de>

Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:

>> Exactly.  I would _want_ to push to both with single action when
>> I say "git push ko-private".  Actually I have _never_ felt need
>> to, but Linus wanted it first and I think it makes sense.
>
> Hmmm. Isn't this a solution for a very special use-case?
> You even can not specify different push lines for the 2 URLs.
> I think you want an alias name for a group of remotes here?

Perhaps.  

The "push to multiple places" is mostly for Linus backing things
up, and I tend to agree that your "alias" notation makes things
appear to be more general.  However, I do not think you would
want to push to two different places with different branch
mappings, so I suspect that generality is not buying you much
while making things more easily misconfigured.

> I suppose "branch.<branch name>.origin" is still the way to go for
> specifying the upstream?

Probably "origin" is a better name for it; I was assuming
"branch.<branch name>.remote = foo" refers to a [remote "foo"]
section and means "when on this branch, pull from foo and merge
from it".

^ permalink raw reply

* Re: The git newbie experience
From: Junio C Hamano @ 2006-05-14 22:09 UTC (permalink / raw)
  To: Tommi Virtanen; +Cc: git
In-Reply-To: <446778B8.7080201@inoi.fi>

Tommi Virtanen <tv@inoi.fi> writes:

> My best idea so far is to add a "git commit -A" option, that
> essentially does the "update-index --refresh". Whenever index
> has a file state != HEAD, update-index it. The modified unrelated
> files will have index state == HEAD. Or altering "git commit -a"
> to do that.

I am not sure what you are trying to achieve by --refresh.  It
does not update the object names in the index.  Maybe you are
thinking about --again, but that is when you did something to
the index yourself, so it would not buy you much in the "novice
faced with merge" case.

Anyway, I think the time to commit is too late to save somebody
who does not understand the index.  How would you explain why
you sometimes need to use -A and sometimes -a?  That is why I
suggested to make "git pull" and "git merge" refuse to work if
there are local changes for novice users, where the definition
of novice is "git commit -a" is the only way to make a commit.
We can have [core] novice = yes in .git/config for that.

If somebody does not understand the index, if the merge is
prevented because the local change does conflict with it, how
would you explain why sometimes you can merge and sometimes you
cannot?

But if you insist going that route, I would say we could make
"git commit -a" on a merge commit to do a bit more magic.

For example, we could make -a do something special for a merge
by looking at the presense of .git/MERGE_HEAD.

	- if "commit -a", and without .git/MERGE_HEAD, just grab
          all the local modifications that are not in index yet,
          and commit it.

	- upon "commit -a", and when .git/MERGE_HEAD exists,
          grab the paths that ls-files -u reports, update-index
          them.  Other automerged paths are already registered
          in the index.

> Except, trying to solve usability problems by _adding_ options
> is just insane.

I am not sure if it is "usability" but additional option to
simplify things does not sound right, I'd agree.

> For example, we had a case where we absolutely _had_ to keep
> an ugly workaround in the tree, in a file not otherwise
> edited, but we definitely did not want to commit the kludge,
> at least not to the branch that was really being worked on. So
> such restricted mode would just have meant either people could
> not merge, or they had to use index anyway.

Your example is a very ill-thought out one.

If you are leaving the uncommitable kludge around, you cannot be
using "commit -a" with the normal non-merge workflow.  Why
would you worry about not being able to do "commit -a" on a
merge then?

For the beginning user without index, I would rewrite your
scenario like this.

- Jack is a beginning user of git and does not (want to) understand
  the index (right now).

- Jack works on branch X, say his HEAD points to X1. He has an edited,
  uncommitted files with the names A, B and C.

- Jack wants to pull new changes made by others to his branch.
  But "git merge" invoked from "git pull" says he needs to stash
  away the local changes to do the merge.

- Jack stashes away what he has been working on and cleans up
  his mess.

  git diff >P.diff
  git checkout HEAD A B C

- Jack then pulls.  There are merge conflicts in files D, E, ..., Z.

- Jack resolves the merge conflicts and is ready to commit the resulting
  merge. Note files A, B and C do not have his unfinished work.

  There is no "if Jack does this or that" problem; he says "git
  commit -a" because that is the only "commit" command he knows
  about.

- Jack then reapplies what he stashed away with "git apply P.diff"
  and keeps working.

Maybe "git stash" command that does "git diff --full-index" with
some frills, and "git unstash" command which does an equivalent
of "git am -3" would help this workflow (bare "git apply" does
not do the three-way merge like am does).

^ permalink raw reply

* Re: Branch relationships
From: Josef Weidendorfer @ 2006-05-14 22:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr72w2thu.fsf@assigned-by-dhcp.cox.net>

On Sunday 14 May 2006 23:20, Junio C Hamano wrote:
> Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:
> >>   ; my private build areas on the kernel.org machines
> >>    [remote "ko-private"]
> >>         url = "x86-64-build.kernel.org:git"
> >>         url = "i386-build.kernel.org:git"
> >>         push = master:origin
> >> ...
> >
> > specifies that "git push" should push to both URLs?
> 
> Exactly.  I would _want_ to push to both with single action when
> I say "git push ko-private".  Actually I have _never_ felt need
> to, but Linus wanted it first and I think it makes sense.

Hmmm. Isn't this a solution for a very special use-case?
You even can not specify different push lines for the 2 URLs.
I think you want an alias name for a group of remotes here?
Why not

 [remotealias "ko-private"]
   remote = "ko-private-x86-64"
   remote = "ko-private-i386"

Neverless, this wasn't the thing I was after.

> That is what "branch.foo.remote = this-remote" is about.

OK. Sorry, I somehow missed this.

> When 
> working on foo branch, use what is described in
> remote."this-remote" section for "git pull".
> remote."this-remote" would have url and one or more fetch lines,
> and as usual the first fetch line would say "merge this thing".
> This gives the continuity in semantics while migrating from
> .git/remotes/foo to [remote "foo"] section of the config file.

The only thing I wanted to discuss in this thread is exactly the
limitation of "as usual the first fetch line..." by the branch
attribute "origin", which lead me to the alternative "tracksremote"
attribute. Sorry about any confusion.

I suppose "branch.<branch name>.origin" is still the way to go for
specifying the upstream?

Josef

^ permalink raw reply

* Re: The git newbie experience
From: Junio C Hamano @ 2006-05-14 21:26 UTC (permalink / raw)
  To: Tommi Virtanen; +Cc: git
In-Reply-To: <446778B8.7080201@inoi.fi>

Tommi Virtanen <tv@inoi.fi> writes:

> (21:08:34) ***gitster personally considers getting more users a very
> high priority but agrees that from usability point of view, having a
> mode to expose "stripped down" set of features for simple needs would be
> beneficial.
>
> That I can 100% agree with.

Sorry, I personally consider it is not a very high priority; the
above is a typo (otherwise "but agrees" does not make _any_
sense).

The rest of the message I'll find a time to comment on
separately, but I am in the middle of migrating the development
environment, so it might take some time.

Oh, and please send patches cc'ed to the list at least for a few
days, because I am fighting with fetchmail/getmail configuration
right now and might lose a few mails while doing so.

^ permalink raw reply

* Re: Branch relationships
From: Junio C Hamano @ 2006-05-14 21:20 UTC (permalink / raw)
  To: Josef Weidendorfer; +Cc: git
In-Reply-To: <200605142249.17508.Josef.Weidendorfer@gmx.de>

Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:

> On Sunday 14 May 2006 19:36, Junio C Hamano wrote:
>> Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:
>> 
>> > On Saturday 13 May 2006 23:22, you wrote:
>> >>  * remotes/ information from .git/config (js/fetchconfig)
>> >> ...
>> >>    [branch "master"]
>> >> 	remote = "ko-private"
>
> I still do not understand the semantic of this line.
> Is this supposed to do "git pull ko-private" as default pull
> action and "git push ko-private" as default push?
>
> So using
>
>>   ; my private build areas on the kernel.org machines
>>    [remote "ko-private"]
>>         url = "x86-64-build.kernel.org:git"
>>         url = "i386-build.kernel.org:git"
>>         push = master:origin
>> ...
>
> specifies that "git push" should push to both URLs?

Exactly.  I would _want_ to push to both with single action when
I say "git push ko-private".  Actually I have _never_ felt need
to, but Linus wanted it first and I think it makes sense.

> This is really confusing: Is the remote "ko-private" now at
> "x86-64-build.kernel.org:git" or at "i386-build.kernel.org:git" ?

Neither.

Perhaps reading my comment on #git log from yesterday or so,
where I talk about ".git/branches is about configuration
per-local branches, and .git/remotes is about giving a
short-hand to remote repositories that are used often",
might be helpful.  

> Neverless, I missed the info "Which branch should be merged in a default
> pull after fetching the given branches from remote". I understand that
> this is not needed in your workflow, as you have no upstream.

Not with git but in other projects I do.

That is what "branch.foo.remote = this-remote" is about.  When
working on foo branch, use what is described in
remote."this-remote" section for "git pull".
remote."this-remote" would have url and one or more fetch lines,
and as usual the first fetch line would say "merge this thing".
This gives the continuity in semantics while migrating from
.git/remotes/foo to [remote "foo"] section of the config file.

>> >  [branch "ko-master"]
>> >     tracksremote = "master of ko-private"
>> >
>> > This also would specify that we are not allowed to commit on "ko-master".
>> 
>> For my workflow, it is "master of ko"; your notation expresses
>> the same constraints more explicitly by being more special
>> purpose
>
> Why is this more special purpose?

It is more special purpose than just saying "do not touch this
myself".  You are saying "do not touch this myself because it
tracks a remote".  I do not think it is necessary to state the
reason.

> Perhaps an option "when pulling into this branch from a remote, also fetch
> all branches tracked from this remote", or another one "when fetching/pulling
> into this branch, update all other branches, too".

You already have "when fetching from this remote, fetch all
these branches and store them in the tracking branches", and
that is what .git/remotes/ is about.  The [remote] section in
the configuration file has the same semantics.  What problem are
you trying to fix by doing things differently?

^ permalink raw reply

* [PATCH] include header to define uint32_t, necessary on Mac OS X
From: Ben Clifford @ 2006-05-14 20:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, Dennis Stosberg, git
In-Reply-To: <Pine.LNX.4.64.0605142056320.3038@mundungus.clifford.ac>


From: Ben Clifford <benc@hawaga.org.uk>
Date: Sun, 14 May 2006 21:34:56 +0100
Subject: [PATCH] include header to define uint32_t, necessary on Mac OS X

---

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

2ee926ab9da67ef2a6ca28bb70954a33d65ba466
diff --git a/pack-objects.c b/pack-objects.c
index 1b9e7a1..5466b15 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -10,6 +10,7 @@ #include "csum-file.h"
  #include "tree-walk.h"
  #include <sys/time.h>
  #include <signal.h>
+#include <stdint.h>

  static const char pack_usage[] = "git-pack-objects [-q] [--no-reuse-delta] [--non-empty] [--local] [--incremental] [--window=N] [--depth=N] {--stdout | base-name} < object-list";

diff --git a/sha1_file.c b/sha1_file.c
index 631a605..3372ebc 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -13,6 +13,7 @@ #include "blob.h"
  #include "commit.h"
  #include "tag.h"
  #include "tree.h"
+#include <stdint.h>

  #ifndef O_NOATIME
  #if defined(__linux__) && (defined(__i386__) || defined(__PPC__))
-- 
1.3.2.g5f7f2-dirty

^ permalink raw reply related

* Re: [PATCH] Fix git-pack-objects for 64-bit platforms
From: Ben Clifford @ 2006-05-14 20:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, Dennis Stosberg, git
In-Reply-To: <7viroav534.fsf@assigned-by-dhcp.cox.net>


> It takes the uint32_t version but matches another place to use
> that type instead of "int" (which would not matter in next 10
> years perhaps).

This use of uint32_t breaks the build for me on Mac OS X. Including
<stdint.h> seems to make it work again. See following patch.

-- 

^ permalink raw reply

* Branch relationships (was:Re: git diff: support "-U" and "--unified" options properly)
From: Josef Weidendorfer @ 2006-05-14 20:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7j4o33wc.fsf@assigned-by-dhcp.cox.net>

On Sunday 14 May 2006 19:36, Junio C Hamano wrote:
> Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:
> 
> > On Saturday 13 May 2006 23:22, you wrote:
> >>  * remotes/ information from .git/config (js/fetchconfig)
> >> ...
> >>    [branch "master"]
> >> 	remote = "ko-private"

I still do not understand the semantic of this line.
Is this supposed to do "git pull ko-private" as default pull
action and "git push ko-private" as default push?

So using

>   ; my private build areas on the kernel.org machines
>    [remote "ko-private"]
>         url = "x86-64-build.kernel.org:git"
>         url = "i386-build.kernel.org:git"
>         push = master:origin
> ...

specifies that "git push" should push to both URLs?
This is really confusing: Is the remote "ko-private" now at
"x86-64-build.kernel.org:git" or at "i386-build.kernel.org:git" ?

Probably its better to have
  [branch "master"]
    push-remote = "ko-private-x86-64 ko-private-i386"
and entries for 2 remotes.

Neverless, I missed the info "Which branch should be merged in a default
pull after fetching the given branches from remote". I understand that
this is not needed in your workflow, as you have no upstream.

> >  [branch "ko-master"]
> >     tracksremote = "master of ko-private"
> >
> > This also would specify that we are not allowed to commit on "ko-master".
> 
> For my workflow, it is "master of ko"; your notation expresses
> the same constraints more explicitly by being more special
> purpose

Why is this more special purpose?
IMHO the only difference is that your proposal puts this information into
the remote, and I am putting it to the branch as attribute.

I chose it this way because I always thought that the Pull-lines in .git/remotes
where only about giving a default fetch action, ie. they are optionally. But
even if I do not want to have a remote branch fetched on default, I want to put
the information "XXX of remote RRR is the upstream of local YYY" into some place
to be able to do a pull when on that branch. 

> : "This tracks that one so never touch it any way other 
> than fetching into it" (we may not even allow checking out
> "ko-master" -- I dunno).  
> 
> One issue you might want to think about is it is far more
> efficient to fetch multiple branches from the same git://... URL
> is than fetching them one by one.  The push has exactly the same
> property.

Perhaps an option "when pulling into this branch from a remote, also fetch
all branches tracked from this remote", or another one "when fetching/pulling
into this branch, update all other branches, too".

> Another thing is the above talks only about constraints, and the
> user has to go all over the config file to find "xxx to
> ko-private" in order to figure out what happens when he says
> "pull ko-private".

I wasn't intending to change the current semantics of the remote section,
which specifies what to do when pulling/fetching/pushing from/into this
remote.

But you are right; this could get inconsistent. Better find a way to
not be able to specify an inconsistent config in the first place.

However, such an inconsistent config is already possible today:

 [remote "r1"]
    fetch = master:master
 [remote "r2"]
    fetch = master:master

This is not really better than

 [branch "master"]
    tracksremote "master of r1"
 [remote "r1"]
    fetch = "notmaster:master"

We already print out a warning when updating a branch tracking a remote
is not a fast-forward.
So I would leave it as it is, and allow both ways.

This all would be mood if we use refs/remotes consequently: existance of
a branch "refs/remotes/r1/master" implicitly specifies that this is
a branch tracking a remote, where the remote is named "r1" and the
remote branch is "master". 

With

 [core]
   useSeparateRemote ; bool

 [r1]
   url = ...
   fetch = master branch2 branch3

a "git fetch r1" would track given remote branches in "remotes/r1/<branch>",
and a

 [branch "master"]
   origin = "remotes/r1/master"

would work even without the "fetch" line in the remote section.
Here, we can allow both ways without getting inconsistent, as we implicitly
have

 [branch "remotes/r1/master"]
   tracksremote = "master of r1"

Regarding syntax: Instead of the "... of ..." values I would prefer two keys
"remotebranch"/"remote".

Josef

^ permalink raw reply

* Re: Howto get the merge-base ?
From: Linus Torvalds @ 2006-05-14 18:40 UTC (permalink / raw)
  To: Bertrand Jacquin; +Cc: Git Mailing List
In-Reply-To: <4fb292fa0605141021r20cefaa0he592b9c713ede333@mail.gmail.com>



On Sun, 14 May 2006, Bertrand Jacquin wrote:
> 
> I'm trying to know which commit it the parent of a merge.
> For exemple if I do that :
> 
>   o Merge
>  / \
> /   \
> |   |
> |   o Commit D
> |   |
> |   o Commit C
> |   |
> o   | Commit B
> \  /
>  \/
>  o Commit A
>  |
>  o Init
> 
> How could I know that ``Commit A'' is the merge-base of ``Merge'' ?

Well, Junio already answered, but I'd like to comment a bit further.

There may not be "one" merge-base. There can be several:

		Merge
		  |
		  A
		 / \
		B   C
		|\ /|
		| X |
		|/ \|
		D   E
		 \ /
		  F
		  |

Here the merge (A) has two equally good merge bases: D and E.

If you want all of these merge-bases, you need to add the "--all" flag to 
git-merge-base.

On the git archive, try this trivia shell pipeline:

	git-rev-list --parents HEAD |
		sed -n '/^[0-9a-f]* [0-9a-f]* [0-9a-f]*$/ p' |
		while read a b c
		do
			echo $a: $(git merge-base --all $b $c)
		done | less -S

and you'll be able to easily pick up examples of where there are real 
multiple merge bases. For example, commit 4da8cbc2.. has that.

If you want to examine _why_ it has multiple merge-bases, do:

	gitk 4da8cbc2 --not 5910e997 52b70d56

(where the two "not" commits are the merge bases for it) which shows that 
merge and the criss-crossing nature of the history leading up to it.

NOTE! Most of the time, multiple merge bases do not really matter all that 
much, and you'd get the same merge regardless of which one you would 
choose as the base. Still, they _can_ matter.

			Linus

^ permalink raw reply

* The git newbie experience
From: Tommi Virtanen @ 2006-05-14 18:36 UTC (permalink / raw)
  To: git

Here's my thoughts from teaching half a dozen people git recently:

Minimal newbie command set
--------------------------

The complete set of "newbie commands" for useful development work should
be as small as possible, for fast learning.

People can always look up new things when they want to, but if they
don't get the simple things going quickly they will forever see git
as "that overcomplex thing I tried to use once".

Concretely: explain the indexless "git commit -a" case first,
so people don't need update-index right away. Most new docs are
pretty good at this, already.

Fix the cases where "git commit -a" is not enough. Here's a case I
ran into:

- Jack is a beginning user of git and does not (want to) understand
  the index (right now).
- Jack works on branch X, say his HEAD points to X1. He has an edited,
  uncommitted files with the names A, B and C.
- Jack wants to pull new changes made by others to his branch.
  There are merge conflicts in files D, E, ..., Z.
- Jack resolves the merge conflicts and is ready to commit the resulting
  merge. Note files A, B and C should not be committed.

  1. if Jack says "git commit -a", then A, B and C will be committed
     also.

  2. if Jack says "git commit", then the current state of the index will
     be committed. That is, the commit will not contain the proper
     merged state of files D, E, ..., Z.

  3. if Jack says "git commit D E ... Z", things work correctly. But
     Jack does not want to type or copy-paste that much, and that's
     horribly error-prone anyway. If the leaves one file out, or
     accidentally adds B there too, the merge goes wrong.

  4. if Jack says "git update-index --refresh" and then "git commit",
     things work correctly. But Jack doesn't (want to) know about the
     index.

My best idea so far is to add a "git commit -A" option, that
essentially does the "update-index --refresh". Whenever index
has a file state != HEAD, update-index it. The modified unrelated
files will have index state == HEAD. Or altering "git commit -a"
to do that.

Except, trying to solve usability problems by _adding_ options
is just insane.

I'd really like to see a way to use git without caring about the
index, and just having things work. I can appreciate the index
is useful, and possibly even necessary to work on projects the size
of the Linux kernel, but I really wish it would default to being
_only_ an optimization, not the central bit of user interface to
prepare commits.

Basic git use _should_ be as easy as basic svn/bzr/hg use. Anything
else will just mean git is not used outside of what codebases Linus
has dictatorship over. (At least not after bzr is more done or hg
more well known; right now git has a very nice feature set, but the
others are catching up fast.)

This is a part that Subversion got right. Basic use needs to be simple.
(If you catch me in agitated mood, I'd claim it's the _only_ part
Subversion got right;-)


And to reply to a comment on IRC I missed at the time:

(21:05:35) gitster: as gittus said much earlier, refusing index is like
refusing git.  We might be able to implement "index-less" mode in which
things like merge and am refuse to operate when you have any change from
HEAD in the working tree. Then new users can always do "commit -a".
(21:06:04) gitster: "git-repo-config core.newuser yes" perhaps?

If you do it that way, you only make git unnecessarily hard to use for
newbies. For example, we had a case where we absolutely _had_ to keep
an ugly workaround in the tree, in a file not otherwise edited, but
we definitely did not want to commit the kludge, at least not to the
branch that was really being worked on. So such restricted mode would
just have meant either people could not merge, or they had to use index
anyway. That's a point where people who have a choice make on, and stop
trying to use git.

(21:08:34) ***gitster personally considers getting more users a very
high priority but agrees that from usability point of view, having a
mode to expose "stripped down" set of features for simple needs would be
beneficial.

That I can 100% agree with.


Branch management
-----------------

"master" and "origin" are good enough for the really simple use, but
that starts to fail fast when you add in more branches.

The remotes/* branch support is really nice, but should be used better.
Here's a bunch of wild ideas:

- When cloning a repository, just clone all the non-remote heads to
local remotes/* heads. See what name the remote HEAD points to, store
that locally also as a refs/heads/master, set local HEAD to it. Note,
origin is gone, and is now called remotes/master.

- Alternatively (and I think I like this more): When cloning a
repository, just clone all the non-remote heads to local remotes/*
heads. See what the remote HEAD points to, store that locally
also as a refs/heads/* head, set local HEAD to it. Note, origin and
master are both gone, and accessible via remotes/X and refs/heads/X
(where X is the name remote HEAD pointed to).

- Currently, when people start tracking a new remote branch, they end
up editing .git/remotes/origin and adding new Pull: lines. If they
intend to work on the branch, they also clone the branch locally, and
add a Push: line. Make this simpler. Here's a rough sketch:

  "git track [--read-only] REMOTE [BRANCH | --all]"

  Without --all, git track would:
  - abort if refs/remotes/foo exists
  - add "Pull: foo:remotes/foo" to .git/remotes/origin (or the
    equivalent config file)
  - if --read-only is not given:
    - add "Push: foo:foo" to .git/remotes/origin
  - fetch foo from origin to remotes/foo
  - if --read-only is not given:
    - run "git branch foo remotes/foo"

  With --all, it would do the same, but for everything the REMOTE has
  in refs/remotes/. Exactly when to abort is a bit harder to define,
  but still..

- Or, if that's too much, at least make peek-remote understand
.git/remotes/* shortcuts, so finding out what branches exist is a bit
simpler.

^ permalink raw reply

* Simplify "git reset --hard"
From: Linus Torvalds @ 2006-05-14 18:20 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0605141040210.3866@g5.osdl.org>


Now that the one-way merge strategy does the right thing wrt files that do 
not exist in the result, just remove all the random crud we did in "git 
reset" to do this all by hand.

Instead, just pass in "-u" to git-read-tree when we do a hard reset, and 
depend on git-read-tree to update the working tree appropriately.

This basically means that git reset turns into

	# Always update the HEAD ref
	git update-ref HEAD "$rev"

	case "--soft"
		# do nothing to index/working tree
	case "--hard"
		# read index _and_ update working tree
		git-read-tree --reset -u "$rev"
	case "--mixed"
		# update just index, report on working tree differences
		git-read-tree --reset "$rev"
		git-update-index --refresh

which is what it was always semantically doing, it just did it in a
rather strange way because it was written to not expect git-read-tree to 
do anything to the working tree.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

NOTE! The switch to use "git-read-tree -u" does actually result in a real 
change: we will now remove files that were in the index but not in HEAD 
before (ie files added with "git add"). I'd argue that this is a bug-fix.

 git-reset.sh |   50 ++++----------------------------------------------
 1 files changed, 4 insertions(+), 46 deletions(-)

diff --git a/git-reset.sh b/git-reset.sh
index 6cb073c..0ee3e3e 100755
--- a/git-reset.sh
+++ b/git-reset.sh
@@ -6,6 +6,7 @@ USAGE='[--mixed | --soft | --hard]  [<co
 tmp=${GIT_DIR}/reset.$$
 trap 'rm -f $tmp-*' 0 1 2 3 15
 
+update=
 reset_type=--mixed
 case "$1" in
 --mixed | --soft | --hard)
@@ -23,24 +24,7 @@ # We need to remember the set of paths t
 # behind before a hard reset, so that we can remove them.
 if test "$reset_type" = "--hard"
 then
-	{
-		git-ls-files --stage -z
-		git-rev-parse --verify HEAD 2>/dev/null &&
-		git-ls-tree -r -z HEAD
-	} | perl -e '
-	    use strict;
-	    my %seen;
-	    $/ = "\0";
-	    while (<>) {
-		chomp;
-		my ($info, $path) = split(/\t/, $_);
-		next if ($info =~ / tree /);
-		if (!$seen{$path}) {
-			$seen{$path} = 1;
-			print "$path\0";
-		}
-	    }
-	' >$tmp-exists
+	update=-u
 fi
 
 # Soft reset does not touch the index file nor the working tree
@@ -54,7 +38,7 @@ then
 		die "Cannot do a soft reset in the middle of a merge."
 	fi
 else
-	git-read-tree --reset "$rev" || exit
+	git-read-tree --reset $update "$rev" || exit
 fi
 
 # Any resets update HEAD to the head being switched to.
@@ -68,33 +52,7 @@ git-update-ref HEAD "$rev"
 
 case "$reset_type" in
 --hard )
-	# Hard reset matches the working tree to that of the tree
-	# being switched to.
-	git-checkout-index -f -u -q -a
-	git-ls-files --cached -z |
-	perl -e '
-		use strict;
-		my (%keep, $fh);
-		$/ = "\0";
-		while (<STDIN>) {
-			chomp;
-			$keep{$_} = 1;
-		}
-		open $fh, "<", $ARGV[0]
-			or die "cannot open $ARGV[0]";
-		while (<$fh>) {
-			chomp;
-			if (! exists $keep{$_}) {
-				# it is ok if this fails -- it may already
-				# have been culled by checkout-index.
-				unlink $_;
-				while (s|/[^/]*$||) {
-					rmdir($_) or last;
-				}
-			}
-		}
-	' $tmp-exists
-	;;
+	;; # Nothing else to do
 --soft )
 	;; # Nothing else to do
 --mixed )

^ permalink raw reply related

* Re: Howto get the merge-base ?
From: Junio C Hamano @ 2006-05-14 18:12 UTC (permalink / raw)
  To: Bertrand Jacquin; +Cc: git
In-Reply-To: <4fb292fa0605141104j6c73c1eao5a8eeea9ad1b6282@mail.gmail.com>

"Bertrand Jacquin" <beber.mailing@gmail.com> writes:

> On 5/14/06, Junio C Hamano <junkio@cox.net> wrote:
>> "Bertrand Jacquin" <beber.mailing@gmail.com> writes:
>>
>> > I'm trying to know which commit it the parent of a merge.
>> > For exemple if I do that :
>> >
>> >   o Merge
>> >  / \
>> > /   \
>> > |   |
>> > |   o Commit D
>> > |   |
>> > |   o Commit C
>> > |   |
>> > o   | Commit B
>> > \  /
>> >  \/
>> >  o Commit A
>> >  |
>> >  o Init
>> >
> No, that's just on the following of git-send-mail-commit.sh thread (or
> something near). To make a readable merge mail with diffstat and
> summury.

For that you do not want merge-base.  If your mainline was A-B
and you merged a side branch B-C-D with the merge, then to
people who tracked your head (that's the audience of "merge
mail", I presume) they just need to see:

	git diff --stat M^1..M
	git rev-list --no-merges --pretty M^1..M | git shortlog

The diffstat is "what damage was inflicted on the branch you
have been following with this merge", so the diff between the
trees before and after the merge is what you want.  And the
rev-list piped to shortlog is to show "what commits were _not_
present on this branch before the merge, but are present after
the merge" (it could be spelled M^1..M^2 but the above would
handle octopus as well).

^ permalink raw reply

* Re: Howto get the merge-base ?
From: Bertrand Jacquin @ 2006-05-14 18:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v3bfc33hm.fsf@assigned-by-dhcp.cox.net>

On 5/14/06, Junio C Hamano <junkio@cox.net> wrote:
> "Bertrand Jacquin" <beber.mailing@gmail.com> writes:
>
> > I'm trying to know which commit it the parent of a merge.
> > For exemple if I do that :
> >
> >   o Merge
> >  / \
> > /   \
> > |   |
> > |   o Commit D
> > |   |
> > |   o Commit C
> > |   |
> > o   | Commit B
> > \  /
> >  \/
> >  o Commit A
> >  |
> >  o Init
> >
> > How could I know that ``Commit A'' is the merge-base of ``Merge'' ?
>
> > I try to get this git-merge-base but result is strange and quiet
> > mysterious as he return me always second args I passed to.
>
> It is mysterious to me because you did not say what you gave as
> arguments ;-).

I was using git-merge-base ``merge-ish'' ``comit-ish''. Docs about it
is atrocious so was trying many but not the one good. Blam me.

> If I am reading you correctly, you already have a "Merge"
> commit, made by you or somebody else, and are trying to figure
> out where the merge base was.  If that is the case:
>
>         git-merge-base Merge^1 Merge^2
>
> in other words
>
>         git-merge-base CommitB CommitD

That's ok :)

> is what you are looking for?

Yes ! Thanks :)

> But what do you need that information for?  To reproduce
> somebody else's merge?

No, that's just on the following of git-send-mail-commit.sh thread (or
something near). To make a readable merge mail with diffstat and
summury.

-- 
Beber
#e.fr@freenode

^ permalink raw reply

* Re: Allow one-way tree merge to remove old files
From: Junio C Hamano @ 2006-05-14 17:51 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0605141040210.3866@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> For some random reason (probably just because nobody noticed), the one-way 
> merge strategy didn't mark deleted files as deleted, so if you used
>
> 	git-read-tree -m -u <newtree>
>
> it would update the files that got changed in the index, but it would not 
> delete the files that got deleted.

Good catch.  I think it is a leftover from the days we did not
have -u there; returning 0 was a way to say "there is no cache
entry resulting from this round of path-merge".

^ permalink raw reply

* Re: Howto get the merge-base ?
From: Junio C Hamano @ 2006-05-14 17:44 UTC (permalink / raw)
  To: Bertrand Jacquin; +Cc: git
In-Reply-To: <4fb292fa0605141021r20cefaa0he592b9c713ede333@mail.gmail.com>

"Bertrand Jacquin" <beber.mailing@gmail.com> writes:

> I'm trying to know which commit it the parent of a merge.
> For exemple if I do that :
>
>   o Merge
>  / \
> /   \
> |   |
> |   o Commit D
> |   |
> |   o Commit C
> |   |
> o   | Commit B
> \  /
>  \/
>  o Commit A
>  |
>  o Init
>
> How could I know that ``Commit A'' is the merge-base of ``Merge'' ?

> I try to get this git-merge-base but result is strange and quiet
> mysterious as he return me always second args I passed to.

It is mysterious to me because you did not say what you gave as
arguments ;-).

If I am reading you correctly, you already have a "Merge"
commit, made by you or somebody else, and are trying to figure
out where the merge base was.  If that is the case:

	git-merge-base Merge^1 Merge^2

in other words

	git-merge-base CommitB CommitD        

is what you are looking for?

But what do you need that information for?  To reproduce
somebody else's merge?

^ permalink raw reply

* Allow one-way tree merge to remove old files
From: Linus Torvalds @ 2006-05-14 17:43 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


For some random reason (probably just because nobody noticed), the one-way 
merge strategy didn't mark deleted files as deleted, so if you used

	git-read-tree -m -u <newtree>

it would update the files that got changed in the index, but it would not 
delete the files that got deleted.

This should fix it, and I can't imagine that anybody depends on the old 
strange "update only existing files" behaviour.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
diff --git a/read-tree.c b/read-tree.c
index e926e4c..11157f4 100644
--- a/read-tree.c
+++ b/read-tree.c
@@ -684,7 +684,7 @@ static int oneway_merge(struct cache_ent
 			     merge_size);
 
 	if (!a)
-		return 0;
+		return deleted_entry(old, NULL);
 	if (old && same(old, a)) {
 		return keep_entry(old);
 	}

^ permalink raw reply related

* Re: git diff: support "-U" and "--unified" options properly
From: Junio C Hamano @ 2006-05-14 17:36 UTC (permalink / raw)
  To: Josef Weidendorfer; +Cc: git
In-Reply-To: <200605141457.17314.Josef.Weidendorfer@gmx.de>

Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:

> On Saturday 13 May 2006 23:22, you wrote:
>>  * remotes/ information from .git/config (js/fetchconfig)
>> ...
>>    [branch "master"]
>> 	remote = "ko-private"
>
> Why is this line needed? In this example, what is the relationship
> of local "master" with the remote? I think it is enough to specify
> the local upstream branch:
>
>  [branch "master"]
>     origin = "ko-master"

I confused you by not describing the flow. 

There are four repositories involved:

 a. siamese:/git (my local development repo)
 b. x86-64.kernel.org:~/git (my kernel.org build repo)
 c. i386.kernel.org:~/git   (ditto but for i386)
 d. kernel.org:/pub/scm/git/git.git (public distribution point)

The workflow is:

 1. maint/master/next/pu are prepared on a.

 2. maint/master/next/pu are pushed to b. and c. as
    maint/origin/next/pu.  b. and c. always have their "master"
    checked out.

 3. I go to b. and c., and "pull . origin" to start my build
    there on all four branches.

 4. When things go well, I come back to a.

 5. Then maint/master/next/pu are pushed to d. without renaming.

 6. To keep track of what have been pushed, maint/master/next/pu
    from d. are fetched to a., with ko- prefixed.  There is no
    "when on this branch" involved in this step.  Regardless of
    which branch I currently on, the fetch goes fine.  I never
    check out ko-* branches on a, nor merge from ko-* branches.

 7. Go back to step 1 and start the next cycle.  ko-maint, ko-master
    and ko-next reminds me not to rewind maint/master/next while I
    shuffle the changes to discard botched commits beyond them.

> So we need
>
>  [branch "ko-master"]
>     tracksremote = "master of ko-private"
>
> This also would specify that we are not allowed to commit on "ko-master".

For my workflow, it is "master of ko"; your notation expresses
the same constraints more explicitly by being more special
purpose: "This tracks that one so never touch it any way other
than fetching into it" (we may not even allow checking out
"ko-master" -- I dunno).  

One issue you might want to think about is it is far more
efficient to fetch multiple branches from the same git://... URL
is than fetching them one by one.  The push has exactly the same
property.

Another thing is the above talks only about constraints, and the
user has to go all over the config file to find "xxx to
ko-private" in order to figure out what happens when he says
"pull ko-private".

>> ...
>>    [remote "ko"]
>>    	url = "kernel.org:/pub/scm/git/git.git"
>>       push = master:master
>> ...

>> 	fetch = master:ko-master
>

> These specifications more or less are independent from the above,
> as it specifies the defaults when fetching/pushing to the specified remote.

Not really; and what you introduced can conflict with [remote]
specification.  If you have [branch "ko-master"] that says it
tracks remote "master" from "ko" repository, your [remote "ko"]
should have not say "fetch = foobla:ko-master", so in that sense
it is redundant and inviting later inconsistency.  The only
information you added with "tracksremote" is the branch is used
to track what is on remote (implying we'd better not touch it
ourselves), so I'd say this would make sense

	[branch "ko-master"]
        	tracksremote ; bool!
	[remote "ko"]
        	url = git://git.kernel.org/pub/scm/git/git.git
        	fetch = master:ko-master

or alternatively this would:

	[branch "ko-master"]
		tracksremote = "master of ko"
	[remote "ko"]
        	url = git://git.kernel.org/pub/scm/git/git.git

^ permalink raw reply

* Howto get the merge-base ?
From: Bertrand Jacquin @ 2006-05-14 17:21 UTC (permalink / raw)
  To: Git Mailing List

Hi,

I'm trying to know which commit it the parent of a merge.
For exemple if I do that :

   o Merge
  / \
 /   \
 |   |
 |   o Commit D
 |   |
 |   o Commit C
 |   |
 o   | Commit B
 \  /
  \/
  o Commit A
  |
  o Init

How could I know that ``Commit A'' is the merge-base of ``Merge'' ?

I try to get this git-merge-base but result is strange and quiet
mysterious as he return me always second args I passed to. I'm using
git 1.3.2

-- 
Beber
#e.fr@freenode

^ permalink raw reply

* Re: [PATCH 5/5] diff: parse U/u/unified options with an optional integer arg
From: Linus Torvalds @ 2006-05-14 16:33 UTC (permalink / raw)
  To: Eric Wong; +Cc: git
In-Reply-To: <11476199631085-git-send-email-normalperson@yhbt.net>



On Sun, 14 May 2006, Eric Wong wrote:
> 
> -u (lowercase) now accepts an optional arg, like -U (GNU diff
> -u also has this behavior).

Actually, modern GNU "diff -u5" will say

	diff: `-5' option is obsolete; use `-U 5'
	diff: Try `diff --help' for more information.

and exit.

I'm not entirely sure why, but I think it's because "u" can be mixed (ie 
with something like "-urN"), while "U" cannot. The GNU diff rule seems to 
be that simple arguments can be mixed together, but arguments with 
parameters cannot. Which sounds sane.

		Linus

^ permalink raw reply

* [PATCH 3/5] gitopt: convert setup_revisions() and friends
From: Eric Wong @ 2006-05-14 15:19 UTC (permalink / raw)
  To: git; +Cc: Eric Wong
In-Reply-To: <11476199622462-git-send-email-normalperson@yhbt.net>

	diff_opt_parse => diff_opt_handle

I've added --raw to diff_opt_handle() for consistency's sake

Lightly tested, some bugs in the original series of submitted
patches were fixed wrt argument parsing for -C,-M,-B).

Passes all tests so that's a good sign.

Signed-off-by: Eric Wong <normalperson@yhbt.net>

---

 builtin-diff.c      |  152 +++++++-------
 builtin-log.c       |    7 -
 commit.c            |   14 +
 diff-files.c        |   25 +-
 diff-index.c        |   18 +-
 diff-stages.c       |   56 +++--
 diff-tree.c         |   15 +
 diff.c              |  183 ++++++++++++-----
 diff.h              |    7 -
 gitopt/diff-files.h |   45 ++++
 gitopt/diff-index.h |   22 ++
 gitopt/diff-tree.h  |   22 ++
 gitopt/diff.h       |   19 ++
 http-push.c         |    2 
 rev-list.c          |   51 +++--
 revision.c          |  542 +++++++++++++++++++++++++++------------------------
 revision.h          |    4 
 17 files changed, 704 insertions(+), 480 deletions(-)
 create mode 100644 gitopt/diff-files.h
 create mode 100644 gitopt/diff-index.h
 create mode 100644 gitopt/diff-tree.h
 create mode 100644 gitopt/diff.h

946397f743f363198d154efaa74d5cc5bd9d6aae
diff --git a/builtin-diff.c b/builtin-diff.c
index d3ac581..3aefd3c 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -12,6 +12,9 @@ #include "diffcore.h"
 #include "revision.h"
 #include "log-tree.h"
 #include "builtin.h"
+#include "gitopt/diff-index.h"
+#include "gitopt/diff-files.h"
+#include "gitopt/diff-tree.h"
 
 /* NEEDSWORK: struct object has place for name but we _do_
  * know mode when we extracted the blob out of a tree, which
@@ -25,26 +28,25 @@ struct blobinfo {
 static const char builtin_diff_usage[] =
 "diff <options> <rev>{0,2} -- <path>*";
 
+static int extra_diff_flags[LAST_DIFF_EXTRA_ID] = { 0 };
+
 static int builtin_diff_files(struct rev_info *revs,
 			      int argc, const char **argv)
 {
-	int silent = 0;
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--base"))
-			revs->max_count = 1;
-		else if (!strcmp(arg, "--ours"))
-			revs->max_count = 2;
-		else if (!strcmp(arg, "--theirs"))
-			revs->max_count = 3;
-		else if (!strcmp(arg, "-q"))
-			silent = 1;
-		else if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
+	int i;
+	struct df_extra_opts dfeo = { revs, 0 };
+
+	if (argc)
+		usage(builtin_diff_usage);
+
+	for (i = 1; i < ARRAY_SIZE(extra_diff_flags); i++) {
+		if (extra_diff_flags[i]) {
+			if (diff_files_opt_handler(NULL, i, &dfeo) > 0)
+				continue;
 			usage(builtin_diff_usage);
-		argv++; argc--;
+		}
 	}
+
 	/*
 	 * Make sure there are NO revision (i.e. pending object) parameter,
 	 * specified rev.max_count is reasonable (0 <= n <= 3), and
@@ -65,7 +67,7 @@ static int builtin_diff_files(struct rev
 	 */
 	if (revs->diffopt.output_format == DIFF_FORMAT_NO_OUTPUT)
 		revs->diffopt.output_format = DIFF_FORMAT_RAW;
-	return run_diff_files(revs, silent);
+	return run_diff_files(revs, dfeo.silent);
 }
 
 static void stuff_change(struct diff_options *opt,
@@ -107,14 +109,9 @@ static int builtin_diff_b_f(struct rev_i
 	/* Blob vs file in the working tree*/
 	struct stat st;
 
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+	if (argc)
+		usage(builtin_diff_usage);
+
 	if (lstat(path, &st))
 		die("'%s': %s", path, strerror(errno));
 	if (!(S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)))
@@ -135,14 +132,9 @@ static int builtin_diff_blobs(struct rev
 	/* Blobs */
 	unsigned mode = canon_mode(S_IFREG | 0644);
 
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+	if (argc)
+		usage(builtin_diff_usage);
+
 	stuff_change(&revs->diffopt,
 		     mode, mode,
 		     blob[0].sha1, blob[1].sha1,
@@ -155,17 +147,19 @@ static int builtin_diff_blobs(struct rev
 static int builtin_diff_index(struct rev_info *revs,
 			      int argc, const char **argv)
 {
-	int cached = 0;
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--cached"))
-			cached = 1;
-		else if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
+	int i, cached = 0;
+
+	if (argc)
+		usage(builtin_diff_usage);
+
+	for (i = 1; i < ARRAY_SIZE(extra_diff_flags); i++) {
+		if (extra_diff_flags[i]) {
+			if (diff_index_opt_handler(NULL, i, &cached) > 0)
+				continue;
 			usage(builtin_diff_usage);
-		argv++; argc--;
+		}
 	}
+
 	/*
 	 * Make sure there is one revision (i.e. pending object),
 	 * and there is no revision filtering parameters.
@@ -183,14 +177,9 @@ static int builtin_diff_tree(struct rev_
 {
 	const unsigned char *(sha1[2]);
 	int swap = 1;
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+
+	if (argc)
+		usage(builtin_diff_usage);
 
 	/* We saw two trees, ent[0] and ent[1].
 	 * unless ent[0] is unintesting, they are swapped
@@ -212,14 +201,9 @@ static int builtin_diff_combined(struct 
 	const unsigned char (*parent)[20];
 	int i;
 
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+	if (argc)
+		usage(builtin_diff_usage);
+
 	if (!revs->dense_combined_merges && !revs->combine_merges)
 		revs->dense_combined_merges = revs->combine_merges = 1;
 	parent = xmalloc(ents * sizeof(*parent));
@@ -243,6 +227,27 @@ static void add_head(struct rev_info *re
 	add_object(obj, &revs->pending_objects, NULL, "HEAD");
 }
 
+static struct opt_spec * diff_combined_ost()
+{
+	struct opt_spec *rv, *tmp;
+
+	tmp = combine_opt_spec(diff_files_ost, diff_tree_ost);
+	rv = combine_opt_spec(tmp, diff_index_ost);
+	free(tmp);
+	return rv;
+}
+
+/* just set flags in here for use by the builtin_diff_* functions  */
+static int diff_combined_handler(struct gitopt_iterator *gi,
+				const int id, void *args)
+{
+	if (id > 0 && id < LAST_DIFF_EXTRA_ID) {
+		extra_diff_flags[id] = 1;
+		return 1;
+	}
+	return 0;
+}
+
 int cmd_diff(int argc, const char **argv, char **envp)
 {
 	struct rev_info rev;
@@ -250,6 +255,9 @@ int cmd_diff(int argc, const char **argv
 	int ents = 0, blobs = 0, paths = 0;
 	const char *path = NULL;
 	struct blobinfo blob[2];
+	struct exec_args *b;
+	struct opt_spec *ost = diff_combined_ost();
+	struct gitopt_extra ge = { ost, diff_combined_handler, NULL };
 
 	/*
 	 * We could get N tree-ish in the rev.pending_objects list.
@@ -275,22 +283,14 @@ int cmd_diff(int argc, const char **argv
 	init_revisions(&rev);
 	rev.diffopt.output_format = DIFF_FORMAT_PATCH;
 
-	argc = setup_revisions(argc, argv, &rev, NULL);
+	b = setup_revisions(argc, argv, &rev, NULL, &ge);
+	free(ost);
+
 	/* Do we have --cached and not have a pending object, then
 	 * default to HEAD by hand.  Eek.
 	 */
-	if (!rev.pending_objects) {
-		int i;
-		for (i = 1; i < argc; i++) {
-			const char *arg = argv[i];
-			if (!strcmp(arg, "--"))
-				break;
-			else if (!strcmp(arg, "--cached")) {
-				add_head(&rev);
-				break;
-			}
-		}
-	}
+	if (!rev.pending_objects && extra_diff_flags[opt_cached])
+		add_head(&rev);
 
 	for (list = rev.pending_objects; list; list = list->next) {
 		struct object *obj = list->item;
@@ -340,17 +340,17 @@ int cmd_diff(int argc, const char **argv
 	if (!ents) {
 		switch (blobs) {
 		case 0:
-			return builtin_diff_files(&rev, argc, argv);
+			return builtin_diff_files(&rev, b->argc, b->argv);
 			break;
 		case 1:
 			if (paths != 1)
 				usage(builtin_diff_usage);
-			return builtin_diff_b_f(&rev, argc, argv, blob, path);
+			return builtin_diff_b_f(&rev, b->argc, b->argv, blob, path);
 			break;
 		case 2:
 			if (paths)
 				usage(builtin_diff_usage);
-			return builtin_diff_blobs(&rev, argc, argv, blob);
+			return builtin_diff_blobs(&rev, b->argc, b->argv, blob);
 			break;
 		default:
 			usage(builtin_diff_usage);
@@ -359,10 +359,10 @@ int cmd_diff(int argc, const char **argv
 	else if (blobs)
 		usage(builtin_diff_usage);
 	else if (ents == 1)
-		return builtin_diff_index(&rev, argc, argv);
+		return builtin_diff_index(&rev, b->argc, b->argv);
 	else if (ents == 2)
-		return builtin_diff_tree(&rev, argc, argv, ent);
+		return builtin_diff_tree(&rev, b->argc, b->argv, ent);
 	else
-		return builtin_diff_combined(&rev, argc, argv, ent, ents);
+		return builtin_diff_combined(&rev, b->argc, b->argv, ent, ents);
 	usage(builtin_diff_usage);
 }
diff --git a/builtin-log.c b/builtin-log.c
index 69f2911..3cfc8ac 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -14,14 +14,15 @@ static int cmd_log_wc(int argc, const ch
 		      struct rev_info *rev)
 {
 	struct commit *commit;
+	struct exec_args *b;
 
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
 	rev->verbose_header = 1;
-	argc = setup_revisions(argc, argv, rev, "HEAD");
+	b = setup_revisions(argc, argv, rev, "HEAD", NULL);
 
-	if (argc > 1)
-		die("unrecognized argument: %s", argv[1]);
+	if (b->argc > 1)
+		die("unrecognized argument: %s", b->argv[0]);
 
 	prepare_revision_walk(rev);
 	setup_pager();
diff --git a/commit.c b/commit.c
index 2717dd8..2343729 100644
--- a/commit.c
+++ b/commit.c
@@ -24,19 +24,19 @@ const char *commit_type = "commit";
 
 enum cmit_fmt get_commit_format(const char *arg)
 {
-	if (!*arg)
+	if (!arg)
 		return CMIT_FMT_DEFAULT;
-	if (!strcmp(arg, "=raw"))
+	if (!strcmp(arg, "raw"))
 		return CMIT_FMT_RAW;
-	if (!strcmp(arg, "=medium"))
+	if (!strcmp(arg, "medium"))
 		return CMIT_FMT_MEDIUM;
-	if (!strcmp(arg, "=short"))
+	if (!strcmp(arg, "short"))
 		return CMIT_FMT_SHORT;
-	if (!strcmp(arg, "=full"))
+	if (!strcmp(arg, "full"))
 		return CMIT_FMT_FULL;
-	if (!strcmp(arg, "=fuller"))
+	if (!strcmp(arg, "fuller"))
 		return CMIT_FMT_FULLER;
-	if (!strcmp(arg, "=oneline"))
+	if (!strcmp(arg, "oneline"))
 		return CMIT_FMT_ONELINE;
 	die("invalid --pretty format");
 }
diff --git a/diff-files.c b/diff-files.c
index b9d193d..431487b 100644
--- a/diff-files.c
+++ b/diff-files.c
@@ -7,6 +7,7 @@ #include "cache.h"
 #include "diff.h"
 #include "commit.h"
 #include "revision.h"
+#include "gitopt/diff-files.h"
 
 static const char diff_files_usage[] =
 "git-diff-files [-q] [-0/-1/2/3 |-c|--cc] [<common diff options>] [<path>...]"
@@ -15,26 +16,18 @@ COMMON_DIFF_OPTIONS_HELP;
 int main(int argc, const char **argv)
 {
 	struct rev_info rev;
-	int silent = 0;
+	struct exec_args *b;
+	struct df_extra_opts dfeo = { &rev, 0 };
+	struct gitopt_extra ge = { diff_files_ost, diff_files_opt_handler,
+				   &dfeo };
 
 	git_config(git_diff_config);
 	init_revisions(&rev);
 	rev.abbrev = 0;
 
-	argc = setup_revisions(argc, argv, &rev, NULL);
-	while (1 < argc && argv[1][0] == '-') {
-		if (!strcmp(argv[1], "--base"))
-			rev.max_count = 1;
-		else if (!strcmp(argv[1], "--ours"))
-			rev.max_count = 2;
-		else if (!strcmp(argv[1], "--theirs"))
-			rev.max_count = 3;
-		else if (!strcmp(argv[1], "-q"))
-			silent = 1;
-		else
-			usage(diff_files_usage);
-		argv++; argc--;
-	}
+	b = setup_revisions(argc, argv, &rev, NULL, &ge);
+	if (b->argc)
+		usage(diff_files_usage);
 	/*
 	 * Make sure there are NO revision (i.e. pending object) parameter,
 	 * rev.max_count is reasonable (0 <= n <= 3),
@@ -50,5 +43,5 @@ int main(int argc, const char **argv)
 	 */
 	if (rev.diffopt.output_format == DIFF_FORMAT_NO_OUTPUT)
 		rev.diffopt.output_format = DIFF_FORMAT_RAW;
-	return run_diff_files(&rev, silent);
+	return run_diff_files(&rev, dfeo.silent);
 }
diff --git a/diff-index.c b/diff-index.c
index 8c9f601..987a00a 100644
--- a/diff-index.c
+++ b/diff-index.c
@@ -2,6 +2,7 @@ #include "cache.h"
 #include "diff.h"
 #include "commit.h"
 #include "revision.h"
+#include "gitopt/diff-index.h"
 
 static const char diff_cache_usage[] =
 "git-diff-index [-m] [--cached] "
@@ -12,21 +13,18 @@ int main(int argc, const char **argv)
 {
 	struct rev_info rev;
 	int cached = 0;
-	int i;
+	struct exec_args *b;
+	struct gitopt_extra ge = { diff_index_ost,
+				diff_index_opt_handler, &cached };
 
 	git_config(git_diff_config);
 	init_revisions(&rev);
 	rev.abbrev = 0;
 
-	argc = setup_revisions(argc, argv, &rev, NULL);
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-			
-		if (!strcmp(arg, "--cached"))
-			cached = 1;
-		else
-			usage(diff_cache_usage);
-	}
+	b = setup_revisions(argc, argv, &rev, NULL, &ge);
+	if (b->argc)
+		usage(diff_cache_usage);
+
 	/*
 	 * Make sure there is one revision (i.e. pending object),
 	 * and there is no revision filtering parameters.
diff --git a/diff-stages.c b/diff-stages.c
index dcd20e7..05129c5 100644
--- a/diff-stages.c
+++ b/diff-stages.c
@@ -4,6 +4,13 @@
 
 #include "cache.h"
 #include "diff.h"
+#include "gitopt.h"
+
+enum diff_stages_ost { opt_r = 1 };
+static const struct opt_spec diff_stages_ost[] = {
+	{ 0,	'r',	0,	0,	opt_r },
+	{ 0, 0 }
+};
 
 static struct diff_options diff_options;
 
@@ -59,40 +66,47 @@ int main(int ac, const char **av)
 	int stage1, stage2;
 	const char *prefix = setup_git_directory();
 	const char **pathspec = NULL;
+	struct exec_args *b;
+	struct opt_spec *ost;
+	struct gitopt_iterator gi;
+	int i;
 
 	git_config(git_diff_config);
 	read_cache();
 	diff_setup(&diff_options);
-	while (1 < ac && av[1][0] == '-') {
-		const char *arg = av[1];
-		if (!strcmp(arg, "-r"))
-			; /* as usual */
-		else {
-			int diff_opt_cnt;
-			diff_opt_cnt = diff_opt_parse(&diff_options,
-						      av+1, ac-1);
-			if (diff_opt_cnt < 0)
-				usage(diff_stages_usage);
-			else if (diff_opt_cnt) {
-				av += diff_opt_cnt;
-				ac -= diff_opt_cnt;
-				continue;
-			}
+
+	ost = combine_opt_spec(diff_ost, diff_stages_ost);
+	gitopt_iter_setup(&gi, ac, av);
+
+	for (b=gi.b; (i=gitopt_iter_parse(&gi, ost)); gitopt_iter_next(&gi)) {
+		switch (i) {
+		case GITOPT_NON_OPTION:
+			b->argv[b->argc++] = gi.argv[gi.pos];
+		case GITOPT_DD:
+		case opt_r: /* as usual */
+			break;
+		case GITOPT_ERROR:
+			usage(diff_stages_usage);
+		default:
+			i = diff_opt_handler(&gi, i, &diff_options);
+			if (i > 0)
+				gi.pos += i - 1;
 			else
 				usage(diff_stages_usage);
 		}
-		ac--; av++;
 	}
+	gitopt_iter_done(&gi);
+	free(ost);
 
-	if (ac < 3 ||
-	    sscanf(av[1], "%d", &stage1) != 1 ||
+	if (b->argc < 2 ||
+	    sscanf(b->argv[0], "%d", &stage1) != 1 ||
 	    ! (0 <= stage1 && stage1 <= 3) ||
-	    sscanf(av[2], "%d", &stage2) != 1 ||
+	    sscanf(b->argv[1], "%d", &stage2) != 1 ||
 	    ! (0 <= stage2 && stage2 <= 3))
 		usage(diff_stages_usage);
 
-	av += 3; /* The rest from av[0] are for paths restriction. */
-	pathspec = get_pathspec(prefix, av);
+	b->argv += 2; /* The rest from b->argv[0] are for paths restriction. */
+	pathspec = get_pathspec(prefix, b->argv);
 
 	if (diff_setup_done(&diff_options) < 0)
 		usage(diff_stages_usage);
diff --git a/diff-tree.c b/diff-tree.c
index 7207867..b5c6072 100644
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -2,6 +2,7 @@ #include "cache.h"
 #include "diff.h"
 #include "commit.h"
 #include "log-tree.h"
+#include "gitopt/diff-tree.h"
 
 static struct rev_info log_tree_opt;
 
@@ -66,23 +67,19 @@ int main(int argc, const char **argv)
 	static struct rev_info *opt = &log_tree_opt;
 	struct object_list *list;
 	int read_stdin = 0;
+	struct exec_args *b;
+	struct gitopt_extra ge = { diff_tree_ost, diff_tree_opt_handler,
+				&read_stdin };
 
 	git_config(git_diff_config);
 	nr_sha1 = 0;
 	init_revisions(opt);
 	opt->abbrev = 0;
 	opt->diff = 1;
-	argc = setup_revisions(argc, argv, opt, NULL);
+	b = setup_revisions(argc, argv, opt, NULL, &ge);
 
-	while (--argc > 0) {
-		const char *arg = *++argv;
-
-		if (!strcmp(arg, "--stdin")) {
-			read_stdin = 1;
-			continue;
-		}
+	if (b->argc)
 		usage(diff_tree_usage);
-	}
 
 	/*
 	 * NOTE! "setup_revisions()" will have inserted the revisions
diff --git a/diff.c b/diff.c
index 7a7b839..7d88dc5 100644
--- a/diff.c
+++ b/diff.c
@@ -10,6 +10,48 @@ #include "diff.h"
 #include "diffcore.h"
 #include "delta.h"
 #include "xdiff-interface.h"
+#include "gitopt.h"
+
+static int diff_scoreopt_parse(const int id, const char *opt);
+
+enum diff_ost_ids {
+	opt_p = GITOPT_DIFF_BASE, opt_raw, opt_patch_with_raw,
+	opt_stat, opt_patch_with_stat,
+	opt_z, opt_l, opt_full_index, opt_name_only, opt_name_status,
+	opt_R, opt_S, opt_s, opt_O, opt_diff_filter,
+	opt_pickaxe_all, opt_pickaxe_regex,
+	opt_B, opt_M, opt_C, opt_find_copies_harder, opt_abbrev,
+	opt_binary
+};
+
+const struct opt_spec diff_ost[] = {
+	{ 0,			'p',	0,	0,	opt_p },
+	{ "unified",		'u',	0,	0,	opt_p },
+	{ "raw",		0,	0,	0,	opt_raw },
+	{ "patch-with-raw",	0,	0,	0,	opt_patch_with_raw },
+	{ "stat",		0,	0,	0,	opt_stat },
+	{ "patch-with-stat",	0,	0,	0,	opt_patch_with_stat },
+	{ 0,			'z',	0,	0,	opt_z },
+	{ 0,			'l',	0,	ARG_INT,	opt_l },
+	{ "full-index",		0,	0,	0,	opt_full_index },
+	{ "name-only",		0,	0,	0,	opt_name_only },
+	{ "name-status",	0,	0,	0,	opt_name_status },
+	{ 0,			'R',	0,	0,	opt_R },
+	{ 0,			'S',	0,	ARG_ONE,	opt_S },
+	{ 0,			's',	0,	0,	opt_s },
+	{ 0,			'O',	0,	ARG_ONE, opt_O },
+	{ "diff-filter",	0,	0,	ARG_ONE, opt_diff_filter },
+	{ "pickaxe-all",	0,	0,	0,	opt_pickaxe_all },
+	{ "pickaxe-regex",	0,	0,	0,	opt_pickaxe_regex },
+	{ 0,			'B',	0,	ARG_OPT,	opt_B },
+	{ 0,			'M',	0,	ARG_OPT,	opt_M },
+	{ 0,			'C',	0,	ARG_OPT,	opt_C },
+	{ "find-copies-harder",	0,	0,	0,	opt_find_copies_harder},
+	{ "abbrev",		0,	0,	ARG_OPTINT, opt_abbrev },
+	{ "binary",		0,	0,	0,	opt_binary },
+	{ 0, 0 }
+};
+
 
 static int use_size_cache;
 
@@ -1222,79 +1264,102 @@ int diff_setup_done(struct diff_options 
 	return 0;
 }
 
-int diff_opt_parse(struct diff_options *options, const char **av, int ac)
+int diff_opt_handler(struct gitopt_iterator *gi, const int id, void *args)
 {
-	const char *arg = av[0];
-	if (!strcmp(arg, "-p") || !strcmp(arg, "-u"))
+	struct diff_options *options = (struct diff_options *)args;
+
+	switch (id) {
+	case opt_p:
 		options->output_format = DIFF_FORMAT_PATCH;
-	else if (!strcmp(arg, "--patch-with-raw")) {
+		break;
+	case opt_raw:
+		options->output_format = DIFF_FORMAT_RAW;
+		break;
+	case opt_patch_with_raw:
 		options->output_format = DIFF_FORMAT_PATCH;
 		options->with_raw = 1;
-	}
-	else if (!strcmp(arg, "--stat"))
+		break;
+	case opt_stat:
 		options->output_format = DIFF_FORMAT_DIFFSTAT;
-	else if (!strcmp(arg, "--patch-with-stat")) {
+		break;
+	case opt_patch_with_stat:
 		options->output_format = DIFF_FORMAT_PATCH;
 		options->with_stat = 1;
-	}
-	else if (!strcmp(arg, "-z"))
+		break;
+	case opt_z:
 		options->line_termination = 0;
-	else if (!strncmp(arg, "-l", 2))
-		options->rename_limit = strtoul(arg+2, NULL, 10);
-	else if (!strcmp(arg, "--full-index"))
+		break;
+	case opt_l:
+		options->rename_limit = strtoul(gi->ea->argv[0], NULL, 10);
+		break;
+	case opt_full_index:
 		options->full_index = 1;
-	else if (!strcmp(arg, "--binary")) {
+		break;
+	case opt_binary:
 		options->output_format = DIFF_FORMAT_PATCH;
 		options->full_index = options->binary = 1;
-	}
-	else if (!strcmp(arg, "--name-only"))
+		break;
+	case opt_name_only:
 		options->output_format = DIFF_FORMAT_NAME;
-	else if (!strcmp(arg, "--name-status"))
+		break;
+	case opt_name_status:
 		options->output_format = DIFF_FORMAT_NAME_STATUS;
-	else if (!strcmp(arg, "-R"))
+		break;
+	case opt_R:
 		options->reverse_diff = 1;
-	else if (!strncmp(arg, "-S", 2))
-		options->pickaxe = arg + 2;
-	else if (!strcmp(arg, "-s"))
+		break;
+	case opt_S:
+		options->pickaxe = gi->ea->argv[0];
+		break;
+	case opt_s:
 		options->output_format = DIFF_FORMAT_NO_OUTPUT;
-	else if (!strncmp(arg, "-O", 2))
-		options->orderfile = arg + 2;
-	else if (!strncmp(arg, "--diff-filter=", 14))
-		options->filter = arg + 14;
-	else if (!strcmp(arg, "--pickaxe-all"))
+		break;
+	case opt_O:
+		options->orderfile = gi->ea->argv[0];
+		break;
+	case opt_diff_filter:
+		options->filter = gi->ea->argv[0];
+		break;
+	case opt_pickaxe_all:
 		options->pickaxe_opts = DIFF_PICKAXE_ALL;
-	else if (!strcmp(arg, "--pickaxe-regex"))
+		break;
+	case opt_pickaxe_regex:
 		options->pickaxe_opts = DIFF_PICKAXE_REGEX;
-	else if (!strncmp(arg, "-B", 2)) {
-		if ((options->break_opt =
-		     diff_scoreopt_parse(arg)) == -1)
+		break;
+	case opt_B:
+		if (gi->ea->argc && (options->break_opt =
+		     diff_scoreopt_parse(id, gi->ea->argv[1])) == -1)
 			return -1;
-	}
-	else if (!strncmp(arg, "-M", 2)) {
-		if ((options->rename_score =
-		     diff_scoreopt_parse(arg)) == -1)
+		break;
+	case opt_M:
+		if (gi->ea->argc && (options->rename_score =
+		     diff_scoreopt_parse(id, gi->ea->argv[1])) == -1)
 			return -1;
 		options->detect_rename = DIFF_DETECT_RENAME;
-	}
-	else if (!strncmp(arg, "-C", 2)) {
-		if ((options->rename_score =
-		     diff_scoreopt_parse(arg)) == -1)
+		break;
+	case opt_C:
+		if (gi->ea->argc && (options->rename_score =
+		     diff_scoreopt_parse(id, gi->ea->argv[1])) == -1)
 			return -1;
 		options->detect_rename = DIFF_DETECT_COPY;
-	}
-	else if (!strcmp(arg, "--find-copies-harder"))
+		break;
+	case opt_find_copies_harder:
 		options->find_copies_harder = 1;
-	else if (!strcmp(arg, "--abbrev"))
-		options->abbrev = DEFAULT_ABBREV;
-	else if (!strncmp(arg, "--abbrev=", 9)) {
-		options->abbrev = strtoul(arg + 9, NULL, 10);
-		if (options->abbrev < MINIMUM_ABBREV)
-			options->abbrev = MINIMUM_ABBREV;
-		else if (40 < options->abbrev)
-			options->abbrev = 40;
-	}
-	else
+		break;
+	case opt_abbrev:
+		if (gi->ea->argc == 1)
+			options->abbrev = DEFAULT_ABBREV;
+		else {
+			options->abbrev = strtoul(gi->ea->argv[1], NULL, 10);
+			if (options->abbrev < MINIMUM_ABBREV)
+				options->abbrev = MINIMUM_ABBREV;
+			else if (40 < options->abbrev)
+				options->abbrev = 40;
+		}
+		break;
+	default:
 		return 0;
+	}
 	return 1;
 }
 
@@ -1304,9 +1369,13 @@ static int parse_num(const char **cp_p)
 	int ch, dot;
 	const char *cp = *cp_p;
 
+	if (!cp)
+		return 0;
+
 	num = 0;
 	scale = 1;
 	dot = 0;
+
 	for(;;) {
 		ch = *cp;
 		if ( !dot && ch == '.' ) {
@@ -1334,21 +1403,15 @@ static int parse_num(const char **cp_p)
 	return (num >= scale) ? MAX_SCORE : (MAX_SCORE * num / scale);
 }
 
-int diff_scoreopt_parse(const char *opt)
+static int diff_scoreopt_parse(const int id, const char *opt)
 {
-	int opt1, opt2, cmd;
-
-	if (*opt++ != '-')
-		return -1;
-	cmd = *opt++;
-	if (cmd != 'M' && cmd != 'C' && cmd != 'B')
-		return -1; /* that is not a -M, -C nor -B option */
+	int opt1, opt2;
 
 	opt1 = parse_num(&opt);
-	if (cmd != 'B')
+	if (id != opt_B)
 		opt2 = 0;
 	else {
-		if (*opt == 0)
+		if (!opt || *opt == 0)
 			opt2 = 0;
 		else if (*opt != '/')
 			return -1; /* we expect -B80/99 or -B80 */
@@ -1357,7 +1420,7 @@ int diff_scoreopt_parse(const char *opt)
 			opt2 = parse_num(&opt);
 		}
 	}
-	if (*opt != 0)
+	if (opt && *opt != 0)
 		return -1;
 	return opt1 | (opt2 << 16);
 }
diff --git a/diff.h b/diff.h
index d052608..fa44d1b 100644
--- a/diff.h
+++ b/diff.h
@@ -5,6 +5,7 @@ #ifndef DIFF_H
 #define DIFF_H
 
 #include "tree-walk.h"
+#include "gitopt.h"
 
 struct rev_info;
 struct diff_options;
@@ -96,15 +97,13 @@ extern void diff_change(struct diff_opti
 extern void diff_unmerge(struct diff_options *,
 			 const char *path);
 
-extern int diff_scoreopt_parse(const char *opt);
-
 #define DIFF_SETUP_REVERSE      	1
 #define DIFF_SETUP_USE_CACHE		2
 #define DIFF_SETUP_USE_SIZE_CACHE	4
 
 extern int git_diff_config(const char *var, const char *value);
 extern void diff_setup(struct diff_options *);
-extern int diff_opt_parse(struct diff_options *, const char **, int);
+extern int diff_opt_handler(struct gitopt_iterator *, const int, void *);
 extern int diff_setup_done(struct diff_options *);
 
 #define DIFF_DETECT_RENAME	1
@@ -117,6 +116,8 @@ extern void diffcore_std(struct diff_opt
 
 extern void diffcore_std_no_resolve(struct diff_options *);
 
+extern const struct opt_spec diff_ost[];
+
 #define COMMON_DIFF_OPTIONS_HELP \
 "\ncommon diff options:\n" \
 "  -z            output diff-raw with lines terminated with NUL.\n" \
diff --git a/gitopt/diff-files.h b/gitopt/diff-files.h
new file mode 100644
index 0000000..85c84ba
--- /dev/null
+++ b/gitopt/diff-files.h
@@ -0,0 +1,45 @@
+#ifndef GITOPT_DIFF_FILES_H
+#define GITOPT_DIFF_FILES_H
+
+#include "../gitopt/diff.h"
+#include "../revision.h"
+
+struct df_extra_opts {
+	struct rev_info *rev;
+	int silent;
+};
+
+static const struct opt_spec diff_files_ost[] = {
+	{ "base",	0,	0,	0,	opt_base },
+	{ "ours",	0,	0,	0,	opt_ours },
+	{ "theirs",	0,	0,	0,	opt_theirs },
+	{ 0,		'q',	0,	0,	opt_q },
+	{ 0, 0 }
+};
+
+static int diff_files_opt_handler(struct gitopt_iterator *gi,
+				const int id, void *args)
+{
+	int rv = 1;
+	struct df_extra_opts *dfeo = (struct df_extra_opts *)args;
+
+	switch (id) {
+	case opt_base:
+		dfeo->rev->max_count = 1;
+		break;
+	case opt_ours:
+		dfeo->rev->max_count = 2;
+		break;
+	case opt_theirs:
+		dfeo->rev->max_count = 3;
+		break;
+	case opt_q:
+		dfeo->silent = 1;
+		break;
+	default:
+		rv = 0;
+	}
+	return rv;
+}
+
+#endif
diff --git a/gitopt/diff-index.h b/gitopt/diff-index.h
new file mode 100644
index 0000000..aff0944
--- /dev/null
+++ b/gitopt/diff-index.h
@@ -0,0 +1,22 @@
+#ifndef GITOPT_DIFF_INDEX_H
+#define GITOPT_DIFF_INDEX_H
+
+#include "../gitopt/diff.h"
+
+static const struct opt_spec diff_index_ost[] = {
+	{ "cached",	0,	0,	0,	opt_cached },
+	{ 0, 0 }
+};
+
+static int diff_index_opt_handler(struct gitopt_iterator *gi,
+				const int id, void *args)
+{
+	if (id == opt_cached) {
+		*(int *)args = 1;
+		return 1;
+	}
+	return 0;
+}
+
+#endif
+
diff --git a/gitopt/diff-tree.h b/gitopt/diff-tree.h
new file mode 100644
index 0000000..cfee9b5
--- /dev/null
+++ b/gitopt/diff-tree.h
@@ -0,0 +1,22 @@
+#ifndef GITOPT_DIFF_TREE_H
+#define GITOPT_DIFF_TREE_H
+
+#include "../gitopt/diff.h"
+
+static const struct opt_spec diff_tree_ost[] = {
+	{ "stdin",	0,	0,	0,	opt_stdin },
+	{ 0, 0 }
+};
+
+/* inline to suppress a warning as it's not used in builtin_diff_tree */
+static inline int diff_tree_opt_handler(struct gitopt_iterator *gi,
+				const int id, void *args)
+{
+	if (id == opt_stdin) {
+		*(int *)args = 1;
+		return 1;
+	}
+	return 0;
+}
+
+#endif
diff --git a/gitopt/diff.h b/gitopt/diff.h
new file mode 100644
index 0000000..abc2c7a
--- /dev/null
+++ b/gitopt/diff.h
@@ -0,0 +1,19 @@
+#ifndef GITOPT_DIFF_H
+#define GITOPT_DIFF_H
+
+#include "../gitopt.h"
+
+enum diff_extra_ids {
+	/* diff-files */
+	opt_base = 1, opt_ours, opt_theirs, opt_q,
+
+	/* diff-tree */
+	opt_stdin,
+
+	/* diff-index */
+	opt_cached,
+
+	LAST_DIFF_EXTRA_ID,
+};
+
+#endif /* GITOPT_DIFF_H */
diff --git a/http-push.c b/http-push.c
index b4327d9..9c16f3b 100644
--- a/http-push.c
+++ b/http-push.c
@@ -2499,7 +2499,7 @@ int main(int argc, char **argv)
 			commit_argc++;
 		}
 		init_revisions(&revs);
-		setup_revisions(commit_argc, commit_argv, &revs, NULL);
+		setup_revisions(commit_argc, commit_argv, &revs, NULL, NULL);
 		free(new_sha1_hex);
 		if (old_sha1_hex) {
 			free(old_sha1_hex);
diff --git a/rev-list.c b/rev-list.c
index 8b0ec38..7baeb52 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -291,34 +291,49 @@ static void mark_edges_uninteresting(str
 	}
 }
 
+enum rev_list_ids { opt_header = 1, opt_timestamp, opt_bisect };
+
+static const struct opt_spec rev_list_ost[] = {
+	{ "header",	0,	0,	0,	opt_header },
+	{ "timestamp",	0,	0,	0,	opt_timestamp },
+	{ "bisect",	0,	0,	0,	opt_bisect },
+	{ 0, 0 }
+};
+
+static int rev_list_opt_handler(struct gitopt_iterator *gi,
+				const int id, void *args)
+{
+	int rv = 1;
+	switch (id) {
+	case opt_header:
+		revs.verbose_header = 1;
+		break;
+	case opt_timestamp:
+		show_timestamp = 1;
+		break;
+	case opt_bisect:
+		bisect_list = 1;
+		break;
+	default:
+		rv = 0;
+	}
+	return rv;
+}
+
 int main(int argc, const char **argv)
 {
 	struct commit_list *list;
-	int i;
+	struct exec_args *b;
+	struct gitopt_extra ge = { rev_list_ost, rev_list_opt_handler, NULL };
 
 	init_revisions(&revs);
 	revs.abbrev = 0;
 	revs.commit_format = CMIT_FMT_UNSPECIFIED;
-	argc = setup_revisions(argc, argv, &revs, NULL);
-
-	for (i = 1 ; i < argc; i++) {
-		const char *arg = argv[i];
+	b = setup_revisions(argc, argv, &revs, NULL, &ge);
 
-		if (!strcmp(arg, "--header")) {
-			revs.verbose_header = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--timestamp")) {
-			show_timestamp = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--bisect")) {
-			bisect_list = 1;
-			continue;
-		}
+	if (b->argc)
 		usage(rev_list_usage);
 
-	}
 	if (revs.commit_format != CMIT_FMT_UNSPECIFIED) {
 		/* The command line has a --pretty  */
 		hdr_termination = '\n';
diff --git a/revision.c b/revision.c
index 2294b16..aeaf52c 100644
--- a/revision.c
+++ b/revision.c
@@ -6,6 +6,7 @@ #include "commit.h"
 #include "diff.h"
 #include "refs.h"
 #include "revision.h"
+#include "gitopt.h"
 
 static char *path_name(struct name_path *path, const char *name)
 {
@@ -534,6 +535,136 @@ void init_revisions(struct rev_info *rev
 	diff_setup(&revs->diffopt);
 }
 
+enum setup_revision_ost_ids {
+	opt_max_count = GITOPT_SR_BASE, opt_max_age, opt_min_age,
+	opt_since, opt_until, opt_all, opt_not, opt_default,
+	opt_topo_order, opt_date_order, opt_parents, opt_dense,
+	opt_sparse, opt_remove_empty, opt_no_merges, opt_boundary,
+	opt_objects, opt_objects_edge, opt_unpacked,
+	opt_r, opt_t, opt_m, opt_c, opt_cc, opt_v,
+	opt_pretty, opt_root, opt_no_commit_id, opt_always,
+	opt_abbrev, opt_no_abbrev, opt_abbrev_commit, opt_full_diff
+};
+
+static const struct opt_spec setup_revisions_ost[] = {
+	{ "max-count",		'n',	0,	ARG_INT,	opt_max_count },
+	{ 0,			' ',	0,	ARG_INT,	opt_max_count },
+	{ "max-age",		0,	0,	ARG_INT,	opt_max_age },
+	{ "min-age",		0,	0,	ARG_INT,	opt_min_age },
+	{ "since",		0,	0,	ARG_ONE,	opt_since },
+	{ "after",		0,	0,	ARG_ONE,	opt_since },
+	{ "before",		0,	0,	ARG_ONE,	opt_until },
+	{ "until",		0,	0,	ARG_ONE,	opt_until },
+	{ "all",		0,	0,	0,		opt_all },
+	{ "not",		0,	0,	0,		opt_not },
+	{ "default",		0,	0,	ARG_ONE,	opt_default },
+	{ "topo-order",		0,	0,	0,	opt_topo_order },
+	{ "date-order",		0,	0,	0,	opt_date_order },
+	{ "parents",		0,	0,	0,	opt_parents },
+	{ "dense",		0,	0,	0,	opt_dense },
+	{ "sparse",		0,	0,	0,	opt_sparse },
+	{ "remove-empty",	0,	0,	0,	opt_remove_empty },
+	{ "no-merges",		0,	0,	0,	opt_no_merges },
+	{ "boundary",		0,	0,	0,	opt_boundary },
+	{ "objects",		0,	0,	0,	opt_objects },
+	{ "objects-edge",	0,	0,	0,	opt_objects_edge },
+	{ "unpacked",		0,	0,	0,	opt_unpacked },
+	{ 0,			'r',	0,	0,	opt_r },
+	{ 0,			't',	0,	0,	opt_t },
+	{ 0,			'm',	0,	0,	opt_m },
+	{ 0,			'c',	0,	0,	opt_c },
+	{ "cc",			0,	0,	0,	opt_cc },
+	{ 0,			'v',	0,	0,	opt_v },
+	{ "pretty",		0,	0,	ARG_OPT,	opt_pretty },
+	{ "root",		0,	0,	0,	opt_root },
+	{ "no-commit-id",	0,	0,	0,	opt_no_commit_id },
+	{ "always",		0,	0,	0,	opt_always },
+	{ "no-abbrev",		0,	0,	0,	opt_no_abbrev },
+	{ "abbrev",		0,	0,	0,	opt_abbrev },
+	{ "abbrev-commit",	0,	0,	0,	opt_abbrev_commit },
+	{ "full-diff",		0,	0,	0,	opt_full_diff },
+	{ 0, 0 }
+};
+
+static int setup_revision(struct gitopt_iterator *gi, struct rev_info *revs,
+			int flags, int seen_dashdash)
+{
+	const char *arg = gi->argv[gi->pos];
+	unsigned char sha1[20];
+	struct object *object;
+	char *dotdot;
+	int local_flags;
+
+	dotdot = strstr(arg, "..");
+	if (dotdot) {
+		unsigned char from_sha1[20];
+		const char *next = dotdot + 2;
+		const char *this = arg;
+		*dotdot = 0;
+		if (!*next)
+			next = "HEAD";
+		if (dotdot == arg)
+			this = "HEAD";
+		if (!get_sha1(this, from_sha1) &&
+		    !get_sha1(next, sha1)) {
+			struct object *exclude;
+			struct object *include;
+
+			exclude = get_reference(revs, this, from_sha1,
+						flags ^ UNINTERESTING);
+			include = get_reference(revs, next, sha1, flags);
+			if (!exclude || !include)
+				die("Invalid revision range %s..%s", arg, next);
+
+			if (!seen_dashdash) {
+				*dotdot = '.';
+				verify_non_filename(revs->prefix, arg);
+			}
+			add_pending_object(revs, exclude, this);
+			add_pending_object(revs, include, next);
+			return 1;
+		}
+		*dotdot = '.';
+	}
+
+	dotdot = strstr(arg, "^@");
+	if (dotdot && !dotdot[2]) {
+		*dotdot = 0;
+		if (add_parents_only(revs, arg, flags))
+			return 1;
+		*dotdot = '^';
+	}
+	local_flags = 0;
+	if (*arg == '^') {
+		local_flags = UNINTERESTING;
+		arg++;
+	}
+	if (get_sha1(arg, sha1) < 0) {
+		int j;
+
+		if (seen_dashdash || local_flags)
+			die("bad revision '%s'", arg);
+
+		/* If we didn't have a "--":
+		 * (1) all filenames must exist;
+		 * (2) all rev-args must not be interpretable
+		 *     as a valid filename.
+		 * but the latter we have checked in the main loop.
+		 */
+		for (j = gi->pos; j < gi->argc; j++)
+			verify_filename(revs->prefix, gi->argv[j]);
+
+		revs->prune_data = get_pathspec(revs->prefix,
+						gi->argv + gi->pos);
+		return 1;
+	}
+	if (!seen_dashdash)
+		verify_non_filename(revs->prefix, arg);
+	object = get_reference(revs, arg, sha1, flags ^ local_flags);
+	add_pending_object(revs, object, arg);
+	return 1;
+}
+
 /*
  * Parse revision information, filling in the "rev_info" structure,
  * and removing the used arguments from the argument list.
@@ -541,11 +672,14 @@ void init_revisions(struct rev_info *rev
  * Returns the number of arguments left that weren't recognized
  * (which are also moved to the head of the argument list)
  */
-int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def)
+struct exec_args *setup_revisions(int argc, const char **argv,
+					struct rev_info *revs, const char *def,
+					const struct gitopt_extra *ge)
 {
-	int i, flags, seen_dashdash;
-	const char **unrecognized = argv + 1;
-	int left = 1;
+	int i, id, flags, seen_dashdash;
+	struct gitopt_iterator gi;
+	struct exec_args *ea, *b;
+	struct opt_spec *ost;
 
 	/* First, search for "--" */
 	seen_dashdash = 0;
@@ -561,261 +695,158 @@ int setup_revisions(int argc, const char
 	}
 
 	flags = 0;
-	for (i = 1; i < argc; i++) {
-		struct object *object;
-		const char *arg = argv[i];
-		unsigned char sha1[20];
-		char *dotdot;
-		int local_flags;
+	ost = combine_opt_spec(setup_revisions_ost, diff_ost);
+	if (ge) {
+		struct opt_spec *tmp = ost;
+		ost = combine_opt_spec(tmp, ge->ost);
+		free(tmp);
+	}
 
-		if (*arg == '-') {
-			int opts;
-			if (!strncmp(arg, "--max-count=", 12)) {
-				revs->max_count = atoi(arg + 12);
-				continue;
-			}
-			/* accept -<digit>, like traditional "head" */
-			if ((*arg == '-') && isdigit(arg[1])) {
-				revs->max_count = atoi(arg + 1);
-				continue;
-			}
-			if (!strcmp(arg, "-n")) {
-				if (argc <= i + 1)
-					die("-n requires an argument");
-				revs->max_count = atoi(argv[++i]);
-				continue;
-			}
-			if (!strncmp(arg,"-n",2)) {
-				revs->max_count = atoi(arg + 2);
-				continue;
-			}
-			if (!strncmp(arg, "--max-age=", 10)) {
-				revs->max_age = atoi(arg + 10);
-				continue;
-			}
-			if (!strncmp(arg, "--since=", 8)) {
-				revs->max_age = approxidate(arg + 8);
-				continue;
-			}
-			if (!strncmp(arg, "--after=", 8)) {
-				revs->max_age = approxidate(arg + 8);
-				continue;
-			}
-			if (!strncmp(arg, "--min-age=", 10)) {
-				revs->min_age = atoi(arg + 10);
-				continue;
-			}
-			if (!strncmp(arg, "--before=", 9)) {
-				revs->min_age = approxidate(arg + 9);
-				continue;
-			}
-			if (!strncmp(arg, "--until=", 8)) {
-				revs->min_age = approxidate(arg + 8);
-				continue;
-			}
-			if (!strcmp(arg, "--all")) {
-				handle_all(revs, flags);
-				continue;
-			}
-			if (!strcmp(arg, "--not")) {
-				flags ^= UNINTERESTING;
-				continue;
-			}
-			if (!strcmp(arg, "--default")) {
-				if (++i >= argc)
-					die("bad --default argument");
-				def = argv[i];
-				continue;
-			}
-			if (!strcmp(arg, "--topo-order")) {
-				revs->topo_order = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--date-order")) {
-				revs->lifo = 0;
-				revs->topo_order = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--parents")) {
-				revs->parents = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--dense")) {
-				revs->dense = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--sparse")) {
-				revs->dense = 0;
-				continue;
-			}
-			if (!strcmp(arg, "--remove-empty")) {
-				revs->remove_empty_trees = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--no-merges")) {
-				revs->no_merges = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--boundary")) {
-				revs->boundary = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--objects")) {
-				revs->tag_objects = 1;
-				revs->tree_objects = 1;
-				revs->blob_objects = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--objects-edge")) {
-				revs->tag_objects = 1;
-				revs->tree_objects = 1;
-				revs->blob_objects = 1;
-				revs->edge_hint = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--unpacked")) {
-				revs->unpacked = 1;
-				continue;
-			}
-			if (!strcmp(arg, "-r")) {
-				revs->diff = 1;
-				revs->diffopt.recursive = 1;
-				continue;
-			}
-			if (!strcmp(arg, "-t")) {
-				revs->diff = 1;
-				revs->diffopt.recursive = 1;
-				revs->diffopt.tree_in_recursive = 1;
-				continue;
-			}
-			if (!strcmp(arg, "-m")) {
-				revs->ignore_merges = 0;
-				continue;
-			}
-			if (!strcmp(arg, "-c")) {
-				revs->diff = 1;
-				revs->dense_combined_merges = 0;
-				revs->combine_merges = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--cc")) {
-				revs->diff = 1;
-				revs->dense_combined_merges = 1;
-				revs->combine_merges = 1;
-				continue;
-			}
-			if (!strcmp(arg, "-v")) {
-				revs->verbose_header = 1;
-				continue;
-			}
-			if (!strncmp(arg, "--pretty", 8)) {
-				revs->verbose_header = 1;
-				revs->commit_format = get_commit_format(arg+8);
-				continue;
-			}
-			if (!strcmp(arg, "--root")) {
-				revs->show_root_diff = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--no-commit-id")) {
-				revs->no_commit_id = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--always")) {
-				revs->always_show_header = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--no-abbrev")) {
-				revs->abbrev = 0;
-				continue;
-			}
-			if (!strcmp(arg, "--abbrev")) {
+	gitopt_iter_setup(&gi, argc, argv);
+	ea = gi.ea;
+	b = gi.b;
+	for (; (id = gitopt_iter_parse(&gi, ost)); gitopt_iter_next(&gi)) {
+		switch (id) {
+		case opt_max_count:
+			revs->max_count = atoi(ea->argv[0]);
+			break;
+		case opt_max_age:
+			revs->max_age = atoi(ea->argv[0]);
+			break;
+		case opt_min_age:
+			revs->min_age = atoi(ea->argv[0]);
+			break;
+		case opt_since:
+			revs->max_age = approxidate(ea->argv[0]);
+			break;
+		case opt_until:
+			revs->min_age = approxidate(ea->argv[0]);
+			break;
+		case opt_all:
+			handle_all(revs, flags);
+			break;
+		case opt_not:
+			flags ^= UNINTERESTING;
+			break;
+		case opt_default:
+			if (!ea->argc)
+				die("bad --default argument");
+			def = ea->argv[0];
+			break;
+		case opt_topo_order:
+			revs->topo_order = 1;
+			break;
+		case opt_date_order:
+			revs->lifo = 0;
+			revs->topo_order = 1;
+			break;
+		case opt_parents:
+			revs->parents = 1;
+			break;
+		case opt_dense:
+			revs->dense = 1;
+			break;
+		case opt_sparse:
+			revs->dense = 0;
+			break;
+		case opt_remove_empty:
+			revs->remove_empty_trees = 1;
+			break;
+		case opt_no_merges:
+			revs->no_merges = 1;
+			break;
+		case opt_boundary:
+			revs->boundary = 1;
+			break;
+		case opt_objects_edge:
+			revs->edge_hint = 1; /* fall through */
+		case opt_objects:
+			revs->tag_objects = 1;
+			revs->tree_objects = 1;
+			revs->blob_objects = 1;
+			break;
+		case opt_unpacked:
+			revs->unpacked = 1;
+			break;
+		case opt_r:
+			revs->diff = 1;
+			revs->diffopt.recursive = 1;
+			break;
+		case opt_t:
+			revs->diff = 1;
+			revs->diffopt.recursive = 1;
+			revs->diffopt.tree_in_recursive = 1;
+			break;
+		case opt_m:
+			revs->ignore_merges = 0;
+			break;
+		case opt_c:
+			revs->diff = 1;
+			revs->dense_combined_merges = 0;
+			revs->combine_merges = 1;
+			break;
+		case opt_cc:
+			revs->diff = 1;
+			revs->dense_combined_merges = 1;
+			revs->combine_merges = 1;
+			break;
+		case opt_v:
+			revs->verbose_header = 1;
+			break;
+		case opt_pretty:
+			revs->verbose_header = 1;
+			revs->commit_format = get_commit_format(ea->argv[1]);
+			break;
+		case opt_root:
+			revs->show_root_diff = 1;
+			break;
+		case opt_no_commit_id:
+			revs->no_commit_id = 1;
+			break;
+		case opt_always:
+			revs->always_show_header = 1;
+			break;
+		case opt_no_abbrev:
+			revs->abbrev = 0;
+			break;
+		case opt_abbrev_commit:
+			revs->abbrev_commit = 1;
+			break;
+		case opt_full_diff:
+			revs->diff = 1;
+			revs->full_diff = 1;
+			break;
+		case opt_abbrev:
+			if (ea->argc == 1)
 				revs->abbrev = DEFAULT_ABBREV;
-				continue;
+			else {
+				revs->abbrev = strtoul(ea->argv[1], NULL, 10);
+				if (revs->abbrev < MINIMUM_ABBREV)
+					revs->abbrev = MINIMUM_ABBREV;
+				else if (40 < revs->abbrev)
+					revs->abbrev = 40;
 			}
-			if (!strcmp(arg, "--abbrev-commit")) {
-				revs->abbrev_commit = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--full-diff")) {
-				revs->diff = 1;
-				revs->full_diff = 1;
-				continue;
-			}
-			opts = diff_opt_parse(&revs->diffopt, argv+i, argc-i);
-			if (opts > 0) {
-				revs->diff = 1;
-				i += opts - 1;
-				continue;
-			}
-			*unrecognized++ = arg;
-			left++;
-			continue;
-		}
-		dotdot = strstr(arg, "..");
-		if (dotdot) {
-			unsigned char from_sha1[20];
-			const char *next = dotdot + 2;
-			const char *this = arg;
-			*dotdot = 0;
-			if (!*next)
-				next = "HEAD";
-			if (dotdot == arg)
-				this = "HEAD";
-			if (!get_sha1(this, from_sha1) &&
-			    !get_sha1(next, sha1)) {
-				struct object *exclude;
-				struct object *include;
-
-				exclude = get_reference(revs, this, from_sha1, flags ^ UNINTERESTING);
-				include = get_reference(revs, next, sha1, flags);
-				if (!exclude || !include)
-					die("Invalid revision range %s..%s", arg, next);
-
-				if (!seen_dashdash) {
-					*dotdot = '.';
-					verify_non_filename(revs->prefix, arg);
-				}
-				add_pending_object(revs, exclude, this);
-				add_pending_object(revs, include, next);
-				continue;
-			}
-			*dotdot = '.';
-		}
-		dotdot = strstr(arg, "^@");
-		if (dotdot && !dotdot[2]) {
-			*dotdot = 0;
-			if (add_parents_only(revs, arg, flags))
-				continue;
-			*dotdot = '^';
-		}
-		local_flags = 0;
-		if (*arg == '^') {
-			local_flags = UNINTERESTING;
-			arg++;
-		}
-		if (get_sha1(arg, sha1)) {
-			int j;
-
-			if (seen_dashdash || local_flags)
-				die("bad revision '%s'", arg);
-
-			/* If we didn't have a "--":
-			 * (1) all filenames must exist;
-			 * (2) all rev-args must not be interpretable
-			 *     as a valid filename.
-			 * but the latter we have checked in the main loop.
-			 */
-			for (j = i; j < argc; j++)
-				verify_filename(revs->prefix, argv[j]);
-
-			revs->prune_data = get_pathspec(revs->prefix, argv + i);
 			break;
+		case GITOPT_NON_OPTION:
+			/* fprintf(stderr,"no: %s\n",gi.argv[gi.pos]); */
+			i = setup_revision(&gi, revs, flags, seen_dashdash);
+			gi.pos += i - 1;
+			break;
+		default:
+			/* fprintf(stderr,"do: %s\n",gi.argv[gi.pos]); */
+			i = diff_opt_handler(&gi, id, &revs->diffopt);
+			if (i > 0)
+				revs->diff = 1;
+			else if (i < 0) /* error */
+				b->argv[b->argc++] = gi.argv[gi.pos++];
+			else if (ge) {
+				/* i = 0: not a diff_opt, try other handler */
+				i = ge->opt_handler(&gi, id, ge->args);
+				if (i <= 0) /* error */
+					b->argv[b->argc++] = gi.argv[gi.pos++];
+			}
+			gi.pos += i - 1;
 		}
-		if (!seen_dashdash)
-			verify_non_filename(revs->prefix, arg);
-		object = get_reference(revs, arg, sha1, flags ^ local_flags);
-		add_pending_object(revs, object, arg);
 	}
 	if (def && !revs->pending_objects) {
 		unsigned char sha1[20];
@@ -844,7 +875,8 @@ int setup_revisions(int argc, const char
 	revs->diffopt.abbrev = revs->abbrev;
 	diff_setup_done(&revs->diffopt);
 
-	return left;
+	free(ost);
+	return b;
 }
 
 void prepare_revision_walk(struct rev_info *revs)
diff --git a/revision.h b/revision.h
index 48d7b4c..28211b6 100644
--- a/revision.h
+++ b/revision.h
@@ -81,7 +81,9 @@ extern int rev_same_tree_as_empty(struct
 extern int rev_compare_tree(struct rev_info *, struct tree *t1, struct tree *t2);
 
 extern void init_revisions(struct rev_info *revs);
-extern int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def);
+extern struct exec_args *setup_revisions(int, const char **argv,
+					struct rev_info *revs, const char *def,
+					const struct gitopt_extra *);
 extern void prepare_revision_walk(struct rev_info *revs);
 extern struct commit *get_revision(struct rev_info *revs);
 
-- 
1.3.2.g102e322

^ permalink raw reply related

* [PATCH 4/5] commit: allow --pretty= args to be abbreviated
From: Eric Wong @ 2006-05-14 15:19 UTC (permalink / raw)
  To: git; +Cc: Eric Wong
In-Reply-To: <11476199622462-git-send-email-normalperson@yhbt.net>

Signed-off-by: Eric Wong <normalperson@yhbt.net>

---

 commit.c |   42 +++++++++++++++++++++++++++++-------------
 1 files changed, 29 insertions(+), 13 deletions(-)

b6190a2b46acc250277fd55e33e0a7216b7fad75
diff --git a/commit.c b/commit.c
index 2343729..a786371 100644
--- a/commit.c
+++ b/commit.c
@@ -22,23 +22,39 @@ struct sort_node
 
 const char *commit_type = "commit";
 
+struct cmt_fmt_map {
+	const char *n;
+	enum cmit_fmt v;
+} cmt_fmts[] = {
+	{ "raw",	CMIT_FMT_RAW },
+	{ "medium",	CMIT_FMT_MEDIUM },
+	{ "short",	CMIT_FMT_SHORT },
+	{ "full",	CMIT_FMT_FULL },
+	{ "fuller",	CMIT_FMT_FULLER },
+	{ "oneline",	CMIT_FMT_ONELINE },
+};
+
 enum cmit_fmt get_commit_format(const char *arg)
 {
+	int i, found = -1;
 	if (!arg)
 		return CMIT_FMT_DEFAULT;
-	if (!strcmp(arg, "raw"))
-		return CMIT_FMT_RAW;
-	if (!strcmp(arg, "medium"))
-		return CMIT_FMT_MEDIUM;
-	if (!strcmp(arg, "short"))
-		return CMIT_FMT_SHORT;
-	if (!strcmp(arg, "full"))
-		return CMIT_FMT_FULL;
-	if (!strcmp(arg, "fuller"))
-		return CMIT_FMT_FULLER;
-	if (!strcmp(arg, "oneline"))
-		return CMIT_FMT_ONELINE;
-	die("invalid --pretty format");
+	for (i = 0; i < ARRAY_SIZE(cmt_fmts); i++) {
+		if (!strcmp(arg, cmt_fmts[i].n))
+			return cmt_fmts[i].v;
+	}
+
+	/* look for abbreviations */
+	for (i = 0; i < ARRAY_SIZE(cmt_fmts); i++) {
+		if (strstr(cmt_fmts[i].n, arg)) {
+			if (found >= 0)
+				die("invalid --pretty format: %s", arg);
+			found = i;
+		}
+	}
+	if (found >= 0)
+		return cmt_fmts[found].v;
+	die("invalid --pretty format: %s", arg);
 }
 
 static struct commit *check_commit(struct object *obj,
-- 
1.3.2.g102e322

^ permalink raw reply related

* [PATCH 5/5] diff: parse U/u/unified options with an optional integer arg
From: Eric Wong @ 2006-05-14 15:19 UTC (permalink / raw)
  To: git; +Cc: Eric Wong
In-Reply-To: <11476199622462-git-send-email-normalperson@yhbt.net>

Looks like Linus and I both think GIT_DIFF_OPTS="--unified=5" is
silly, but it still continues to work.

This was originally bundled into my first series gitopt patches,
and now Linus has a more correct/complete one that affects
combine-diff and works with uppercase -U  This patch
combines the Linus one with my gitopt version.

-u (lowercase) now accepts an optional arg, like -U (GNU diff
-u also has this behavior).

This uses the built-in gitopt parsers to do optional
integer argument handling.

Signed-off-by: Eric Wong <normalperson@yhbt.net>

---

 combine-diff.c |    1 +
 diff.c         |   11 ++++++++---
 diff.h         |    1 +
 3 files changed, 10 insertions(+), 3 deletions(-)

102e3227de7c7f08a69096d5094ee31128bf9819
diff --git a/combine-diff.c b/combine-diff.c
index 8a8fe38..64b20cc 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -608,6 +608,7 @@ static int show_patch_diff(struct combin
 	int abbrev = opt->full_index ? 40 : DEFAULT_ABBREV;
 	mmfile_t result_file;
 
+	context = opt->context;
 	/* Read the result of merge first */
 	if (!working_tree_file)
 		result = grab_blob(elem->sha1, &result_size);
diff --git a/diff.c b/diff.c
index 7d88dc5..6ee612f 100644
--- a/diff.c
+++ b/diff.c
@@ -15,7 +15,7 @@ #include "gitopt.h"
 static int diff_scoreopt_parse(const int id, const char *opt);
 
 enum diff_ost_ids {
-	opt_p = GITOPT_DIFF_BASE, opt_raw, opt_patch_with_raw,
+	opt_u = GITOPT_DIFF_BASE, opt_p, opt_raw, opt_patch_with_raw,
 	opt_stat, opt_patch_with_stat,
 	opt_z, opt_l, opt_full_index, opt_name_only, opt_name_status,
 	opt_R, opt_S, opt_s, opt_O, opt_diff_filter,
@@ -26,7 +26,8 @@ enum diff_ost_ids {
 
 const struct opt_spec diff_ost[] = {
 	{ 0,			'p',	0,	0,	opt_p },
-	{ "unified",		'u',	0,	0,	opt_p },
+	{ "unified",		'u',	0,	ARG_OPTINT,	opt_u },
+	{ 0,			'U',	0,	ARG_OPTINT,	opt_u },
 	{ "raw",		0,	0,	0,	opt_raw },
 	{ "patch-with-raw",	0,	0,	0,	opt_patch_with_raw },
 	{ "stat",		0,	0,	0,	opt_stat },
@@ -600,7 +601,7 @@ static void builtin_diff(const char *nam
 
 		ecbdata.label_path = lbl;
 		xpp.flags = XDF_NEED_MINIMAL;
-		xecfg.ctxlen = 3;
+		xecfg.ctxlen = o->context;
 		xecfg.flags = XDL_EMIT_FUNCNAMES;
 		if (!diffopts)
 			;
@@ -1224,6 +1225,7 @@ void diff_setup(struct diff_options *opt
 	options->line_termination = '\n';
 	options->break_opt = -1;
 	options->rename_limit = -1;
+	options->context = 3;
 
 	options->change = diff_change;
 	options->add_remove = diff_addremove;
@@ -1269,6 +1271,9 @@ int diff_opt_handler(struct gitopt_itera
 	struct diff_options *options = (struct diff_options *)args;
 
 	switch (id) {
+	case opt_u:
+		if (gi->ea->argc > 1)
+			options->context = strtol(gi->ea->argv[1], NULL, 10);
 	case opt_p:
 		options->output_format = DIFF_FORMAT_PATCH;
 		break;
diff --git a/diff.h b/diff.h
index fa44d1b..b9d7573 100644
--- a/diff.h
+++ b/diff.h
@@ -33,6 +33,7 @@ struct diff_options {
 		 full_index:1,
 		 silent_on_remove:1,
 		 find_copies_harder:1;
+	int context;
 	int break_opt;
 	int detect_rename;
 	int line_termination;
-- 
1.3.2.g102e322

^ permalink raw reply related


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