Git development
 help / color / mirror / Atom feed
* user-manual updates.
From: J. Bruce Fields @ 2007-05-07  5:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Some miscellaneous user-manual updates, also available from

	git://linux-nfs.org/~bfields/git.git maint

--b.

^ permalink raw reply

* Re: [PATCH] connect: display connection progress
From: Michael S. Tsirkin @ 2007-05-07  4:54 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Michael S. Tsirkin, Junio C Hamano, git
In-Reply-To: <20070506222123.GB2439@steel.home>

> @@ -511,10 +537,19 @@ static int git_tcp_connect_sock(char *host)
>  
>  		if (connect(sockfd, (struct sockaddr *)&sa, sizeof sa) < 0) {
>  			saved_errno = errno;
> +			fprintf(stderr, "%s[%d: %s]: net=%s, errno=%s\n",
> +				host,
> +				cnt,
> +				inet_ntoa(*(struct in_addr *)&sa.sin_addr),
> +				hstrerror(h_errno),
> +				strerror(saved_errno));
>  			close(sockfd);
>  			sockfd = -1;
>  			continue;
>  		}
> +		fprintf(stderr, "using %s[%s]\n",
> +			host,
> +			inet_ntoa(*(struct in_addr *)&sa.sin_addr));
>  		break;
>  	}

My manual says:
	The inet_ntoa() function shall convert the Internet host address
	specified by in to a string in the Internet standard dot notation.

does it work for IPv6?

-- 
MST

^ permalink raw reply

* Re: [PATCH] connect: display connection progress
From: Michael S. Tsirkin @ 2007-05-07  4:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael S. Tsirkin, git
In-Reply-To: <7vhcqpbs78.fsf@assigned-by-dhcp.cox.net>

> Quoting Junio C Hamano <junkio@cox.net>:
> Subject: Re: [PATCH] connect: display connection progress
> 
> "Michael S. Tsirkin" <mst@dev.mellanox.co.il> writes:
> 
> > Make git notify the user about host resolution/connection attempts.  This
> > is useful both as a progress indicator on slow links, and helps reassure the
> > user there are no DNS/firewall problems.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@dev.mellanox.co.il>
> >
> > ---
> >
> > I find the following useful.
> > This currently only covers native git protocol. I expect it would
> > be easy to extend this to other protocols, if there's interest.
> > Opinions?
> 
> I think giving this kind of feedback makes a lot of sense, from
> both the "assurance" point of view and also debuggability.
> 
> But please do this only under verbose, or squelch it if "quiet"
> is asked.

Squelching it if quiet is set makes more sense to me.
I'll do that.

-- 
MST

^ permalink raw reply

* Re: [PATCH 2/3] dir.c: Omit non-excluded directories with dir->show_ignored
From: Michael Spang @ 2007-05-07  2:35 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Linus Torvalds
In-Reply-To: <alpine.LFD.0.98.0705061239460.25245@woody.linux-foundation.org>

This makes "git-ls-files --others --directory --ignored" behave
as documented and consequently also fixes "git-clean -d -X".
Previously, git-clean would remove non-excluded directories
even when using the -X option.

Signed-off-by: Michael Spang <mspang@uwaterloo.ca>
---

This fixes the style issue noted by Linus.


 dir.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/dir.c b/dir.c
index d306352..11fab7f 100644
--- a/dir.c
+++ b/dir.c
@@ -448,6 +448,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 
 		while ((de = readdir(fdir)) != NULL) {
 			int len;
+			int exclude;
 
 			if ((de->d_name[0] == '.') &&
 			    (de->d_name[1] == 0 ||
@@ -461,7 +462,9 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 			memcpy(fullname + baselen, de->d_name, len+1);
 			if (simplify_away(fullname, baselen + len, simplify))
 				continue;
-			if (excluded(dir, fullname) != dir->show_ignored) {
+
+			exclude = excluded(dir, fullname);
+			if (exclude != dir->show_ignored) {
 				if (!dir->show_ignored || DTYPE(de) != DT_DIR) {
 					continue;
 				}
@@ -484,6 +487,8 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 				len++;
 				switch (treat_directory(dir, fullname, baselen + len, simplify)) {
 				case show_directory:
+					if (exclude != dir->show_ignored)
+						continue;
 					break;
 				case recurse_into_directory:
 					contents += read_directory_recursive(dir,

^ permalink raw reply related

* Re: 'upstream' branches.
From: Junio C Hamano @ 2007-05-07  1:20 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Alex Riesen, git
In-Reply-To: <1178440759.17680.112.camel@shinybook.infradead.org>

David Woodhouse <dwmw2@infradead.org> writes:

> You're speaking from the point of view of the git implementation.
> From the point of view of the _user_, I would violently disagree :)

I would not view this as an implementation issue.  If you or
anybody disagrees, I think that is a disagreement at more
conceptual level.

> Having pulled that into my local repository, how do I then set it up to
> push the latest commit of refs/remotes/*/linus into the 'linus' branch
> of the origin, when I push back to my public tree on the server? Or do
> you expect _everyone_ who pulls from that public tree to also do stuff
> like:
>> 	git log master --not remotes/a/linus remotes/b/linus remotes/c/linus

Of course not.  Why are you even _PUBLISHING_ what your
upstreams' origins are to begin with?  I think you are simply
being silly.

So let's step back a bit, so I can clarify why I said "silly" --
I am not Linus and usually try not to say things like that ;-).

First, I think everybody by now understands why rewinding a
branch in a published repository is a bad idea, and agrees that
(at least) Linus's tip never rewinds but always goes forward.
I see you also subscribe to the school of thought:

> Can't I instruct it to _merge_ the 'linus' branch of each remote into my
> own 'linus' branch? Of course that merge would only ever be a
> fast-forward or a no-op, in practice.

By this, you are effectively getting the origin as seen by other
people, and taking the most advanced one as the union of the
origins.

But step back and think about the reason why you would even want
to know about the origin of each of your buddies (I earlier said
"upstream" in this message, but because there is no inherent
up/down in the distributed development model, I think it is more
correct to call them your mtd buddies).

Earlier I said that it would make sense for you to keep track of
the tip and "the tip of Linus as seen by the buddy" for _each_
of your mtd buddies, by doing:

	[remote "A"]
        	fetch = refs/heads/master:refs/remotes/A/master
                fetch = refs/heads/linus:refs/remotes/A/linus

for 'A', 'B', and 'C', your mtd buddies.  It would make sense
because the log between A/linus and A/master represents what A
did, and what have not been incorporated in the Linus tree yet
from A's point of view.  You can do

	$ git log remotes/A/linus..remotes/A/master

for that (same for B and C).  Also, diff between these would
represent the change A made as a whole:

	$ git diff remotes/A/linus..remotes/A/master

But your arrangement is a bit different.  You allow the same
branch refs/heads/linus to be updated/overwritten by A, B and C.
We could teach special semantics of "fast forward or nothing",
perhaps using '*' like this:

	[remote "A"]
        	fetch = refs/heads/master:refs/heads/A
                fetch = *refs/heads/linus:refs/heads/linus
	[remote "B"]
        	fetch = refs/heads/master:refs/heads/B
                fetch = *refs/heads/linus:refs/heads/linus

as you suggest, but I do not think it buys you much.  The tip of
Linus's repository B or C has may much more advanced than what A
based his work on, so your 'linus' may be soemthing A has not
seen yet.  However, even then:

	$ git log linus..A

would continue to work.  On the other hand, the earlier "diff"
now needs to be written like this:

	$ git diff $(git merge-base linus A)..A

Because this is the right thing to do in regular cases anyway,
we even have a short-hand for that in the "three dot" form:

	$ git diff linus...A

I think you already know these two things: "git-log linus..A is
the right way to ask what A did relative to Linus, even when
'linus' is ahead of what A based his work on" and "the three-dot
notation linus...A is the right thing to use when 'linus' could
be ahead of what A is based on".  Otherwise you would not be
asking for the "fast forward or nothing" fetch, as its result
would be hard to use without these characteristics.

But if you know them, and if you do not care exactly which
commit from Linus what each of your buddies thought was at
Linus's tip (and you obviously don't, as "fast forward or
nothing" would lose information for two people and keep only the
most advanced one), then you would also know that there is not
much point fetching the origin from your buddies.  You can fetch
and keep track of where Linus's tip is directly from Linus
yourself, and the above "git log linus..A" and "git diff
linus...A" would work.  Then there is no risk of confusion.  If
one of A, B, or C had a wrong commit that claims to from Linus,
having separate tracking branch on your end is necessary to
figure out which one has screwed up -- "fast forward or nothing"
would not help.

Having said that, I think "fast-forward or nothing" might make
sense in one special case.  If the kernel project _were_ more
regidly structured such that you were a third-stratum developer
who can only interact with second-stratum people and not allowed
to fetch directly from first-stratum repository (i.e. Linus's).
Then, the best guess you could make where the tip of Linus's
repository is by learning second-hand from the repositories of
second-stratum you fetch, and keeping track of their origins,
and picking the most advanced one among them.

But the kernel project is not structured that way.

You also _could_ argue that your fetching directly from Linus is
one extra fetch, and you do not _care_ where the real Linus's
tip is.  Both of these are correct, if the only thing you care
about in this application is to inspect the progress your mtd
buddies A, B and C are making.  Even when all of them are way
behind from Linus's tree, "log linus..A"/"diff linus...A" would
work just fine.  I do not think it is unreasonable to want to
maintain a single 'linus' branch by picking the most advanced
among the different 'linus' branches you get from different
repositories.

But at that point, I think it is such a specialized application
that you should be scripting that outside of git-core.

Oh, and to cut-down the message roundtrip (although I do not
think you would make such a silly argument, I do very much
anticipate somebody else would).  I would not buy "SCM tool
should do that work for me, not me doing that work for SCM"
argument on that last point.  It is like saying "why doesn't
your editor fill a completed program when I open a new file
whose name is 'hello.c', and instead have me type it all?  It
should be clear that I want to write a "hello world" program,
and the tool should be helping me".

^ permalink raw reply

* Re: [PATCH] Teach bash to complete branch names in some places.
From: Karl Hasselström @ 2007-05-07  1:11 UTC (permalink / raw)
  To: Yann Dirson; +Cc: Catalin Marinas, git
In-Reply-To: <20070506231357.31547.25929.stgit@gandelf.nowhere.earth>

On 2007-05-07 01:13:57 +0200, Yann Dirson wrote:

> Unfortunately, the current completion framework does not seem
> flexible enough to get branch completion after "-b" flags, but I
> feel things are already better this way.

Perhaps another round of stealing stuff from git's bash completion is
in order?

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* Re: What's in git.git (stable)
From: Jakub Narebski @ 2007-05-07  0:59 UTC (permalink / raw)
  To: git
In-Reply-To: <7vejlucozk.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

>   - "git diff $commit1:$path2 $commit2:$path2" can now report
>     mode changes between the two blobs.

I think that actually it is enough to have $tree1:$path1 $tree2:$path2
(and it should be $commit1:$path1 not $path2 nevertheless).

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH v3] Support ent:relative_path
From: Dana How @ 2007-05-07  0:37 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Git Mailing List, Shawn O. Pearce, danahow
In-Reply-To: <7v6475bi69.fsf@assigned-by-dhcp.cox.net>

On 5/6/07, Junio C Hamano <junkio@cox.net> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Sun, 6 May 2007, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >>
> >> > On Sat, 5 May 2007, Junio C Hamano wrote:
> >> >
> >> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >> >>
> >> >> >> (a) In a bare repository, I believe
> >> >> >> setup.c:setup_git_directory_gently() determines the prefix to be
> >> >> >> NULL.  This means my patch will see ALL paths as absolute, except
> >> >> >> :../path which will result in an error.
> >> >> >
> >> >> > My point was that it feels inconsistent to take the current path into
> >> >> > account in one case, but not in the other.
> >> >>
> >> >> I do not understand your reasoning.  In a bare repository you cannot
> >> >> even be in a subdirectory to begin with.
> >> >
> >> > Exactly! That is my point. If you can do it in a working directory, but
> >> > also with a bare repository, I find it highly confusing and inconsistent
> >> > to have different meaning.
> >>
> >> Sorry.  Now you confused me further.  I can do:
> >>
> >>      cd Documentation
> >>         git diff v1.5.0 v1.5.1 -- git.txt
> >>
> >> Is that confusing, inconsistent and bad for the users?
> >
> > Well, I am partly at fault that you _can_ execute git-diff outside of a
> > repository.
> >
> > But given the _arguments_ you give to git-diff as above, I'd expect it to
> > actually care about the working directory. IOW I would _not_ expect this
> > to work outside of a working directory (even if it does).
>
> Oh, I was not thinking about "outside of repository" use.  I was
> talking about your earlier "bare repository vs inside worktree
> vs inside a subdirectory of worktree" point.  In a bare
> repository,
>
>         git diff v1.5.0 v1.5.1 -- Documentation/git.txt
>
> is the only form that makes sense, as you cannot say "I am
> interested in Documentation/" by _being_ in that subdirectory.
> In a repository with worktree, you can, and we let you do so.
>
> I would expect v1.5.0:Documentation/git.txt notation would be
> the only sane variant that would make sense to name that blob in
> a bare repository for the same reason.  I do not expect anybody
> to complain because we do not allow him to say v1.5.0:git.txt in
> a bare repository, either.
>
> Also I sympathize with people who would wish to (eventually) be
> able to do:
>
>         $ cd Documentation/
>         $ git show v1.5.0:git.txt
>
> in a repository with worktree, by making the "relative path" the
> default behaviour.  They would need to do either one of:
>
>         $ git show v1.5.0:/git.c
>         $ git show v1.5.0:../git.c
>
> if we ever made the "relative path" the default.  As long as you
> make sure that you make:
>
>         $ git show v1.5.0:/git.c
>
> work the same way in a bare repository _if_ we make the
> "relative path" the default, I do not see any inconsistency
> problem there.
>
> A bare repository and a repository with working tree are
> different.  In the former, you cannot say "I am interested in
> this subtree" by _being_ in a subdirectory; in the latter you
> can.  Taking advantage of that and allowing the user to express
> himself better (only) in the latter is not an inconsistency.
> Not being able to do that in a bare repository comes from what a
> bare repository inherently is.

Each of the examples given already works as described above
in the current patch [if you change one #define to enable
"relative is default"].  I will get around to fixing the one
error I described elsewhere and re-submit.

Thanks,
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

^ permalink raw reply

* Re: [PATCH v3] Support ent:relative_path
From: Junio C Hamano @ 2007-05-07  0:18 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Dana How, Git Mailing List, Shawn O. Pearce
In-Reply-To: <Pine.LNX.4.64.0705070110040.4167@racer.site>

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

> On Sun, 6 May 2007, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> > On Sat, 5 May 2007, Junio C Hamano wrote:
>> >
>> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> >> 
>> >> >> (a) In a bare repository, I believe 
>> >> >> setup.c:setup_git_directory_gently() determines the prefix to be 
>> >> >> NULL.  This means my patch will see ALL paths as absolute, except 
>> >> >> :../path which will result in an error.
>> >> >
>> >> > My point was that it feels inconsistent to take the current path into 
>> >> > account in one case, but not in the other.
>> >> 
>> >> I do not understand your reasoning.  In a bare repository you cannot 
>> >> even be in a subdirectory to begin with.
>> >
>> > Exactly! That is my point. If you can do it in a working directory, but 
>> > also with a bare repository, I find it highly confusing and inconsistent 
>> > to have different meaning.
>> 
>> Sorry.  Now you confused me further.  I can do:
>> 
>> 	cd Documentation
>>         git diff v1.5.0 v1.5.1 -- git.txt
>> 
>> Is that confusing, inconsistent and bad for the users?
>
> Well, I am partly at fault that you _can_ execute git-diff outside of a 
> repository.
>
> But given the _arguments_ you give to git-diff as above, I'd expect it to 
> actually care about the working directory. IOW I would _not_ expect this 
> to work outside of a working directory (even if it does).

Oh, I was not thinking about "outside of repository" use.  I was
talking about your earlier "bare repository vs inside worktree
vs inside a subdirectory of worktree" point.  In a bare
repository,

	git diff v1.5.0 v1.5.1 -- Documentation/git.txt

is the only form that makes sense, as you cannot say "I am
interested in Documentation/" by _being_ in that subdirectory.
In a repository with worktree, you can, and we let you do so.

I would expect v1.5.0:Documentation/git.txt notation would be
the only sane variant that would make sense to name that blob in
a bare repository for the same reason.  I do not expect anybody
to complain because we do not allow him to say v1.5.0:git.txt in
a bare repository, either.

Also I sympathize with people who would wish to (eventually) be
able to do:

	$ cd Documentation/
	$ git show v1.5.0:git.txt

in a repository with worktree, by making the "relative path" the
default behaviour.  They would need to do either one of:

	$ git show v1.5.0:/git.c
	$ git show v1.5.0:../git.c

if we ever made the "relative path" the default.  As long as you
make sure that you make:

	$ git show v1.5.0:/git.c

work the same way in a bare repository _if_ we make the
"relative path" the default, I do not see any inconsistency
problem there.

A bare repository and a repository with working tree are
different.  In the former, you cannot say "I am interested in
this subtree" by _being_ in a subdirectory; in the latter you
can.  Taking advantage of that and allowing the user to express
himself better (only) in the latter is not an inconsistency.
Not being able to do that in a bare repository comes from what a
bare repository inherently is.

^ permalink raw reply

* Re: [FAQ?] Rationale for git's way to manage the index
From: Johannes Schindelin @ 2007-05-06 23:51 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqbqgxak1i.fsf@bauges.imag.fr>

Hi,

On Sun, 6 May 2007, Matthieu Moy wrote:

> Linus Torvalds <torvalds@linux-foundation.org> writes:
> 
> >  - You fundamentally cannot do it any other way.
> >
> >    Not doing it the way git does it (point to the content) means that the 
> >    index-replacement has to point to something else, namely a "file ID". 
> 
> Well, git's index still tells more than "the content FOOBAR exists,
> somewhere". It also "contains", if not "points to", the file name.

As you pointed out yourself, the index _has_ an idea of the content of 
that file. So, arguably, it does not point to _that_ file, but rather to 
that file _with a certain content_.

> > What's so hard with adding that "-a" to "git commit"? You don't even need 
> > it on the status line, the status is relevant and understandable (and 
> > actually tells you more) even without it.
> 
> Off course, I don't have strong argument against it. The biggest
> annoyance is that my fingers are used to "commit -m message", and now
> type "commit -a message", but ...

Just another reason to hate CVS. Because it trained people to do that. If 
it was not for the training by CVS, I would have strongly opposed to the 
introduction of the "-m" switch to commit. It _encourages_ bad commit 
messages.

Now, with Git I usually let git-commit start up the editor. Because then I 
am actually encouraged to make up my mind, and put down a meaningful 
message, which might not only help _others_ to understand why I did it, 
and how, but also _myself_ (after a few months).

> The reason why I'm posting this is that I was wondering whether "commit 
> -a" not being the default was supposed to be a message like "you 
> shouln't use it too often".

IMHO yes, that is the message.

In addition to being nice to people used to the behaviour of "git commit" 
_without_ other arguments.

Ciao,
Dscho

^ permalink raw reply

* Re: [FAQ?] Rationale for git's way to manage the index
From: Johannes Schindelin @ 2007-05-06 23:42 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqk5vlamav.fsf@bauges.imag.fr>

Hi Matthieu,

On Sun, 6 May 2007, Matthieu Moy wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Sun, 6 May 2007, Matthieu Moy wrote:
> >
> >> [...]
> >>
> >> % git satus -a
> >> % git commit -a -m "..."
> >> 
> >> In the former case, I have more commands to type, and in the second
> >> case, I loose part of the stat-cache benefit: If I run "git status -a"
> >> twice, the second run will actually diff all the files touched since
> >> the last run, since "git status -a" actually updated a temporary
> >> index, and discarded it afterwards, so it doesn't update the stat
> >> information in the index (while "git status" would have).
> >
> > Have you tried "git status" _without_ "-a"?
> 
> Reading my message (including the last 5 words of the sentence you're 
> quoting) would have told you that ;-).

Okay, I rephrase the (badly worded) question:

Why did you use "-a" with "git status" to begin with? It's useless.

> >> In both cases, I can't really see the benefit.
> >
> > The benefit is a clear distinguishing between DWIM and low level. The 
> > index contains _exactly_ what you told it to contain.
> 
> In other systems, commit commits _exactly_ the content of files on
> disk. And most people seem happy with that.

Because they do not realize that the file _names_ are actually only a key, 
not the value.

With Git, it is possible to stage changes, but also to have a dirty stage.

Think, for example, about debugging a program. Many programs have 
Makefiles, which define CFLAGS without "-g". Now you want to debug. Since 
gdb acquired the bad habit of not working properly at all without that 
flag (which is especially apparent when single stepping jumps around 
wildly in the source code), you _have_ to change the Makefile to include 
"-g" with the CFLAGS.

But you don't want to commit _that_. It is no useful change for the 
project. Submitting such a patch makes you look foolish. So, you leave it 
out of the commit.

And to make you _aware_ that it is a real possibility, and often a 
desirable one, git-commit makes you specify "-a" when you are _sure_ that 
you want to commit _all_ of your changes to the tracked files.

With CVS (which has been bashed on a lot on this list, and rightfully so), 
after a mistaken "cvs commit" _with_ irrelevant changes, like the change 
to the Makefile I illustrated above), you have two options:

	- leave it as it is (possibly undoing the change in a subsequent 
	  commit), or

	- edit the files, which often leads to an inconsistent repository. 
	  Yeah, sure, you can checkout the newest state, but you cannot 
	  reproduce known older states.

> > By forcing users to use "-a" with "git commit",
> 
> Does this mean that the normal way to use "commit" is to use "-a"?

Well, I use it quite a lot. But 30% of the time, I prefer to commit with 
specific filenames, so I can be sure _what_ I commit. FWIW, I picked up on 
that practice when using CVS...

There are even about 20% of the time, when I use "git commit" _without_ 
any parameters, because I used "git add" to tell Git that I resolved some 
conflicts, or that I want this file to be committed, while other files 
should not be committed.

> > you make it clear that a separate update steo is involved,
> 
> Well, with those kind of arguments, I could have my web browser not do
> DNS resolution for me, because it would make it clear that a separate
> step from HTTP request is involved.

No. _You_ never need to tell the browser _not_ to resolve via DNS.

But _you_ sometimes _need_ to commit with _different_ parameters than 
"-a". You might not realize that _now_. But at least specifying "-a" 
everytime you do your thing gives you a _chance_ to realize it.

> > and if you made an error (which you see from the file list), you can
> > abort, and start over with the original index.
> 
> You don't necessarily see your error from the file list:
> 
> % vi foo.c
> % git add foo.c
> % vi foo.c
> % git commit -m foo

As others have commented, "-m" is a _bad_ option. Yes, for ease of use, it 
is provided.

But how useful is a commit message which consists of less than five words?

It does _not_ tell you,

	- what the _conceptual_ change was,
	- _why_ it was done,
	- _how_ it was done, and
	- what the rationale of the committer was, for the case that 
	  people try to come up with a cleverer patch, to prevent 
	  unnecessary rethinking.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] Teach bash to complete branch names in some places.
From: Yann Dirson @ 2007-05-06 23:13 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git


This patch causes bash completion to look for branch names instead of
patch names for "branch" and "rebase".

Unfortunately, the current completion framework does not seem flexible
enough to get branch completion after "-b" flags, but I feel things
are already better this way.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 contrib/stgit-completion.bash |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/contrib/stgit-completion.bash b/contrib/stgit-completion.bash
index 760fc2f..d497098 100644
--- a/contrib/stgit-completion.bash
+++ b/contrib/stgit-completion.bash
@@ -98,6 +98,12 @@ _all_other_patches ()
         | grep -v "^$(cat $g/patches/$b/current 2> /dev/null)$"
 }
 
+_all_branches ()
+{
+    local g=$(_gitdir)
+    [ "$g" ] && (cd .git/patches/ && echo *)
+}
+
 # List the command options
 _cmd_options ()
 {
@@ -137,6 +143,11 @@ _complete_patch_range_options ()
     COMPREPLY=($(compgen -W "$options" -- "$cur"))
 }
 
+_complete_branch ()
+{
+     COMPREPLY=($(compgen -W "$(_cmd_options $1) $($2)" -- "${COMP_WORDS[COMP_CWORD]}"))
+}
+
 # Generate completions for options from the given list.
 _complete_options ()
 {
@@ -211,6 +222,9 @@ _stg ()
         sync)   _stg_patches $command _applied_patches ;;
         # working-copy commands
         diff)   _stg_patches_options $command _applied_patches "-r --range" ;;
+	# commands that usually raher accept branches
+	branch) _complete_branch $command _all_branches ;;
+	rebase) _complete_branch $command _all_branches ;;
         # all the other commands
         *)      _stg_common $command ;;
     esac

^ permalink raw reply related

* Re: [PATCH] Document "commit --only".
From: Nicolas Pitre @ 2007-05-06 23:13 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <11784859184184-git-send-email-Matthieu.Moy@imag.fr>

On Sun, 6 May 2007, Matthieu Moy wrote:

> The documentation was there earlier, but removed by
> 4170a19587280eeb3663a47a6fd993910de78076.
> 
> That option being the default now, it could perhaps be actually
> removed, but since it is there, and mentionned in the SYNOPSIS, it
> should be actually documented.

I don't think this is a good idea either.

> +-o|--only::
> +	Commit only the files specified on the command line.
> +	This format cannot be used during a merge. This is the
> +	default.

This looks really confusing.  This is the default _only_ if files are 
specified on the command line.

IMHO I'd rather remove the option from the synopsis and flag it as 
scheduled for removal in the source instead.


Nicolas

^ permalink raw reply

* Re: [PATCH v3] Support ent:relative_path
From: Johannes Schindelin @ 2007-05-06 23:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Dana How, Git Mailing List, Shawn O. Pearce
In-Reply-To: <7v1whtdfd2.fsf@assigned-by-dhcp.cox.net>

Hi,

On Sun, 6 May 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Sat, 5 May 2007, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >> 
> >> >> (a) In a bare repository, I believe 
> >> >> setup.c:setup_git_directory_gently() determines the prefix to be 
> >> >> NULL.  This means my patch will see ALL paths as absolute, except 
> >> >> :../path which will result in an error.
> >> >
> >> > My point was that it feels inconsistent to take the current path into 
> >> > account in one case, but not in the other.
> >> 
> >> I do not understand your reasoning.  In a bare repository you cannot 
> >> even be in a subdirectory to begin with.
> >
> > Exactly! That is my point. If you can do it in a working directory, but 
> > also with a bare repository, I find it highly confusing and inconsistent 
> > to have different meaning.
> 
> Sorry.  Now you confused me further.  I can do:
> 
> 	cd Documentation
>         git diff v1.5.0 v1.5.1 -- git.txt
> 
> Is that confusing, inconsistent and bad for the users?

Well, I am partly at fault that you _can_ execute git-diff outside of a 
repository.

But given the _arguments_ you give to git-diff as above, I'd expect it to 
actually care about the working directory. IOW I would _not_ expect this 
to work outside of a working directory (even if it does).

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v3] Support ent:relative_path
From: Johannes Schindelin @ 2007-05-06 23:09 UTC (permalink / raw)
  To: Martin Waitz; +Cc: Junio C Hamano, Shawn O. Pearce, Dana How, Git Mailing List
In-Reply-To: <20070506185259.GF30511@admingilde.org>

Hi,

On Sun, 6 May 2007, Martin Waitz wrote:

> On Sun, May 06, 2007 at 02:59:55AM +0200, Johannes Schindelin wrote:
> > > On Sat, May 05, 2007 at 01:17:35PM -0700, Junio C Hamano wrote:
> > > > > we could also introduce "<tree-ish>/<path>" for absolute path entries.
> > > > 
> > > > When you name the tree-ish with usual "branch name", where does
> > > > the branch name end and pathname start?  What happens when there
> > > > is an ambiguity, and how costly to detect such an ambiguity to
> > > > begin with?
> > > 
> > > well, if you know that it starts with a tree-ish there is no
> > > ambiguity [...]
> > 
> > Wrong. For example, mw/submodules~10 _is_ a tree-ish (if you have a branch 
> > named "mw/submodules").
> 
> so what?

So what: it proves that your case is wrong. mw/submodules~10/README would 
be _severely_ confused.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Document the fact that commit -a is the way to go for simple operations
From: Nicolas Pitre @ 2007-05-06 23:07 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <11784859173386-git-send-email-Matthieu.Moy@imag.fr>

On Sun, 6 May 2007, Matthieu Moy wrote:

> 
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>  Documentation/git-commit.txt |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> index 2895225..d024c03 100644
> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -54,7 +54,8 @@ OPTIONS
>  -a|--all::
>  	Tell the command to automatically stage files that have
>  	been modified and deleted, but new files you have not
> -	told git about are not affected.
> +	told git about are not affected.  You should use it for most
> +        trivial operations.

I don't think this patch is worth it.  If anything it might create more 
confusion.

There are many ways to commit without -a which still could be considered 
amongst "most trivial operations".


Nicolas

^ permalink raw reply

* [PATCH 7/6] todo: Remove "Gitweb diff on merge commits" entry
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski
In-Reply-To: <11784930091585-git-send-email-jnareb@gmail.com>

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch applies to 'todo' branch!

Perhaps TODO entry should be replaced (updated) to say about format
of gitweb diff for merge commits (diffstat in HTML, improving difftree
for merges), instead of just deleting it.

 TODO |   11 -----------
 1 files changed, 0 insertions(+), 11 deletions(-)

diff --git a/TODO b/TODO
index 6c603db..305363a 100644
--- a/TODO
+++ b/TODO
@@ -85,17 +85,6 @@ the box.
 
 [jc: limbo?]
 
-* Gitweb diff on merge commits
-
-From: Linus Torvalds <torvalds@osdl.org>
-Subject: Re: git show and gitweb gives different result for kernel
-Message-ID: <Pine.LNX.4.64.0610061202060.3952@g5.osdl.org>
-
-Maybe allow gitweb to show diff with any parent and diff --cc,
-not just diff with the first parent for a merge.
-
-[jc: Jakub is interested in it]
-
 * Delegate gitweb part to somebody else.
 
 * Use gitattributes for more things.
-- 
1.5.1.3

^ permalink raw reply related

* [PATCH 4/6] gitweb: Make it possible to use pre-parsed info in git_difftree_body
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski
In-Reply-To: <11784930091585-git-send-email-jnareb@gmail.com>

Make it possible to use pre-parsed, or generated by hand, difftree
info in git_difftree_body, similarly to how was and is it done in
git_patchset_body.

Use just introduced feature in git_commitdiff to parse difftree info
(raw diff output) only once: difftree info is now parsed in
git_commitdiff directly, and parsed information is passed to both
git_difftree_body and git_patchset_body. (Till now only git_blobdiff
made use of git_patchset_body ability to use pre-parsed or hand
generated info.) Additionally this makes rename info for combined diff
with renames (or copies) calculated only once in git_difftree_body;
the $difftree is modified and git_patchset_body makes use of added
info.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This corrects what was mentioned in the preceding commit (patch):

  Generating "src" file name for renames in combined diff was separated
  into fill_from_file_info subroutine; git_difftree_body was modified to
  use it. Currently git_difftree_body and git_patchset_body fills this
  info separately.

Now git_difftree_body fills this info, and git_patchset_body uses it
in the 'commitdiff' view (git_commitdiff subroutine).

 gitweb/gitweb.perl |  139 +++++++++++++++++++++++++++-------------------------
 1 files changed, 73 insertions(+), 66 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 53ae0b8..b3e2e07 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2330,7 +2330,13 @@ sub git_difftree_body {
 	my $alternate = 1;
 	my $patchno = 0;
 	foreach my $line (@{$difftree}) {
-		my %diff = parse_difftree_raw_line($line);
+		my $diff;
+		if (ref($line) eq "HASH") {
+			# pre-parsed (or generated by hand)
+			$diff = $line;
+		} else {
+			$diff = parse_difftree_raw_line($line);
+		}
 
 		if ($alternate) {
 			print "<tr class=\"dark\">\n";
@@ -2339,21 +2345,22 @@ sub git_difftree_body {
 		}
 		$alternate ^= 1;
 
-		if (exists $diff{'nparents'}) { # combined diff
+		if (exists $diff->{'nparents'}) { # combined diff
 
-			fill_from_file_info(\%diff, @parents);
+			fill_from_file_info($diff, @parents)
+				unless exists $diff->{'from_file'};
 
-			if ($diff{'to_id'} ne ('0' x 40)) {
+			if ($diff->{'to_id'} ne ('0' x 40)) {
 				# file exists in the result (child) commit
 				print "<td>" .
-				      $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
-				                             file_name=>$diff{'to_file'},
+				      $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
+				                             file_name=>$diff->{'to_file'},
 				                             hash_base=>$hash),
-				              -class => "list"}, esc_path($diff{'to_file'})) .
+				              -class => "list"}, esc_path($diff->{'to_file'})) .
 				      "</td>\n";
 			} else {
 				print "<td>" .
-				      esc_path($diff{'to_file'}) .
+				      esc_path($diff->{'to_file'}) .
 				      "</td>\n";
 			}
 
@@ -2368,11 +2375,11 @@ sub git_difftree_body {
 
 			my $has_history = 0;
 			my $not_deleted = 0;
-			for (my $i = 0; $i < $diff{'nparents'}; $i++) {
+			for (my $i = 0; $i < $diff->{'nparents'}; $i++) {
 				my $hash_parent = $parents[$i];
-				my $from_hash = $diff{'from_id'}[$i];
-				my $from_path = $diff{'from_file'}[$i];
-				my $status = $diff{'status'}[$i];
+				my $from_hash = $diff->{'from_id'}[$i];
+				my $from_path = $diff->{'from_file'}[$i];
+				my $status = $diff->{'status'}[$i];
 
 				$has_history ||= ($status ne 'A');
 				$not_deleted ||= ($status ne 'D');
@@ -2388,17 +2395,17 @@ sub git_difftree_body {
 					              "blob" . ($i+1)) .
 					      " | </td>\n";
 				} else {
-					if ($diff{'to_id'} eq $from_hash) {
+					if ($diff->{'to_id'} eq $from_hash) {
 						print "<td class=\"link nochange\">";
 					} else {
 						print "<td class=\"link\">";
 					}
 					print $cgi->a({-href => href(action=>"blobdiff",
-					                             hash=>$diff{'to_id'},
+					                             hash=>$diff->{'to_id'},
 					                             hash_parent=>$from_hash,
 					                             hash_base=>$hash,
 					                             hash_parent_base=>$hash_parent,
-					                             file_name=>$diff{'to_file'},
+					                             file_name=>$diff->{'to_file'},
 					                             file_parent=>$from_path)},
 					              "diff" . ($i+1)) .
 					      " | </td>\n";
@@ -2408,15 +2415,15 @@ sub git_difftree_body {
 			print "<td class=\"link\">";
 			if ($not_deleted) {
 				print $cgi->a({-href => href(action=>"blob",
-				                             hash=>$diff{'to_id'},
-				                             file_name=>$diff{'to_file'},
+				                             hash=>$diff->{'to_id'},
+				                             file_name=>$diff->{'to_file'},
 				                             hash_base=>$hash)},
 				              "blob");
 				print " | " if ($has_history);
 			}
 			if ($has_history) {
 				print $cgi->a({-href => href(action=>"history",
-				                             file_name=>$diff{'to_file'},
+				                             file_name=>$diff->{'to_file'},
 				                             hash_base=>$hash)},
 				              "history");
 			}
@@ -2429,29 +2436,29 @@ sub git_difftree_body {
 
 		my ($to_mode_oct, $to_mode_str, $to_file_type);
 		my ($from_mode_oct, $from_mode_str, $from_file_type);
-		if ($diff{'to_mode'} ne ('0' x 6)) {
-			$to_mode_oct = oct $diff{'to_mode'};
+		if ($diff->{'to_mode'} ne ('0' x 6)) {
+			$to_mode_oct = oct $diff->{'to_mode'};
 			if (S_ISREG($to_mode_oct)) { # only for regular file
 				$to_mode_str = sprintf("%04o", $to_mode_oct & 0777); # permission bits
 			}
-			$to_file_type = file_type($diff{'to_mode'});
+			$to_file_type = file_type($diff->{'to_mode'});
 		}
-		if ($diff{'from_mode'} ne ('0' x 6)) {
-			$from_mode_oct = oct $diff{'from_mode'};
+		if ($diff->{'from_mode'} ne ('0' x 6)) {
+			$from_mode_oct = oct $diff->{'from_mode'};
 			if (S_ISREG($to_mode_oct)) { # only for regular file
 				$from_mode_str = sprintf("%04o", $from_mode_oct & 0777); # permission bits
 			}
-			$from_file_type = file_type($diff{'from_mode'});
+			$from_file_type = file_type($diff->{'from_mode'});
 		}
 
-		if ($diff{'status'} eq "A") { # created
+		if ($diff->{'status'} eq "A") { # created
 			my $mode_chng = "<span class=\"file_status new\">[new $to_file_type";
 			$mode_chng   .= " with mode: $to_mode_str" if $to_mode_str;
 			$mode_chng   .= "]</span>";
 			print "<td>";
-			print $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
-			                             hash_base=>$hash, file_name=>$diff{'file'}),
-			              -class => "list"}, esc_path($diff{'file'}));
+			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
+			                             hash_base=>$hash, file_name=>$diff->{'file'}),
+			              -class => "list"}, esc_path($diff->{'file'}));
 			print "</td>\n";
 			print "<td>$mode_chng</td>\n";
 			print "<td class=\"link\">";
@@ -2461,17 +2468,17 @@ sub git_difftree_body {
 				print $cgi->a({-href => "#patch$patchno"}, "patch");
 				print " | ";
 			}
-			print $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
-			                             hash_base=>$hash, file_name=>$diff{'file'})},
+			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
+			                             hash_base=>$hash, file_name=>$diff->{'file'})},
 			              "blob");
 			print "</td>\n";
 
-		} elsif ($diff{'status'} eq "D") { # deleted
+		} elsif ($diff->{'status'} eq "D") { # deleted
 			my $mode_chng = "<span class=\"file_status deleted\">[deleted $from_file_type]</span>";
 			print "<td>";
-			print $cgi->a({-href => href(action=>"blob", hash=>$diff{'from_id'},
-			                             hash_base=>$parent, file_name=>$diff{'file'}),
-			               -class => "list"}, esc_path($diff{'file'}));
+			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'from_id'},
+			                             hash_base=>$parent, file_name=>$diff->{'file'}),
+			               -class => "list"}, esc_path($diff->{'file'}));
 			print "</td>\n";
 			print "<td>$mode_chng</td>\n";
 			print "<td class=\"link\">";
@@ -2481,22 +2488,22 @@ sub git_difftree_body {
 				print $cgi->a({-href => "#patch$patchno"}, "patch");
 				print " | ";
 			}
-			print $cgi->a({-href => href(action=>"blob", hash=>$diff{'from_id'},
-			                             hash_base=>$parent, file_name=>$diff{'file'})},
+			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'from_id'},
+			                             hash_base=>$parent, file_name=>$diff->{'file'})},
 			              "blob") . " | ";
 			if ($have_blame) {
 				print $cgi->a({-href => href(action=>"blame", hash_base=>$parent,
-				                             file_name=>$diff{'file'})},
+				                             file_name=>$diff->{'file'})},
 				              "blame") . " | ";
 			}
 			print $cgi->a({-href => href(action=>"history", hash_base=>$parent,
-			                             file_name=>$diff{'file'})},
+			                             file_name=>$diff->{'file'})},
 			              "history");
 			print "</td>\n";
 
-		} elsif ($diff{'status'} eq "M" || $diff{'status'} eq "T") { # modified, or type changed
+		} elsif ($diff->{'status'} eq "M" || $diff->{'status'} eq "T") { # modified, or type changed
 			my $mode_chnge = "";
-			if ($diff{'from_mode'} != $diff{'to_mode'}) {
+			if ($diff->{'from_mode'} != $diff->{'to_mode'}) {
 				$mode_chnge = "<span class=\"file_status mode_chnge\">[changed";
 				if ($from_file_type ne $to_file_type) {
 					$mode_chnge .= " from $from_file_type to $to_file_type";
@@ -2511,9 +2518,9 @@ sub git_difftree_body {
 				$mode_chnge .= "]</span>\n";
 			}
 			print "<td>";
-			print $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
-			                             hash_base=>$hash, file_name=>$diff{'file'}),
-			              -class => "list"}, esc_path($diff{'file'}));
+			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
+			                             hash_base=>$hash, file_name=>$diff->{'file'}),
+			              -class => "list"}, esc_path($diff->{'file'}));
 			print "</td>\n";
 			print "<td>$mode_chnge</td>\n";
 			print "<td class=\"link\">";
@@ -2522,70 +2529,70 @@ sub git_difftree_body {
 				$patchno++;
 				print $cgi->a({-href => "#patch$patchno"}, "patch") .
 				      " | ";
-			} elsif ($diff{'to_id'} ne $diff{'from_id'}) {
+			} elsif ($diff->{'to_id'} ne $diff->{'from_id'}) {
 				# "commit" view and modified file (not onlu mode changed)
 				print $cgi->a({-href => href(action=>"blobdiff",
-				                             hash=>$diff{'to_id'}, hash_parent=>$diff{'from_id'},
+				                             hash=>$diff->{'to_id'}, hash_parent=>$diff->{'from_id'},
 				                             hash_base=>$hash, hash_parent_base=>$parent,
-				                             file_name=>$diff{'file'})},
+				                             file_name=>$diff->{'file'})},
 				              "diff") .
 				      " | ";
 			}
-			print $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
-			                             hash_base=>$hash, file_name=>$diff{'file'})},
+			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
+			                             hash_base=>$hash, file_name=>$diff->{'file'})},
 			               "blob") . " | ";
 			if ($have_blame) {
 				print $cgi->a({-href => href(action=>"blame", hash_base=>$hash,
-				                             file_name=>$diff{'file'})},
+				                             file_name=>$diff->{'file'})},
 				              "blame") . " | ";
 			}
 			print $cgi->a({-href => href(action=>"history", hash_base=>$hash,
-			                             file_name=>$diff{'file'})},
+			                             file_name=>$diff->{'file'})},
 			              "history");
 			print "</td>\n";
 
-		} elsif ($diff{'status'} eq "R" || $diff{'status'} eq "C") { # renamed or copied
+		} elsif ($diff->{'status'} eq "R" || $diff->{'status'} eq "C") { # renamed or copied
 			my %status_name = ('R' => 'moved', 'C' => 'copied');
-			my $nstatus = $status_name{$diff{'status'}};
+			my $nstatus = $status_name{$diff->{'status'}};
 			my $mode_chng = "";
-			if ($diff{'from_mode'} != $diff{'to_mode'}) {
+			if ($diff->{'from_mode'} != $diff->{'to_mode'}) {
 				# mode also for directories, so we cannot use $to_mode_str
 				$mode_chng = sprintf(", mode: %04o", $to_mode_oct & 0777);
 			}
 			print "<td>" .
 			      $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-			                             hash=>$diff{'to_id'}, file_name=>$diff{'to_file'}),
-			              -class => "list"}, esc_path($diff{'to_file'})) . "</td>\n" .
+			                             hash=>$diff->{'to_id'}, file_name=>$diff->{'to_file'}),
+			              -class => "list"}, esc_path($diff->{'to_file'})) . "</td>\n" .
 			      "<td><span class=\"file_status $nstatus\">[$nstatus from " .
 			      $cgi->a({-href => href(action=>"blob", hash_base=>$parent,
-			                             hash=>$diff{'from_id'}, file_name=>$diff{'from_file'}),
-			              -class => "list"}, esc_path($diff{'from_file'})) .
-			      " with " . (int $diff{'similarity'}) . "% similarity$mode_chng]</span></td>\n" .
+			                             hash=>$diff->{'from_id'}, file_name=>$diff->{'from_file'}),
+			              -class => "list"}, esc_path($diff->{'from_file'})) .
+			      " with " . (int $diff->{'similarity'}) . "% similarity$mode_chng]</span></td>\n" .
 			      "<td class=\"link\">";
 			if ($action eq 'commitdiff') {
 				# link to patch
 				$patchno++;
 				print $cgi->a({-href => "#patch$patchno"}, "patch") .
 				      " | ";
-			} elsif ($diff{'to_id'} ne $diff{'from_id'}) {
+			} elsif ($diff->{'to_id'} ne $diff->{'from_id'}) {
 				# "commit" view and modified file (not only pure rename or copy)
 				print $cgi->a({-href => href(action=>"blobdiff",
-				                             hash=>$diff{'to_id'}, hash_parent=>$diff{'from_id'},
+				                             hash=>$diff->{'to_id'}, hash_parent=>$diff->{'from_id'},
 				                             hash_base=>$hash, hash_parent_base=>$parent,
-				                             file_name=>$diff{'to_file'}, file_parent=>$diff{'from_file'})},
+				                             file_name=>$diff->{'to_file'}, file_parent=>$diff->{'from_file'})},
 				              "diff") .
 				      " | ";
 			}
-			print $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
-			                             hash_base=>$parent, file_name=>$diff{'to_file'})},
+			print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
+			                             hash_base=>$parent, file_name=>$diff->{'to_file'})},
 			              "blob") . " | ";
 			if ($have_blame) {
 				print $cgi->a({-href => href(action=>"blame", hash_base=>$hash,
-				                             file_name=>$diff{'to_file'})},
+				                             file_name=>$diff->{'to_file'})},
 				              "blame") . " | ";
 			}
 			print $cgi->a({-href => href(action=>"history", hash_base=>$hash,
-			                            file_name=>$diff{'to_file'})},
+			                            file_name=>$diff->{'to_file'})},
 			              "history");
 			print "</td>\n";
 
@@ -4401,7 +4408,7 @@ sub git_commitdiff {
 			chomp $line;
 			# empty line ends raw part of diff-tree output
 			last unless $line;
-			push @difftree, $line;
+			push @difftree, scalar parse_difftree_raw_line($line);
 		}
 
 	} elsif ($format eq 'plain') {
-- 
1.5.1.3

^ permalink raw reply related

* [PATCH 6/6] gitweb: Show combined diff for merge commits in 'commit' view
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski
In-Reply-To: <11784930091585-git-send-email-jnareb@gmail.com>

When commit shown is a merge commit (has more than one parent),
display combined difftree output (result of git-diff-tree -c).
Earlier (since commit 549ab4a30703012ff3a12b5455d319216805a8db)
difftree output (against first parent) was not printed for merges.

Examples of non-trivial merges:
  5bac4a671907604b5fb4e24ff682d5b0e8431931 (includes rename)
  addafaf92eeb86033da91323d0d3ad7a496dae83 (five parents)
  95f97567c1887d77f3a46b42d8622c76414d964d (evil merge)

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
The same note as for the patch before apply: for trivial merges there
is no difftree output. But for 'commit' view it doesn't matter much
because there was no difftree output at all for merges.

I plan to add HTML diffstat (using divs with set background color and
set width for bar graph for diff stats) for merges for 'commit' view
(diffstat to first parent, aka. "damages"), and text diffstat for
'commitdiff_plain' view. It would be controlled by new %feature.
But if somebody else want to implement this, feel free.


Examples are included to make it possible to check combined diff
output in gitweb: the sha1 of commits should be turned into
hyperlinks (committags support), and you can click on them to go to
'commit' view.

 gitweb/gitweb.perl |   20 ++++++++------------
 1 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c0e2473..53e6dce 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4026,14 +4026,13 @@ sub git_commit {
 		$parent = "--root";
 	}
 	my @difftree;
-	if (@$parents <= 1) {
-		# difftree output is not printed for merges
-		open my $fd, "-|", git_cmd(), "diff-tree", '-r', "--no-commit-id",
-			@diff_opts, $parent, $hash, "--"
-			or die_error(undef, "Open git-diff-tree failed");
-		@difftree = map { chomp; $_ } <$fd>;
-		close $fd or die_error(undef, "Reading git-diff-tree failed");
-	}
+	open my $fd, "-|", git_cmd(), "diff-tree", '-r', "--no-commit-id",
+		@diff_opts, 
+		(@$parents <= 1 ? $parent : '-c'),
+		$hash, "--"
+		or die_error(undef, "Open git-diff-tree failed");
+	@difftree = map { chomp; $_ } <$fd>;
+	close $fd or die_error(undef, "Reading git-diff-tree failed");
 
 	# non-textual hash id's can be cached
 	my $expires;
@@ -4111,10 +4110,7 @@ sub git_commit {
 	git_print_log($co{'comment'});
 	print "</div>\n";
 
-	if (@$parents <= 1) {
-		# do not output difftree/whatchanged for merges
-		git_difftree_body(\@difftree, $hash, $parent);
-	}
+	git_difftree_body(\@difftree, $hash, @$parents);
 
 	git_footer_html();
 }
-- 
1.5.1.3

^ permalink raw reply related

* [PATCH 5/6] gitweb: Show combined diff for merge commits in 'commitdiff' view
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski
In-Reply-To: <11784930091585-git-send-email-jnareb@gmail.com>

When 'commitdiff' action is requested without 'hp' (hash parent)
parameter, and commit given by 'h' (hash) parameter is merge commit,
show merge as combined diff.

Earlier for merge commits without 'hp' parameter diff to first parent
was shown.

Note that in compact combined (--cc) format 'uninteresting' hunks
omission mechanism can make that there is no patch corresponding to
line in raw format (difftree) output. That is why (at least for now)
we use --combined and not --cc format for showing commitdiff for merge
commits.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Note the following fragment of combined diff format description from
git-diff-tree(1) man page; note the final sentence

-c::
        This flag changes the way a merge commit is displayed
        (which means it is useful only when the command is given
        one <tree-ish>, or '--stdin').  It shows the differences
        from each of the parents to the merge result simultaneously
        instead of showing pairwise diff between a parent and the
        result one at a time (which is what the '-m' option does).
        Furthermore, _it lists only files which were modified
        from all parents_.

This means that now 'commitdiff' would show empty diff for all trival
(tree-level) merges, which I think is a majority of merges. Is showing
empty diff better than diff to first parent for merges, then? Or do we
need some way to show from which parent was final version of a file
taken?

Currently there is no way to generate 'commitdiff' view to one of the
parents by clicking some link directly from 'commitdiff' view: you
have to go via 'commit' view (click on commit subject, which functions
as switch between 'commitdiff' and 'commit' views, then on "diff" link
next to one of the parents in commit header). Perhaps this can be
improved with improving difftree/whatchanged output for combined
diff.

 gitweb/gitweb.perl |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b3e2e07..c0e2473 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4391,8 +4391,10 @@ sub git_commitdiff {
 		}
 	}
 
+	my $hash_parent_param = $hash_parent;
 	if (!defined $hash_parent) {
-		$hash_parent = $co{'parent'} || '--root';
+		$hash_parent_param =
+			@{$co{'parents'}} > 1 ? '-c' : $co{'parent'} || '--root';
 	}
 
 	# read commitdiff
@@ -4401,7 +4403,7 @@ sub git_commitdiff {
 	if ($format eq 'html') {
 		open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
 			"--no-commit-id", "--patch-with-raw", "--full-index",
-			$hash_parent, $hash, "--"
+			$hash_parent_param, $hash, "--"
 			or die_error(undef, "Open git-diff-tree failed");
 
 		while (my $line = <$fd>) {
@@ -4413,7 +4415,7 @@ sub git_commitdiff {
 
 	} elsif ($format eq 'plain') {
 		open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
-			'-p', $hash_parent, $hash, "--"
+			'-p', $hash_parent_param, $hash, "--"
 			or die_error(undef, "Open git-diff-tree failed");
 
 	} else {
@@ -4469,10 +4471,10 @@ TEXT
 
 	# write patch
 	if ($format eq 'html') {
-		git_difftree_body(\@difftree, $hash, $hash_parent);
+		git_difftree_body(\@difftree, $hash, $hash_parent || @{$co{'parents'}});
 		print "<br/>\n";
 
-		git_patchset_body($fd, \@difftree, $hash, $hash_parent);
+		git_patchset_body($fd, \@difftree, $hash, $hash_parent || @{$co{'parents'}});
 		close $fd;
 		print "</div>\n"; # class="page_body"
 		git_footer_html();
-- 
1.5.1.3

^ permalink raw reply related

* [PATCH 3/6] gitweb: Add combined diff support to git_patchset_body
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski
In-Reply-To: <11784930091585-git-send-email-jnareb@gmail.com>

Calling convention for combined diff similar to the one for
git_difftree_body subroutine: difftree info (first parameter) must be
result of calling git-diff-tree with -c/--cc option, and all parents
of a commit must be passed as last parameters. See also description in
  "gitweb: Add combined diff support to git_difftree_body"

This ability is not used yet.

Generating "src" file name for renames in combined diff was separated
into fill_from_file_info subroutine; git_difftree_body was modified to
use it. Currently git_difftree_body and git_patchset_body fills this
info separately.

The from-file line in two-line from-file/to-file header is not
hyperlinked: there can be more than one "from"/"src" file. This
differs from HTML output of ordinary (not combined) diff.

format_diff_line subroutine needs extra $from/$to parameters to format
combined diff patch line correctly.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Currently the following extended git diff header line in combined diff

	new file mode <mode>

gets its <mode> (filetype) explained like for appropriate extended
header lines for non-combined (ordinary) diff

	old mode <mode>
	new mode <mode>
	deleted file mode <mode>
	new file mode <mode>
	index <hash>..<hash> <mode>

while similar line

	deleted file mode <mode>,<mode>

does not get <mode> explained. Perhaps we should remove this addition,
or make explanation using tooltip (and perhaps <abbr> HTML element).

 gitweb/gitweb.perl |  221 ++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 181 insertions(+), 40 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c6a2fef..53ae0b8 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -897,19 +897,34 @@ sub format_subject_html {
 sub format_diff_line {
 	my $line = shift;
 	my ($from, $to) = @_;
-	my $char = substr($line, 0, 1);
 	my $diff_class = "";
 
 	chomp $line;
 
-	if ($char eq '+') {
-		$diff_class = " add";
-	} elsif ($char eq "-") {
-		$diff_class = " rem";
-	} elsif ($char eq "@") {
-		$diff_class = " chunk_header";
-	} elsif ($char eq "\\") {
-		$diff_class = " incomplete";
+	if ($from && $to && ref($from->{'href'}) eq "ARRAY") {
+		# combined diff
+		my $prefix = substr($line, 0, scalar @{$from->{'href'}});
+		if ($line =~ m/^\@{3}/) {
+			$diff_class = " chunk_header";
+		} elsif ($line =~ m/^\\/) {
+			$diff_class = " incomplete";
+		} elsif ($prefix =~ tr/+/+/) {
+			$diff_class = " add";
+		} elsif ($prefix =~ tr/-/-/) {
+			$diff_class = " rem";
+		}
+	} else {
+		# assume ordinary diff
+		my $char = substr($line, 0, 1);
+		if ($char eq '+') {
+			$diff_class = " add";
+		} elsif ($char eq '-') {
+			$diff_class = " rem";
+		} elsif ($char eq '@') {
+			$diff_class = " chunk_header";
+		} elsif ($char eq "\\") {
+			$diff_class = " incomplete";
+		}
 	}
 	$line = untabify($line);
 	if ($from && $to && $line =~ m/^\@{2} /) {
@@ -930,6 +945,39 @@ sub format_diff_line {
 		$line = "<span class=\"chunk_info\">@@ $from_text $to_text @@</span>" .
 		        "<span class=\"section\">" . esc_html($section, -nbsp=>1) . "</span>";
 		return "<div class=\"diff$diff_class\">$line</div>\n";
+	} elsif ($from && $to && $line =~ m/^\@{3}/) {
+		my ($prefix, $ranges, $section) = $line =~ m/^(\@+) (.*?) \@+(.*)$/;
+		my (@from_text, @from_start, @from_nlines, $to_text, $to_start, $to_nlines);
+
+		@from_text = split(' ', $ranges);
+		for (my $i = 0; $i < @from_text; ++$i) {
+			($from_start[$i], $from_nlines[$i]) =
+				(split(',', substr($from_text[$i], 1)), 0);
+		}
+
+		$to_text   = pop @from_text;
+		$to_start  = pop @from_start;
+		$to_nlines = pop @from_nlines;
+
+		$line = "<span class=\"chunk_info\">$prefix ";
+		for (my $i = 0; $i < @from_text; ++$i) {
+			if ($from->{'href'}[$i]) {
+				$line .= $cgi->a({-href=>"$from->{'href'}[$i]#l$from_start[$i]",
+				                  -class=>"list"}, $from_text[$i]);
+			} else {
+				$line .= $from_text[$i];
+			}
+			$line .= " ";
+		}
+		if ($to->{'href'}) {
+			$line .= $cgi->a({-href=>"$to->{'href'}#l$to_start",
+			                  -class=>"list"}, $to_text);
+		} else {
+			$line .= $to_text;
+		}
+		$line .= " $prefix</span>" .
+		         "<span class=\"section\">" . esc_html($section, -nbsp=>1) . "</span>";
+		return "<div class=\"diff$diff_class\">$line</div>\n";
 	}
 	return "<div class=\"diff$diff_class\">" . esc_html($line, -nbsp=>1) . "</div>\n";
 }
@@ -2233,6 +2281,39 @@ sub git_print_tree_entry {
 ## ......................................................................
 ## functions printing large fragments of HTML
 
+sub fill_from_file_info {
+	my ($diff, @parents) = @_;
+
+	$diff->{'from_file'} = [ ];
+	$diff->{'from_file'}[$diff->{'nparents'} - 1] = undef;
+	for (my $i = 0; $i < $diff->{'nparents'}; $i++) {
+		if ($diff->{'status'}[$i] eq 'R' ||
+		    $diff->{'status'}[$i] eq 'C') {
+			$diff->{'from_file'}[$i] =
+				git_get_path_by_hash($parents[$i], $diff->{'from_id'}[$i]);
+		}
+	}
+
+	return $diff;
+}
+
+# parameters can be strings, or references to arrays of strings
+sub from_ids_eq {
+	my ($a, $b) = @_;
+
+	if (ref($a) eq "ARRAY" && ref($b) eq "ARRAY" && @$a == @$b) {
+		for (my $i = 0; $i < @$a; ++$i) {
+			return 0 unless ($a->[$i] eq $b->[$i]);
+		}
+		return 1;
+	} elsif (!ref($a) && !ref($b)) {
+		return $a eq $b;
+	} else {
+		return 0;
+	}
+}
+
+
 sub git_difftree_body {
 	my ($difftree, $hash, @parents) = @_;
 	my ($parent) = $parents[0];
@@ -2260,6 +2341,8 @@ sub git_difftree_body {
 
 		if (exists $diff{'nparents'}) { # combined diff
 
+			fill_from_file_info(\%diff, @parents);
+
 			if ($diff{'to_id'} ne ('0' x 40)) {
 				# file exists in the result (child) commit
 				print "<td>" .
@@ -2288,16 +2371,12 @@ sub git_difftree_body {
 			for (my $i = 0; $i < $diff{'nparents'}; $i++) {
 				my $hash_parent = $parents[$i];
 				my $from_hash = $diff{'from_id'}[$i];
-				my $from_path = undef;
+				my $from_path = $diff{'from_file'}[$i];
 				my $status = $diff{'status'}[$i];
 
 				$has_history ||= ($status ne 'A');
 				$not_deleted ||= ($status ne 'D');
 
-				if ($status eq 'R' || $status eq 'C') {
-					$from_path = git_get_path_by_hash($hash_parent, $from_hash);
-				}
-
 				if ($status eq 'A') {
 					print "<td  class=\"link\" align=\"right\"> | </td>\n";
 				} elsif ($status eq 'D') {
@@ -2517,7 +2596,8 @@ sub git_difftree_body {
 }
 
 sub git_patchset_body {
-	my ($fd, $difftree, $hash, $hash_parent) = @_;
+	my ($fd, $difftree, $hash, @hash_parents) = @_;
+	my ($hash_parent) = $hash_parents[0];
 
 	my $patch_idx = 0;
 	my $patch_number = 0;
@@ -2555,6 +2635,9 @@ sub git_patchset_body {
 			if ($patch_line =~ m/^index ([0-9a-fA-F]{40})..([0-9a-fA-F]{40})/) {
 				$from_id = $1;
 				$to_id   = $2;
+			} elsif ($patch_line =~ m/^index ((?:[0-9a-fA-F]{40},)+[0-9a-fA-F]{40})..([0-9a-fA-F]{40})/) {
+				$from_id = [ split(',', $1) ];
+				$to_id   = $2;
 			}
 
 			push @diff_header, $patch_line;
@@ -2564,8 +2647,8 @@ sub git_patchset_body {
 		# check if current patch belong to current raw line
 		# and parse raw git-diff line if needed
 		if (defined $diffinfo &&
-		    $diffinfo->{'from_id'} eq $from_id &&
-		    $diffinfo->{'to_id'}   eq $to_id) {
+		    from_ids_eq($diffinfo->{'from_id'}, $from_id) &&
+		    $diffinfo->{'to_id'} eq $to_id) {
 			# this is split patch
 			print "<div class=\"patch cont\">\n";
 		} else {
@@ -2579,15 +2662,34 @@ sub git_patchset_body {
 			} else {
 				$diffinfo = parse_difftree_raw_line($difftree->[$patch_idx]);
 			}
-			$from{'file'} = $diffinfo->{'from_file'} || $diffinfo->{'file'};
-			$to{'file'}   = $diffinfo->{'to_file'}   || $diffinfo->{'file'};
-			if ($diffinfo->{'status'} ne "A") { # not new (added) file
-				$from{'href'} = href(action=>"blob", hash_base=>$hash_parent,
-				                     hash=>$diffinfo->{'from_id'},
-				                     file_name=>$from{'file'});
+			if ($diffinfo->{'nparents'}) {
+				# combined diff
+				$from{'file'} = [];
+				$from{'href'} = [];
+				fill_from_file_info($diffinfo, @hash_parents)
+					unless exists $diffinfo->{'from_file'};
+				for (my $i = 0; $i < $diffinfo->{'nparents'}; $i++) {
+					$from{'file'}[$i] = $diffinfo->{'from_file'}[$i] || $diffinfo->{'to_file'};
+					if ($diffinfo->{'status'}[$i] ne "A") { # not new (added) file
+						$from{'href'}[$i] = href(action=>"blob",
+						                         hash_base=>$hash_parents[$i],
+						                         hash=>$diffinfo->{'from_id'}[$i],
+						                         file_name=>$from{'file'}[$i]);
+					} else {
+						$from{'href'}[$i] = undef;
+					}
+				}
 			} else {
-				delete $from{'href'};
+				$from{'file'} = $diffinfo->{'from_file'} || $diffinfo->{'file'};
+				if ($diffinfo->{'status'} ne "A") { # not new (added) file
+					$from{'href'} = href(action=>"blob", hash_base=>$hash_parent,
+					                     hash=>$diffinfo->{'from_id'},
+					                     file_name=>$from{'file'});
+				} else {
+					delete $from{'href'};
+				}
 			}
+			$to{'file'} = $diffinfo->{'to_file'} || $diffinfo->{'file'};
 			if ($diffinfo->{'status'} ne "D") { # not deleted file
 				$to{'href'} = href(action=>"blob", hash_base=>$hash,
 				                   hash=>$diffinfo->{'to_id'},
@@ -2602,19 +2704,34 @@ sub git_patchset_body {
 
 		# print "git diff" header
 		$patch_line = shift @diff_header;
-		$patch_line =~ s!^(diff (.*?) )"?a/.*$!$1!;
-		if ($from{'href'}) {
-			$patch_line .= $cgi->a({-href => $from{'href'}, -class => "path"},
-			                       'a/' . esc_path($from{'file'}));
-		} else { # file was added
-			$patch_line .= 'a/' . esc_path($from{'file'});
-		}
-		$patch_line .= ' ';
-		if ($to{'href'}) {
-			$patch_line .= $cgi->a({-href => $to{'href'}, -class => "path"},
-			                       'b/' . esc_path($to{'file'}));
-		} else { # file was deleted
-			$patch_line .= 'b/' . esc_path($to{'file'});
+		if ($diffinfo->{'nparents'}) {
+
+			# combined diff
+			$patch_line =~ s!^(diff (.*?) )"?.*$!$1!;
+			if ($to{'href'}) {
+				$patch_line .= $cgi->a({-href => $to{'href'}, -class => "path"},
+				                       esc_path($to{'file'}));
+			} else { # file was deleted
+				$patch_line .= esc_path($to{'file'});
+			}
+
+		} else {
+
+			$patch_line =~ s!^(diff (.*?) )"?a/.*$!$1!;
+			if ($from{'href'}) {
+				$patch_line .= $cgi->a({-href => $from{'href'}, -class => "path"},
+				                       'a/' . esc_path($from{'file'}));
+			} else { # file was added
+				$patch_line .= 'a/' . esc_path($from{'file'});
+			}
+			$patch_line .= ' ';
+			if ($to{'href'}) {
+				$patch_line .= $cgi->a({-href => $to{'href'}, -class => "path"},
+				                       'b/' . esc_path($to{'file'}));
+			} else { # file was deleted
+				$patch_line .= 'b/' . esc_path($to{'file'});
+			}
+
 		}
 		print "<div class=\"diff header\">$patch_line</div>\n";
 
@@ -2631,14 +2748,37 @@ sub git_patchset_body {
 				$patch_line .= $cgi->a({-href=>$to{'href'}, -class=>"path"},
 				                       esc_path($to{'file'}));
 			}
-			# match <mode>
+			# match single <mode>
 			if ($patch_line =~ m/\s(\d{6})$/) {
 				$patch_line .= '<span class="info"> (' .
 				               file_type_long($1) .
 				               ')</span>';
 			}
 			# match <hash>
-			if ($patch_line =~ m/^index/) {
+			if ($patch_line =~ m/^index [0-9a-fA-F]{40},[0-9a-fA-F]{40}/) {
+				# can match only for combined diff
+				$patch_line = 'index ';
+				for (my $i = 0; $i < $diffinfo->{'nparents'}; $i++) {
+					if ($from{'href'}[$i]) {
+						$patch_line .= $cgi->a({-href=>$from{'href'}[$i],
+						                        -class=>"hash"},
+						                       substr($diffinfo->{'from_id'}[$i],0,7));
+					} else {
+						$patch_line .= '0' x 7;
+					}
+					# separator
+					$patch_line .= ',' if ($i < $diffinfo->{'nparents'} - 1);
+				}
+				$patch_line .= '..';
+				if ($to{'href'}) {
+					$patch_line .= $cgi->a({-href=>$to{'href'}, -class=>"hash"},
+					                       substr($diffinfo->{'to_id'},0,7));
+				} else {
+					$patch_line .= '0' x 7;
+				}
+
+			} elsif ($patch_line =~ m/^index [0-9a-fA-F]{40}..[0-9a-fA-F]{40}/) {
+				# can match only for ordinary diff
 				my ($from_link, $to_link);
 				if ($from{'href'}) {
 					$from_link = $cgi->a({-href=>$from{'href'}, -class=>"hash"},
@@ -2674,7 +2814,8 @@ sub git_patchset_body {
 		}
 		next PATCH if ($patch_line =~ m/^diff /);
 		#assert($patch_line =~ m/^---/) if DEBUG;
-		if ($from{'href'} && $patch_line =~ m!^--- "?a/!) {
+		if (!$diffinfo->{'nparents'} && # not from-file line for combined diff
+		    $from{'href'} && $patch_line =~ m!^--- "?a/!) {
 			$patch_line = '--- a/' .
 			              $cgi->a({-href=>$from{'href'}, -class=>"path"},
 			                      esc_path($from{'file'}));
-- 
1.5.1.3

^ permalink raw reply related

* [PATCH 2/6] gitweb: Add combined diff support to git_difftree_body
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski
In-Reply-To: <11784930091585-git-send-email-jnareb@gmail.com>

You have to pass all parents as final parameters of git_difftree_body
subroutine; the number of parents of a diff must be equal to the
number derived from parsing git-diff-tree output, raw combined diff
for git_difftree_body to display combined diff correctly (but it is
not checked).

Currently the possibility of displaying diffree of combined diff is
not used in gitweb code; git_difftree_body is always caled for
ordinary diff, and with only one parent.


Description of output for combined diff:
----------------------------------------

The difftree table for combined diff starts with a cell with pathname
of changed blob (changed file), which if possible is hidden link
(class="list") to the 'blob' view of final version (if it exists),
like for difftree for ordinary diff. If file was deleted in the final
commit then filename is not hyperlinked.

There is no cell with single file status (new, deleted, mode change,
rename), as for combined diff as there is no single status: different
parents might have different status.

If git_difftree_body was called from git_commitdiff (for 'commitdiff'
action) there is inner link to anchor to appropriate fragment (patch)
in patchset body; the "patch" link does not replace "diff" link like
for ordinary diff.

Each of "diff" links is in separate cell, contrary to output for
ordinary diff in which all links are (at least for now) in a single
cell.

For each parent, if file was not present we leave cell empty. If file
was deleted in the result, we provide link to 'blob' view. Otherwise
we provide link to 'commitdiff' view, even if patch (diff) consist
only of extended diff header, and contents is not changed (pure
rename, pure mode change). The only difference is that link to
"blobdiff" view with no contents change is with 'nochange' class.

At last, there is provided link to current version of file as "blob"
link, if the file was not deleted in the result, and lik to history of
a file, if there exists one. (The link to file history might be
confused, at least for now, by renames.)

Note that git-diff-tree raw output dor combined diff does not provide
filename before change for renames and copies; we use
git_get_path_by_hash to get "src" filename for renames (this means
additional call to git-ls-tree for a _whole_ tree).

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Currently the work to find filename _before_ rename for renames (and
copies) in combined diff is done during difftree generation (link
generation). It is possible to change it to have this calculation done
on link target, in 'blobdiff' and 'blob' views which lack 'fp' or 'f'
parameter, and have 'hpb' and 'hp' or 'hb' and 'h' parameters needed
to find path by hash. Nevertheless renames in combined diff format are
rare I think, and non-empty combined diff for merge commit
(non-trivial merge result) is also usually rare.

[The above paragraph should perhaps be added to commit message, but is
quite long even without it.]


This is preliminary version: difftree output for combined diff is a
bit crude, and does not display all the information (mode changes,
renames, etc.). Nevertheless I think it is a good start.

CSS styling is a bit crude.

Patches (or corrections) and ideas how should HTML output of difftree
(whatchanged-like output) for combined diff look like are very
welcome.

 gitweb/gitweb.css  |   17 +++++++
 gitweb/gitweb.perl |  121 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 136 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 2b023bd..e795b70 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -181,6 +181,23 @@ table.diff_tree {
 	font-family: monospace;
 }
 
+table.combined.diff_tree td {
+	padding-right: 24px;
+}
+
+table.combined.diff_tree td.link {
+	padding: 0px 2px;
+}
+
+table.combined.diff_tree td.nochange a {
+	color: #6666ff;
+}
+
+table.combined.diff_tree td.nochange a:hover,
+table.combined.diff_tree td.nochange a:visited {
+	color: #d06666;
+}
+
 table.blame {
 	border-collapse: collapse;
 }
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index dfba399..c6a2fef 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1015,6 +1015,30 @@ sub git_get_hash_by_path {
 	return $3;
 }
 
+# get path of entry with given hash at given tree-ish (ref)
+# used to get 'from' filename for combined diff (merge commit) for renames
+sub git_get_path_by_hash {
+	my $base = shift || return;
+	my $hash = shift || return;
+
+	local $/ = "\0";
+
+	open my $fd, "-|", git_cmd(), "ls-tree", '-r', '-t', '-z', $base
+		or return undef;
+	while (my $line = <$fd>) {
+		chomp $line;
+
+		#'040000 tree 595596a6a9117ddba9fe379b6b012b558bac8423	gitweb'
+		#'100644 blob e02e90f0429be0d2a69b76571101f20b8f75530f	gitweb/README'
+		if ($line =~ m/(?:[0-9]+) (?:.+) $hash\t(.+)$/) {
+			close $fd;
+			return $1;
+		}
+	}
+	close $fd;
+	return undef;
+}
+
 ## ......................................................................
 ## git utility functions, directly accessing git repository
 
@@ -2210,7 +2234,8 @@ sub git_print_tree_entry {
 ## functions printing large fragments of HTML
 
 sub git_difftree_body {
-	my ($difftree, $hash, $parent) = @_;
+	my ($difftree, $hash, @parents) = @_;
+	my ($parent) = $parents[0];
 	my ($have_blame) = gitweb_check_feature('blame');
 	print "<div class=\"list_head\">\n";
 	if ($#{$difftree} > 10) {
@@ -2218,7 +2243,9 @@ sub git_difftree_body {
 	}
 	print "</div>\n";
 
-	print "<table class=\"diff_tree\">\n";
+	print "<table class=\"" .
+	      (@parents > 1 ? "combined " : "") .
+	      "diff_tree\">\n";
 	my $alternate = 1;
 	my $patchno = 0;
 	foreach my $line (@{$difftree}) {
@@ -2231,6 +2258,96 @@ sub git_difftree_body {
 		}
 		$alternate ^= 1;
 
+		if (exists $diff{'nparents'}) { # combined diff
+
+			if ($diff{'to_id'} ne ('0' x 40)) {
+				# file exists in the result (child) commit
+				print "<td>" .
+				      $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
+				                             file_name=>$diff{'to_file'},
+				                             hash_base=>$hash),
+				              -class => "list"}, esc_path($diff{'to_file'})) .
+				      "</td>\n";
+			} else {
+				print "<td>" .
+				      esc_path($diff{'to_file'}) .
+				      "</td>\n";
+			}
+
+			if ($action eq 'commitdiff') {
+				# link to patch
+				$patchno++;
+				print "<td class=\"link\">" .
+				      $cgi->a({-href => "#patch$patchno"}, "patch") .
+				      " | " .
+				      "</td>\n";
+			}
+
+			my $has_history = 0;
+			my $not_deleted = 0;
+			for (my $i = 0; $i < $diff{'nparents'}; $i++) {
+				my $hash_parent = $parents[$i];
+				my $from_hash = $diff{'from_id'}[$i];
+				my $from_path = undef;
+				my $status = $diff{'status'}[$i];
+
+				$has_history ||= ($status ne 'A');
+				$not_deleted ||= ($status ne 'D');
+
+				if ($status eq 'R' || $status eq 'C') {
+					$from_path = git_get_path_by_hash($hash_parent, $from_hash);
+				}
+
+				if ($status eq 'A') {
+					print "<td  class=\"link\" align=\"right\"> | </td>\n";
+				} elsif ($status eq 'D') {
+					print "<td class=\"link\">" .
+					      $cgi->a({-href => href(action=>"blob",
+					                             hash_base=>$hash,
+					                             hash=>$from_hash,
+					                             file_name=>$from_path)},
+					              "blob" . ($i+1)) .
+					      " | </td>\n";
+				} else {
+					if ($diff{'to_id'} eq $from_hash) {
+						print "<td class=\"link nochange\">";
+					} else {
+						print "<td class=\"link\">";
+					}
+					print $cgi->a({-href => href(action=>"blobdiff",
+					                             hash=>$diff{'to_id'},
+					                             hash_parent=>$from_hash,
+					                             hash_base=>$hash,
+					                             hash_parent_base=>$hash_parent,
+					                             file_name=>$diff{'to_file'},
+					                             file_parent=>$from_path)},
+					              "diff" . ($i+1)) .
+					      " | </td>\n";
+				}
+			}
+
+			print "<td class=\"link\">";
+			if ($not_deleted) {
+				print $cgi->a({-href => href(action=>"blob",
+				                             hash=>$diff{'to_id'},
+				                             file_name=>$diff{'to_file'},
+				                             hash_base=>$hash)},
+				              "blob");
+				print " | " if ($has_history);
+			}
+			if ($has_history) {
+				print $cgi->a({-href => href(action=>"history",
+				                             file_name=>$diff{'to_file'},
+				                             hash_base=>$hash)},
+				              "history");
+			}
+			print "</td>\n";
+
+			print "</tr>\n";
+			next; # instead of 'else' clause, to avoid extra indent
+		}
+		# else ordinary diff
+
 		my ($to_mode_oct, $to_mode_str, $to_file_type);
 		my ($from_mode_oct, $from_mode_str, $from_file_type);
 		if ($diff{'to_mode'} ne ('0' x 6)) {
-- 
1.5.1.3

^ permalink raw reply related

* [PATCH 1/6] gitweb: Add parsing of raw combined diff format to parse_difftree_raw_line
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski
In-Reply-To: <11784930091585-git-send-email-jnareb@gmail.com>

Add parsing line of raw combined diff ("git diff-tree -c/-cc" output)
as described in section "diff format for merges" in diff-format.txt
to parse_difftree_raw_line subroutine.

Returned hash (or hashref) has for combined diff 'nparents' key which
holds number of parents in a merge. At keys 'from_mode' and 'from_id'
there are arrayrefs holding modes and ids, respectively. There is no
'similarity' value, and there is only 'to_file' value and no
'from_file' value.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ba5cc43..dfba399 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1495,6 +1495,17 @@ sub parse_difftree_raw_line {
 			$res{'file'} = unquote($7);
 		}
 	}
+	# '::100755 100755 100755 60e79ca1b01bc8b057abe17ddab484699a7f5fdb 94067cc5f73388f33722d52ae02f44692bc07490 94067cc5f73388f33722d52ae02f44692bc07490 MR	git-gui/git-gui.sh'
+	# combined diff (for merge commit)
+	elsif ($line =~ s/^(::+)((?:[0-7]{6} )+)((?:[0-9a-fA-F]{40} )+)([a-zA-Z]+)\t(.*)$//) {
+		$res{'nparents'}  = length($1);
+		$res{'from_mode'} = [ split(' ', $2) ];
+		$res{'to_mode'} = pop @{$res{'from_mode'}};
+		$res{'from_id'} = [ split(' ', $3) ];
+		$res{'to_id'} = pop @{$res{'from_id'}};
+		$res{'status'} = [ split('', $4) ];
+		$res{'to_file'} = unquote($5);
+	}
 	# 'c512b523472485aef4fff9e57b229d9d243c967f'
 	elsif ($line =~ m/^([0-9a-fA-F]{40})$/) {
 		$res{'commit'} = $1;
-- 
1.5.1.3

^ permalink raw reply related

* [PATCH 0/6] gitweb: Add combined diff support
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski

This series of patches adds support for showing combined diff for
merge commits in gitweb, instead of using diff with the first parent
(for 'commitdiff' view), or not showing difftree (for 'commit' view).

We use diff -c (diff --combined) and not diff --cc; the latter would
require modifying git_difftree_body and git_patchset_body for the
possibility that there is line in raw diff output format (in difftree)
that doesn't correspond to any patch: the patch part was removed due
to hunks simplification.

See also notes in individual patches.


Diffstat:
=========

 gitweb/gitweb.css  |   17 ++
 gitweb/gitweb.perl |  480 +++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 394 insertions(+), 103 deletions(-)


Table of contents:
==================

 [PATCH 1/6] gitweb: Add parsing of raw combined diff format to parse_difftree_raw_line
 [PATCH 2/6] gitweb: Add combined diff support to git_difftree_body
 [PATCH 3/6] gitweb: Add combined diff support to git_patchset_body
 [PATCH 4/6] gitweb: Make it possible to use pre-parsed info in git_difftree_body

Those patches prepare for adding support for combined diff for
'commitdiff' and 'commit' views.


 [PATCH 5/6] gitweb: Show combined diff for merge commits in 'commitdiff' view
 [PATCH 6/6] gitweb: Show combined diff for merge commits in 'commit' view

Those patches add (preliminary) generating combined diff for merge
commits for 'commitdiff' view (if there was not provided commit to
compare againts), and for 'commit' view.


 [PATCH 7/6] todo: todo: Remove "Gitweb diff on merge commits" entry

This patch updates TODO list in 'todo' branch.

-- 
Jakub Narebski

^ permalink raw reply

* Re: [FAQ?] Rationale for git's way to manage the index
From: Julian Phillips @ 2007-05-06 22:53 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqbqgxak1i.fsf@bauges.imag.fr>

On Sun, 6 May 2007, Matthieu Moy wrote:

> The reason why I'm posting this is that I was wondering whether
> "commit -a" not being the default was supposed to be a message like
> "you shouln't use it too often".

Well, personally I practically never use it, I find that having a 
separation between what the current state of my tree is and what will be 
comitted to be one of the really "oh wow, why doens't everything else do 
this?" features.  However, i tend to be working on more than one thing at 
once, and switch between them - so I commit work on A while work on B is 
still unfinished, then start C, finish B some point later and commit it, 
and then I can finish C.  Git is the first VCS that supports a butterfly 
mind :P.

> It seems it isn't. I'll just get used to "commit -a" (and probably
> alias it), and discover the actual benefits of the index little by
> little.

"git add -i" - this is a feature I have wanted since I started using 
version control ...

-- 
Julian

  ---
Your good nature will bring you unbounded happiness.

^ permalink raw reply


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