git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH gitweb] Visually indicating patch size with horizontal bars
@ 2005-10-27 20:39 Chris Shoemaker
  2005-10-27 22:02 ` Junio C Hamano
                   ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Chris Shoemaker @ 2005-10-27 20:39 UTC (permalink / raw)
  To: git


I really like gitweb (thanks Kay!), but I thought it would be nice to
have a visual indication of patch size.  I found this helpful when
scanning though the shortlogs.

To see what it looks like with the gitweb for gitweb (meta-gitweb?)
goto:

http://www.codesifter.com/cgi-bin/gitweb.cgi?p=gitweb.git;a=shortlog

I rather like the look of what I've hacked up (the enclosed patch),
but it should be considered as just a prototype: it only affects the
shortlog, it's horribly inefficient, and I don't really do perl.  :)

If anyone thinks this is a good feature, then please tell me an
efficient way to get some heuristic of the patch size.

Right now, I'm using: 

GIT_DIFF_OPTS='-U 0' $gitbin/git-diff-tree -p $hash | wc -l

which is pretty slow.  Any suggestions?

-chris


Subject: [PATCH] initial hack at horizontal bars indicating patch size

---

 gitweb.cgi |   38 +++++++++++++++++++++++++++++++++++++-
 1 files changed, 37 insertions(+), 1 deletions(-)

c8d45f9a3cfdd7080a57e0de315f3ab9475f60bf
diff --git a/gitweb.cgi b/gitweb.cgi
--- a/gitweb.cgi
+++ b/gitweb.cgi
@@ -53,6 +53,9 @@ if (defined $action) {
 	} elsif ($action eq "opml") {
 		git_opml();
 		exit;
+	} elsif ($action eq "bar.png") {
+	    git_bar_png();
+	    exit;
 	}
 }
 
@@ -358,6 +361,16 @@ sub git_get_type {
 	return $type;
 }
 
+sub git_get_commit_size {
+	my $hash = shift;
+
+	open my $fd, "-|", "GIT_DIFF_OPTS='-U 0' $gitbin/git-diff-tree -p $hash | wc -l" or return;
+	my $size = <$fd>;
+	close $fd or return;
+	chomp $size;
+	return $size;
+}
+
 sub git_read_hash {
 	my $path = shift;
 
@@ -719,6 +732,21 @@ sub git_logo {
 		"\x12\x1c\x9a\xfe\x00\x00\x00\x00\x49\x45\x4e\x44\xae\x42\x60\x82";
 }
 
+# git_bar_png (cached in browser for one day)
+sub git_bar_png {
+	print $cgi->header(-type => 'image/png', -expires => '+1d');
+        # cat bar.png | hexdump -e '"q" 16/1 "w%02x"  "q . \n"' | 
+        #    sed 's/w/\\x/g' | sed 's/q/"/g'
+print "\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52" .
+"\x00\x00\x00\x01\x00\x00\x00\x0c\x08\x02\x00\x00\x00\x2c\xe9\x40" .
+"\x00\x00\x00\x00\x3b\x49\x44\x41\x54\x08\x1d\x01\x30\x00\xcf\xff" .
+"\x00\xba\xba\xff\x02\xf1\xf1\x00\x02\xf2\xf2\x00\x02\xf1\xf2\x00" .
+"\x02\xf2\xf1\x00\x02\xf1\xf1\x00\x02\xf2\xf1\x00\x02\xf1\xf1\x00" .
+"\x02\xf1\xf2\x00\x02\xf1\xf1\x00\x02\xf2\xf2\x00\x02\xf2\xf1\x00" .
+"\x45\x85\x17\x49\x14\x70\x67\xdb\x00\x00\x00\x00\x49\x45\x4e\x44" .
+"\xae\x42\x60\x82";
+}
+
 sub get_file_owner {
 	my $path = shift;
 
@@ -2280,8 +2308,16 @@ sub git_shortlog {
 		      "<td class=\"link\">" .
 		      $cgi->a({-href => "$my_uri?p=$project;a=commit;h=$commit"}, "commit") .
 		      " | " . $cgi->a({-href => "$my_uri?p=$project;a=commitdiff;h=$commit"}, "commitdiff") .
-		      "</td>\n" .
+		      "</td>\n";
+		my $scale = 100;
+		my $stretch = 32;
+		# commits of size 1.7*$scale will be $stretch pixels wide 
+		my $size = int(log((git_get_commit_size($commit)+$scale)/$scale)*$stretch);
+		print "<td class=\"bar\">" .
+		      "<img src=\"$my_uri?a=bar.png\" width=\"$size\" height=\"12\"/>" .
+		      "</td>" .
 		      "</tr>";
+
 	}
 	if ($#revlist >= (100 * ($page+1)-1)) {
 		print "<tr>\n" .

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-27 20:39 [PATCH gitweb] Visually indicating patch size with horizontal bars Chris Shoemaker
@ 2005-10-27 22:02 ` Junio C Hamano
  2005-10-27 23:48   ` Chris Shoemaker
  2005-10-28  1:16   ` Martin Langhoff
  2005-10-28  1:56 ` Kay Sievers
  2005-10-28  9:16 ` Josef Weidendorfer
  2 siblings, 2 replies; 27+ messages in thread
From: Junio C Hamano @ 2005-10-27 22:02 UTC (permalink / raw)
  To: Chris Shoemaker; +Cc: git

Chris Shoemaker <c.shoemaker@cox.net> writes:

> If anyone thinks this is a good feature, then please tell me an
> efficient way to get some heuristic of the patch size.
>
> Right now, I'm using: 
>
> GIT_DIFF_OPTS='-U 0' $gitbin/git-diff-tree -p $hash | wc -l
>
> which is pretty slow.  Any suggestions?

* do we really want to know the number of lines?  sometimes the
  number of pahts that are affected is more useful than number
  of lines when assessing the damage, which can be done with
  'git-diff-tree --name-only'.

* cache the result -- they never change.

An interesting question is what to do with merges, but probably
we can just ignore it for now.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-27 22:02 ` Junio C Hamano
@ 2005-10-27 23:48   ` Chris Shoemaker
  2005-10-28  0:12     ` Linus Torvalds
  2005-10-28  1:16   ` Martin Langhoff
  1 sibling, 1 reply; 27+ messages in thread
From: Chris Shoemaker @ 2005-10-27 23:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thu, Oct 27, 2005 at 03:02:10PM -0700, Junio C Hamano wrote:
> Chris Shoemaker <c.shoemaker@cox.net> writes:
> 
> > If anyone thinks this is a good feature, then please tell me an
> > efficient way to get some heuristic of the patch size.
> >
> > Right now, I'm using: 
> >
> > GIT_DIFF_OPTS='-U 0' $gitbin/git-diff-tree -p $hash | wc -l
> >
> > which is pretty slow.  Any suggestions?
> 
> * do we really want to know the number of lines?  sometimes the
>   number of pahts that are affected is more useful than number
>   of lines when assessing the damage, which can be done with
>   'git-diff-tree --name-only'.

That only shows the top-level names, so when 100s of files changes in
a subdir it looks just like one entry.  It's ok when there's no
subdirs, but it just doesn't work when 95% of the code is under,
e.g. src/.

> 
> * cache the result -- they never change.

True.  Maybe gitk and gitweb can share a cache containing the tree
diffs.  Or maybe git-core can cache tree diffs?

> 
> An interesting question is what to do with merges, but probably
> we can just ignore it for now.

It's trivial to, e.g. use a different image for merges, maybe based on
# of parents?

But, in general, is there interest in a visual indicator of commit
size and/or type in gitweb?

-chris

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-27 23:48   ` Chris Shoemaker
@ 2005-10-28  0:12     ` Linus Torvalds
  2005-10-28  0:50       ` Chris Shoemaker
  0 siblings, 1 reply; 27+ messages in thread
From: Linus Torvalds @ 2005-10-28  0:12 UTC (permalink / raw)
  To: Chris Shoemaker; +Cc: Junio C Hamano, git



On Thu, 27 Oct 2005, Chris Shoemaker wrote:
> > 
> > * do we really want to know the number of lines?  sometimes the
> >   number of pahts that are affected is more useful than number
> >   of lines when assessing the damage, which can be done with
> >   'git-diff-tree --name-only'.
> 
> That only shows the top-level names, so when 100s of files changes in
> a subdir it looks just like one entry.  It's ok when there's no
> subdirs, but it just doesn't work when 95% of the code is under,
> e.g. src/.

Add the "-r" flag to do the recursive thing, ie

	git-diff-tree -r --name-only

should do the right thing.

> True.  Maybe gitk and gitweb can share a cache containing the tree
> diffs.  Or maybe git-core can cache tree diffs?

Creating them is fast enough if there is no IO. Make sure your project is 
packed, and you should be ok.

The expensive part is the "-p" thing to create patches. If you avoid the 
patch creation, you should be ok.

> But, in general, is there interest in a visual indicator of commit
> size and/or type in gitweb?

I kind of like it, but I'm not sure how useful it is, and maybe it does 
really want the whole patch size (not just how many files it touches). 
That's where caching might save your *ss.

		Linus

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-28  0:12     ` Linus Torvalds
@ 2005-10-28  0:50       ` Chris Shoemaker
  2005-10-28  1:08         ` Martin Langhoff
                           ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Chris Shoemaker @ 2005-10-28  0:50 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git

On Thu, Oct 27, 2005 at 05:12:33PM -0700, Linus Torvalds wrote:
> Add the "-r" flag to do the recursive thing, ie
> 
> 	git-diff-tree -r --name-only
> 
> should do the right thing.

Ah, yes, it does.  Thanks.

> > True.  Maybe gitk and gitweb can share a cache containing the tree
> > diffs.  Or maybe git-core can cache tree diffs?
> 
> Creating them is fast enough if there is no IO. Make sure your project is 
> packed, and you should be ok.
> 
> The expensive part is the "-p" thing to create patches. If you avoid the 
> patch creation, you should be ok.

git-diff-tree -r --name-only is pretty quick and it actually does a
halfway reasonable job of representing damage-potential.

> > But, in general, is there interest in a visual indicator of commit
> > size and/or type in gitweb?
> 
> I kind of like it, but I'm not sure how useful it is, and maybe it does 
> really want the whole patch size (not just how many files it touches). 

Hard to say.  Neither one is going to be perfect, so I'm ok with
settling for the cheap one if it's halfway reasonable.  I think I'll
mock up the merge indicator and see if there's any value added there.

So, what's the best way to detect merges?  Maybe see if
'git-cat-file commit $hash | grep ^parent | wc -l' is greater than 1?

> That's where caching might save your *ss.

Ok, but that cache would live inside GIT_DIR an be shared with gitk,
right?

-chris

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-28  0:50       ` Chris Shoemaker
@ 2005-10-28  1:08         ` Martin Langhoff
  2005-10-28  1:13         ` H. Peter Anvin
  2005-10-28  9:31         ` Junio C Hamano
  2 siblings, 0 replies; 27+ messages in thread
From: Martin Langhoff @ 2005-10-28  1:08 UTC (permalink / raw)
  To: Chris Shoemaker; +Cc: Linus Torvalds, Junio C Hamano, git

On 10/28/05, Chris Shoemaker <c.shoemaker@cox.net> wrote:
> So, what's the best way to detect merges?  Maybe see if
> 'git-cat-file commit $hash | grep ^parent | wc -l' is greater than 1?
>
> > That's where caching might save your *ss.
>
> Ok, but that cache would live inside GIT_DIR an be shared with gitk,
> right?

gitweb should have any caches it wants, regardless of gitk, methinks.

I very rarely run gitk and gitweb on the same repo. The repos where I
run gitk are all development repos, on my desktop machine or laptop.
gitweb runs only on the webserver where I publish those...

So it may be practical to have a common cache format, but unlikely
that both programs will use the same cached data in practice...


martin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-28  0:50       ` Chris Shoemaker
  2005-10-28  1:08         ` Martin Langhoff
@ 2005-10-28  1:13         ` H. Peter Anvin
  2005-10-28  8:29           ` Andreas Ericsson
  2005-10-28  9:31         ` Junio C Hamano
  2 siblings, 1 reply; 27+ messages in thread
From: H. Peter Anvin @ 2005-10-28  1:13 UTC (permalink / raw)
  To: Chris Shoemaker; +Cc: Linus Torvalds, Junio C Hamano, git

Chris Shoemaker wrote:
> 
> Ok, but that cache would live inside GIT_DIR an be shared with gitk,
> right?
> 

That would be bad.  Don't assume that the person running gitweb (or 
gitk, for that matter) has write permission.

	-hpa

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-27 22:02 ` Junio C Hamano
  2005-10-27 23:48   ` Chris Shoemaker
@ 2005-10-28  1:16   ` Martin Langhoff
  2005-10-28  2:38     ` Linus Torvalds
  1 sibling, 1 reply; 27+ messages in thread
From: Martin Langhoff @ 2005-10-28  1:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Chris Shoemaker, git

On 10/28/05, Junio C Hamano <junkio@cox.net> wrote:
> > which is pretty slow.  Any suggestions?
>
> * do we really want to know the number of lines?

What about both? And sugar (rename detection) on top! ;-)

If you try an find the largest commit (by line count) in the gitweb
revision history, you bump into the gitweb.pl -> gitweb.cgi rename.

cheers,


martin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-27 20:39 [PATCH gitweb] Visually indicating patch size with horizontal bars Chris Shoemaker
  2005-10-27 22:02 ` Junio C Hamano
@ 2005-10-28  1:56 ` Kay Sievers
  2005-10-28  2:38   ` Chris Shoemaker
  2005-10-28  9:16 ` Josef Weidendorfer
  2 siblings, 1 reply; 27+ messages in thread
From: Kay Sievers @ 2005-10-28  1:56 UTC (permalink / raw)
  To: Chris Shoemaker; +Cc: git

On Thu, Oct 27, 2005 at 04:39:45PM -0400, Chris Shoemaker wrote:
> 
> I really like gitweb (thanks Kay!), but I thought it would be nice to
> have a visual indication of patch size.  I found this helpful when
> scanning though the shortlogs.

This looks nice, but if the patch size tells you something important,
your commit subjects are probably too short or wrong. :)

> To see what it looks like with the gitweb for gitweb (meta-gitweb?)
> goto:
> 
> http://www.codesifter.com/cgi-bin/gitweb.cgi?p=gitweb.git;a=shortlog
> 
> I rather like the look of what I've hacked up (the enclosed patch),
> but it should be considered as just a prototype: it only affects the
> shortlog, it's horribly inefficient, and I don't really do perl.  :)
> 
> If anyone thinks this is a good feature, then please tell me an
> efficient way to get some heuristic of the patch size.

You may try to use CSS instead of an embedded picture to draw the bar,
just like the RSS logo in the footer, which is simple CSS rendered in the
browser.

Kay

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-28  1:16   ` Martin Langhoff
@ 2005-10-28  2:38     ` Linus Torvalds
  2005-10-28  3:52       ` Junio C Hamano
  0 siblings, 1 reply; 27+ messages in thread
From: Linus Torvalds @ 2005-10-28  2:38 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Junio C Hamano, Chris Shoemaker, git



On Fri, 28 Oct 2005, Martin Langhoff wrote:
>
> On 10/28/05, Junio C Hamano <junkio@cox.net> wrote:
> > > which is pretty slow.  Any suggestions?
> >
> > * do we really want to know the number of lines?
> 
> What about both? And sugar (rename detection) on top! ;-)

Well, if you do full copy detection (and break detection), then 
git-diff-tree will actually have effectively calculated the size of the 
diff of each file. It just doesn't print them (well, it does a percentage 
for the renames/copies).

So you could make git-diff-tree tell you how big the patch was, without 
actually generating a patch at all. It will be quite a bit more expensive 
than just a plain "git-diff-tree -r --name-only", but if you cache the 
result is might be quite acceptable.

Caching the result might be as simple as just telling the caching 
web-server that the result is static and never changes - no need to 
cache things inside of gitweb itself. Just set expiration to "never".

Anybody wants to add a new output format to git-diff-tree that outputs how 
big the changes are in absolute terms (rather than the "similarity index", 
which is obviously relative to the original size of the file in question)?

		Linus

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-28  1:56 ` Kay Sievers
@ 2005-10-28  2:38   ` Chris Shoemaker
  2005-11-01 23:30     ` Petr Baudis
  0 siblings, 1 reply; 27+ messages in thread
From: Chris Shoemaker @ 2005-10-28  2:38 UTC (permalink / raw)
  To: Kay Sievers; +Cc: git

On Fri, Oct 28, 2005 at 03:56:42AM +0200, Kay Sievers wrote:
> On Thu, Oct 27, 2005 at 04:39:45PM -0400, Chris Shoemaker wrote:
> > 
> > I really like gitweb (thanks Kay!), but I thought it would be nice to
> > have a visual indication of patch size.  I found this helpful when
> > scanning though the shortlogs.
> 
> This looks nice, but if the patch size tells you something important,
> your commit subjects are probably too short or wrong. :)

Yeah, some people write lousy commit subjects.  But me?  Nooo,
/never/.  :)

> You may try to use CSS instead of an embedded picture to draw the bar,
> just like the RSS logo in the footer, which is simple CSS rendered in the
> browser.

I'll look into that, but the cost wasn't in the image; it was in the
width calculation.

Here's a side-by-side comparison.  Open two browser tabs and flip between them:

http://www.codesifter.com/cgi-bin/gitweb-difftreeP.cgi?p=git.git;a=shortlog
http://www.codesifter.com/cgi-bin/gitweb-difftreeNames.cgi?p=git.git;a=shortlog

I've used a project you all are familar with, and that has more than
two files.  The first page uses 'git-diff-tree -p $hash|wc -l'.  The
second page uses 'git-diff-tree -r --name-only|wc -l'.  (Oh and I have
a merge indicator now.)

How do they compare for showing damage-potential?  I think they both
do a reasonable job.  I think the full patch diff is a bit better, but
it does cost.

-chris

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-28  2:38     ` Linus Torvalds
@ 2005-10-28  3:52       ` Junio C Hamano
  2005-10-28 16:02         ` Linus Torvalds
  0 siblings, 1 reply; 27+ messages in thread
From: Junio C Hamano @ 2005-10-28  3:52 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Martin Langhoff, Chris Shoemaker, git

Linus Torvalds <torvalds@osdl.org> writes:

> Well, if you do full copy detection (and break detection), then 
> git-diff-tree will actually have effectively calculated the size of the 
> diff of each file. It just doesn't print them (well, it does a percentage 
> for the renames/copies).

Unbroken in-place edit would never go through diffcore-rename,
so that is a gross overstatement.

But we could if we wanted to.  I do not know how useful it would
be, but if somebody wants to do it, I think the best strategy is
to do as a separate diffcore backend that comes after
diffcore_rename() runs, and do the similarity estimator only on
filepairs that rename/copy did not touch.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-28  1:13         ` H. Peter Anvin
@ 2005-10-28  8:29           ` Andreas Ericsson
  0 siblings, 0 replies; 27+ messages in thread
From: Andreas Ericsson @ 2005-10-28  8:29 UTC (permalink / raw)
  To: git

H. Peter Anvin wrote:
> Chris Shoemaker wrote:
> 
>>
>> Ok, but that cache would live inside GIT_DIR an be shared with gitk,
>> right?
>>
> 
> That would be bad.  Don't assume that the person running gitweb (or 
> gitk, for that matter) has write permission.
> 

Not necessarily in the archive, but it could support a --cache-dir 
option. If no cache-dir directive is used it could try GIT_DIR/cache and 
go on as usual if that fails too.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-27 20:39 [PATCH gitweb] Visually indicating patch size with horizontal bars Chris Shoemaker
  2005-10-27 22:02 ` Junio C Hamano
  2005-10-28  1:56 ` Kay Sievers
@ 2005-10-28  9:16 ` Josef Weidendorfer
  2 siblings, 0 replies; 27+ messages in thread
From: Josef Weidendorfer @ 2005-10-28  9:16 UTC (permalink / raw)
  To: git

On Thursday 27 October 2005 22:39, Chris Shoemaker wrote:
> 
> I really like gitweb (thanks Kay!), but I thought it would be nice to
> have a visual indication of patch size.  I found this helpful when
> scanning though the shortlogs.

Looks nice.
What about splitting this up into red (removed lines)
and green (added lines) bars?

Josef

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-28  0:50       ` Chris Shoemaker
  2005-10-28  1:08         ` Martin Langhoff
  2005-10-28  1:13         ` H. Peter Anvin
@ 2005-10-28  9:31         ` Junio C Hamano
  2 siblings, 0 replies; 27+ messages in thread
From: Junio C Hamano @ 2005-10-28  9:31 UTC (permalink / raw)
  To: Chris Shoemaker; +Cc: git

Chris Shoemaker <c.shoemaker@cox.net> writes:

> Ok, but that cache would live inside GIT_DIR an be shared with gitk,
> right?

It is up to gitk.  If your cache file format is simple, concise
and easy to access, then it might be useful for gitk to take
advantage of it.  Although I doubt many people would run gitk
and gitweb on the same repository (usually the former is run on
the private developer repository and the latter public one).

Caching the 'git-diff-tree -p | git-apply --numstat' output
might be useful and compact enough.  I often wonder if the
commit page (i.e. gitweb?p=$repository;a=commit;h=$sha1) might
be more useful if it had diffstat drawing on each blob line at
the end of the page, and the output from the above pipe can be
used for that.

I wonder how big that thing would become if we cache it for the
whole history, using something simple and lightweight like
berkeley db or dbm, 20-byte commit ID as the key (for now,
ignoring merges, but we could use 40-byte commit-parent ID pair
as the key) and a list of the number of insertions and deletions
for affected paths as the value.  If we can do it quickly
enough, you could put the cache update in post-update hook, so
that every time you push into the public repository the
patch-size cache is updated for gitweb's use.  This can be done
by the repository owner, and gitweb can stay read-only consumer
of the information.

Just in case people find this useful, here is a patch to
implement git-apply --numstat.

    ------------
[PATCH] git-apply --numstat

The new option, --numstat, shows number of inserted and deleted
lines for each path.  It is similar to --stat output but is
meant to be more machine friendly by giving number of added and
deleted lines and unabbreviated paths.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

git diff
diff --git a/apply.c b/apply.c
index e5c0b7d..73dfd0c 100644
--- a/apply.c
+++ b/apply.c
@@ -13,18 +13,20 @@
 //  --check turns on checking that the working tree matches the
 //    files that are being modified, but doesn't apply the patch
 //  --stat does just a diffstat, and doesn't actually apply
+//  --numstat does numeric diffstat, and doesn't actually apply
 //  --index-info shows the old and new index info for paths if available.
 //
 static int check_index = 0;
 static int write_index = 0;
 static int diffstat = 0;
+static int numstat = 0;
 static int summary = 0;
 static int check = 0;
 static int apply = 1;
 static int show_index_info = 0;
 static int line_termination = '\n';
 static const char apply_usage[] =
-"git-apply [--stat] [--summary] [--check] [--index] [--apply] [--index-info] [-z] <patch>...";
+"git-apply [--stat] [--numstat] [--summary] [--check] [--index] [--apply] [--index-info] [-z] <patch>...";
 
 /*
  * For "diff-stat" like behaviour, we keep track of the biggest change
@@ -1317,6 +1319,20 @@ static void stat_patch_list(struct patch
 	printf(" %d files changed, %d insertions(+), %d deletions(-)\n", files, adds, dels);
 }
 
+static void numstat_patch_list(struct patch *patch)
+{
+	for ( ; patch; patch = patch->next) { 
+		const char *name;
+		name = patch->old_name ? patch->old_name : patch->new_name;
+		printf("%d\t%d\t", patch->lines_added, patch->lines_deleted);
+		if (line_termination && quote_c_style(name, NULL, NULL, 0))
+			quote_c_style(name, NULL, stdout, 0);
+		else
+			fputs(name, stdout);
+		putchar('\n');
+	}
+}
+
 static void show_file_mode_name(const char *newdelete, unsigned int mode, const char *name)
 {
 	if (mode)
@@ -1650,6 +1666,9 @@ static int apply_patch(int fd)
 	if (diffstat)
 		stat_patch_list(list);
 
+	if (numstat)
+		numstat_patch_list(list);
+	
 	if (summary)
 		summary_patch_list(list);
 
@@ -1683,6 +1702,11 @@ int main(int argc, char **argv)
 			diffstat = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--numstat")) {
+			apply = 0;
+			numstat = 1;
+			continue;
+		}
 		if (!strcmp(arg, "--summary")) {
 			apply = 0;
 			summary = 1;

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-28  3:52       ` Junio C Hamano
@ 2005-10-28 16:02         ` Linus Torvalds
  0 siblings, 0 replies; 27+ messages in thread
From: Linus Torvalds @ 2005-10-28 16:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Martin Langhoff, Chris Shoemaker, git



On Thu, 27 Oct 2005, Junio C Hamano wrote:
>
> Linus Torvalds <torvalds@osdl.org> writes:
> 
> > Well, if you do full copy detection (and break detection), then 
> > git-diff-tree will actually have effectively calculated the size of the 
> > diff of each file. It just doesn't print them (well, it does a percentage 
> > for the renames/copies).
> 
> Unbroken in-place edit would never go through diffcore-rename,
> so that is a gross overstatement.

Well, the break detection will have _calculated_ the diff size.

The point being that all the work has been done - it's just not printed 
out.

		Linus

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-10-28  2:38   ` Chris Shoemaker
@ 2005-11-01 23:30     ` Petr Baudis
  2005-11-01 23:33       ` Martin Langhoff
  0 siblings, 1 reply; 27+ messages in thread
From: Petr Baudis @ 2005-11-01 23:30 UTC (permalink / raw)
  To: Chris Shoemaker; +Cc: Kay Sievers, git

Dear diary, on Fri, Oct 28, 2005 at 04:38:33AM CEST, I got a letter
where Chris Shoemaker <c.shoemaker@cox.net> told me that...
> Here's a side-by-side comparison.  Open two browser tabs and flip between them:
> 
> http://www.codesifter.com/cgi-bin/gitweb-difftreeP.cgi?p=git.git;a=shortlog
> http://www.codesifter.com/cgi-bin/gitweb-difftreeNames.cgi?p=git.git;a=shortlog
> 
> I've used a project you all are familar with, and that has more than
> two files.  The first page uses 'git-diff-tree -p $hash|wc -l'.  The
> second page uses 'git-diff-tree -r --name-only|wc -l'.  (Oh and I have
> a merge indicator now.)
> 
> How do they compare for showing damage-potential?  I think they both
> do a reasonable job.  I think the full patch diff is a bit better, but
> it does cost.

What about having the color indicate the number of affected files (let's
say on a blue..red scale) and the width the size of patch?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-11-01 23:30     ` Petr Baudis
@ 2005-11-01 23:33       ` Martin Langhoff
  2005-11-01 23:43         ` Petr Baudis
  2005-11-02  0:12         ` Chris Shoemaker
  0 siblings, 2 replies; 27+ messages in thread
From: Martin Langhoff @ 2005-11-01 23:33 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Chris Shoemaker, Kay Sievers, git

On 11/2/05, Petr Baudis <pasky@suse.cz> wrote:
> What about having the color indicate the number of affected files (let's
> say on a blue..red scale) and the width the size of patch?

I'm a /little bit/ colour blind on the red scale -- so I vote for 2
bars, each half the heigth of the current bar.  ;-)

martin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-11-01 23:33       ` Martin Langhoff
@ 2005-11-01 23:43         ` Petr Baudis
  2005-11-02  8:08           ` Andreas Ericsson
  2005-11-02  0:12         ` Chris Shoemaker
  1 sibling, 1 reply; 27+ messages in thread
From: Petr Baudis @ 2005-11-01 23:43 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Chris Shoemaker, Kay Sievers, git

Dear diary, on Wed, Nov 02, 2005 at 12:33:38AM CET, I got a letter
where Martin Langhoff <martin.langhoff@gmail.com> told me that...
> On 11/2/05, Petr Baudis <pasky@suse.cz> wrote:
> > What about having the color indicate the number of affected files (let's
> > say on a blue..red scale) and the width the size of patch?
> 
> I'm a /little bit/ colour blind on the red scale -- so I vote for 2
> bars, each half the heigth of the current bar.  ;-)

That's certainly possible as well (if you make each of the bars of
different color), but for most people not equally visually obvious.
Perhaps we could have a knob at the bottom of the page, but that isn't
very satisfying a solution either... :-(

Another possibility is to make the height dynamic and in proportion with
the number of affected files. Or combine both the color and dynamic
height. I believe changing the color to red would make it appear as
black for the red-color-blind people?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-11-01 23:33       ` Martin Langhoff
  2005-11-01 23:43         ` Petr Baudis
@ 2005-11-02  0:12         ` Chris Shoemaker
  2005-11-02  0:26           ` Kay Sievers
  2005-12-05  0:04           ` Petr Baudis
  1 sibling, 2 replies; 27+ messages in thread
From: Chris Shoemaker @ 2005-11-02  0:12 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Petr Baudis, Kay Sievers, git

On Wed, Nov 02, 2005 at 12:33:38PM +1300, Martin Langhoff wrote:
> On 11/2/05, Petr Baudis <pasky@suse.cz> wrote:
> > What about having the color indicate the number of affected files (let's
> > say on a blue..red scale) and the width the size of patch?
> 
> I'm a /little bit/ colour blind on the red scale -- so I vote for 2
> bars, each half the heigth of the current bar.  ;-)

I was going to use two bars for add vs. delete, but this could work,
too.  I'm intending on getting back to this ASAP, but for now my
cvsimport problems are higher priority (see other post).

-chris

> 
> martin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-11-02  0:12         ` Chris Shoemaker
@ 2005-11-02  0:26           ` Kay Sievers
  2005-12-05  0:04           ` Petr Baudis
  1 sibling, 0 replies; 27+ messages in thread
From: Kay Sievers @ 2005-11-02  0:26 UTC (permalink / raw)
  To: Chris Shoemaker; +Cc: Martin Langhoff, Petr Baudis, git

On Tue, Nov 01, 2005 at 07:12:06PM -0500, Chris Shoemaker wrote:
> On Wed, Nov 02, 2005 at 12:33:38PM +1300, Martin Langhoff wrote:
> > On 11/2/05, Petr Baudis <pasky@suse.cz> wrote:
> > > What about having the color indicate the number of affected files (let's
> > > say on a blue..red scale) and the width the size of patch?
> > 
> > I'm a /little bit/ colour blind on the red scale -- so I vote for 2
> > bars, each half the heigth of the current bar.  ;-)
> 
> I was going to use two bars for add vs. delete, but this could work,
> too.  I'm intending on getting back to this ASAP, but for now my
> cvsimport problems are higher priority (see other post).

Guys, I'm not convinced, that we should make gitweb look like Konqueror. :)

Kay

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-11-01 23:43         ` Petr Baudis
@ 2005-11-02  8:08           ` Andreas Ericsson
  2005-11-02 10:37             ` Johannes Schindelin
  0 siblings, 1 reply; 27+ messages in thread
From: Andreas Ericsson @ 2005-11-02  8:08 UTC (permalink / raw)
  To: git

Petr Baudis wrote:
> 
> Another possibility is to make the height dynamic and in proportion with
> the number of affected files. Or combine both the color and dynamic
> height. I believe changing the color to red would make it appear as
> black for the red-color-blind people?
> 

Color-blindness doesn't work like that. There are no "red-color-blind" 
people. It's either red-blue, red-green or blue-green and the problem 
lies in differing those colors from each other when they're close 
together (and, usually, intermixed). Red-green color-blindness is by far 
the most common so it would be wise not to use those.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-11-02  8:08           ` Andreas Ericsson
@ 2005-11-02 10:37             ` Johannes Schindelin
  2005-11-02 12:19               ` Andreas Ericsson
  0 siblings, 1 reply; 27+ messages in thread
From: Johannes Schindelin @ 2005-11-02 10:37 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: git

Hi,

On Wed, 2 Nov 2005, Andreas Ericsson wrote:

> Color-blindness doesn't work like that. There are no "red-color-blind" 
> people.

I do exist. I have problems focusing on red text or objects. Agreed, it is 
no "blindness", but it is not too seldom either.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-11-02 10:37             ` Johannes Schindelin
@ 2005-11-02 12:19               ` Andreas Ericsson
  2005-11-02 12:43                 ` Johannes Schindelin
  0 siblings, 1 reply; 27+ messages in thread
From: Andreas Ericsson @ 2005-11-02 12:19 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin wrote:
> Hi,
> 
> On Wed, 2 Nov 2005, Andreas Ericsson wrote:
> 
> 
>>Color-blindness doesn't work like that. There are no "red-color-blind" 
>>people.
> 
> 
> I do exist. I have problems focusing on red text or objects. Agreed, it is 
> no "blindness", but it is not too seldom either.
> 

Is that irrespective of background color?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-11-02 12:19               ` Andreas Ericsson
@ 2005-11-02 12:43                 ` Johannes Schindelin
  0 siblings, 0 replies; 27+ messages in thread
From: Johannes Schindelin @ 2005-11-02 12:43 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: git

Hi,

On Wed, 2 Nov 2005, Andreas Ericsson wrote:

> Johannes Schindelin wrote:
> > 
> > On Wed, 2 Nov 2005, Andreas Ericsson wrote:
> > 
> > 
> > > Color-blindness doesn't work like that. There are no "red-color-blind"
> > > people.
> > 
> > 
> > I do exist. I have problems focusing on red text or objects. Agreed, it is
> > no "blindness", but it is not too seldom either.
> > 
> 
> Is that irrespective of background color?

Mostly. (I don't remember the exact outcome of the test, but I am 
definitely not color blind).

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-11-02  0:12         ` Chris Shoemaker
  2005-11-02  0:26           ` Kay Sievers
@ 2005-12-05  0:04           ` Petr Baudis
  2005-12-05  1:03             ` Chris Shoemaker
  1 sibling, 1 reply; 27+ messages in thread
From: Petr Baudis @ 2005-12-05  0:04 UTC (permalink / raw)
  To: Chris Shoemaker; +Cc: Martin Langhoff, Kay Sievers, git

Dear diary, on Wed, Nov 02, 2005 at 01:12:06AM CET, I got a letter
where Chris Shoemaker <c.shoemaker@cox.net> said that...
> On Wed, Nov 02, 2005 at 12:33:38PM +1300, Martin Langhoff wrote:
> > On 11/2/05, Petr Baudis <pasky@suse.cz> wrote:
> > > What about having the color indicate the number of affected files (let's
> > > say on a blue..red scale) and the width the size of patch?
> > 
> > I'm a /little bit/ colour blind on the red scale -- so I vote for 2
> > bars, each half the heigth of the current bar.  ;-)
> 
> I was going to use two bars for add vs. delete, but this could work,
> too.  I'm intending on getting back to this ASAP, but for now my
> cvsimport problems are higher priority (see other post).

Is there any progress, by the way?

If you didn't manage to finish it, no big deal - but it would be great
to have at least the last version you screenshotted, since IIRC I
couldn't find that one either, and I would like to play with it a bit.

Thanks,

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH gitweb] Visually indicating patch size with horizontal bars
  2005-12-05  0:04           ` Petr Baudis
@ 2005-12-05  1:03             ` Chris Shoemaker
  0 siblings, 0 replies; 27+ messages in thread
From: Chris Shoemaker @ 2005-12-05  1:03 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Martin Langhoff, Kay Sievers, git

On Mon, Dec 05, 2005 at 01:04:42AM +0100, Petr Baudis wrote:
> Dear diary, on Wed, Nov 02, 2005 at 01:12:06AM CET, I got a letter
> where Chris Shoemaker <c.shoemaker@cox.net> said that...
> > On Wed, Nov 02, 2005 at 12:33:38PM +1300, Martin Langhoff wrote:
> > > On 11/2/05, Petr Baudis <pasky@suse.cz> wrote:
> > > > What about having the color indicate the number of affected files (let's
> > > > say on a blue..red scale) and the width the size of patch?
> > > 
> > > I'm a /little bit/ colour blind on the red scale -- so I vote for 2
> > > bars, each half the heigth of the current bar.  ;-)
> > 
> > I was going to use two bars for add vs. delete, but this could work,
> > too.  I'm intending on getting back to this ASAP, but for now my
> > cvsimport problems are higher priority (see other post).
> 
> Is there any progress, by the way?

A little.  I decided to follow Junio's suggestion of caching the
result of "git-diff-tree -r -p $commit | git-apply --numstat" in a
BerkeleyDB.  (I liked the idea of reusing the cached results on the
commit page, too.)  I got a script to populate the cache, then I
suspect could be easily adapting into a commit-hook.  Then I started
working on the gitweb part and tried to follow another suggestion
(Kay's, I think.) to use CSS instead of (yet another) embedded .png.

This is where I got hung up: I discovered something strange (to me, at
least) about CSS/html: I'm using the <td></td> in the fifth column of
the shortlog.  I tried to use an anchor tag for the added count and
one for the deleted count.  Setting "display:block" and the different
background-colors works (produces stacked horizontal bars), as does
setting various widths (an essential point), but *ONLY* using "width"
in the CSS.  Using width anchor attribute simply doesn't work.

Honestly, html/css is not my strong suit and neither is perl, although
the BerkeleyDB perl API seemed simple enough.

> If you didn't manage to finish it, no big deal - but it would be great
> to have at least the last version you screenshotted, since IIRC I
> couldn't find that one either, and I would like to play with it a bit.

I'm happy for anyone to take this over.  Since my excursion into css
didn't really work, I'd suggest starting with the gitweb-difftreeP.cgi
version.  I will send you (and anyone else who asks) that file and the
cache population script.

-chris

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2005-12-05  1:03 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-10-27 20:39 [PATCH gitweb] Visually indicating patch size with horizontal bars Chris Shoemaker
2005-10-27 22:02 ` Junio C Hamano
2005-10-27 23:48   ` Chris Shoemaker
2005-10-28  0:12     ` Linus Torvalds
2005-10-28  0:50       ` Chris Shoemaker
2005-10-28  1:08         ` Martin Langhoff
2005-10-28  1:13         ` H. Peter Anvin
2005-10-28  8:29           ` Andreas Ericsson
2005-10-28  9:31         ` Junio C Hamano
2005-10-28  1:16   ` Martin Langhoff
2005-10-28  2:38     ` Linus Torvalds
2005-10-28  3:52       ` Junio C Hamano
2005-10-28 16:02         ` Linus Torvalds
2005-10-28  1:56 ` Kay Sievers
2005-10-28  2:38   ` Chris Shoemaker
2005-11-01 23:30     ` Petr Baudis
2005-11-01 23:33       ` Martin Langhoff
2005-11-01 23:43         ` Petr Baudis
2005-11-02  8:08           ` Andreas Ericsson
2005-11-02 10:37             ` Johannes Schindelin
2005-11-02 12:19               ` Andreas Ericsson
2005-11-02 12:43                 ` Johannes Schindelin
2005-11-02  0:12         ` Chris Shoemaker
2005-11-02  0:26           ` Kay Sievers
2005-12-05  0:04           ` Petr Baudis
2005-12-05  1:03             ` Chris Shoemaker
2005-10-28  9:16 ` Josef Weidendorfer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).