Git development
 help / color / mirror / Atom feed
* Re: git push refspec problem
From: James @ 2007-11-07 15:23 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Johannes Schindelin, git
In-Reply-To: <20071107152003.GL18057@artemis.corp>

On Nov 7, 2007, at 10:20 AM, Pierre Habouzit wrote:

> On Wed, Nov 07, 2007 at 03:11:46PM +0000, Johannes Schindelin wrote:
>> Hi,
>>
>> On Wed, 7 Nov 2007, James wrote:
>>
>>>       fetch = +refs/heads/*:refs/remotes/origin/*
>>
>> This is a refspec.
>>
>>>       push = ssh://james@my.server.com/home/james/scm/git/project.git/
>>
>> This is a URL.  It does not specify any refs.  But "push =" expects  
>> a URL.
>>
>> You probably want to setup a different remote if you want to push  
>> to a
>> different URL than you are fetching from.
>
>  Oh, there is no way to pull through git:// and push to ssh://
> perfectly knowint it's the same physical repository so that the fetch
> doesn't have the ssh-handhshake-overhead ?


There has to be *some* way of pulling through git and pushing through  
ssh with a simple "git push".  :-P  I'm doing it manually, after all.   
I could have sworn I've read how to do its somewhere but have since  
forgotten.


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

^ permalink raw reply

* Re: git push refspec problem
From: Johannes Gilger @ 2007-11-07 15:22 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: James, git
In-Reply-To: <Pine.LNX.4.64.0711071510480.4362@racer.site>

Johannes Schindelin wrote:
> Hi,
> 
> On Wed, 7 Nov 2007, James wrote:
> 
>>        fetch = +refs/heads/*:refs/remotes/origin/*
> 
> This is a refspec.
> 
>>        push = ssh://james@my.server.com/home/james/scm/git/project.git/
> 
> This is a URL.  It does not specify any refs.  But "push =" expects a URL.

I think Johannes meant to say "But 'push =' expects a refspec." (the
manpage even says so).

About your problem: If you want to pull from a git:// repository and
push to another with ssh:// (or in general when having two different
repositories for pushing and fetching) in my novice understanding
you would need two remotes. In your case, can't you just use your
ssh-url for fetching as well?

Regards,
Jojo

-- 
Johannes Gilger <heipei@hackvalue.de>
http://hackvalue.de/heipei/
GPG-Key: 0x42F6DE81
GPG-Fingerprint: BB49 F967 775E BB52 3A81  882C 58EE B178 42F6 DE81

^ permalink raw reply

* Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
From: Yin Ping @ 2007-11-07 15:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsswjeom.fsf@gitster.siamese.dyndns.org>

On 11/6/07, Junio C Hamano <gitster@pobox.com> wrote:
> "Yin Ping" <pkufranky@gmail.com> writes:
>
> > However, in some cases these messages are helpful. And a third kind of
> > cases is that people care about only part of all submodules.
> >
> > So, maybe some an switch can be used to turn this on or off (default
> > off)?
>
> I personally think that is overengineering.  Isn't having/not
> having the submodule directory cloned a good enough indicator
> of which submodules are interested in by the user?
>
Good suggestion. I'll give my new patch later.

-- 
franky

^ permalink raw reply

* Re: git push refspec problem
From: Pierre Habouzit @ 2007-11-07 15:20 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: James, git
In-Reply-To: <Pine.LNX.4.64.0711071510480.4362@racer.site>

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

On Wed, Nov 07, 2007 at 03:11:46PM +0000, Johannes Schindelin wrote:
> Hi,
> 
> On Wed, 7 Nov 2007, James wrote:
> 
> >        fetch = +refs/heads/*:refs/remotes/origin/*
> 
> This is a refspec.
> 
> >        push = ssh://james@my.server.com/home/james/scm/git/project.git/
> 
> This is a URL.  It does not specify any refs.  But "push =" expects a URL.
> 
> You probably want to setup a different remote if you want to push to a 
> different URL than you are fetching from.

  Oh, there is no way to pull through git:// and push to ssh://
perfectly knowint it's the same physical repository so that the fetch
doesn't have the ssh-handhshake-overhead ?


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

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

^ permalink raw reply

* Re: [bug in next ?] git-fetch/git-push issue
From: Pierre Habouzit @ 2007-11-07 15:11 UTC (permalink / raw)
  To: Nicolas Pitre, Daniel Barkalow, Jeff King, Git ML
In-Reply-To: <20071105175654.GD6205@artemis.corp>

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


oh and while we're at it, someone reminded me on IRC that when you:

  git push origin :somebranch

it does not removes origin/somebranch from your branches.  Now that git
push updates our knowledge of the remote branch, I believe that it
should also try to perform the equivalent of a:

  git branch -r -d origin/somebranch

And to be fair, I'd also say that git fetch <some-remote> should
complain about remote heads that match <some-remote> refspec that have
no corresponding reference _on_ the remote so that the user knows that
the branches have been removed.

I wanted to write a patch about that long time ago, but my plate is
already full with the diff option things.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: git push refspec problem
From: Johannes Schindelin @ 2007-11-07 15:11 UTC (permalink / raw)
  To: James; +Cc: git
In-Reply-To: <7B37E361-9606-447C-B853-001182688AFA@nc.rr.com>

Hi,

On Wed, 7 Nov 2007, James wrote:

>        fetch = +refs/heads/*:refs/remotes/origin/*

This is a refspec.

>        push = ssh://james@my.server.com/home/james/scm/git/project.git/

This is a URL.  It does not specify any refs.  But "push =" expects a URL.

You probably want to setup a different remote if you want to push to a 
different URL than you are fetching from.

Hth,
Dscho

^ permalink raw reply

* Re: [PATCH] Make git-clean a builtin
From: Johannes Schindelin @ 2007-11-07 15:04 UTC (permalink / raw)
  To: Shawn Bohrer; +Cc: gitster, git
In-Reply-To: <20071107145434.GB6768@mediacenter.austin.rr.com>

Hi,

On Wed, 7 Nov 2007, Shawn Bohrer wrote:

> On Wed, Nov 07, 2007 at 11:10:45AM +0000, Johannes Schindelin wrote:
> > 
> > you still have quite a number of instances where you wrap just one line 
> > into curly brackets:
> > 
> > 	if (bla) {
> > 		[just one line]
> > 	}
> 
> Crap.  OK I count one instance unless you count:
> 
> 	if (foo) {
> 		one_line();
> 	} else if (bar) {
> 		one_line();
> 		two_lines();
> 	} else {
> 		something_else();
> 	}

I do count them.  Personally, I find it highly distracting and ugly.  
Besides, we have the convention of putting the "}" not into the same line 
as "else".  (See keyword "uncuddling" in the list archives.)

While it may be true that some parts of the code follow these rules less 
strictly, it does not mean that we should introduce more of that kind.

BTW there are plenty of examples in the existing code which illustrate our 
implicit coding conventions.

> Now I suppose I can get rid of the curly braces here as well but I 
> personally find that strange and ugly.  So is there an official 
> guideline on if else statements?

Not yet ;-)  I can add it to the tentative v3 of Documentation/CodingStyle 
or CodingConventions or however the list would like to name it.

Ciao,
Dscho

^ permalink raw reply

* git push refspec problem
From: James @ 2007-11-07 15:01 UTC (permalink / raw)
  To: git

Hi,

I'm trying to set up my git configuration file in one of my projects  
so that I can do a simple "git push" to update my project on the git  
server.

Currently, I run the following command to push my updates (and it  
works just fine):

git push james@my.server.com:~/scm/git/project.git/

In my .git/config file, I've added a line for push, as follows:

[remote "origin"]
         url = git://my.server.com/project.git
         fetch = +refs/heads/*:refs/remotes/origin/*
         push = ssh://james@my.server.com/home/james/scm/git/ 
project.git/

When I run a "git push", it comes back with this error:

fatal: remote part of refspec is not a valid name in ssh://james@my.server.com/home/james/scm/git/project.git/

I've looked at this git push documentation:

http://www.kernel.org/pub/software/scm/git/docs/git-push.html

and it seems like my refspec is indeed correct.  (or so I thought ;))   
Any ideas on what I'm doing wrong?

Thanks!
.james

^ permalink raw reply

* [PATCH v2] Add Documentation/CodingStyle
From: Johannes Schindelin @ 2007-11-07 14:59 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Junio C Hamano, Ralf Wildenhues, git
In-Reply-To: <47317CD7.5040506@op5.se>


Even if our code is quite a good documentation for our coding style,
some people seem to prefer a document describing it.

The part about the shell scripts is clearly just copied from one of
Junio's helpful mails, and some parts were added from comments by
Junio and Andreas Ericsson.

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

	On Wed, 7 Nov 2007, Andreas Ericsson wrote:

	> Perhaps with this addendum?
	> 
	> - Think very, very hard before introducing a new dependency into git. This
	>  means you should stay away from scripting languages not already used in
	>  the git core command set unless your command is clearly separate from it,
	>  such as an importer to convert random-scm-X repositories to git.

	I edited it minimally.

 Documentation/CodingStyle |  103 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 103 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/CodingStyle

diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
new file mode 100644
index 0000000..38a3d9f
--- /dev/null
+++ b/Documentation/CodingStyle
@@ -0,0 +1,103 @@
+As a popular project, we also have some guidelines to keep to the
+code.  For git in general, two rough rules are:
+
+ - Most importantly, we never say "It's in POSIX; we'll happily
+   screw your system that does not conform."  We live in the
+   real world.
+
+ - However, we often say "Let's stay away from that construct,
+   it's not even in POSIX".
+
+ - In spite of the above two rules, we sometimes say "Although
+   this is not in POSIX, it (is so convenient | makes the code
+   much more readable | has other good characteristics) and
+   practically all the platforms we care about support it, so
+   let's use it".  Again, we live in the real world, and it is
+   sometimes a judgement call, decided based more on real world
+   constraints people face than what the paper standard says.
+
+
+As for more concrete guidelines, just imitate the existing code
+(this is a good guideline, no matter which project you are contributing
+to...).  But if you must have some list of rules, here they are.
+
+For shell scripts specifically (not exhaustive):
+
+ - We prefer $( ... ) for command substitution; unlike ``, it
+   properly nests.  It should have been the way Bourne spelled
+   it from day one, but unfortunately isn't.
+
+ - We use ${parameter-word} and its [-=?+] siblings, and their
+   colon'ed "unset or null" form.
+
+ - We use ${parameter#word} and its [#%] siblings, and their
+   doubled "longest matching" form.
+
+ - We use Arithmetic Expansion $(( ... )).
+
+ - No "Substring Expansion" ${parameter:offset:length}.
+
+ - No shell arrays.
+
+ - No strlen ${#parameter}.
+
+ - No regexp ${parameter/pattern/string}.
+
+ - We do not use Process Substitution <(list) or >(list).
+
+ - We prefer "test" over "[ ... ]".
+
+ - We do not write noiseword "function" in front of shell
+   functions.
+
+For C programs:
+
+ - Use tabs to indent, and interpret tabs as taking up to 8 spaces
+
+ - Try to keep to 80 characters per line
+
+ - When declaring pointers, the star sides with the variable name, i.e.
+   "char *string", not "char* string" or "char * string".  This makes
+   it easier to understand "char *string, c;"
+
+ - Do not use curly brackets unnecessarily.  I.e.
+
+	if (bla) {
+		x = 1;
+	}
+
+   is frowned upon.  A gray area is when the statement extends over a
+   few lines, and/or you have a lengthy comment atop of it.
+
+ - Try to make your code understandable.  You may put comments in, but
+   comments invariably tend to stale out when the code they were
+   describing changes.  Often splitting a function into two makes the
+   intention of the code much clearer.
+
+   Double negation is often harder to understand than no negation at
+   all.
+
+   Some clever tricks, like using the !! operator with arithmetic
+   constructs, can be extremely confusing to others.  Avoid them,
+   unless there is a compelling reason to use them.
+
+ - Use the API.  No, really.  We have a strbuf (variable length string),
+   several arrays with the ALLOC_GROW() macro, a path_list for sorted
+   string lists, a hash map (mapping struct objects) named
+   "struct decorate", amongst other things.
+
+ - When you come up with an API, document it.
+
+ - #include system headers in git-compat-util.h.  Some headers on some
+   systems show subtle breakages when you change the order, so it is
+   best to keep them in one place.
+
+ - if you are planning a new command, consider writing it in shell or
+   perl first, so that changes in semantics can be easily changed and
+   discussed.  Many git commands started out like that, and a few are
+   still scripts.
+
+ - Avoid introducing a new dependency into git. This means you usually
+   should stay away from scripting languages not already used in the git
+   core command set (unless your command is clearly separate from it,
+   such as an importer to convert random-scm-X repositories to git).
-- 
1.5.3.5.1597.g7191

^ permalink raw reply related

* Re: stgit: cleaning up after using git branch delete commands
From: Catalin Marinas @ 2007-11-07 14:57 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Git Mailing List
In-Reply-To: <9e4733910711070606t2c558ac9ob4c729d5baca8fb9@mail.gmail.com>

"Jon Smirl" <jonsmirl@gmail.com> wrote:
> I've used git commands to delete several branches that had stgit
> active on it.  Doing that has left a bunch of clutter in the .git
> directory. Is there a stgit command to remove all the clutter from
> branches that no longer exist? I'd like to use the branch names again
> but the clutter is interfering.

You can create the branch back with GIT and run "stg branch --delete
--force", though I don't guarantee it will work (BTW, I only recently
relaxed the branch deletion rules in StGIT so that it doesn't complain
of missing files and completes the operation, so you should use the
latest HEAD).

-- 
Catalin

^ permalink raw reply

* Re: [PATCH] Add Documentation/CodingStyle
From: Johannes Schindelin @ 2007-11-07 14:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ralf Wildenhues, git
In-Reply-To: <7v640ega5q.fsf@gitster.siamese.dyndns.org>

Hi,

On Tue, 6 Nov 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
> > new file mode 100644
> > index 0000000..622b80b
> > --- /dev/null
> > +++ b/Documentation/CodingStyle
> > @@ -0,0 +1,87 @@
> > +As a popular project, we also have some guidelines to keep to the
> > +code.  For git in general, two rough rules are:
> > +
> > + - Most importantly, we never say "It's in POSIX; we'll happily
> > +   screw your system that does not conform."  We live in the
> > +   real world.
> > +
> > + - However, we often say "Let's stay away from that construct,
> > +   it's not even in POSIX".
> > +
> 
> I am not sure if we want to have CodingStyle document, but the
> above are not CodingStyle issues.

Would you like to call it CodingConventions?

> If we are to write this down, I'd like to have the more
> important third rule, which is:
> 
>  - In spite of the above two rules, we sometimes say "Although
>    this is not in POSIX, it (is so convenient | makes the code
>    much more readable | has other good characteristics) and
>    practically all the platforms we care about support it, so
>    let's use it".  Again, we live in the real world, and it is
>    sometimes a judgement call, decided based more on real world
>    constraints people face than what the paper standard says.

Okay, will add.

> > +For C programs:
> > +
> > + - Use tabs to increment, and interpret tabs as taking up to 8 spaces
> 
> What's the character for decrement?  DEL? ;-)

Hehe.  As you undoubtedly guessed, I meant "indent"...

> > +   Double negation is often harder to understand than no negation at
> > +   all.
> > +
> > +   Some clever tricks, like using the !! operator with arithmetic
> > +   constructs, can be extremely confusing to others.  Avoid them,
> > +   unless there is a compelling reason to use them.
> 
> I actually think (!!var) idiom is already established in our
> codebase.

Yep, but when there are easier variants, AFAICT they were preferred.

> > + - Use the API.  No, really.  We have a strbuf (variable length string),
> > +   several arrays with the ALLOC_GROW() macro, a path_list for sorted
> > +   string lists, a hash map (mapping struct objects) named
> > +   "struct decorate", amongst other things.
> 
>  - When you come up with an API, document it.

Okay.

> > + - if you are planning a new command, consider writing it in shell or
> > +   perl first, so that changes in semantics can be easily changed and
> > +   discussed.  Many git commands started out like that, and a few are
> > +   still scripts.
> 
> No Python allowed?

Well, maybe we should allow that again.  Although I hoped we are over that 
now, as it would complicate the msysGit efforts tremendously.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Make git-clean a builtin
From: Shawn Bohrer @ 2007-11-07 14:54 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: gitster, git
In-Reply-To: <Pine.LNX.4.64.0711071110040.4362@racer.site>

On Wed, Nov 07, 2007 at 11:10:45AM +0000, Johannes Schindelin wrote:
> 
> you still have quite a number of instances where you wrap just one line 
> into curly brackets:
> 
> 	if (bla) {
> 		[just one line]
> 	}

Crap.  OK I count one instance unless you count:

	if (foo) {
		one_line();
	} else if (bar) {
		one_line();
		two_lines();
	} else {
		something_else();
	}

Now I suppose I can get rid of the curly braces here as well but I
personally find that strange and ugly.  So is there an official guideline
on if else statements?

Of course I'll fix the other one I missed and send a new patch.

^ permalink raw reply

* Re: [PATCH 0/5] some shell portability fixes
From: Johannes Schindelin @ 2007-11-07 14:47 UTC (permalink / raw)
  To: Mike Ralphson; +Cc: Ralf Wildenhues, Mike Hommey, Junio C Hamano, git
In-Reply-To: <e2b179460711070617h7e9af64egcde5122808a4d924@mail.gmail.com>

Hi,

On Wed, 7 Nov 2007, Mike Ralphson wrote:

> Junio C Hamano wrote on Tue, Nov 06, 2007 at 09:46:35PM CET:
> > [2/5] Gaah, AIX sed X-<.  I am not opposed to this patch but
> >       would want to get Yays from people with non GNU sed.  Is
> >       busybox sed good enough to grok our scripts these days?
> >       Please ask help and collect Acks at least from folks on
> >       Solaris, MacOS, FBSD, and OBSD.
> 
> On Nov 6, 2007 11:25 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > As Junio commented in the part you did not quote, there are better shells
> > in Solaris.  Use those.
> 
> Equally GNU sed is available as a drop-in rpm for AIX. I wonder if it
> would be worth adding
> Makefile support for a PATH prefix for the git scripts, so they could
> prepend (in this case)
> something like /opt/freeware/bin or /usr/linux/bin ?
> 
> In our AIX environment many GNU tools are installed but I can't
> guarantee they come first
> in the paths of the git users.
> 
> I'm willing to work up a patch if there's any interest.

Would that be a task for configure?  Because I am not sure if the GNU 
tools are installed in the same place on all AIX boxen...

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Make git-clean a builtin
From: Matthieu Moy @ 2007-11-07 14:45 UTC (permalink / raw)
  To: Bill Lear; +Cc: Johannes Schindelin, Shawn Bohrer, gitster, git
In-Reply-To: <18225.48553.44088.269677@lisa.zopyra.com>

Bill Lear <rael@zopyra.com> writes:

> I've always found this a thoughtful practice.  It helps ensure nobody writes:
>
>        if (bla)
>            just_one_line();
>            /* perhaps a comment, other stuff ... */
>            just_another_line();

It also simplify patches for cases like

 	if (bla) {
 		just_one_line();
+		another_added_line();
 	}

instead of

- 	if (bla)
+ 	if (bla) {
 		just_one_line();
+		another_added_line();
+	}

But it seems people here prefer not putting the braces in this case.

-- 
Matthieu

^ permalink raw reply

* Re: [PATCH 0/5] some shell portability fixes
From: Mike Ralphson @ 2007-11-07 14:17 UTC (permalink / raw)
  To: Johannes Schindelin, Ralf Wildenhues; +Cc: Mike Hommey, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711062324590.4362@racer.site>

Junio C Hamano wrote on Tue, Nov 06, 2007 at 09:46:35PM CET:
> [2/5] Gaah, AIX sed X-<.  I am not opposed to this patch but
>       would want to get Yays from people with non GNU sed.  Is
>       busybox sed good enough to grok our scripts these days?
>       Please ask help and collect Acks at least from folks on
>       Solaris, MacOS, FBSD, and OBSD.

On Nov 6, 2007 11:25 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> As Junio commented in the part you did not quote, there are better shells
> in Solaris.  Use those.

Equally GNU sed is available as a drop-in rpm for AIX. I wonder if it
would be worth adding
Makefile support for a PATH prefix for the git scripts, so they could
prepend (in this case)
something like /opt/freeware/bin or /usr/linux/bin ?

In our AIX environment many GNU tools are installed but I can't
guarantee they come first
in the paths of the git users.

I'm willing to work up a patch if there's any interest.

Mike

^ permalink raw reply

* Re: [PATCH] Make git-clean a builtin
From: Johannes Schindelin @ 2007-11-07 14:17 UTC (permalink / raw)
  To: Bill Lear; +Cc: Shawn Bohrer, gitster, git
In-Reply-To: <18225.48553.44088.269677@lisa.zopyra.com>

Hi,

On Wed, 7 Nov 2007, Bill Lear wrote:

> On Wednesday, November 7, 2007 at 11:10:45 (+0000) Johannes Schindelin writes:
>
> > you still have quite a number of instances where you wrap just one 
> > line into curly brackets:
> >
> >	if (bla) {
> >		[just one line]
> >	}
> 
> I've always found this a thoughtful practice.  It helps ensure nobody 
> writes:
> 
>        if (bla)
>            just_one_line();
>            /* perhaps a comment, other stuff ... */
>            just_another_line();

But if there is only one line and you fail to add curly brackets when 
adding a second line, well, uhm, then I cannot help you with anything.

BTW I was talking about _one_ line, not a line and another one with a 
comment.

> It also is nice for others who come along and extend the branch from 
> just one line to multiple ones, as the brackets are already in place.

The fact is: these lines will stay single lines most likely for eternity.

> Why do you find it objectionable?

It distracts.  It's ugly.  It's unnecessary.

Ciao,
Dscho

^ permalink raw reply

* stgit: cleaning up after using git branch delete commands
From: Jon Smirl @ 2007-11-07 14:06 UTC (permalink / raw)
  To: Git Mailing List

I've used git commands to delete several branches that had stgit
active on it.  Doing that has left a bunch of clutter in the .git
directory. Is there a stgit command to remove all the clutter from
branches that no longer exist? I'd like to use the branch names again
but the clutter is interfering.

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [PATCH] Make git-clean a builtin
From: Bill Lear @ 2007-11-07 13:29 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Shawn Bohrer, gitster, git
In-Reply-To: <Pine.LNX.4.64.0711071110040.4362@racer.site>

On Wednesday, November 7, 2007 at 11:10:45 (+0000) Johannes Schindelin writes:
>Hi,
>
>you still have quite a number of instances where you wrap just one line 
>into curly brackets:
>
>	if (bla) {
>		[just one line]
>	}

I've always found this a thoughtful practice.  It helps ensure nobody writes:

       if (bla)
           just_one_line();
           /* perhaps a comment, other stuff ... */
           just_another_line();

which I've seen happen countless times.  It also is nice for others who
come along and extend the branch from just one line to multiple ones,
as the brackets are already in place.

Why do you find it objectionable?


Bill

^ permalink raw reply

* Re: [PATCH] Make git-clean a builtin
From: Johannes Schindelin @ 2007-11-07 11:10 UTC (permalink / raw)
  To: Shawn Bohrer; +Cc: gitster, git
In-Reply-To: <11944127311587-git-send-email-shawn.bohrer@gmail.com>

Hi,

you still have quite a number of instances where you wrap just one line 
into curly brackets:

	if (bla) {
		[just one line]
	}

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.
From: Johannes Schindelin @ 2007-11-07 11:08 UTC (permalink / raw)
  To: Mike Hommey
  Cc: Robin Rosenberg, Junio C Hamano, Steven Grimm, Pierre Habouzit,
	git
In-Reply-To: <20071107081608.GA19066@glandium.org>

Hi,

On Wed, 7 Nov 2007, Mike Hommey wrote:

> On Tue, Nov 06, 2007 at 10:25:48PM +0000, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> > On Tue, 6 Nov 2007, Robin Rosenberg wrote:
> > 
> > > tisdag 06 november 2007 skrev Mike Hommey:
> > > > Maybe the documentation could emphasise on how to undo things when 
> > > > the user makes mistakes. Sometimes, saving your repo can be as 
> > > > simple as git reset --hard HEAD@{1}. This is not, unfortunately, a 
> > > > works-for-all-cases command.
> > > 
> > > Yea, git-undo(7). 
> > 
> > In related news, I know a few users who need an un-rm-rf.  Anyone?
> 
> The fact is you can do harm to your repo with things you wouldn't expect 
> to break things, except maybe you gave bad arguments or so. It's quite 
> easy to fuck up with git-rebase, or to merge the wrong commits, etc.

I don't see how these commands are dangerous.  Usually you just look into 
the reflog, pick the one commit you started with, and reset --hard.

The _only_ commands I find dangerous are "git stash clear" and "git reflog 
--expire=0".  Funnily, people want to do that all the time.

Like recently, on the IRC channel, where somebody lost patches "during a 
rebase", by "rm -rf .dotest".

There will be a point where nobody can help.  But before that, reflogs are 
your friend.  But you must not do "reset --hard HEAD@{1}" blindly.  You 
have to look first what the reflogs are.

Ciao,
Dscho

^ permalink raw reply

* Re: [ANNOUNCE] cgit v0.7
From: Lars Hjemli @ 2007-11-07 10:52 UTC (permalink / raw)
  To: Patrick Aljord; +Cc: git list
In-Reply-To: <6b6419750711061039l26290561wd2abe07035a8679c@mail.gmail.com>

On Nov 6, 2007 7:39 PM, Patrick Aljord <patcito@gmail.com> wrote:
> Looks great, thanks for the new release.

Thanks!

> It would be great if when a commit message contains something such as
> #1234 it would automatically link to the bug tracker page of that bug
> (like Trac does) by preconfiguring the bugtracker URL.

Yeah, it shouldn't be to difficult either. We could just add a couple
of repo-options to cgitrc, maybe something like this:

  repo.bugtracker-regex=(#[0-9]+)
  repo.bugtracker-url=http://bugs.freedesktop.org/show_bug.cgi?id=%1

and then use regexec() combined with the interpolate() function in git
to perform the substitution. There could also be similar global
options, i.e.:

  bugtracker-regex=(#[0-9]+)
  bugtracker-url=http://bugs.freedesktop.org/show_bug.cgi?id=%1

which would be used if the per-repo options weren't specified.

I'll put it on my todo-list (and patches are always appreciated).

--
larsh

^ permalink raw reply

* [PATCH] hooks--update: allow deleting a tag through git push <remote> :<tag>
From: Gerrit Pape @ 2007-11-07 10:34 UTC (permalink / raw)
  To: git, Junio C Hamano

The update hook interpretes deleting a tag, no matter if annotated or not,
through git push <remote> :<tag> as unannotated tag, and declines it by
default:

 $ git push origin :atag
 deleting 'refs/tags/atag'
 *** The un-annotated tag, atag, is not allowed in this repository
 *** Use 'git tag [ -a | -s ]' for tags you want to propagate.
 ng refs/tags/atag hook declined
 error: hooks/update exited with error code 1
 error: hook declined to update refs/tags/atag
 error: failed to push to 'monolith:/git/qm/test-repo'

With this patch deleting a tag is allowed unconditionally, just as
deleting a branch.

Signed-off-by: Gerrit Pape <pape@smarden.org>
---
 templates/hooks--update |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/templates/hooks--update b/templates/hooks--update
index 65e8c32..a109b1b 100644
--- a/templates/hooks--update
+++ b/templates/hooks--update
@@ -43,7 +43,7 @@ fi
 # --- Check types
 # if $newrev is 0000...0000, it's a commit to delete a branch
 if [ "$newrev" = "0000000000000000000000000000000000000000" ]; then
-	newrev_type=commit
+	newrev_type=delete
 else
 	newrev_type=$(git-cat-file -t $newrev)
 fi
@@ -58,13 +58,16 @@ case "$refname","$newrev_type" in
 			exit 1
 		fi
 		;;
+	refs/tags/*,delete)
+		# delete tag
+		;;
 	refs/tags/*,tag)
 		# annotated tag
 		;;
-	refs/heads/*,commit)
+	refs/heads/*,commit | refs/heads/*,delete)
 		# branch
 		;;
-	refs/remotes/*,commit)
+	refs/remotes/*,commit | refs/remotes/*,delete)
 		# tracking branch
 		;;
 	*)
-- 
1.5.3.5

^ permalink raw reply related

* [PATCH] gitk: Swap positions of 'next' and 'prev' buttons in the 'Find' section.
From: Johannes Sixt @ 2007-11-07 10:22 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: Git Mailing List

The button order 'prev' left of 'next' feels more natural than the other
way round, in particular, compared to the order of the forward and backward
arrows that are in the line above.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
  gitk |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index 01a8541..727f4a0 100755
--- a/gitk
+++ b/gitk
@@ -767,7 +767,7 @@ proc makewindow {} {
      button .tf.lbar.fnext -text "next" -command {dofind 1 1} -font uifont
      button .tf.lbar.fprev -text "prev" -command {dofind -1 1} -font uifont
      label .tf.lbar.flab2 -text " commit " -font uifont
-    pack .tf.lbar.flabel .tf.lbar.fnext .tf.lbar.fprev .tf.lbar.flab2 \
+    pack .tf.lbar.flabel .tf.lbar.fprev .tf.lbar.fnext .tf.lbar.flab2 \
  	-side left -fill y
      set gdttype "containing:"
      set gm [tk_optionMenu .tf.lbar.gdttype gdttype \
-- 
1.5.3.5.1298.g228d6-dirty

^ permalink raw reply related

* [PATCH DIFF-CLEANUP 1/2] Make the diff_options bitfields be an unsigned with explicit masks.
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-6-git-send-email-madcoder@debian.org>

reverse_diff was a bit-value in disguise, it's merged in the flags now.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-blame.c      |   10 ++--
 builtin-diff-files.c |    4 +-
 builtin-diff-index.c |    4 +-
 builtin-diff-tree.c  |    9 ++--
 builtin-diff.c       |   12 +++---
 builtin-log.c        |   24 ++++------
 combine-diff.c       |   10 ++--
 diff-lib.c           |   23 +++++-----
 diff.c               |  123 ++++++++++++++++++++++++-------------------------
 diff.h               |   40 ++++++++++-------
 log-tree.c           |    6 +--
 merge-recursive.c    |    2 +-
 patch-ids.c          |    2 +-
 revision.c           |   22 +++++-----
 tree-diff.c          |   14 +++---
 15 files changed, 155 insertions(+), 150 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index 55a3c0b..eaf9c15 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -335,7 +335,7 @@ static struct origin *find_origin(struct scoreboard *sb,
 	 * same and diff-tree is fairly efficient about this.
 	 */
 	diff_setup(&diff_opts);
-	diff_opts.recursive = 1;
+	DIFF_OPT_SET(&diff_opts, RECURSIVE);
 	diff_opts.detect_rename = 0;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	paths[0] = origin->path;
@@ -409,7 +409,7 @@ static struct origin *find_rename(struct scoreboard *sb,
 	const char *paths[2];
 
 	diff_setup(&diff_opts);
-	diff_opts.recursive = 1;
+	DIFF_OPT_SET(&diff_opts, RECURSIVE);
 	diff_opts.detect_rename = DIFF_DETECT_RENAME;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = origin->path;
@@ -1075,7 +1075,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
 		return 1; /* nothing remains for this target */
 
 	diff_setup(&diff_opts);
-	diff_opts.recursive = 1;
+	DIFF_OPT_SET(&diff_opts, RECURSIVE);
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 
 	paths[0] = NULL;
@@ -1093,7 +1093,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
 	if ((opt & PICKAXE_BLAME_COPY_HARDEST)
 	    || ((opt & PICKAXE_BLAME_COPY_HARDER)
 		&& (!porigin || strcmp(target->path, porigin->path))))
-		diff_opts.find_copies_harder = 1;
+		DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
 
 	if (is_null_sha1(target->commit->object.sha1))
 		do_diff_cache(parent->tree->object.sha1, &diff_opts);
@@ -1102,7 +1102,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
 			       target->commit->tree->object.sha1,
 			       "", &diff_opts);
 
-	if (!diff_opts.find_copies_harder)
+	if (!DIFF_OPT_TST(&diff_opts, FIND_COPIES_HARDER))
 		diffcore_std(&diff_opts);
 
 	retval = 0;
diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 6cb30c8..046b7e3 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -31,5 +31,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
 	if (!rev.diffopt.output_format)
 		rev.diffopt.output_format = DIFF_FORMAT_RAW;
 	result = run_diff_files_cmd(&rev, argc, argv);
-	return rev.diffopt.exit_with_status ? rev.diffopt.has_changes: result;
+	if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
+		return DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
+	return result;
 }
diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 81e7167..556c506 100644
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -44,5 +44,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
 		return -1;
 	}
 	result = run_diff_index(&rev, cached);
-	return rev.diffopt.exit_with_status ? rev.diffopt.has_changes: result;
+	if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
+		return DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
+	return result;
 }
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 0b591c8..e71841a 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -118,12 +118,12 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	}
 
 	if (!read_stdin)
-		return opt->diffopt.exit_with_status ?
-		    opt->diffopt.has_changes: 0;
+		return DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS)
+			&& DIFF_OPT_TST(&opt->diffopt, HAS_CHANGES);
 
 	if (opt->diffopt.detect_rename)
 		opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
-				       DIFF_SETUP_USE_CACHE);
+							   DIFF_SETUP_USE_CACHE);
 	while (fgets(line, sizeof(line), stdin)) {
 		unsigned char sha1[20];
 
@@ -134,5 +134,6 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 		else
 			diff_tree_stdin(line);
 	}
-	return opt->diffopt.exit_with_status ? opt->diffopt.has_changes: 0;
+	return DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS)
+		&& DIFF_OPT_TST(&opt->diffopt, HAS_CHANGES);
 }
diff --git a/builtin-diff.c b/builtin-diff.c
index 80392a8..59b9c6e 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -35,7 +35,7 @@ static void stuff_change(struct diff_options *opt,
 	    !hashcmp(old_sha1, new_sha1) && (old_mode == new_mode))
 		return;
 
-	if (opt->reverse_diff) {
+	if (DIFF_OPT_TST(opt, REVERSE_DIFF)) {
 		unsigned tmp;
 		const unsigned char *tmp_u;
 		const char *tmp_c;
@@ -257,13 +257,13 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 		if (diff_setup_done(&rev.diffopt) < 0)
 			die("diff_setup_done failed");
 	}
-	rev.diffopt.allow_external = 1;
-	rev.diffopt.recursive = 1;
+	DIFF_OPT_SET(&rev.diffopt, ALLOW_EXTERNAL);
+	DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
 
 	/* If the user asked for our exit code then don't start a
 	 * pager or we would end up reporting its exit code instead.
 	 */
-	if (!rev.diffopt.exit_with_status)
+	if (!DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
 		setup_pager();
 
 	/* Do we have --cached and not have a pending object, then
@@ -367,8 +367,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	else
 		result = builtin_diff_combined(&rev, argc, argv,
 					     ent, ents);
-	if (rev.diffopt.exit_with_status)
-		result = rev.diffopt.has_changes;
+	if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
+		result = DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
 
 	if (1 < rev.diffopt.skip_stat_unmatch)
 		refresh_index_quietly();
diff --git a/builtin-log.c b/builtin-log.c
index b6ca04a..72745cd 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -55,13 +55,13 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
 	rev->verbose_header = 1;
-	rev->diffopt.recursive = 1;
+	DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
 	rev->show_root_diff = default_show_root;
 	rev->subject_prefix = fmt_patch_subject_prefix;
 	argc = setup_revisions(argc, argv, rev, "HEAD");
 	if (rev->diffopt.pickaxe || rev->diffopt.filter)
 		rev->always_show_header = 0;
-	if (rev->diffopt.follow_renames) {
+	if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
 		rev->always_show_header = 0;
 		if (rev->diffopt.nr_paths != 1)
 			usage("git logs can only follow renames on one pathname at a time");
@@ -308,11 +308,9 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 			struct tag *t = (struct tag *)o;
 
 			printf("%stag %s%s\n\n",
-					diff_get_color(rev.diffopt.color_diff,
-						DIFF_COMMIT),
+					diff_get_color_opt(&rev.diffopt, DIFF_COMMIT),
 					t->tag,
-					diff_get_color(rev.diffopt.color_diff,
-						DIFF_RESET));
+					diff_get_color_opt(&rev.diffopt, DIFF_RESET));
 			ret = show_object(o->sha1, 1);
 			objects[i].item = (struct object *)t->tagged;
 			i--;
@@ -320,11 +318,9 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 		}
 		case OBJ_TREE:
 			printf("%stree %s%s\n\n",
-					diff_get_color(rev.diffopt.color_diff,
-						DIFF_COMMIT),
+					diff_get_color_opt(&rev.diffopt, DIFF_COMMIT),
 					name,
-					diff_get_color(rev.diffopt.color_diff,
-						DIFF_RESET));
+					diff_get_color_opt(&rev.diffopt, DIFF_RESET));
 			read_tree_recursive((struct tree *)o, "", 0, 0, NULL,
 					show_tree_object);
 			break;
@@ -620,7 +616,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.combine_merges = 0;
 	rev.ignore_merges = 1;
 	rev.diffopt.msg_sep = "";
-	rev.diffopt.recursive = 1;
+	DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
 
 	rev.subject_prefix = fmt_patch_subject_prefix;
 	rev.extra_headers = extra_headers;
@@ -728,8 +724,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (!rev.diffopt.output_format)
 		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY | DIFF_FORMAT_PATCH;
 
-	if (!rev.diffopt.text)
-		rev.diffopt.binary = 1;
+	if (!DIFF_OPT_TST(&rev.diffopt, TEXT))
+		DIFF_OPT_SET(&rev.diffopt, BINARY);
 
 	if (!output_directory && !use_stdout)
 		output_directory = prefix;
@@ -887,7 +883,7 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 	revs.diff = 1;
 	revs.combine_merges = 0;
 	revs.ignore_merges = 1;
-	revs.diffopt.recursive = 1;
+	DIFF_OPT_SET(&revs.diffopt, RECURSIVE);
 
 	if (add_pending_commit(head, &revs, 0))
 		die("Unknown commit %s", head);
diff --git a/combine-diff.c b/combine-diff.c
index fe5a2a1..3cab04b 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -664,7 +664,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 	int mode_differs = 0;
 	int i, show_hunks;
 	int working_tree_file = is_null_sha1(elem->sha1);
-	int abbrev = opt->full_index ? 40 : DEFAULT_ABBREV;
+        int abbrev = DIFF_OPT_TST(opt, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
 	mmfile_t result_file;
 
 	context = opt->context;
@@ -784,7 +784,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 
 	if (show_hunks || mode_differs || working_tree_file) {
 		const char *abb;
-		int use_color = opt->color_diff;
+		int use_color = DIFF_OPT_TST(opt, COLOR_DIFF);
 		const char *c_meta = diff_get_color(use_color, DIFF_METAINFO);
 		const char *c_reset = diff_get_color(use_color, DIFF_RESET);
 		int added = 0;
@@ -836,7 +836,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			dump_quoted_path("+++ /dev/", "null", c_meta, c_reset);
 		else
 			dump_quoted_path("+++ b/", elem->path, c_meta, c_reset);
-		dump_sline(sline, cnt, num_parent, opt->color_diff);
+		dump_sline(sline, cnt, num_parent, DIFF_OPT_TST(opt, COLOR_DIFF));
 	}
 	free(result);
 
@@ -929,8 +929,8 @@ void diff_tree_combined(const unsigned char *sha1,
 
 	diffopts = *opt;
 	diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
-	diffopts.recursive = 1;
-	diffopts.allow_external = 0;
+	DIFF_OPT_SET(&diffopts, RECURSIVE);
+	DIFF_OPT_CLR(&diffopts, ALLOW_EXTERNAL);
 
 	show_log_first = !!rev->loginfo && !rev->no_commit_id;
 	needsep = 0;
diff --git a/diff-lib.c b/diff-lib.c
index da55713..69b5dc9 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -121,7 +121,7 @@ static int queue_diff(struct diff_options *o,
 	} else {
 		struct diff_filespec *d1, *d2;
 
-		if (o->reverse_diff) {
+		if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
 			unsigned tmp;
 			const char *tmp_c;
 			tmp = mode1; mode1 = mode2; mode2 = tmp;
@@ -188,8 +188,7 @@ static int handle_diff_files_args(struct rev_info *revs,
 		else if (!strcmp(argv[1], "-n") ||
 				!strcmp(argv[1], "--no-index")) {
 			revs->max_count = -2;
-			revs->diffopt.exit_with_status = 1;
-			revs->diffopt.no_index = 1;
+			revs->diffopt.flags |= DIFF_OPT_EXIT_WITH_STATUS | DIFF_OPT_NO_INDEX;
 		}
 		else if (!strcmp(argv[1], "-q"))
 			*silent = 1;
@@ -207,7 +206,7 @@ static int handle_diff_files_args(struct rev_info *revs,
 		if (!is_in_index(revs->diffopt.paths[0]) ||
 					!is_in_index(revs->diffopt.paths[1])) {
 			revs->max_count = -2;
-			revs->diffopt.no_index = 1;
+			DIFF_OPT_SET(&revs->diffopt, NO_INDEX);
 		}
 	}
 
@@ -258,7 +257,7 @@ int setup_diff_no_index(struct rev_info *revs,
 			break;
 		} else if (i < argc - 3 && !strcmp(argv[i], "--no-index")) {
 			i = argc - 3;
-			revs->diffopt.exit_with_status = 1;
+			DIFF_OPT_SET(&revs->diffopt, EXIT_WITH_STATUS);
 			break;
 		}
 	if (argc != i + 2 || (!is_outside_repo(argv[i + 1], nongit, prefix) &&
@@ -296,7 +295,7 @@ int setup_diff_no_index(struct rev_info *revs,
 	else
 		revs->diffopt.paths = argv + argc - 2;
 	revs->diffopt.nr_paths = 2;
-	revs->diffopt.no_index = 1;
+	DIFF_OPT_SET(&revs->diffopt, NO_INDEX);
 	revs->max_count = -2;
 	if (diff_setup_done(&revs->diffopt) < 0)
 		die("diff_setup_done failed");
@@ -310,7 +309,7 @@ int run_diff_files_cmd(struct rev_info *revs, int argc, const char **argv)
 	if (handle_diff_files_args(revs, argc, argv, &silent_on_removed))
 		return -1;
 
-	if (revs->diffopt.no_index) {
+	if (DIFF_OPT_TST(&revs->diffopt, NO_INDEX)) {
 		if (revs->diffopt.nr_paths != 2)
 			return error("need two files/directories with --no-index");
 		if (queue_diff(&revs->diffopt, revs->diffopt.paths[0],
@@ -346,7 +345,8 @@ int run_diff_files(struct rev_info *revs, int silent_on_removed)
 		struct cache_entry *ce = active_cache[i];
 		int changed;
 
-		if (revs->diffopt.quiet && revs->diffopt.has_changes)
+		if (DIFF_OPT_TST(&revs->diffopt, QUIET) &&
+			DIFF_OPT_TST(&revs->diffopt, HAS_CHANGES))
 			break;
 
 		if (!ce_path_match(ce, revs->prune_data))
@@ -442,7 +442,7 @@ int run_diff_files(struct rev_info *revs, int silent_on_removed)
 			continue;
 		}
 		changed = ce_match_stat(ce, &st, 0);
-		if (!changed && !revs->diffopt.find_copies_harder)
+		if (!changed && !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
 			continue;
 		oldmode = ntohl(ce->ce_mode);
 		newmode = ntohl(ce_mode_from_stat(ce, st.st_mode));
@@ -561,7 +561,7 @@ static int show_modified(struct rev_info *revs,
 
 	oldmode = old->ce_mode;
 	if (mode == oldmode && !hashcmp(sha1, old->sha1) &&
-	    !revs->diffopt.find_copies_harder)
+	    !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
 		return 0;
 
 	mode = ntohl(mode);
@@ -581,7 +581,8 @@ static int diff_cache(struct rev_info *revs,
 		struct cache_entry *ce = *ac;
 		int same = (entries > 1) && ce_same_name(ce, ac[1]);
 
-		if (revs->diffopt.quiet && revs->diffopt.has_changes)
+		if (DIFF_OPT_TST(&revs->diffopt, QUIET) &&
+			DIFF_OPT_TST(&revs->diffopt, HAS_CHANGES))
 			break;
 
 		if (!ce_path_match(ce, pathspec))
diff --git a/diff.c b/diff.c
index 6bb902f..97c9a59 100644
--- a/diff.c
+++ b/diff.c
@@ -827,10 +827,10 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
 	}
 
 	/* Find the longest filename and max number of changes */
-	reset = diff_get_color(options->color_diff, DIFF_RESET);
-	set = diff_get_color(options->color_diff, DIFF_PLAIN);
-	add_c = diff_get_color(options->color_diff, DIFF_FILE_NEW);
-	del_c = diff_get_color(options->color_diff, DIFF_FILE_OLD);
+	reset = diff_get_color_opt(options, DIFF_RESET);
+	set   = diff_get_color_opt(options, DIFF_PLAIN);
+	add_c = diff_get_color_opt(options, DIFF_FILE_NEW);
+	del_c = diff_get_color_opt(options, DIFF_FILE_OLD);
 
 	for (i = 0; i < data->nr; i++) {
 		struct diffstat_file *file = data->files[i];
@@ -1256,8 +1256,8 @@ static void builtin_diff(const char *name_a,
 	mmfile_t mf1, mf2;
 	const char *lbl[2];
 	char *a_one, *b_two;
-	const char *set = diff_get_color(o->color_diff, DIFF_METAINFO);
-	const char *reset = diff_get_color(o->color_diff, DIFF_RESET);
+	const char *set = diff_get_color_opt(o, DIFF_METAINFO);
+	const char *reset = diff_get_color_opt(o, DIFF_RESET);
 
 	a_one = quote_two("a/", name_a + (*name_a == '/'));
 	b_two = quote_two("b/", name_b + (*name_b == '/'));
@@ -1290,7 +1290,7 @@ static void builtin_diff(const char *name_a,
 			goto free_ab_and_return;
 		if (complete_rewrite) {
 			emit_rewrite_diff(name_a, name_b, one, two,
-					o->color_diff);
+					DIFF_OPT_TST(o, COLOR_DIFF));
 			o->found_changes = 1;
 			goto free_ab_and_return;
 		}
@@ -1299,13 +1299,13 @@ static void builtin_diff(const char *name_a,
 	if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
 		die("unable to read files to diff");
 
-	if (!o->text &&
+	if (!DIFF_OPT_TST(o, TEXT) &&
 	    (diff_filespec_is_binary(one) || diff_filespec_is_binary(two))) {
 		/* Quite common confusing case */
 		if (mf1.size == mf2.size &&
 		    !memcmp(mf1.ptr, mf2.ptr, mf1.size))
 			goto free_ab_and_return;
-		if (o->binary)
+		if (DIFF_OPT_TST(o, BINARY))
 			emit_binary_diff(&mf1, &mf2);
 		else
 			printf("Binary files %s and %s differ\n",
@@ -1328,7 +1328,7 @@ static void builtin_diff(const char *name_a,
 		memset(&xecfg, 0, sizeof(xecfg));
 		memset(&ecbdata, 0, sizeof(ecbdata));
 		ecbdata.label_path = lbl;
-		ecbdata.color_diff = o->color_diff;
+		ecbdata.color_diff = DIFF_OPT_TST(o, COLOR_DIFF);
 		ecbdata.found_changesp = &o->found_changes;
 		xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
 		xecfg.ctxlen = o->context;
@@ -1344,11 +1344,11 @@ static void builtin_diff(const char *name_a,
 		ecb.outf = xdiff_outf;
 		ecb.priv = &ecbdata;
 		ecbdata.xm.consume = fn_out_consume;
-		if (o->color_diff_words)
+		if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))
 			ecbdata.diff_words =
 				xcalloc(1, sizeof(struct diff_words_data));
 		xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
-		if (o->color_diff_words)
+		if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))
 			free_diff_words_data(&ecbdata);
 	}
 
@@ -1422,7 +1422,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
 	data.xm.consume = checkdiff_consume;
 	data.filename = name_b ? name_b : name_a;
 	data.lineno = 0;
-	data.color_diff = o->color_diff;
+	data.color_diff = DIFF_OPT_TST(o, COLOR_DIFF);
 
 	if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
 		die("unable to read files to diff");
@@ -1866,7 +1866,7 @@ static void run_diff_cmd(const char *pgm,
 			 struct diff_options *o,
 			 int complete_rewrite)
 {
-	if (!o->allow_external)
+	if (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))
 		pgm = NULL;
 	else {
 		const char *cmd = external_diff_attr(name);
@@ -1964,9 +1964,9 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
 	}
 
 	if (hashcmp(one->sha1, two->sha1)) {
-		int abbrev = o->full_index ? 40 : DEFAULT_ABBREV;
+		int abbrev = DIFF_OPT_TST(o, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
 
-		if (o->binary) {
+		if (DIFF_OPT_TST(o, BINARY)) {
 			mmfile_t mf;
 			if ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) ||
 			    (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))
@@ -2058,7 +2058,10 @@ void diff_setup(struct diff_options *options)
 
 	options->change = diff_change;
 	options->add_remove = diff_addremove;
-	options->color_diff = diff_use_color_default;
+	if (diff_use_color_default)
+		DIFF_OPT_SET(options, COLOR_DIFF);
+	else
+		DIFF_OPT_CLR(options, COLOR_DIFF);
 	options->detect_rename = diff_detect_rename_default;
 }
 
@@ -2077,7 +2080,7 @@ int diff_setup_done(struct diff_options *options)
 	if (count > 1)
 		die("--name-only, --name-status, --check and -s are mutually exclusive");
 
-	if (options->find_copies_harder)
+	if (DIFF_OPT_TST(options, FIND_COPIES_HARDER))
 		options->detect_rename = DIFF_DETECT_COPY;
 
 	if (options->output_format & (DIFF_FORMAT_NAME |
@@ -2101,12 +2104,12 @@ int diff_setup_done(struct diff_options *options)
 				      DIFF_FORMAT_SHORTSTAT |
 				      DIFF_FORMAT_SUMMARY |
 				      DIFF_FORMAT_CHECKDIFF))
-		options->recursive = 1;
+		DIFF_OPT_SET(options, RECURSIVE);
 	/*
 	 * Also pickaxe would not work very well if you do not say recursive
 	 */
 	if (options->pickaxe)
-		options->recursive = 1;
+		DIFF_OPT_SET(options, RECURSIVE);
 
 	if (options->detect_rename && options->rename_limit < 0)
 		options->rename_limit = diff_rename_limit_default;
@@ -2128,9 +2131,9 @@ int diff_setup_done(struct diff_options *options)
 	 * to have found.  It does not make sense not to return with
 	 * exit code in such a case either.
 	 */
-	if (options->quiet) {
+	if (DIFF_OPT_TST(options, QUIET)) {
 		options->output_format = DIFF_FORMAT_NO_OUTPUT;
-		options->exit_with_status = 1;
+		DIFF_OPT_SET(options, EXIT_WITH_STATUS);
 	}
 
 	/*
@@ -2138,7 +2141,7 @@ int diff_setup_done(struct diff_options *options)
 	 * upon the first hit.  We need to run diff as usual.
 	 */
 	if (options->pickaxe || options->filter)
-		options->quiet = 0;
+		DIFF_OPT_CLR(options, QUIET);
 
 	return 0;
 }
@@ -2201,15 +2204,12 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->output_format |= DIFF_FORMAT_PATCH;
 	else if (!strcmp(arg, "--raw"))
 		options->output_format |= DIFF_FORMAT_RAW;
-	else if (!strcmp(arg, "--patch-with-raw")) {
+	else if (!strcmp(arg, "--patch-with-raw"))
 		options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW;
-	}
-	else if (!strcmp(arg, "--numstat")) {
+	else if (!strcmp(arg, "--numstat"))
 		options->output_format |= DIFF_FORMAT_NUMSTAT;
-	}
-	else if (!strcmp(arg, "--shortstat")) {
+	else if (!strcmp(arg, "--shortstat"))
 		options->output_format |= DIFF_FORMAT_SHORTSTAT;
-	}
 	else if (!prefixcmp(arg, "--stat")) {
 		char *end;
 		int width = options->stat_width;
@@ -2241,33 +2241,30 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->output_format |= DIFF_FORMAT_CHECKDIFF;
 	else if (!strcmp(arg, "--summary"))
 		options->output_format |= DIFF_FORMAT_SUMMARY;
-	else if (!strcmp(arg, "--patch-with-stat")) {
+	else if (!strcmp(arg, "--patch-with-stat"))
 		options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT;
-	}
 	else if (!strcmp(arg, "-z"))
 		options->line_termination = 0;
 	else if (!prefixcmp(arg, "-l"))
 		options->rename_limit = strtoul(arg+2, NULL, 10);
 	else if (!strcmp(arg, "--full-index"))
-		options->full_index = 1;
+		DIFF_OPT_SET(options, FULL_INDEX);
 	else if (!strcmp(arg, "--binary")) {
 		options->output_format |= DIFF_FORMAT_PATCH;
-		options->binary = 1;
-	}
-	else if (!strcmp(arg, "-a") || !strcmp(arg, "--text")) {
-		options->text = 1;
+		DIFF_OPT_SET(options, BINARY);
 	}
+	else if (!strcmp(arg, "-a") || !strcmp(arg, "--text"))
+		DIFF_OPT_SET(options, TEXT);
 	else if (!strcmp(arg, "--name-only"))
 		options->output_format |= DIFF_FORMAT_NAME;
 	else if (!strcmp(arg, "--name-status"))
 		options->output_format |= DIFF_FORMAT_NAME_STATUS;
 	else if (!strcmp(arg, "-R"))
-		options->reverse_diff = 1;
+		DIFF_OPT_SET(options, REVERSE_DIFF);
 	else if (!prefixcmp(arg, "-S"))
 		options->pickaxe = arg + 2;
-	else if (!strcmp(arg, "-s")) {
+	else if (!strcmp(arg, "-s"))
 		options->output_format |= DIFF_FORMAT_NO_OUTPUT;
-	}
 	else if (!prefixcmp(arg, "-O"))
 		options->orderfile = arg + 2;
 	else if (!prefixcmp(arg, "--diff-filter="))
@@ -2277,28 +2274,25 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 	else if (!strcmp(arg, "--pickaxe-regex"))
 		options->pickaxe_opts = DIFF_PICKAXE_REGEX;
 	else if (!prefixcmp(arg, "-B")) {
-		if ((options->break_opt =
-		     diff_scoreopt_parse(arg)) == -1)
+		if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)
 			return -1;
 	}
 	else if (!prefixcmp(arg, "-M")) {
-		if ((options->rename_score =
-		     diff_scoreopt_parse(arg)) == -1)
+		if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
 			return -1;
 		options->detect_rename = DIFF_DETECT_RENAME;
 	}
 	else if (!prefixcmp(arg, "-C")) {
 		if (options->detect_rename == DIFF_DETECT_COPY)
-			options->find_copies_harder = 1;
-		if ((options->rename_score =
-		     diff_scoreopt_parse(arg)) == -1)
+			DIFF_OPT_SET(options, FIND_COPIES_HARDER);
+		if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
 			return -1;
 		options->detect_rename = DIFF_DETECT_COPY;
 	}
 	else if (!strcmp(arg, "--find-copies-harder"))
-		options->find_copies_harder = 1;
+		DIFF_OPT_SET(options, FIND_COPIES_HARDER);
 	else if (!strcmp(arg, "--follow"))
-		options->follow_renames = 1;
+		DIFF_OPT_SET(options, FOLLOW_RENAMES);
 	else if (!strcmp(arg, "--abbrev"))
 		options->abbrev = DEFAULT_ABBREV;
 	else if (!prefixcmp(arg, "--abbrev=")) {
@@ -2309,9 +2303,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 			options->abbrev = 40;
 	}
 	else if (!strcmp(arg, "--color"))
-		options->color_diff = 1;
+		DIFF_OPT_SET(options, COLOR_DIFF);
 	else if (!strcmp(arg, "--no-color"))
-		options->color_diff = 0;
+		DIFF_OPT_CLR(options, COLOR_DIFF);
 	else if (!strcmp(arg, "-w") || !strcmp(arg, "--ignore-all-space"))
 		options->xdl_opts |= XDF_IGNORE_WHITESPACE;
 	else if (!strcmp(arg, "-b") || !strcmp(arg, "--ignore-space-change"))
@@ -2319,17 +2313,17 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 	else if (!strcmp(arg, "--ignore-space-at-eol"))
 		options->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;
 	else if (!strcmp(arg, "--color-words"))
-		options->color_diff = options->color_diff_words = 1;
+		options->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;
 	else if (!strcmp(arg, "--no-renames"))
 		options->detect_rename = 0;
 	else if (!strcmp(arg, "--exit-code"))
-		options->exit_with_status = 1;
+		DIFF_OPT_SET(options, EXIT_WITH_STATUS);
 	else if (!strcmp(arg, "--quiet"))
-		options->quiet = 1;
+		DIFF_OPT_SET(options, QUIET);
 	else if (!strcmp(arg, "--ext-diff"))
-		options->allow_external = 1;
+		DIFF_OPT_SET(options, ALLOW_EXTERNAL);
 	else if (!strcmp(arg, "--no-ext-diff"))
-		options->allow_external = 0;
+		DIFF_OPT_CLR(options, ALLOW_EXTERNAL);
 	else
 		return 0;
 	return 1;
@@ -3084,7 +3078,7 @@ static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
 			 * to determine how many paths were dirty only
 			 * due to stat info mismatch.
 			 */
-			if (!diffopt->no_index)
+			if (!DIFF_OPT_TST(diffopt, NO_INDEX))
 				diffopt->skip_stat_unmatch++;
 			diff_free_filepair(p);
 		}
@@ -3095,10 +3089,10 @@ static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
 
 void diffcore_std(struct diff_options *options)
 {
-	if (options->quiet)
+	if (DIFF_OPT_TST(options, QUIET))
 		return;
 
-	if (options->skip_stat_unmatch && !options->find_copies_harder)
+	if (options->skip_stat_unmatch && !DIFF_OPT_TST(options, FIND_COPIES_HARDER))
 		diffcore_skip_stat_unmatch(options);
 	if (options->break_opt != -1)
 		diffcore_break(options->break_opt);
@@ -3113,7 +3107,10 @@ void diffcore_std(struct diff_options *options)
 	diff_resolve_rename_copy();
 	diffcore_apply_filter(options->filter);
 
-	options->has_changes = !!diff_queued_diff.nr;
+	if (diff_queued_diff.nr)
+		DIFF_OPT_SET(options, HAS_CHANGES);
+	else
+		DIFF_OPT_CLR(options, HAS_CHANGES);
 }
 
 
@@ -3137,7 +3134,7 @@ void diff_addremove(struct diff_options *options,
 	 * Before the final output happens, they are pruned after
 	 * merged into rename/copy pairs as appropriate.
 	 */
-	if (options->reverse_diff)
+	if (DIFF_OPT_TST(options, REVERSE_DIFF))
 		addremove = (addremove == '+' ? '-' :
 			     addremove == '-' ? '+' : addremove);
 
@@ -3152,7 +3149,7 @@ void diff_addremove(struct diff_options *options,
 		fill_filespec(two, sha1, mode);
 
 	diff_queue(&diff_queued_diff, one, two);
-	options->has_changes = 1;
+	DIFF_OPT_SET(options, HAS_CHANGES);
 }
 
 void diff_change(struct diff_options *options,
@@ -3164,7 +3161,7 @@ void diff_change(struct diff_options *options,
 	char concatpath[PATH_MAX];
 	struct diff_filespec *one, *two;
 
-	if (options->reverse_diff) {
+	if (DIFF_OPT_TST(options, REVERSE_DIFF)) {
 		unsigned tmp;
 		const unsigned char *tmp_c;
 		tmp = old_mode; old_mode = new_mode; new_mode = tmp;
@@ -3178,7 +3175,7 @@ void diff_change(struct diff_options *options,
 	fill_filespec(two, new_sha1, new_mode);
 
 	diff_queue(&diff_queued_diff, one, two);
-	options->has_changes = 1;
+	DIFF_OPT_SET(options, HAS_CHANGES);
 }
 
 void diff_unmerge(struct diff_options *options,
diff --git a/diff.h b/diff.h
index 4546aad..6ff2b0e 100644
--- a/diff.h
+++ b/diff.h
@@ -43,26 +43,32 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 
 #define DIFF_FORMAT_CALLBACK	0x1000
 
+#define DIFF_OPT_RECURSIVE           (1 <<  0)
+#define DIFF_OPT_TREE_IN_RECURSIVE   (1 <<  1)
+#define DIFF_OPT_BINARY              (1 <<  2)
+#define DIFF_OPT_TEXT                (1 <<  3)
+#define DIFF_OPT_FULL_INDEX          (1 <<  4)
+#define DIFF_OPT_SILENT_ON_REMOVE    (1 <<  5)
+#define DIFF_OPT_FIND_COPIES_HARDER  (1 <<  6)
+#define DIFF_OPT_FOLLOW_RENAMES      (1 <<  7)
+#define DIFF_OPT_COLOR_DIFF          (1 <<  8)
+#define DIFF_OPT_COLOR_DIFF_WORDS    (1 <<  9)
+#define DIFF_OPT_HAS_CHANGES         (1 << 10)
+#define DIFF_OPT_QUIET               (1 << 11)
+#define DIFF_OPT_NO_INDEX            (1 << 12)
+#define DIFF_OPT_ALLOW_EXTERNAL      (1 << 13)
+#define DIFF_OPT_EXIT_WITH_STATUS    (1 << 14)
+#define DIFF_OPT_REVERSE_DIFF        (1 << 15)
+#define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
+#define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
+#define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)
+
 struct diff_options {
 	const char *filter;
 	const char *orderfile;
 	const char *pickaxe;
 	const char *single_follow;
-	unsigned recursive:1,
-		 tree_in_recursive:1,
-		 binary:1,
-		 text:1,
-		 full_index:1,
-		 silent_on_remove:1,
-		 find_copies_harder:1,
-		 follow_renames:1,
-		 color_diff:1,
-		 color_diff_words:1,
-		 has_changes:1,
-		 quiet:1,
-		 no_index:1,
-		 allow_external:1,
-		 exit_with_status:1;
+	unsigned flags;
 	int context;
 	int break_opt;
 	int detect_rename;
@@ -71,7 +77,6 @@ struct diff_options {
 	int output_format;
 	int pickaxe_opts;
 	int rename_score;
-	int reverse_diff;
 	int rename_limit;
 	int setup;
 	int abbrev;
@@ -105,6 +110,9 @@ enum color_diff {
 	DIFF_WHITESPACE = 7,
 };
 const char *diff_get_color(int diff_use_color, enum color_diff ix);
+#define diff_get_color_opt(o, ix) \
+	diff_get_color(DIFF_OPT_TST((o), COLOR_DIFF), ix)
+
 
 extern const char mime_boundary_leader[];
 
diff --git a/log-tree.c b/log-tree.c
index a34beb0..1f3fcf1 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -245,8 +245,7 @@ void show_log(struct rev_info *opt, const char *sep)
 			opt->diffopt.stat_sep = buffer;
 		}
 	} else if (opt->commit_format != CMIT_FMT_USERFORMAT) {
-		fputs(diff_get_color(opt->diffopt.color_diff, DIFF_COMMIT),
-		      stdout);
+		fputs(diff_get_color_opt(&opt->diffopt, DIFF_COMMIT), stdout);
 		if (opt->commit_format != CMIT_FMT_ONELINE)
 			fputs("commit ", stdout);
 		if (commit->object.flags & BOUNDARY)
@@ -266,8 +265,7 @@ void show_log(struct rev_info *opt, const char *sep)
 			       diff_unique_abbrev(parent->object.sha1,
 						  abbrev_commit));
 		show_decorations(commit);
-		printf("%s",
-		       diff_get_color(opt->diffopt.color_diff, DIFF_RESET));
+		printf("%s", diff_get_color_opt(&opt->diffopt, DIFF_RESET));
 		putchar(opt->commit_format == CMIT_FMT_ONELINE ? ' ' : '\n');
 		if (opt->reflog_info) {
 			show_reflog_message(opt->reflog_info,
diff --git a/merge-recursive.c b/merge-recursive.c
index 6c6f595..9a1e2f2 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -366,7 +366,7 @@ static struct path_list *get_renames(struct tree *tree,
 
 	renames = xcalloc(1, sizeof(struct path_list));
 	diff_setup(&opts);
-	opts.recursive = 1;
+	DIFF_OPT_SET(&opts, RECURSIVE);
 	opts.detect_rename = DIFF_DETECT_RENAME;
 	opts.rename_limit = rename_limit;
 	opts.output_format = DIFF_FORMAT_NO_OUTPUT;
diff --git a/patch-ids.c b/patch-ids.c
index a288fac..3be5d31 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -121,7 +121,7 @@ int init_patch_ids(struct patch_ids *ids)
 {
 	memset(ids, 0, sizeof(*ids));
 	diff_setup(&ids->diffopts);
-	ids->diffopts.recursive = 1;
+	DIFF_OPT_SET(&ids->diffopts, RECURSIVE);
 	if (diff_setup_done(&ids->diffopts) < 0)
 		return error("diff_setup_done failed");
 	return 0;
diff --git a/revision.c b/revision.c
index 931f978..040214f 100644
--- a/revision.c
+++ b/revision.c
@@ -252,7 +252,7 @@ static void file_add_remove(struct diff_options *options,
 	}
 	tree_difference = diff;
 	if (tree_difference == REV_TREE_DIFFERENT)
-		options->has_changes = 1;
+		DIFF_OPT_SET(options, HAS_CHANGES);
 }
 
 static void file_change(struct diff_options *options,
@@ -262,7 +262,7 @@ static void file_change(struct diff_options *options,
 		 const char *base, const char *path)
 {
 	tree_difference = REV_TREE_DIFFERENT;
-	options->has_changes = 1;
+	DIFF_OPT_SET(options, HAS_CHANGES);
 }
 
 static int rev_compare_tree(struct rev_info *revs, struct tree *t1, struct tree *t2)
@@ -272,7 +272,7 @@ static int rev_compare_tree(struct rev_info *revs, struct tree *t1, struct tree
 	if (!t2)
 		return REV_TREE_DIFFERENT;
 	tree_difference = REV_TREE_SAME;
-	revs->pruning.has_changes = 0;
+	DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
 	if (diff_tree_sha1(t1->object.sha1, t2->object.sha1, "",
 			   &revs->pruning) < 0)
 		return REV_TREE_DIFFERENT;
@@ -296,7 +296,7 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct tree *t1)
 	init_tree_desc(&empty, "", 0);
 
 	tree_difference = REV_TREE_SAME;
-	revs->pruning.has_changes = 0;
+	DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
 	retval = diff_tree(&empty, &real, "", &revs->pruning);
 	free(tree);
 
@@ -688,8 +688,8 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->abbrev = DEFAULT_ABBREV;
 	revs->ignore_merges = 1;
 	revs->simplify_history = 1;
-	revs->pruning.recursive = 1;
-	revs->pruning.quiet = 1;
+	DIFF_OPT_SET(&revs->pruning, RECURSIVE);
+	DIFF_OPT_SET(&revs->pruning, QUIET);
 	revs->pruning.add_remove = file_add_remove;
 	revs->pruning.change = file_change;
 	revs->lifo = 1;
@@ -1086,13 +1086,13 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 			}
 			if (!strcmp(arg, "-r")) {
 				revs->diff = 1;
-				revs->diffopt.recursive = 1;
+				DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
 				continue;
 			}
 			if (!strcmp(arg, "-t")) {
 				revs->diff = 1;
-				revs->diffopt.recursive = 1;
-				revs->diffopt.tree_in_recursive = 1;
+				DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
+				DIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE);
 				continue;
 			}
 			if (!strcmp(arg, "-m")) {
@@ -1274,7 +1274,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		revs->diff = 1;
 
 	/* Pickaxe and rename following needs diffs */
-	if (revs->diffopt.pickaxe || revs->diffopt.follow_renames)
+	if (revs->diffopt.pickaxe || DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
 		revs->diff = 1;
 
 	if (revs->topo_order)
@@ -1283,7 +1283,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 	if (revs->prune_data) {
 		diff_tree_setup_paths(revs->prune_data, &revs->pruning);
 		/* Can't prune commits with rename following: the paths change.. */
-		if (!revs->diffopt.follow_renames)
+		if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
 			revs->prune = 1;
 		if (!revs->full_diff)
 			diff_tree_setup_paths(revs->prune_data, &revs->diffopt);
diff --git a/tree-diff.c b/tree-diff.c
index 7c261fd..aa0a100 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -39,7 +39,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
 		show_entry(opt, "+", t2, base, baselen);
 		return 1;
 	}
-	if (!opt->find_copies_harder && !hashcmp(sha1, sha2) && mode1 == mode2)
+	if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) && !hashcmp(sha1, sha2) && mode1 == mode2)
 		return 0;
 
 	/*
@@ -52,10 +52,10 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
 		return 0;
 	}
 
-	if (opt->recursive && S_ISDIR(mode1)) {
+	if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode1)) {
 		int retval;
 		char *newbase = malloc_base(base, baselen, path1, pathlen1);
-		if (opt->tree_in_recursive)
+		if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE))
 			opt->change(opt, mode1, mode2,
 				    sha1, sha2, base, path1);
 		retval = diff_tree_sha1(sha1, sha2, newbase, opt);
@@ -206,7 +206,7 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
 	const char *path;
 	const unsigned char *sha1 = tree_entry_extract(desc, &path, &mode);
 
-	if (opt->recursive && S_ISDIR(mode)) {
+	if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode)) {
 		enum object_type type;
 		int pathlen = tree_entry_len(path, sha1);
 		char *newbase = malloc_base(base, baselen, path, pathlen);
@@ -257,7 +257,7 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, stru
 	int baselen = strlen(base);
 
 	for (;;) {
-		if (opt->quiet && opt->has_changes)
+		if (DIFF_OPT_TST(opt, QUIET) && DIFF_OPT_TST(opt, HAS_CHANGES))
 			break;
 		if (opt->nr_paths) {
 			skip_uninteresting(t1, base, baselen, opt);
@@ -315,7 +315,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 	q->nr = 0;
 
 	diff_setup(&diff_opts);
-	diff_opts.recursive = 1;
+	DIFF_OPT_SET(&diff_opts, RECURSIVE);
 	diff_opts.detect_rename = DIFF_DETECT_RENAME;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = opt->paths[0];
@@ -380,7 +380,7 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha
 	init_tree_desc(&t1, tree1, size1);
 	init_tree_desc(&t2, tree2, size2);
 	retval = diff_tree(&t1, &t2, base, opt);
-	if (opt->follow_renames && diff_might_be_rename()) {
+	if (DIFF_OPT_TST(opt, FOLLOW_RENAMES) && diff_might_be_rename()) {
 		init_tree_desc(&t1, tree1, size1);
 		init_tree_desc(&t2, tree2, size2);
 		try_to_follow_renames(&t1, &t2, base, opt);
-- 
1.5.3.5.1598.gdef4e

^ permalink raw reply related

* [PATCH DIFF-CLEANUP 2/2] Reorder diff_opt_parse options more logically per topics.
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-7-git-send-email-madcoder@debian.org>

This is a line reordering patch _only_.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
    the 10 added lines are five times a blank line and a header for the each
    of the 5 options groups.

    This patch is very useful as it will help checking that I didn't missed
    any option while converting to parseopt, it's not coquetry.

 diff.c |  116 ++++++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 63 insertions(+), 53 deletions(-)

diff --git a/diff.c b/diff.c
index 97c9a59..7a89959 100644
--- a/diff.c
+++ b/diff.c
@@ -2198,6 +2198,8 @@ static int diff_scoreopt_parse(const char *opt);
 int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 {
 	const char *arg = av[0];
+
+	/* Output format options */
 	if (!strcmp(arg, "-p") || !strcmp(arg, "-u"))
 		options->output_format |= DIFF_FORMAT_PATCH;
 	else if (opt_arg(arg, 'U', "unified", &options->context))
@@ -2210,6 +2212,18 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->output_format |= DIFF_FORMAT_NUMSTAT;
 	else if (!strcmp(arg, "--shortstat"))
 		options->output_format |= DIFF_FORMAT_SHORTSTAT;
+	else if (!strcmp(arg, "--check"))
+		options->output_format |= DIFF_FORMAT_CHECKDIFF;
+	else if (!strcmp(arg, "--summary"))
+		options->output_format |= DIFF_FORMAT_SUMMARY;
+	else if (!strcmp(arg, "--patch-with-stat"))
+		options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT;
+	else if (!strcmp(arg, "--name-only"))
+		options->output_format |= DIFF_FORMAT_NAME;
+	else if (!strcmp(arg, "--name-status"))
+		options->output_format |= DIFF_FORMAT_NAME_STATUS;
+	else if (!strcmp(arg, "-s"))
+		options->output_format |= DIFF_FORMAT_NO_OUTPUT;
 	else if (!prefixcmp(arg, "--stat")) {
 		char *end;
 		int width = options->stat_width;
@@ -2237,42 +2251,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->stat_name_width = name_width;
 		options->stat_width = width;
 	}
-	else if (!strcmp(arg, "--check"))
-		options->output_format |= DIFF_FORMAT_CHECKDIFF;
-	else if (!strcmp(arg, "--summary"))
-		options->output_format |= DIFF_FORMAT_SUMMARY;
-	else if (!strcmp(arg, "--patch-with-stat"))
-		options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT;
-	else if (!strcmp(arg, "-z"))
-		options->line_termination = 0;
-	else if (!prefixcmp(arg, "-l"))
-		options->rename_limit = strtoul(arg+2, NULL, 10);
-	else if (!strcmp(arg, "--full-index"))
-		DIFF_OPT_SET(options, FULL_INDEX);
-	else if (!strcmp(arg, "--binary")) {
-		options->output_format |= DIFF_FORMAT_PATCH;
-		DIFF_OPT_SET(options, BINARY);
-	}
-	else if (!strcmp(arg, "-a") || !strcmp(arg, "--text"))
-		DIFF_OPT_SET(options, TEXT);
-	else if (!strcmp(arg, "--name-only"))
-		options->output_format |= DIFF_FORMAT_NAME;
-	else if (!strcmp(arg, "--name-status"))
-		options->output_format |= DIFF_FORMAT_NAME_STATUS;
-	else if (!strcmp(arg, "-R"))
-		DIFF_OPT_SET(options, REVERSE_DIFF);
-	else if (!prefixcmp(arg, "-S"))
-		options->pickaxe = arg + 2;
-	else if (!strcmp(arg, "-s"))
-		options->output_format |= DIFF_FORMAT_NO_OUTPUT;
-	else if (!prefixcmp(arg, "-O"))
-		options->orderfile = arg + 2;
-	else if (!prefixcmp(arg, "--diff-filter="))
-		options->filter = arg + 14;
-	else if (!strcmp(arg, "--pickaxe-all"))
-		options->pickaxe_opts = DIFF_PICKAXE_ALL;
-	else if (!strcmp(arg, "--pickaxe-regex"))
-		options->pickaxe_opts = DIFF_PICKAXE_REGEX;
+
+	/* renames options */
 	else if (!prefixcmp(arg, "-B")) {
 		if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)
 			return -1;
@@ -2289,33 +2269,38 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 			return -1;
 		options->detect_rename = DIFF_DETECT_COPY;
 	}
+	else if (!strcmp(arg, "--no-renames"))
+		options->detect_rename = 0;
+
+	/* xdiff options */
+	else if (!strcmp(arg, "-w") || !strcmp(arg, "--ignore-all-space"))
+		options->xdl_opts |= XDF_IGNORE_WHITESPACE;
+	else if (!strcmp(arg, "-b") || !strcmp(arg, "--ignore-space-change"))
+		options->xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;
+	else if (!strcmp(arg, "--ignore-space-at-eol"))
+		options->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;
+
+	/* flags options */
+	else if (!strcmp(arg, "--binary")) {
+		options->output_format |= DIFF_FORMAT_PATCH;
+		DIFF_OPT_SET(options, BINARY);
+	}
+	else if (!strcmp(arg, "--full-index"))
+		DIFF_OPT_SET(options, FULL_INDEX);
+	else if (!strcmp(arg, "-a") || !strcmp(arg, "--text"))
+		DIFF_OPT_SET(options, TEXT);
+	else if (!strcmp(arg, "-R"))
+		DIFF_OPT_SET(options, REVERSE_DIFF);
 	else if (!strcmp(arg, "--find-copies-harder"))
 		DIFF_OPT_SET(options, FIND_COPIES_HARDER);
 	else if (!strcmp(arg, "--follow"))
 		DIFF_OPT_SET(options, FOLLOW_RENAMES);
-	else if (!strcmp(arg, "--abbrev"))
-		options->abbrev = DEFAULT_ABBREV;
-	else if (!prefixcmp(arg, "--abbrev=")) {
-		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 if (!strcmp(arg, "--color"))
 		DIFF_OPT_SET(options, COLOR_DIFF);
 	else if (!strcmp(arg, "--no-color"))
 		DIFF_OPT_CLR(options, COLOR_DIFF);
-	else if (!strcmp(arg, "-w") || !strcmp(arg, "--ignore-all-space"))
-		options->xdl_opts |= XDF_IGNORE_WHITESPACE;
-	else if (!strcmp(arg, "-b") || !strcmp(arg, "--ignore-space-change"))
-		options->xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;
-	else if (!strcmp(arg, "--ignore-space-at-eol"))
-		options->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;
 	else if (!strcmp(arg, "--color-words"))
 		options->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;
-	else if (!strcmp(arg, "--no-renames"))
-		options->detect_rename = 0;
 	else if (!strcmp(arg, "--exit-code"))
 		DIFF_OPT_SET(options, EXIT_WITH_STATUS);
 	else if (!strcmp(arg, "--quiet"))
@@ -2324,6 +2309,31 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		DIFF_OPT_SET(options, ALLOW_EXTERNAL);
 	else if (!strcmp(arg, "--no-ext-diff"))
 		DIFF_OPT_CLR(options, ALLOW_EXTERNAL);
+
+	/* misc options */
+	else if (!strcmp(arg, "-z"))
+		options->line_termination = 0;
+	else if (!prefixcmp(arg, "-l"))
+		options->rename_limit = strtoul(arg+2, NULL, 10);
+	else if (!prefixcmp(arg, "-S"))
+		options->pickaxe = arg + 2;
+	else if (!strcmp(arg, "--pickaxe-all"))
+		options->pickaxe_opts = DIFF_PICKAXE_ALL;
+	else if (!strcmp(arg, "--pickaxe-regex"))
+		options->pickaxe_opts = DIFF_PICKAXE_REGEX;
+	else if (!prefixcmp(arg, "-O"))
+		options->orderfile = arg + 2;
+	else if (!prefixcmp(arg, "--diff-filter="))
+		options->filter = arg + 14;
+	else if (!strcmp(arg, "--abbrev"))
+		options->abbrev = DEFAULT_ABBREV;
+	else if (!prefixcmp(arg, "--abbrev=")) {
+		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
 		return 0;
 	return 1;
-- 
1.5.3.5.1598.gdef4e

^ 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