Git development
 help / color / mirror / Atom feed
* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Jeff King @ 2009-11-18 17:24 UTC (permalink / raw)
  To: Philip Hofstetter; +Cc: git
In-Reply-To: <aa2993680911180911o7e3af804m4ebdc20096baa609@mail.gmail.com>

On Wed, Nov 18, 2009 at 06:11:36PM +0100, Philip Hofstetter wrote:

> > As I explained above, there is a reason, but I don't think it's rude to
> > have either of those lines. You were, after all, writing a commit
> > message, not an email (and even if you were, it is a failure of the
> > storage format if it can't represent your data correctly). So I think
> > git is to blame here.
> 
> IMHO, another workable solution would be to reject a commit that later
> can't be handled. That way the current attempts at getting an email
> address can remain intact and the (much more) unlikely case that
> somebody begins the commit message with from: will be caught before
> damage is done.

I'm not sure I like that solution for a few reasons:

  1. It creates a bad user experience. You are not unreasonable for
     wanting to put some specific text in your commit message. Having
     git come back and say "oops, I might get confused by this later"
     just seems like an annoyance to the user.

  2. Mailinfo has to deal with data created by older versions of git. So
     in your case, the rebase was a bomb waiting to go off. If we can
     fix it so that an existing bomb doesn't go off, rather than not
     creating the bomb in the first place, then we are better off.

  3. Commit has to know about rules for mailinfo, even versions of
     mailinfo that will exist in the future. Probably the rules aren't
     going to change much, but it is a weakness.

  4. Commit messages can come from other places than "git commit". What
     should we do with a commit message like this that is imported from
     SVN? Reject the import? Munge the message?

Of course all of that presupposes that we can correctly handle the
existing data after the fact. Even with my patch, you still can't write
"From: foo@example.com" as the first line of your commit body. But that
is, IMHO, getting even more unlikely than your "From:" (which already
seems fairly unlikely).

I also think "git commit" would not be the right time for such a
feature. The problem is not that you have this text in your commit
message. The problem is that the "format-patch | am" transport is lossy.
You would do better to have format-patch say "Ah, this is going to
create a bogus email address" and somehow quote it appropriately.

-Peff

^ permalink raw reply

* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Jakub Narebski @ 2009-11-18 17:46 UTC (permalink / raw)
  To: Jeff King; +Cc: Philip Hofstetter, git
In-Reply-To: <20091118172424.GA24416@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I also think "git commit" would not be the right time for such a
> feature. The problem is not that you have this text in your commit
> message. The problem is that the "format-patch | am" transport is lossy.
> You would do better to have format-patch say "Ah, this is going to
> create a bogus email address" and somehow quote it appropriately.

Doesn't mbox format have some way of escaping "From:" (or is it "From ")?
If I remember correctly it uses ">From " or something for that.
git-format-patch could do this also (perhaps only with --rebasing
option).

P.S. As git-format-patch / git-am have hidden --rebasing option,
perhaps git-mailinfo should have it as well (even if it is called
--strict).

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Jeff King @ 2009-11-18 18:42 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Philip Hofstetter, git
In-Reply-To: <m3bpizd8ua.fsf@localhost.localdomain>

On Wed, Nov 18, 2009 at 09:46:43AM -0800, Jakub Narebski wrote:

> > I also think "git commit" would not be the right time for such a
> > feature. The problem is not that you have this text in your commit
> > message. The problem is that the "format-patch | am" transport is lossy.
> > You would do better to have format-patch say "Ah, this is going to
> > create a bogus email address" and somehow quote it appropriately.
> 
> Doesn't mbox format have some way of escaping "From:" (or is it "From ")?
> If I remember correctly it uses ">From " or something for that.
> git-format-patch could do this also (perhaps only with --rebasing
> option).

It's for "From " lines, which are the mbox separator. This would be
somewhat different. It has nothing at all to do with the mail format
itself, but rather is about git treating the body of the message
specially. I don't think a quoting mechanism currently exists to handle
this.

-Peff

^ permalink raw reply

* RE: Hey - A Conceptual Simplication....
From: George Dennie @ 2009-11-18 18:51 UTC (permalink / raw)
  To: 'Jan Krüger'; +Cc: git
In-Reply-To: <20091118142512.1313744e@perceptron>

Thanks Jan, Jason, Jonathan, and Thomas for your response, your thoughts and
concerns are enlightening....

Jan Kruger wrote...
> git config --global alias.commitx '!git add . && git commit'
> git config --global alias.checkoutx '!git clean && git checkout'

Thank you. Being new to git, I did not know that such aliasing was available
within it.

Jason Sewell wrote...
> If you have a bunch of debugging code sitting around in your working tree
after you've tracked down a 
> problem, you don't want to commit all of those printfs, etc. - you want to
commit the fix. This has 
> ramifications from making diffs of history cleaner to making git bisect
actually useful.

One of the concerns I have with the manual pick-n-commit is that you can
forget a file or two. Consequently, unless you do a clean checkout and test
of the commit, you don't know that your publishable version even compiles.
It seems safer to commit the entirety of your work in its working state and
then do a clean checkout from a dedicated publishable branch and manually
merge the changes in that, test, and commit.

It seems the intuitive model is to treat version control as applying to the
whole document, not parts of it. In this respect the document is defined by
the IDE, namely the entire solution, warts and all. When you start
selectively saving parts of the document then you are doing two things,
versioning and publishing; and at the same time. This was a critical flaw in
older version control approaches because the software solution document is a
file system sub-tree.

What you termed the debugging/printf's I would treat as a distinctions
between a debug vs. a release version that may be suitably delineated by
#define's or preferably separate unit tests assemblies. If I must prune
prior to committing; however, then it seems reverting spurious printf's may
offer a more reliable and automatable technique than ensuring that I have
added all the new class files, resource files, text files, sub projects,
etc; that may constitute the "fix." Once so selectively reverted I can test
and commit such a publishable version.

Jason Sewell wrote...
>  Isn't fastidiously maintaining a .gitignore file to contain everything
you *don't* want in the project more confusing 
> than explicitly specifying things you *do* want in the project?

This is git ignore for "cleaning prior to a check" and git ignore for
"adding to index" and is not an either or. You would specify what you don't
want to version tracked as normal but you can also stipulate what you don't
want to be deleted during a clean restore (which should otherwise completely
wipe the folder prior to restoring a specific commit). This would permit
embedding non-version elements within the version tree for whatever reason
you find necessary.

Thomas Rast wrote...
> That would require supernaturally good maintenance of your .gitignore to
avoid adding or (worse) nuking files by accident.

On the contrary, the approach would all but eliminate the possibility of
loss of data since you would not manually (and therefore error prone-ingly)
pruning until after a commit. In fact, one might default automatic commits
(if required) prior to checkouts or at least an alert system when
uncommitted changes exists.

Thanks again for your input.

^ permalink raw reply

* Re: Hey - A Conceptual Simplication....
From: Jakub Narebski @ 2009-11-18 19:40 UTC (permalink / raw)
  To: George Dennie; +Cc: 'Jan Krüger', git
In-Reply-To: <008401ca6880$33d7e550$9b87aff0$@com>

"George Dennie" <gdennie@pospeople.com> writes:

> Thanks Jan, Jason, Jonathan, and Thomas for your response, your thoughts and
> concerns are enlightening....
 
> Jason Sewell wrote...
>
> > If you have a bunch of debugging code sitting around in your working tree
> > after you've tracked down a problem, you don't want to commit all
> > of those printfs, etc. - you want to commit the fix. This has
> > ramifications from making diffs of history cleaner to making git
> > bisect actually useful.
> 
> One of the concerns I have with the manual pick-n-commit is that you can
> forget a file or two.

I don't think that this concern is valid.  

The files which make project are those defined in Makefile or
equivalent project file, _not_ all files (or even all files of
specific type / extension) that do happen to reside in given
directory.  And those files whould be known to git, either added when
importing project into git, or added when they were created.  And if
they are known it is enough to use "git commit -a" to pick all
changes.

So I don't see how you can 'forget a file or two'.

Are those *theoretical* concerns, or is it something that happened to
you doring using git?

> Consequently, unless you do a clean checkout and test
> of the commit, you don't know that your publishable version even compiles.
> It seems safer to commit the entirety of your work in its working state and
> then do a clean checkout from a dedicated publishable branch and manually
> merge the changes in that, test, and commit.

That's what

  git stash --keep-index

is for.  

That, and continuous integration repository, with it's hooks.

> 
> It seems the intuitive model is to treat version control as applying to the
> whole document, not parts of it. In this respect the document is defined by
> the IDE, namely the entire solution, warts and all.

Yes, and IDE has project file which defines which files are in
project, just like version control system has it's tracked files.

> When you start
> selectively saving parts of the document then you are doing two things,
> versioning and publishing; and at the same time. This was a critical flaw in
> older version control approaches because the software solution document is a
> file system sub-tree.

Atomic commits are important, but the distinction between tracked
files, (untracked) ignored files, and files in "limbo" state (neither
tracked nor ignored) is orthogonal to having atomic commits.

> Jason Sewell wrote...
>
> >  Isn't fastidiously maintaining a .gitignore file to contain
> > everything you *don't* want in the project more confusing than
> > explicitly specifying things you *do* want in the project?  
> 
> This is git ignore for "cleaning prior to a check" and git ignore for
> "adding to index" and is not an either or. You would specify what you don't
> want to version tracked as normal but you can also stipulate what you don't
> want to be deleted during a clean restore (which should otherwise completely
> wipe the folder prior to restoring a specific commit). This would permit
> embedding non-version elements within the version tree for whatever reason
> you find necessary.

And this is supposedly easier to use?  I don't think so.

> Thomas Rast wrote...
>
> > That would require supernaturally good maintenance of your
> > .gitignore to avoid adding or (worse) nuking files by accident.
> 
> On the contrary, the approach would all but eliminate the possibility of
> loss of data since you would not manually (and therefore error prone-ingly)
> pruning until after a commit. In fact, one might default automatic commits
> (if required) prior to checkouts or at least an alert system when
> uncommitted changes exists.

What?  I cannot understand you here.

I think that automatic pruning of non-versioned files is _more_ error
prone than manual deleting of files.  And much more error prone that
just keeping non-ignored and non-tracked files.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Hey - A Conceptual Simplication....
From: Jason Sewall @ 2009-11-18 19:52 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: George Dennie, Jan Krüger, git
In-Reply-To: <m37htnd3kb.fsf@localhost.localdomain>

Sorry for the 2x post, George; forgot to include the list in my reply....

On Wed, Nov 18, 2009 at 1:51 PM, George Dennie <gdennie@pospeople.com> wrote:
[some cleanup of quote line wrapping]
> Jason Sewall wrote...
>> If you have a bunch of debugging code sitting around in your
>> working tree after you've tracked down a problem, you don't want to
>> commit all of those printfs, etc. - you want to commit the
>> fix. This has ramifications from making diffs of history cleaner to
>> making git bisect actually useful.

> One of the concerns I have with the manual pick-n-commit is that you
> can forget a file or two. Consequently, unless you do a clean
> checkout and test of the commit, you don't know that your
> publishable version even compiles.  It seems safer to commit the
> entirety of your work in its working state and then do a clean
> checkout from a dedicated publishable branch and manually merge the
> changes in that, test, and commit.

I find git status very useful in preparing a commit; untracked (and
'un-ignored') files are listed right there and I can if there are new
source files that are not present but not tracked.  You could even add
a 'pre-commit hook' to make sure that you don't have any untracked *.c
(or whatever) files before you actually make the commit.

As to 'publishable' version, it's probably a good idea to run 'make
distcheck' or the equivalent before making a release anyway.

> It seems the intuitive model is to treat version control as applying
> to the whole document, not parts of it. In this respect the document
> is defined by the IDE, namely the entire solution, warts and
> all. When you start selectively saving parts of the document then
> you are doing two things, versioning and publishing; and at the same
> time. This was a critical flaw in older version control approaches
> because the software solution document is a file system sub-tree.

I find this leads to big, shapeless commits and, as I mentioned
before, it seriously limits the utility of 'git bisect'.  I also fail
to see how 'selectively saving parts of the document' is versioning
and publishing - what is the publishing part?  The act of committing
is one thing (and 'saving parts of the document' is one conceivable
name for it) and publishing another.  Your workflow may vary, but
before actually 'publishing' (perhaps pushing out to a public repo, or
merging into a public branch), it's probably a good idea to test the
code with whatever system you use anyway.

> What you termed the debugging/printf's I would treat as a
> distinctions between a debug vs. a release version that may be
> suitably delineated by #define's or preferably separate unit tests
> assemblies. If I must prune prior to committing; however, then it
> seems reverting spurious printf's may offer a more reliable and
> automatable technique than ensuring that I have added all the new
> class files, resource files, text files, sub projects, etc; that may
> constitute the "fix." Once so selectively reverted I can test and
> commit such a publishable version.

What if you are hacking away and make changes to several parts of the
code at once?  Making the commits as fine-grained as possible makes it
easier to cherry-pick, bisect, and understand the history.

As to debugging code, I admit I sometimes will use git gui or git add
-p to stage just what I want and then put whatever is 'left over' in a
branch that I might use again later if another bug comes up.  Then I
can reset --hard my 'working' branch and the debugging code is gone.

> Jason Sewell wrote...
>>  Isn't fastidiously maintaining a .gitignore file to contain
>> everything you *don't* want in the project more confusing than
>> explicitly specifying things you *do* want in the project?
>
> This is git ignore for "cleaning prior to a check" and git ignore
> for "adding to index" and is not an either or. You would specify
> what you don't want to version tracked as normal but you can also
> stipulate what you don't want to be deleted during a clean restore
> (which should otherwise completely wipe the folder prior to
> restoring a specific commit). This would permit embedding
> non-version elements within the version tree for whatever reason you
> find necessary.

Perhaps I don't understand your scheme, but it sounds like you're
advocating 2 .gitignores:

* .gitignore_track; with everything you don't automatically staged but
 which can be trashed by your cleaning checkout
* .gitignore_keep; with things you don't want staged but which
  shouldn't be deleted by git during cleaning

That seems even more confusing.  I'm actually having trouble seeing
why you want this untracked-file nuking checkout at all.  Care to give
an example?

> Thomas Rast wrote...
>> That would require supernaturally good maintenance of your
>> .gitignore to
> avoid adding or (worse) nuking files by accident.
>
> On the contrary, the approach would all but eliminate the
> possibility of loss of data since you would not manually (and
> therefore error prone-ingly) pruning until after a commit. In fact,
> one might default automatic commits (if required) prior to checkouts
> or at least an alert system when uncommitted changes exists.

Who is pruning after a commit?  Once nice thing about checkout is that
it will refuse to move to a different commit if there are files that
will get trashed.  Then you can say 'oops, I should stash/commit/nuke
that stuff before I change HEAD.

Jason

^ permalink raw reply

* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Philip Hofstetter @ 2009-11-18 19:57 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20091118172424.GA24416@coredump.intra.peff.net>

Hello,

On Wed, Nov 18, 2009 at 6:24 PM, Jeff King <peff@peff.net> wrote:

>  1. It creates a bad user experience. You are not unreasonable for
>     wanting to put some specific text in your commit message. Having
>     git come back and say "oops, I might get confused by this later"
>     just seems like an annoyance to the user.

agreed, though it's not that bad: when learning git, you will be
confronted with the fact that the commit message has a few things that
are special (well. it's doesn't break git, but the first line should
be < 56 chars in length for example).

Not being able to have From: lines in them that are not describing an
author would then just be one of them.

>  2. Mailinfo has to deal with data created by older versions of git. So
>     in your case, the rebase was a bomb waiting to go off. If we can
>     fix it so that an existing bomb doesn't go off, rather than not
>     creating the bomb in the first place, then we are better off.

This is a very good point. I didn't quite think about that.

>  4. Commit messages can come from other places than "git commit". What
>     should we do with a commit message like this that is imported from
>     SVN? Reject the import? Munge the message?

I would leave that to the tool that does the import. Probably it would
have to munge it. Yes.

I DO see though that implementing the check at commit time would lead
to problems popping up at other places.

> Of course all of that presupposes that we can correctly handle the
> existing data after the fact. Even with my patch, you still can't write
> "From: foo@example.com" as the first line of your commit body. But that

can't you? IMHO it would just attribute the commit to foo@example.com
which can be an equally bad, if not worse thing (I'm saying that
without the needed knowledge about git internals to really be sure, so
take this with a grain of salt)

I just have a bad feeling about trying out heuristics to see whether
thing thing after from: is an email address or not as email addresses
are notoriously hard to detect.

Typing a commit message and applying a patch from an email should be
separate things and should be handled separately. Currently they are
not and this is what's causing the problem in the first place.

Maybe that --strict thing is actually a good thing in the long run,
even though I don't quite like it either :-)

Interesting problem to have though.

Philip

^ permalink raw reply

* Re: Hey - A Conceptual Simplication....
From: Linus Torvalds @ 2009-11-18 20:36 UTC (permalink / raw)
  To: George Dennie; +Cc: git
In-Reply-To: <005a01ca684e$71a1d710$54e58530$@com>



On Wed, 18 Nov 2009, George Dennie wrote:
> 
> The Git model does not seem to go far enough conceptually, for some
> unexplainable reason...

Others already mentioned this, but the concept you missed is the git 
'index', which is actually very central (it is actually the first part of 
git written, before even the object database) but is something that most 
people who get started with git can (and do) ignore.

Now, admittedly, for casual use it's not always clear _why_ the index is 
so central, so the fact that you overlooked it is certainly easy to 
understand. Just take my word for it: to truly understand git, you do need 
to understand the index.

You can ignore it for a long time, because one of the primary reasons for 
it existing is about performance. That happens to be a primary goal of 
git, of course, but some people always think it's "just performance". It's 
way more fundamental than that.

So the way you can start getting used to the index is to think of it as a 
way to avoid having to do a full 'readdir()' on the whole tree to figure 
out what is in there, and avoiding having to read all the files to check 
that their contents still match.

Of course, if that was _all_ the index did, it could be seen purely as a 
cache, and have no semantic visibility at all. And that's not the case: 
the index does have real semantic visibility.

The first time you'll see it is when you decide to stage your changes in 
parts. The index is what allows you to _not_ always commit all your 
changes exactly because git keeps track of something more than _just_ your 
whole current working tree.

A special case (but a really useful one) of the "staging your changes in 
parts" is when you do merges. Now, most people don't do merges like I do 
(what, average of 5 merges per day, day in and day out), so most people 
don't care quite as deeply as I do, but if you ever do a merge where 99% 
merged cleanly, and 1% did not (which is the common case for conflicts), 
you'll really understand why having a system that keeps track of the parts 
that merged cleanly is _critical_. 

So for merges, the index keeps track of what merged cleanly, and what 
didn't, and what the original state for the not-clean stuff was. And as 
somebody who probably does more merges than likely any other human in the 
history of the world, I can state with some authority that any source 
control model that doesn't have this is fundamentally broken.

So the index is really _really_ important. Even if you can ignore it most 
of the time. And the index is why you don't have a model of "always just 
track the exact tree state".

			Linus

^ permalink raw reply

* Re: th/remote-usage
From: Tim Henigan @ 2009-11-18 21:28 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, git
In-Reply-To: <20091118114808.GA13346@progeny.tock>

Hi and thanks for your review.

On Wed, Nov 18, 2009 at 6:48 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Junio C Hamano wrote:
>> [New Topics]
>>
>> * th/remote-usage (2009-11-16) 1 commit.
>>  - git remote: Separate usage strings for subcommands
>
> Glancing at pu^2, I had two small nitpicks: [<options>...] is five
> characters longer than strictly necessary

I based my patch on what I found in other builtin functions (such
as push and diff).  That being said, I don't think that either my
original patch or your updated version is completely correct.
The choices seem to be:
  (1) [<options>...]:  My original based on my interpretation of
      IEEE 1003.1. [1]
  (2) [options]: Your proposal, which drops both the '<>' and '...'.
  (3) <options>:  Used in builtin-diff.c.  Which does not show
      that the options are -- optional.
  (4) [<options>]: What I now believe is correct (based on the
      current implementation of builtin-push.c).  This drops the
      '...' which IEEE 1003.1 defines as allowing multiple options
      to be specified, but it conforms to the conventions in other
      commands.

There does not (yet) seem to be consistency in how options
are presented.  My current plan is to change the patch to
use choice #4, but if Junio has a chance to comment, I will
of course defer to his decision.

I will send an updated patch that implements choice #4 as
soon as I can (should be within the next 12 hours).


> and the argument to git
> remote set-head is not actually optional.

This was obviously an oversight on my part.  I will include the
fix in the next version.


...and from your second email:
> Another option would be to make the strings into static
> variables.

Thanks for the analysis, but I don't plan to include this change
unless specifically requested.


[1]: http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap12.html

^ permalink raw reply

* Re: [PATCH] Give the hunk comment its own color
From: Junio C Hamano @ 2009-11-18 21:56 UTC (permalink / raw)
  To: Jeff King; +Cc: Bert Wesarg, git
In-Reply-To: <20091118142320.GA1220@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> PS I almost complained about your default of "magenta" as the same as
> the meta color before I remembered that magenta meta is a personal
> setting I use.

On black-on-white terminals, cyan tends to be less visible, and I think
that is the whole point of painting the hunk header @@ .. @@ in that
color--- make it less distracting). 

But the function name on the line is not something that should be made
less visible---if that part of the line were a meaningless cruft, we
wouldn't have configurable funcname patterns after all.

I would suggest "normal" as the neutral default.  After all, the purpose
of the funcname in the hunk header is to give context to people who read
patches.

> I'm not sure what is the best way to arrive at a default color for
> something like this. Arguing about it really is almost the definition of
> bikeshedding.  Maybe next year's git survey should contain a special
> section on colors, and majority should rule.  :)

Sorry, but this is no democracy ;-)

^ permalink raw reply

* Re: [PATCH] unset GREP_OPTIONS in test-lib.sh
From: Junio C Hamano @ 2009-11-18 22:05 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: git
In-Reply-To: <1258560919-28054-1-git-send-email-bert.wesarg@googlemail.com>

Bert Wesarg <bert.wesarg@googlemail.com> writes:

> I used to set GREP_OPTIONS to exclude *.orig and *.rej files. But with this
> the test t4252-am-options.sh fails because it calls grep with a .rej file:

Yuck.  Will apply.

That actually makes me worried about a different issue.

Do we kill that environment variable when we call out to external grep in
grep.c?  If not, we should.  An alternative is to teach our internal one
to also honor it, but I personally do not find it too attractive to mimic
the design mistake of GREP_OPTIONS myself.

^ permalink raw reply

* Re: [PATCH 2/2] ls-tree: migrate to parse-options
From: Junio C Hamano @ 2009-11-18 22:19 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: git
In-Reply-To: <4B024A89.2010108@gmail.com>

Stephen Boyd <bebarino@gmail.com> writes:

> Junio C Hamano wrote:
>>
>> @@ -24,7 +24,7 @@ static int chomp_prefix;
>>  static const char *ls_tree_prefix;
>>   static const  char * const ls_tree_usage[] = {
>> -	"git ls-tree [-d] [-r] [-t] [-l | --long] [-z] [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]] <tree-ish> [path...]",
>> +	"git ls-tree <options> <tree-ish> [path...]",
>>  	NULL
>>  };
>>  
>
> Is it [<options>] or [<options>...] or <options> or even
> <options>... though?

Output from "git grep -e '<option' -- '*.c'" indicates that it
should be "[<options>]"; thanks for spotting.

^ permalink raw reply

* Re: [PATCH] Give the hunk comment its own color
From: Jeff King @ 2009-11-18 22:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Bert Wesarg, git
In-Reply-To: <7vaayjebu5.fsf@alter.siamese.dyndns.org>

On Wed, Nov 18, 2009 at 01:56:34PM -0800, Junio C Hamano wrote:

> On black-on-white terminals, cyan tends to be less visible, and I think
> that is the whole point of painting the hunk header @@ .. @@ in that
> color--- make it less distracting).

Hmm. I find cyan-on-white gratingly ugly instead of "less distracting",
but then again, I find black-on-white terminals to be eye-searing in
general.

> I would suggest "normal" as the neutral default.  After all, the purpose
> of the funcname in the hunk header is to give context to people who read
> patches.

I think that is sensible.

-Peff

^ permalink raw reply

* [PATCH] git am/mailinfo: Don't look at in-body headers when rebasing
From: Lukas Sandström @ 2009-11-18 22:45 UTC (permalink / raw)
  To: Jeff King; +Cc: Lukas Sandström, Philip Hofstetter, git
In-Reply-To: <20091118164208.GB15184@coredump.intra.peff.net>

When we are rebasing we know that the header lines in the
patch are good and that we don't need to pick up any headers
from the body of the patch.

This makes it possible to rebase commits whose commit message
start with "From" or "Date".

Test vectors by Jeff King.

Signed-off-by: Lukas Sandström <luksan@gmail.com>
---

Jeff King wrote:
>> Some solutions I can think of are:
>>
>>   1. Improve the header-finding heuristic to actually look for something
>>      more sane, like "From:.*<.*@.*>" (I don't recall off the top of my
>>      head which other headers we handle in this position. Probably
>>      Date, too).
>>
>>   2. Give mailinfo a "--strict" mode to indicate that it is directly
>>      parsing the output of format-patch, and not some random email. Use
>>      --strict when invoking "git am" via "git rebase".
> 
> Solution (2) seemed like a lot of work, so here is the relatively small
> solution (1). I think looking for <.*@.*> is too restrictive, as people
> may be using:
> 

This is an implementation of solution (2). Not much work, but I might
have missed something. git-mailinfo usally breaks when I touch it,
but the testsuite passes with this patch, including the extra test
vectors from Jeff.

The actual change is that mailinfo doesn't look for in-body headers
at all if --no-inbody-headers is passed. git-am now passes this option
to mailinfo when rebasing.

This won't handle the case when a "bad" patch is passed to git-am from
somewhere else than git rebase.

/Lukas


 builtin-mailinfo.c                   |    5 +++++
 git-am.sh                            |   13 ++++++++++---
 t/t5100-mailinfo.sh                  |    6 +++++-
 t/t5100/info0015                     |    5 +++++
 t/t5100/info0015--no-inbody-headers  |    5 +++++
 t/t5100/info0016                     |    5 +++++
 t/t5100/info0016--no-inbody-headers  |    5 +++++
 t/t5100/msg0015                      |    2 ++
 t/t5100/msg0015--no-inbody-headers   |    3 +++
 t/t5100/msg0016                      |    2 ++
 t/t5100/msg0016--no-inbody-headers   |    4 ++++
 t/t5100/patch0015                    |    8 ++++++++
 t/t5100/patch0015--no-inbody-headers |    8 ++++++++
 t/t5100/patch0016                    |    8 ++++++++
 t/t5100/patch0016--no-inbody-headers |    8 ++++++++
 t/t5100/sample.mbox                  |   33
+++++++++++++++++++++++++++++++++
 16 files changed, 116 insertions(+), 4 deletions(-)
 create mode 100644 t/t5100/info0015
 create mode 100644 t/t5100/info0015--no-inbody-headers
 create mode 100644 t/t5100/info0016
 create mode 100644 t/t5100/info0016--no-inbody-headers
 create mode 100644 t/t5100/msg0015
 create mode 100644 t/t5100/msg0015--no-inbody-headers
 create mode 100644 t/t5100/msg0016
 create mode 100644 t/t5100/msg0016--no-inbody-headers
 create mode 100644 t/t5100/patch0015
 create mode 100644 t/t5100/patch0015--no-inbody-headers
 create mode 100644 t/t5100/patch0016
 create mode 100644 t/t5100/patch0016--no-inbody-headers

diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index c90cd31..a81526e 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -26,6 +26,7 @@ static struct strbuf charset = STRBUF_INIT;
 static int patch_lines;
 static struct strbuf **p_hdr_data, **s_hdr_data;
 static int use_scissors;
+static int use_inbody_headers = 1;

 #define MAX_HDR_PARSED 10
 #define MAX_BOUNDARIES 5
@@ -771,6 +772,8 @@ static int handle_commit_msg(struct strbuf *line)
 		return 0;

 	if (still_looking) {
+		if (!use_inbody_headers)
+			still_looking = 0;
 		strbuf_ltrim(line);
 		if (!line->len)
 			return 0;
@@ -1033,6 +1036,8 @@ int cmd_mailinfo(int argc, const char **argv,
const char *prefix)
 			use_scissors = 1;
 		else if (!strcmp(argv[1], "--no-scissors"))
 			use_scissors = 0;
+		else if (!strcmp(argv[1], "--no-inbody-headers"))
+			use_inbody_headers = 0;
 		else
 			usage(mailinfo_usage);
 		argc--; argv++;
diff --git a/git-am.sh b/git-am.sh
index c132f50..96869a2 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -289,7 +289,7 @@ split_patches () {
 prec=4
 dotest="$GIT_DIR/rebase-apply"
 sign= utf8=t keep= skip= interactive= resolved= rebasing= abort=
-resolvemsg= resume= scissors=
+resolvemsg= resume= scissors= no_inbody_headers=
 git_apply_opt=
 committer_date_is_author_date=
 ignore_date=
@@ -322,7 +322,7 @@ do
 	--abort)
 		abort=t ;;
 	--rebasing)
-		rebasing=t threeway=t keep=t scissors=f ;;
+		rebasing=t threeway=t keep=t scissors=f no_inbody_headers=t ;;
 	-d|--dotest)
 		die "-d option is no longer supported.  Do not use."
 		;;
@@ -448,6 +448,7 @@ else
 	echo "$utf8" >"$dotest/utf8"
 	echo "$keep" >"$dotest/keep"
 	echo "$scissors" >"$dotest/scissors"
+	echo "$no_inbody_headers" >"$dotest/no_inbody_headers"
 	echo "$GIT_QUIET" >"$dotest/quiet"
 	echo 1 >"$dotest/next"
 	if test -n "$rebasing"
@@ -495,6 +496,12 @@ t)
 f)
 	scissors=--no-scissors ;;
 esac
+if test "$(cat "$dotest/no_inbody_headers")" = t
+then
+	no_inbody_headers=--no-inbody-headers
+else
+	no_inbody_headers=
+fi
 if test "$(cat "$dotest/quiet")" = t
 then
 	GIT_QUIET=t
@@ -549,7 +556,7 @@ do
 	# by the user, or the user can tell us to do so by --resolved flag.
 	case "$resume" in
 	'')
-		git mailinfo $keep $scissors $utf8 "$dotest/msg" "$dotest/patch" \
+		git mailinfo $keep $no_inbody_headers $scissors $utf8 "$dotest/msg" "$dotest/patch" \
 			<"$dotest/$msgnum" >"$dotest/info" ||
 			stop_here $this

diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index 0279d07..50e13c1 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -11,7 +11,7 @@ test_expect_success 'split sample box' \
 	'git mailsplit -o. "$TEST_DIRECTORY"/t5100/sample.mbox >last &&
 	last=`cat last` &&
 	echo total is $last &&
-	test `cat last` = 14'
+	test `cat last` = 16'

 check_mailinfo () {
 	mail=$1 opt=$2
@@ -30,6 +30,10 @@ do
 		if test -f "$TEST_DIRECTORY"/t5100/msg$mail--scissors
 		then
 			check_mailinfo $mail --scissors
+		fi &&
+		if test -f "$TEST_DIRECTORY"/t5100/msg$mail--use-first-header
+		then
+			check_mailinfo $mail --no-inbody-headers
 		fi
 	'
 done
diff --git a/t/t5100/info0015 b/t/t5100/info0015
new file mode 100644
index 0000000..0114f10
--- /dev/null
+++ b/t/t5100/info0015
@@ -0,0 +1,5 @@
+Author:
+Email:
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/info0015--no-inbody-headers
b/t/t5100/info0015--no-inbody-headers
new file mode 100644
index 0000000..c4d8d77
--- /dev/null
+++ b/t/t5100/info0015--no-inbody-headers
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@example.com
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/info0016 b/t/t5100/info0016
new file mode 100644
index 0000000..38ccd0d
--- /dev/null
+++ b/t/t5100/info0016
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@example.com
+Subject: check bogus body header (date)
+Date: bogus
+
diff --git a/t/t5100/info0016--no-inbody-headers
b/t/t5100/info0016--no-inbody-headers
new file mode 100644
index 0000000..f4857d4
--- /dev/null
+++ b/t/t5100/info0016--no-inbody-headers
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@example.com
+Subject: check bogus body header (date)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/msg0015 b/t/t5100/msg0015
new file mode 100644
index 0000000..9577238
--- /dev/null
+++ b/t/t5100/msg0015
@@ -0,0 +1,2 @@
+- a list
+  - of stuff
diff --git a/t/t5100/msg0015--no-inbody-headers
b/t/t5100/msg0015--no-inbody-headers
new file mode 100644
index 0000000..be5115b
--- /dev/null
+++ b/t/t5100/msg0015--no-inbody-headers
@@ -0,0 +1,3 @@
+From: bogosity
+  - a list
+  - of stuff
diff --git a/t/t5100/msg0016 b/t/t5100/msg0016
new file mode 100644
index 0000000..0d9adad
--- /dev/null
+++ b/t/t5100/msg0016
@@ -0,0 +1,2 @@
+and some content
+
diff --git a/t/t5100/msg0016--no-inbody-headers
b/t/t5100/msg0016--no-inbody-headers
new file mode 100644
index 0000000..1063f51
--- /dev/null
+++ b/t/t5100/msg0016--no-inbody-headers
@@ -0,0 +1,4 @@
+Date: bogus
+
+and some content
+
diff --git a/t/t5100/patch0015 b/t/t5100/patch0015
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0015
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/patch0015--no-inbody-headers
b/t/t5100/patch0015--no-inbody-headers
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0015--no-inbody-headers
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/patch0016 b/t/t5100/patch0016
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0016
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/patch0016--no-inbody-headers
b/t/t5100/patch0016--no-inbody-headers
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0016--no-inbody-headers
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox
index 13fa4ae..de10312 100644
--- a/t/t5100/sample.mbox
+++ b/t/t5100/sample.mbox
@@ -650,3 +650,36 @@ index b0b5d8f..461c47e 100644
  		convert_to_utf8(line, charset.buf);
 --
 1.6.4.1
+From nobody Mon Sep 17 00:00:00 2001
+From: A U Thor <a.u.thor@example.com>
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
+From: bogosity
+  - a list
+  - of stuff
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
+From nobody Mon Sep 17 00:00:00 2001
+From: A U Thor <a.u.thor@example.com>
+Subject: check bogus body header (date)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
+Date: bogus
+
+and some content
+
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
-- 
1.6.4.4

^ permalink raw reply related

* Re: [PATCH 2/2] ls-tree: migrate to parse-options
From: Thiago Farina @ 2009-11-18 23:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Stephen Boyd, git
In-Reply-To: <7veinvcw7w.fsf@alter.siamese.dyndns.org>

On Wed, Nov 18, 2009 at 8:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stephen Boyd <bebarino@gmail.com> writes:
>
>> Junio C Hamano wrote:
>>>
>>> @@ -24,7 +24,7 @@ static int chomp_prefix;
>>>  static const char *ls_tree_prefix;
>>>   static const  char * const ls_tree_usage[] = {
>>> -    "git ls-tree [-d] [-r] [-t] [-l | --long] [-z] [--name-only] [--name-status] [--full-name] [--full-tree] [--abbrev[=<n>]] <tree-ish> [path...]",
>>> +    "git ls-tree <options> <tree-ish> [path...]",
>>>      NULL
>>>  };
>>>
>>
>> Is it [<options>] or [<options>...] or <options> or even
>> <options>... though?
>
> Output from "git grep -e '<option' -- '*.c'" indicates that it
> should be "[<options>]"; thanks for spotting.

$ git grep -e '<option' -- '*.c' | wc -l
4

$ git grep -e '\[options' -- '*.c' | wc -l
42

$ git grep -e '\[<options' -- '*.c' | wc -l
2

Shouldn't be just "[options]"?

^ permalink raw reply

* Re: [PATCH] git am/mailinfo: Don't look at in-body headers when  rebasing
From: Philip Hofstetter @ 2009-11-18 23:47 UTC (permalink / raw)
  To: Lukas Sandström; +Cc: Jeff King, git
In-Reply-To: <4B0478ED.30306@gmail.com>

Hi,

On Wed, Nov 18, 2009 at 11:45 PM, Lukas Sandström <luksan@gmail.com> wrote:

> The actual change is that mailinfo doesn't look for in-body headers
> at all if --no-inbody-headers is passed. git-am now passes this option
> to mailinfo when rebasing.

after all the earlier discussion and a lot of thinking, I have to say,
that IMHO, this is the best option as it doesn't rely on heuristics
and now that you chose a descriptive command line switch, even the
small problem of "why exactly is this switch here?" seems to go away.

As I have no experience in git's codebase at all, I'll leave the
commenting on the patch itself to the people with clue, but
conceptionally, this feels much better than the method 1

> This won't handle the case when a "bad" patch is passed to git-am from
> somewhere else than git rebase.

of course, that leaves the question what "somewhere else" can contain.
If it's just manual calls to git-am, this is a non-issue as it's
easily fixed by the caller. If it's being called from other
higher-level operations though, you might run into the same issue
again.

Here too, I can't really provide any meaningful input though as I just
don't know well enough what really makes git tick.

Just my two cents :-)

Philip

^ permalink raw reply

* RE: Hey - A Conceptual Simplication....
From: George Dennie @ 2009-11-19  2:03 UTC (permalink / raw)
  To: 'Jason Sewall', 'Jakub Narebski'
  Cc: 'Jan Krüger', git
In-Reply-To: <31e9dd080911181152h665d5d9dr5c0736c0ca3234c1@mail.gmail.com>

Thanks Linus, Jason, and Jakub...

Linus Torvalds wrote....
>On Wed, 18 Nov 2009, George Dennie wrote:
>> 
>> The Git model does not seem to go far enough conceptually, for some 
>> unexplainable reason...
>
> Others already mentioned this, but the concept you missed is the git 'index', which is actually very 
> central (it is actually the first part of git written, before even the object database) but is something 
> that most people who get started with git can (and do) ignore.

Uhmmm, subtle. I hear you. Thanks for the heads up. But before that, I just put these two cents down...

One of the persistent problems with software documentation is that it often fails to define the "functional or usage" model, apart from a dry list of commands. I am sure there are many good reasons for this. For one thing, explaining stuff is hard. Now, I have not had occasions to do merges, as such. So I am finding the justification for the index vague. I am wondering whether this might be a great space to describe the functional model of git in a way that more clearly justifies the index...

Specifically, can there be a succinct description of the usage or functional model of Git that necessarily incorporates the index. 

For example, the functional notion of the repository seems well defined: a growing web of immutable commits each created as either an isolated commit or more typically an update and/or merger of one or more pre-existing commits. 

With such a description the rest of the structure becomes almost implicit: Commits may be annotated such as with release number labels. Commits that have not been linked to such as by an update or merger remain dangling like loose threads in the web and are called branches. Branches may be given special labels that the repository will then automatically update so as to refer to the latest commit to that branch.

I don't yet have such a clear model for the index. Yes it is a staging platform, but so is the IDE....I'll do more reading.

Jason Sewell wrote....
> I find this leads to big, shapeless commits and, as I mentioned before, it seriously limits the utility 
> of 'git bisect'.  I also fail to see how 'selectively saving parts of the document' is versioning and 
> publishing - what is the publishing part?  The act of committing is one thing (and 'saving...

The notion of a shapeless commit is curious. Intuitively, I consider a commit as capturing the state of my work at a transactional boundary (i.e. a successful unit test...or even lunch break). However, your characterization of "shape" suggest that you are constructing something other than the immediate functionality of the software. Consequently, your software document is not really the solution files alone but also this commit history that you meticulously craft. 

Further, the participating of the IDE is not to compose within itself the committable document but rather to contribute to such a document in pieces. In fact, the closest metaphor to this process/workflow seems to be submitting articles to a magazine; except you are both the writer and editor/graphic artist; and each edition of the magazine becoming the committable version. 

With this metaphor the index does play a clear role as a layout board of sorts for the complete magazine. And also clearly, the IDE does not "functionally" edit the entire committable document but rather parts of it. Even though it may effectively have the entirety of the index in its working tree; Git requires that it be submitted to the index which is the true committable document. 

It begs the question, why is the working tree (the IDE document) so closely tied to the repository since it really amounts to a scratch pad. In fact, while the index may be attach to the working tree, the repository can be anywhere and have more than one index attached...yeah, I know, having a personal dedicated repository is cheap. (A great example of how expediency, the proximity of the repository, might obscure the functional model by making what is arbitrary and due to convention appear a functional necessity...; if, in fact, my above conclusion is correct of course :)

> What if you are hacking away and make changes to several parts of the code at once?  Making the commits 
> as fine-grained as possible makes it easier to cherry-pick, bisect, and understand the history.

You know Jason, it is often hard to isolate my changes to specific files. I have come to appreciate unit tests as a means of delineating changes. However, clearly the historically record of your solution tree is of substantially value to you. It is something I will have to pay closer attention in my case.

> Perhaps I don't understand your scheme, but it sounds like you're advocating 2 .gitignores:
>
> * .gitignore_track; with everything you don't automatically staged but  which can be trashed by your cleaning checkout
> * .gitignore_keep; with things you don't want staged but which shouldn't be deleted by git during cleaning

Yep, that may be one implementation...but essentially the current .gitignores list exclusionary filters for the "git add ." command. The suggestion was to augment it to also include exclusionary filters for the proposed "git checkout -clean" command.  By perhaps prefixing "+" and "-" symbols to the listed elements you can designate each filter's participation in the "do not add" and "do not delete" activities, respectively. However, this suggest was with the presumption that the work tree was the committable document, but clearly it is not.

> Who is pruning after a commit?  Once nice thing about checkout is that it will refuse to move to a 
> different commit if there are files that will get trashed.  Then you can say 'oops, I should 
> stash/commit/nuke that stuff before I change HEAD.

Not trashing files is a nice thing by checkout. However, are you referring to changes added to the index or changes made in the working tree but not yet added to the index. Base on my current understanding of the functional model, you would be referring to the index since the working tree is little more than a scratch pad. The pruning comment was in recognition that the working tree was not expected to be committable in its entirety.

George.

Thanks again for your input and if you have the time I welcome your response.

^ permalink raw reply

* [PATCH v4] git remote: Separate usage strings for subcommands
From: Tim Henigan @ 2009-11-19  2:59 UTC (permalink / raw)
  To: Junio C Hamano, jrnieder; +Cc: git

When the usage string for a subcommand must be printed,
only print the information relevant to that command.

This commit also removes the options from the first line
of the usage string (replacing them with '<options>'
and lets them be documented in the paragraph below.

Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---

This patch is based on:
http://article.gmane.org/gmane.comp.version-control.git/132953

All usage strings are still only located at the top of file.  However,
separate usage string arrays have been created for each subcommand.

v2 fixed line wrap issues present in v1.

v3 changes include:
 - Changed usage strings to use '<options>...' rather than document
   the options both in the actual string and in the OPT_x array
   (as recommended by Junio).
 - Removed the change made to the usage string array constructed
   in 'cmd_remote'. v2 was broken because that change had made
   the command stop recognizing the '-v' option. Added an extra
   note here which explains that '-v' is only valid when placed
   after 'git remote' and before any 'subcommand'.
 - Updated the man page.

v4 changes include:
 - Changed usage strings to use '<options>' rather than
   '<options>...' based on feedback from Jonathan Nieder.
   See [1] for details.
 - Corrected "git remote set-head" usage string to show the
   required [-a | -d | <branch>] portion of the command.
 - Corrected "git remote update" usage string to show the
   optional, but not otherwise documented [<group> | <remote>]
   portion of the command.
 - Removed 2 more instances of "<subcommand> specific options".

[1] http://thread.gmane.org/gmane.comp.version-control.git/133151/focus=133160


 Documentation/git-remote.txt |   13 +++++----
 builtin-remote.c             |   60 +++++++++++++++++++++++++----------------
 2 files changed, 43 insertions(+), 30 deletions(-)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 82a3d29..ee3dc80 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -9,14 +9,14 @@ git-remote - manage set of tracked repositories
 SYNOPSIS
 --------
 [verse]
-'git remote' [-v | --verbose]
-'git remote add' [-t <branch>] [-m <master>] [-f] [--mirror] <name> <url>
+'git remote' [<options>]
+'git remote add' [<options>] <name> <url>
 'git remote rename' <old> <new>
 'git remote rm' <name>
-'git remote set-head' <name> [-a | -d | <branch>]
-'git remote show' [-n] <name>
-'git remote prune' [-n | --dry-run] <name>
-'git remote update' [-p | --prune] [group | remote]...
+'git remote set-head' <name> [<options>] [-a | -d | <branch>]
+'git remote show' [<options>] <name>
+'git remote prune' [<options>] <name>
+'git remote update' [<options>] [<group> | <remote>]...
 
 DESCRIPTION
 -----------
@@ -30,6 +30,7 @@ OPTIONS
 -v::
 --verbose::
 	Be a little more verbose and show remote url after name.
+	NOTE: This must be placed between `remote` and `subcommand`.
 
 
 COMMANDS
diff --git a/builtin-remote.c b/builtin-remote.c
index 0777dd7..24a3ec0 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -7,18 +7,35 @@
 #include "run-command.h"
 #include "refs.h"
 
+#define REMOTE_BARE_USAGE "git remote [<options>]"
+#define REMOTE_ADD_USAGE "git remote add [<options>] <name> <url>"
+#define REMOTE_RENAME_USAGE "git remote rename <old> <new>"
+#define REMOTE_RM_USAGE "git remote rm <name>"
+#define REMOTE_SETHEAD_USAGE "git remote set-head <name> [-a | -d | <branch>]"
+#define REMOTE_SHOW_USAGE "git remote show [<options>] <name>"
+#define REMOTE_PRUNE_USAGE "git remote prune [<options>] <name>"
+#define REMOTE_UPDATE_USAGE "git remote update [<options>] [<group> | <remote>]..."
+
 static const char * const builtin_remote_usage[] = {
-	"git remote [-v | --verbose]",
-	"git remote add [-t <branch>] [-m <master>] [-f] [--mirror] <name> <url>",
-	"git remote rename <old> <new>",
-	"git remote rm <name>",
-	"git remote set-head <name> [-a | -d | <branch>]",
-	"git remote show [-n] <name>",
-	"git remote prune [-n | --dry-run] <name>",
-	"git remote [-v | --verbose] update [-p | --prune] [group]",
+	REMOTE_BARE_USAGE,
+	REMOTE_ADD_USAGE,
+	REMOTE_RENAME_USAGE,
+	REMOTE_RM_USAGE,
+	REMOTE_SETHEAD_USAGE,
+	REMOTE_SHOW_USAGE,
+	REMOTE_PRUNE_USAGE,
+	REMOTE_UPDATE_USAGE,
 	NULL
 };
 
+static const char * const builtin_remote_add_usage[] = { REMOTE_ADD_USAGE, NULL };
+static const char * const builtin_remote_rename_usage[] = { REMOTE_RENAME_USAGE, NULL };
+static const char * const builtin_remote_rm_usage[] = { REMOTE_RM_USAGE, NULL };
+static const char * const builtin_remote_sethead_usage[] = { REMOTE_SETHEAD_USAGE, NULL };
+static const char * const builtin_remote_show_usage[] = { REMOTE_SHOW_USAGE, NULL };
+static const char * const builtin_remote_prune_usage[] = { REMOTE_PRUNE_USAGE, NULL };
+static const char * const builtin_remote_update_usage[] = { REMOTE_UPDATE_USAGE, NULL };
+
 #define GET_REF_STATES (1<<0)
 #define GET_HEAD_NAMES (1<<1)
 #define GET_PUSH_REF_STATES (1<<2)
@@ -70,7 +87,6 @@ static int add(int argc, const char **argv)
 	int i;
 
 	struct option options[] = {
-		OPT_GROUP("add specific options"),
 		OPT_BOOLEAN('f', "fetch", &fetch, "fetch the remote branches"),
 		OPT_CALLBACK('t', "track", &track, "branch",
 			"branch(es) to track", opt_parse_track),
@@ -79,11 +95,11 @@ static int add(int argc, const char **argv)
 		OPT_END()
 	};
 
-	argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
+	argc = parse_options(argc, argv, NULL, options, builtin_remote_add_usage,
 			     0);
 
 	if (argc < 2)
-		usage_with_options(builtin_remote_usage, options);
+		usage_with_options(builtin_remote_add_usage, options);
 
 	name = argv[0];
 	url = argv[1];
@@ -540,7 +556,7 @@ static int mv(int argc, const char **argv)
 	int i;
 
 	if (argc != 3)
-		usage_with_options(builtin_remote_usage, options);
+		usage_with_options(builtin_remote_rename_usage, options);
 
 	rename.old = argv[1];
 	rename.new = argv[2];
@@ -681,7 +697,7 @@ static int rm(int argc, const char **argv)
 	int i, result;
 
 	if (argc != 2)
-		usage_with_options(builtin_remote_usage, options);
+		usage_with_options(builtin_remote_rm_usage, options);
 
 	remote = remote_get(argv[1]);
 	if (!remote)
@@ -976,7 +992,6 @@ static int show(int argc, const char **argv)
 {
 	int no_query = 0, result = 0, query_flag = 0;
 	struct option options[] = {
-		OPT_GROUP("show specific options"),
 		OPT_BOOLEAN('n', NULL, &no_query, "do not query remotes"),
 		OPT_END()
 	};
@@ -984,7 +999,7 @@ static int show(int argc, const char **argv)
 	struct string_list info_list = { NULL, 0, 0, 0 };
 	struct show_info info;
 
-	argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
+	argc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage,
 			     0);
 
 	if (argc < 1)
@@ -1081,14 +1096,13 @@ static int set_head(int argc, const char **argv)
 	char *head_name = NULL;
 
 	struct option options[] = {
-		OPT_GROUP("set-head specific options"),
 		OPT_BOOLEAN('a', "auto", &opt_a,
 			    "set refs/remotes/<name>/HEAD according to remote"),
 		OPT_BOOLEAN('d', "delete", &opt_d,
 			    "delete refs/remotes/<name>/HEAD"),
 		OPT_END()
 	};
-	argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
+	argc = parse_options(argc, argv, NULL, options, builtin_remote_sethead_usage,
 			     0);
 	if (argc)
 		strbuf_addf(&buf, "refs/remotes/%s/HEAD", argv[0]);
@@ -1114,7 +1128,7 @@ static int set_head(int argc, const char **argv)
 		if (delete_ref(buf.buf, NULL, REF_NODEREF))
 			result |= error("Could not delete %s", buf.buf);
 	} else
-		usage_with_options(builtin_remote_usage, options);
+		usage_with_options(builtin_remote_sethead_usage, options);
 
 	if (head_name) {
 		unsigned char sha1[20];
@@ -1138,16 +1152,15 @@ static int prune(int argc, const char **argv)
 {
 	int dry_run = 0, result = 0;
 	struct option options[] = {
-		OPT_GROUP("prune specific options"),
 		OPT__DRY_RUN(&dry_run),
 		OPT_END()
 	};
 
-	argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
+	argc = parse_options(argc, argv, NULL, options, builtin_remote_prune_usage,
 			     0);
 
 	if (argc < 1)
-		usage_with_options(builtin_remote_usage, options);
+		usage_with_options(builtin_remote_prune_usage, options);
 
 	for (; argc; argc--, argv++)
 		result |= prune_remote(*argv, dry_run);
@@ -1228,13 +1241,12 @@ static int update(int argc, const char **argv)
 	struct string_list list = { NULL, 0, 0, 0 };
 	static const char *default_argv[] = { NULL, "default", NULL };
 	struct option options[] = {
-		OPT_GROUP("update specific options"),
 		OPT_BOOLEAN('p', "prune", &prune,
 			    "prune remotes after fetching"),
 		OPT_END()
 	};
 
-	argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
+	argc = parse_options(argc, argv, NULL, options, builtin_remote_update_usage,
 			     PARSE_OPT_KEEP_ARGV0);
 	if (argc < 2) {
 		argc = 2;
@@ -1334,7 +1346,7 @@ static int show_all(void)
 int cmd_remote(int argc, const char **argv, const char *prefix)
 {
 	struct option options[] = {
-		OPT__VERBOSE(&verbose),
+		OPT_BOOLEAN('v', "verbose", &verbose, "be verbose; must be placed before a subcommand"),
 		OPT_END()
 	};
 	int result;
-- 
1.6.5.2.186.ga4d60

^ permalink raw reply related

* Re: [PATCH v4] git remote: Separate usage strings for subcommands
From: Nanako Shiraishi @ 2009-11-19  3:40 UTC (permalink / raw)
  To: Tim Henigan; +Cc: Junio C Hamano, jrnieder, git
In-Reply-To: <4B04B4A2.8090001@gmail.com>

Quoting Tim Henigan <tim.henigan@gmail.com>

> When the usage string for a subcommand must be printed,
> only print the information relevant to that command.

I think this is a huge improvement.

> diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
> index 82a3d29..ee3dc80 100644
> --- a/Documentation/git-remote.txt
> +++ b/Documentation/git-remote.txt
> @@ -9,14 +9,14 @@ git-remote - manage set of tracked repositories
>  SYNOPSIS
>  --------
>  [verse]
> -'git remote' [-v | --verbose]
> -'git remote add' [-t <branch>] [-m <master>] [-f] [--mirror] <name> <url>
> +'git remote' [<options>]
> +'git remote add' [<options>] <name> <url>
>  'git remote rename' <old> <new>
>  'git remote rm' <name>
> -'git remote set-head' <name> [-a | -d | <branch>]
> -'git remote show' [-n] <name>
> -'git remote prune' [-n | --dry-run] <name>
> -'git remote update' [-p | --prune] [group | remote]...
> +'git remote set-head' <name> [<options>] [-a | -d | <branch>]
> +'git remote show' [<options>] <name>
> +'git remote prune' [<options>] <name>
> +'git remote update' [<options>] [<group> | <remote>]...

Often people look at this part of the manual page to quickly remind 
themselves what options are available, and it is better to keep the 
current text. Some manual pages have to use [options...] when there 
are too many to list, but each subcommand of git-remote doesn't have 
that many options.

> diff --git a/builtin-remote.c b/builtin-remote.c
> index 0777dd7..24a3ec0 100644
> --- a/builtin-remote.c
> +++ b/builtin-remote.c
> @@ -7,18 +7,35 @@
>  #include "run-command.h"
>  #include "refs.h"
>  
> +#define REMOTE_BARE_USAGE "git remote [<options>]"
> +#define REMOTE_ADD_USAGE "git remote add [<options>] <name> <url>"
> +#define REMOTE_RENAME_USAGE "git remote rename <old> <new>"
> +#define REMOTE_RM_USAGE "git remote rm <name>"
> +#define REMOTE_SETHEAD_USAGE "git remote set-head <name> [-a | -d | <branch>]"
> +#define REMOTE_SHOW_USAGE "git remote show [<options>] <name>"
> +#define REMOTE_PRUNE_USAGE "git remote prune [<options>] <name>"
> +#define REMOTE_UPDATE_USAGE "git remote update [<options>] [<group> | <remote>]..."
> +
>  static const char * const builtin_remote_usage[] = {
> -	"git remote [-v | --verbose]",
> -	"git remote add [-t <branch>] [-m <master>] [-f] [--mirror] <name> <url>",
> -	"git remote rename <old> <new>",
> -	"git remote rm <name>",
> -	"git remote set-head <name> [-a | -d | <branch>]",
> -	"git remote show [-n] <name>",
> -	"git remote prune [-n | --dry-run] <name>",
> -	"git remote [-v | --verbose] update [-p | --prune] [group]",
> +	REMOTE_BARE_USAGE,
> +	REMOTE_ADD_USAGE,
> +	REMOTE_RENAME_USAGE,
> +	REMOTE_RM_USAGE,
> +	REMOTE_SETHEAD_USAGE,
> +	REMOTE_SHOW_USAGE,
> +	REMOTE_PRUNE_USAGE,
> +	REMOTE_UPDATE_USAGE,
>  	NULL
>  };

For the same reason, I don't think this is a good change, if these
lines are used to show the first lines of 'git-remote -h' output.

^ permalink raw reply

* Re: [PATCHv3] Add branch management for releases to gitworkflows
From: Nanako Shiraishi @ 2009-11-19  4:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, Raman Gupta, git, skillzero
In-Reply-To: <200911181559.02873.trast@student.ethz.ch>

Quoting Thomas Rast <trast@student.ethz.ch> writes:

> FWIW, you can add my
>
>   Acked-by: Thomas Rast <trast@student.ethz.ch>
>
> to the final (squashed) patch.

Junio, please also add my

   Acked-by: Nanako Shiraishi <nanako3@lavabit.com>

My changes were intended to be squashed into the final single patch, too.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: [ANNOUNCE] codeBeamer MR - Easy ACL for Git
From: david @ 2009-11-19  6:06 UTC (permalink / raw)
  To: Intland Software; +Cc: Petr Baudis, git
In-Reply-To: <4B03F451.4050709@intland.com>

On Wed, 18 Nov 2009, Intland Software wrote:

> More precisely: parts of the source code are actually open, including
> the wiki plugins and the remote clients, for instance. The core source
> is closed, because the same core is also used in our commercial offerings, 
> and
> our commercial license doesn't (currently) allow publishing the
> complete code. We have quite some large customers from the defense space
> that would not be happy if we opened everything ;)

are you sure? did you see the recent memo about OpenSource by the DOD?

David Lang

> We are currently in the midst of rethinking our licensing scheme
> in general, to make things more liberal or to set up some kind of a
> dual license.
>
>> I think a lot of people wonder now, how does this compare to existing
>> solutions; from your announcement I thought it's something like
>> Gitosis/Gitolite, but in fact it seems more similar to Gitorious or
>> GitHub (if it was publicly available, of course); perhaps it would be
>> good idea to present comparison to these on the project homepage.
> Good point. More on this later.
> ---
> Intland
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* [PATCH 1/2] Documentation: fix typos and spelling in replace documentation
From: Christian Couder @ 2009-11-19  6:13 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Michael J Gruber, Jakub Narebski, Johannes Sixt, bill lam,
	Andreas Schwab, Paul Mackerras

This patch fix a missing "s" at the end of an occurence of
"--no-replace-objects" and, while at it, it also improves spelling
and rendering.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-replace.txt |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)

	This is a fix that could go into 'maint'.

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 8adc1ef..69f704f 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -17,31 +17,32 @@ DESCRIPTION
 Adds a 'replace' reference in `.git/refs/replace/`
 
 The name of the 'replace' reference is the SHA1 of the object that is
-replaced. The content of the replace reference is the SHA1 of the
+replaced. The content of the 'replace' reference is the SHA1 of the
 replacement object.
 
-Unless `-f` is given, the replace reference must not yet exist in
+Unless `-f` is given, the 'replace' reference must not yet exist in
 `.git/refs/replace/` directory.
 
-Replace references will be used by default by all git commands except
-those doing reachability traversal (prune, pack transfer and fsck).
+Replacement references will be used by default by all git commands
+except those doing reachability traversal (prune, pack transfer and
+fsck).
 
-It is possible to disable use of replacement refs for any command
-using the --no-replace-objects option just after "git".
+It is possible to disable use of replacement references for any
+command using the `--no-replace-objects` option just after 'git'.
 
-For example if commit "foo" has been replaced by commit "bar":
+For example if commit 'foo' has been replaced by commit 'bar':
 
 ------------------------------------------------
-$ git --no-replace-object cat-file commit foo
+$ git --no-replace-objects cat-file commit foo
 ------------------------------------------------
 
-show information about commit "foo", while:
+shows information about commit 'foo', while:
 
 ------------------------------------------------
 $ git cat-file commit foo
 ------------------------------------------------
 
-show information about commit "bar".
+shows information about commit 'bar'.
 
 OPTIONS
 -------
-- 
1.6.5.1.gaf97d

^ permalink raw reply related

* [PATCH 2/2] Documentation: talk a little bit about GIT_NO_REPLACE_OBJECTS
From: Christian Couder @ 2009-11-19  6:13 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Michael J Gruber, Jakub Narebski, Johannes Sixt, bill lam,
	Andreas Schwab, Paul Mackerras


Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-replace.txt |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

	This patch should go in my series that introduced the
	GIT_NO_REPLACE_OBJECTS env variable.

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 69f704f..65a0da5 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -44,6 +44,9 @@ $ git cat-file commit foo
 
 shows information about commit 'bar'.
 
+The 'GIT_NO_REPLACE_OBJECTS' environment variable can be set to
+achieve the same effect as the `--no-replace-objects` option.
+
 OPTIONS
 -------
 -f::
-- 
1.6.5.1.gaf97d

^ permalink raw reply related

* Re: Hey - A Conceptual Simplication....
From: Björn Steinbrink @ 2009-11-19  7:42 UTC (permalink / raw)
  To: George Dennie
  Cc: 'Jason Sewall', 'Jakub Narebski',
	'Jan Krüger', git
In-Reply-To: <009401ca68bc$7e4b12b0$7ae13810$@com>

On 2009.11.18 21:03:31 -0500, George Dennie wrote:
> Jason Sewell wrote....
> > I find this leads to big, shapeless commits and, as I mentioned
> > before, it seriously limits the utility of 'git bisect'.  I also
> > fail to see how 'selectively saving parts of the document' is
> > versioning and publishing - what is the publishing part?  The act of
> > committing is one thing (and 'saving...
> 
> The notion of a shapeless commit is curious. Intuitively, I consider a
> commit as capturing the state of my work at a transactional boundary
> (i.e. a successful unit test...or even lunch break). However, your
> characterization of "shape" suggest that you are constructing
> something other than the immediate functionality of the software.
> Consequently, your software document is not really the solution files
> alone but also this commit history that you meticulously craft. 

Your "lunch break" as a transaction boundary is a great example of
something that probably most people on this list would consider to
create commits that need rewriting before publishing them. Let's take an
extreme example:

You work on adding a feature to some webmail site that adds colors to
the mail being displayed, using different colors for the headers, quoted
sections and the text from the sender. The colors should be configurable
by the user.

*work*
git commit -m "Go for a coffee"
*work*
git commit -m "Lunch break"
*work*
git commit -m "Meeting"
*work*
git commit -m "Time to go home"

*come back to work*
*work*
git commit -m "Finished the mail coloring support"

This gives you:

* Finished the mail coloring support
|
* Time to go home
|
* Meeting
|
* Lunch break
|
* Go for a coffee

Such a history is basically completely useless. It's (ab)using the VCS
as a plain code dump. In a week, you'll be able to see that you had a
meeting that day, but it doesn't tell you anything about what you did to
the project. And even with less "insane" commit messages, the
"transactional boundaries" are totally arbitrary. They're aligned to
things you did that have absolutely nothing to do with the stuff you're
tracking in your VCS.

A far more useful history might look like this:

* Colorize quoted text in a mail, depending on its quoting depth
|
* Parse mails into a tree structure to represent sections of quoted text
|
* Colorize mail headers
|
* Add support for the user to change the colors used for mails
|
* Add configuration variable for the colors used for mails


At each step, something functionally changed about the software. The
commit messages tell you something about how the software evolved. And
if you get bogus values for the colors in the configuration, you can be
90% sure, by only looking at the commit messages, that you have a bug in
the "Add support for the user to change the colors ..." commit, and not
in one of the others. So you can run "git show $that_commit" to see the
diff of the changes you made in that commit and quickly check them for
your bug.

And while that's not sooo useful for commits that added new
functionality, it's extremely useful for commits that just made small
changes to existing functionality. Finding a bug in a large piece of
code (say 2000 lines) isn't trivial. But if you know that a commit that
changed 5 lines in that code is responsible for the breakage, all you
have to do is to identify the faulty change, which is a lot easier.

And with a large history, where it's not obvious in which commit
something got broken, "git bisect" can help to quickly find the bad
commit. Now consider "git bisect" finding your "Lunch break" commit.
Looking at the commit message tells nothing. The diff is pretty much
arbitrary, might be huge. Not much help. Finding the "Add support for
the user to change the colors ..." commit already tells you something
just because of the commit message. And the diff is about just one
specific change. It's all nicely separated, and that's a huge value.

Using git and producing nice commits is about _documenting_ the history
of your code. And having small, self-contained and well separated
commits is key to that.


And the index can be a great help with that. Given the above example,
you might already have some code to use the configured colors, just for
testing, so things aren't so boring. Maybe even some hack-up of the
code you'll be using later. If that part of the code would be committed
right away, you'd mess up your commit, because it wouldn't be about a
single change anymore, but would also have your testing code in there.
Bad.

But you don't want to throw the testing code away either, because it's
useful right now, and you might need it later, because it might evolve
into the final code used for the actual coloring. So, what now? You have
code that you want to commit, and some code you don't want to commit,
and which needs to go away temporarily, so you can test without it. No
problem, here comes the index.

Say you have:
config.c     # Has changes for the colors
show_mail.c  # Has changes to use the colors
whatever.c   # Has some changes for both

You do:
git add config.c       # Add to the index
git add -p whatever.c  # Only add some hunks to the index

So now the index has what you want to commit, and the working tree still
has everything.

git stash save --keep-index

Now your working tree and index only have the things you want to commit.
You run your unit tests, everythings fine. You commit and get a nice
clean commit, for which you write a useful commit message.

git stash pop

You've got your changes back that you didn't want to commit just yet,
and you can continue working.


Another use-case I have found for myself is to use the index to separate
reviewed and not-yet-reviewed changes. Before I commit, I always review
the diff of the things I'm going to commit. So I start out with "git
diff" and start reading. When I finished reviewing a file, I can do "git
add $that_file", so the diff for that file will no longer be shown by
"git diff". That nicely cuts down the size of the "git diff" output to
things I'm still interested in. Quite useful when you are forced to do a
large commit, because you did some refactoring. If I find a bug during
the review, I can fix that and re-run "git diff", which will only show
changes to me that I didn't declare as "good" already by adding them to
the index.


Sure, it takes some pratice and discipline to generate a nice, useful
history. But that's not much different from writing code. Others will
hate you for writing unreadable spaghetti code, and so will they hate
you for producing a useless history that tells them that you had lunch,
instead of telling them what you did to the code ;-)

Björn

^ permalink raw reply

* [PATCH v2] git am/mailinfo: Don't look at in-body headers when rebasing
From: Lukas Sandström @ 2009-11-19  8:51 UTC (permalink / raw)
  To: Philip Hofstetter, Junio C Hamano; +Cc: Lukas Sandström, Jeff King, git
In-Reply-To: <aa2993680911181547p4cbbf12cq74b482f63e59d007@mail.gmail.com>

When we are rebasing we know that the header lines in the
patch are good and that we don't need to pick up any headers
from the body of the patch.

This makes it possible to rebase commits whose commit message
start with "From" or "Date".

Test vectors by Jeff King.

Signed-off-by: Lukas Sandström <luksan@gmail.com>
---

Argh. I just realized that the change to t5100 in the previous patch
doesn't actually test the new option, since I forgot to change the
+		if test -f "$TEST_DIRECTORY"/t5100/msg$mail--use-first-header
line to "--no-inbody-headers", after my first attempt at an option name.

This time I checked that the tests actually fail when the test is broken.
Still passes, just one line changed since the previous version.

/Lukas

 builtin-mailinfo.c                   |    5 +++++
 git-am.sh                            |   13 ++++++++++---
 t/t5100-mailinfo.sh                  |    6 +++++-
 t/t5100/info0015                     |    5 +++++
 t/t5100/info0015--no-inbody-headers  |    5 +++++
 t/t5100/info0016                     |    5 +++++
 t/t5100/info0016--no-inbody-headers  |    5 +++++
 t/t5100/msg0015                      |    2 ++
 t/t5100/msg0015--no-inbody-headers   |    3 +++
 t/t5100/msg0016                      |    2 ++
 t/t5100/msg0016--no-inbody-headers   |    4 ++++
 t/t5100/patch0015                    |    8 ++++++++
 t/t5100/patch0015--no-inbody-headers |    8 ++++++++
 t/t5100/patch0016                    |    8 ++++++++
 t/t5100/patch0016--no-inbody-headers |    8 ++++++++
 t/t5100/sample.mbox                  |   33 +++++++++++++++++++++++++++++++++
 16 files changed, 116 insertions(+), 4 deletions(-)
 create mode 100644 t/t5100/info0015
 create mode 100644 t/t5100/info0015--no-inbody-headers
 create mode 100644 t/t5100/info0016
 create mode 100644 t/t5100/info0016--no-inbody-headers
 create mode 100644 t/t5100/msg0015
 create mode 100644 t/t5100/msg0015--no-inbody-headers
 create mode 100644 t/t5100/msg0016
 create mode 100644 t/t5100/msg0016--no-inbody-headers
 create mode 100644 t/t5100/patch0015
 create mode 100644 t/t5100/patch0015--no-inbody-headers
 create mode 100644 t/t5100/patch0016
 create mode 100644 t/t5100/patch0016--no-inbody-headers

diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index c90cd31..a81526e 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -26,6 +26,7 @@ static struct strbuf charset = STRBUF_INIT;
 static int patch_lines;
 static struct strbuf **p_hdr_data, **s_hdr_data;
 static int use_scissors;
+static int use_inbody_headers = 1;

 #define MAX_HDR_PARSED 10
 #define MAX_BOUNDARIES 5
@@ -771,6 +772,8 @@ static int handle_commit_msg(struct strbuf *line)
 		return 0;

 	if (still_looking) {
+		if (!use_inbody_headers)
+			still_looking = 0;
 		strbuf_ltrim(line);
 		if (!line->len)
 			return 0;
@@ -1033,6 +1036,8 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)
 			use_scissors = 1;
 		else if (!strcmp(argv[1], "--no-scissors"))
 			use_scissors = 0;
+		else if (!strcmp(argv[1], "--no-inbody-headers"))
+			use_inbody_headers = 0;
 		else
 			usage(mailinfo_usage);
 		argc--; argv++;
diff --git a/git-am.sh b/git-am.sh
index c132f50..96869a2 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -289,7 +289,7 @@ split_patches () {
 prec=4
 dotest="$GIT_DIR/rebase-apply"
 sign= utf8=t keep= skip= interactive= resolved= rebasing= abort=
-resolvemsg= resume= scissors=
+resolvemsg= resume= scissors= no_inbody_headers=
 git_apply_opt=
 committer_date_is_author_date=
 ignore_date=
@@ -322,7 +322,7 @@ do
 	--abort)
 		abort=t ;;
 	--rebasing)
-		rebasing=t threeway=t keep=t scissors=f ;;
+		rebasing=t threeway=t keep=t scissors=f no_inbody_headers=t ;;
 	-d|--dotest)
 		die "-d option is no longer supported.  Do not use."
 		;;
@@ -448,6 +448,7 @@ else
 	echo "$utf8" >"$dotest/utf8"
 	echo "$keep" >"$dotest/keep"
 	echo "$scissors" >"$dotest/scissors"
+	echo "$no_inbody_headers" >"$dotest/no_inbody_headers"
 	echo "$GIT_QUIET" >"$dotest/quiet"
 	echo 1 >"$dotest/next"
 	if test -n "$rebasing"
@@ -495,6 +496,12 @@ t)
 f)
 	scissors=--no-scissors ;;
 esac
+if test "$(cat "$dotest/no_inbody_headers")" = t
+then
+	no_inbody_headers=--no-inbody-headers
+else
+	no_inbody_headers=
+fi
 if test "$(cat "$dotest/quiet")" = t
 then
 	GIT_QUIET=t
@@ -549,7 +556,7 @@ do
 	# by the user, or the user can tell us to do so by --resolved flag.
 	case "$resume" in
 	'')
-		git mailinfo $keep $scissors $utf8 "$dotest/msg" "$dotest/patch" \
+		git mailinfo $keep $no_inbody_headers $scissors $utf8 "$dotest/msg" "$dotest/patch" \
 			<"$dotest/$msgnum" >"$dotest/info" ||
 			stop_here $this

diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index 0279d07..ebc36c1 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -11,7 +11,7 @@ test_expect_success 'split sample box' \
 	'git mailsplit -o. "$TEST_DIRECTORY"/t5100/sample.mbox >last &&
 	last=`cat last` &&
 	echo total is $last &&
-	test `cat last` = 14'
+	test `cat last` = 16'

 check_mailinfo () {
 	mail=$1 opt=$2
@@ -30,6 +30,10 @@ do
 		if test -f "$TEST_DIRECTORY"/t5100/msg$mail--scissors
 		then
 			check_mailinfo $mail --scissors
+		fi &&
+		if test -f "$TEST_DIRECTORY"/t5100/msg$mail--no-inbody-headers
+		then
+			check_mailinfo $mail --no-inbody-headers
 		fi
 	'
 done
diff --git a/t/t5100/info0015 b/t/t5100/info0015
new file mode 100644
index 0000000..0114f10
--- /dev/null
+++ b/t/t5100/info0015
@@ -0,0 +1,5 @@
+Author:
+Email:
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/info0015--no-inbody-headers b/t/t5100/info0015--no-inbody-headers
new file mode 100644
index 0000000..c4d8d77
--- /dev/null
+++ b/t/t5100/info0015--no-inbody-headers
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@example.com
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/info0016 b/t/t5100/info0016
new file mode 100644
index 0000000..38ccd0d
--- /dev/null
+++ b/t/t5100/info0016
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@example.com
+Subject: check bogus body header (date)
+Date: bogus
+
diff --git a/t/t5100/info0016--no-inbody-headers b/t/t5100/info0016--no-inbody-headers
new file mode 100644
index 0000000..f4857d4
--- /dev/null
+++ b/t/t5100/info0016--no-inbody-headers
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@example.com
+Subject: check bogus body header (date)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/msg0015 b/t/t5100/msg0015
new file mode 100644
index 0000000..9577238
--- /dev/null
+++ b/t/t5100/msg0015
@@ -0,0 +1,2 @@
+- a list
+  - of stuff
diff --git a/t/t5100/msg0015--no-inbody-headers b/t/t5100/msg0015--no-inbody-headers
new file mode 100644
index 0000000..be5115b
--- /dev/null
+++ b/t/t5100/msg0015--no-inbody-headers
@@ -0,0 +1,3 @@
+From: bogosity
+  - a list
+  - of stuff
diff --git a/t/t5100/msg0016 b/t/t5100/msg0016
new file mode 100644
index 0000000..0d9adad
--- /dev/null
+++ b/t/t5100/msg0016
@@ -0,0 +1,2 @@
+and some content
+
diff --git a/t/t5100/msg0016--no-inbody-headers b/t/t5100/msg0016--no-inbody-headers
new file mode 100644
index 0000000..1063f51
--- /dev/null
+++ b/t/t5100/msg0016--no-inbody-headers
@@ -0,0 +1,4 @@
+Date: bogus
+
+and some content
+
diff --git a/t/t5100/patch0015 b/t/t5100/patch0015
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0015
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/patch0015--no-inbody-headers b/t/t5100/patch0015--no-inbody-headers
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0015--no-inbody-headers
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/patch0016 b/t/t5100/patch0016
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0016
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/patch0016--no-inbody-headers b/t/t5100/patch0016--no-inbody-headers
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0016--no-inbody-headers
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox
index 13fa4ae..de10312 100644
--- a/t/t5100/sample.mbox
+++ b/t/t5100/sample.mbox
@@ -650,3 +650,36 @@ index b0b5d8f..461c47e 100644
  		convert_to_utf8(line, charset.buf);
 --
 1.6.4.1
+From nobody Mon Sep 17 00:00:00 2001
+From: A U Thor <a.u.thor@example.com>
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
+From: bogosity
+  - a list
+  - of stuff
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
+From nobody Mon Sep 17 00:00:00 2001
+From: A U Thor <a.u.thor@example.com>
+Subject: check bogus body header (date)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
+Date: bogus
+
+and some content
+
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
-- 
1.6.4.4

^ 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