* [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-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-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-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-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 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 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-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: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 ` 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: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-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-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
* 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
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).