[PATCH] gitweb: Adding a `blame' interface.

[PATCH] gitweb: Adding a `blame' interface.

[Florian Forster]

This patch adds an interface for `git-blame' to `gitweb.cgi'. Links to it are
place in `git_blob'.
Internally the code uses `git-annotate' because `git-blame's output differs for
files that have been renamed in the past. However, I like the term `blame'
better.

Signed-off-by: Florian Forster <octo@verplant.org>


---

 gitweb/gitweb.cgi |  108 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 107 insertions(+), 1 deletions(-)

b11522d270365b293197680e43e8feb87328a352
diff --git a/gitweb/gitweb.cgi b/gitweb/gitweb.cgi
index ea21fbe..91c075d 100755
--- a/gitweb/gitweb.cgi
+++ b/gitweb/gitweb.cgi
@@ -203,6 +203,9 @@ if (!defined $action || $action eq "summ
 } elsif ($action eq "tag") {
 	git_tag();
 	exit;
+} elsif ($action eq "blame") {
+	git_blame();
+	exit;
 } else {
 	undef $action;
 	die_error(undef, "Unknown action.");
@@ -1228,6 +1231,107 @@ sub git_tag {
 	git_footer_html();
 }
 
+sub git_blame {
+	my $fd;
+	die_error('404 Not Found', "What file will it be, master?") if (!$file_name);
+	$hash_base ||= git_read_head($project);
+	die_error(undef, "Reading commit failed.") unless ($hash_base);
+	my %co = git_read_commit($hash_base)
+		or die_error(undef, "Reading commit failed.");
+	if (!defined $hash) {
+		$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
+			or die_error(undef, "Error lookup file.");
+	}
+	open ($fd, "-|", "$gitbin/git-annotate", '-l', '-t', '-r', $file_name, $hash_base)
+		or die_error(undef, "Open failed.");
+	git_header_html();
+	print "<div class=\"page_nav\">\n" .
+		$cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=summary")}, "summary") .
+		" | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=shortlog")}, "shortlog") .
+		" | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=log")}, "log") .
+		" | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commit;h=$hash_base")}, "commit") .
+		" | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commitdiff;h=$hash_base")}, "commitdiff") .
+		" | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=tree;h=$co{'tree'};hb=$hash_base")}, "tree") . "<br/>\n";
+	print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$hash;hb=$hash_base;f=$file_name")}, "blob") .
+		" | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blame;f=$file_name")}, "head") . "<br/>\n";
+	print "</div>\n".
+		"<div>" .
+		$cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commit;h=$hash_base"), -class => "title"}, esc_html($co{'title'})) .
+		"</div>\n";
+	print "<div class=\"page_path\"><b>" . esc_html($file_name) . "</b></div>\n";
+	print "<div class=\"page_body\">\n";
+	print <<HTML;
+<table style="border-collapse: collapse;">
+  <tr>
+    <th>Commit</th>
+    <th>Age</th>
+    <th>Author</th>
+    <th>Line</th>
+    <th>Data</th>
+  </tr>
+HTML
+	my @line_class = (qw(light dark));
+	my $line_class_len = scalar (@line_class);
+	my $line_class_num = $#line_class;
+	while (my $line = <$fd>) {
+		my $long_rev;
+		my $short_rev;
+		my $author;
+		my $time;
+		my $lineno;
+		my $data;
+		my $age;
+		my $age_str;
+		my $age_style;
+
+		chomp $line;
+		$line_class_num = ($line_class_num + 1) % $line_class_len;
+
+		if ($line =~ m/^([0-9a-fA-F]{40})\t\(\s*([^\t]+)\t(\d+) \+\d\d\d\d\t(\d+)\)(.*)$/) {
+			$long_rev = $1;
+			$author   = $2;
+			$time     = $3;
+			$lineno   = $4;
+			$data     = $5;
+		} else {
+			print qq(  <tr><td colspan="5" style="color: red; background-color: yellow;">Unable to parse: $line</td></tr>\n);
+			next;
+		}
+		$short_rev  = substr ($long_rev, 0, 8);
+		$age        = time () - $time;
+		$age_str    = age_string ($age);
+		$age_str    =~ s/ /&nbsp;/g;
+		$age_style  = 'font-style: italic;';
+		$age_style .= ' color: #009900; background: transparent;' if ($age < 60*60*24*2);
+		$age_style .= ' font-weight: bold;' if ($age < 60*60*2);
+		$author     = esc_html ($author);
+		$author     =~ s/ /&nbsp;/g;
+		# escape tabs
+		while ((my $pos = index($data, "\t")) != -1) {
+			if (my $count = (8 - ($pos % 8))) {
+				my $spaces = ' ' x $count;
+				$data =~ s/\t/$spaces/;
+			}
+		}
+		$data = esc_html ($data);
+		$data =~ s/ /&nbsp;/g;
+
+		print <<HTML;
+  <tr class="$line_class[$line_class_num]">
+    <td style="font-family: monospace;"><a href="$my_uri?${\esc_param ("p=$project;a=commit;h=$long_rev")}" class="text">$short_rev..</a></td>
+    <td style="$age_style">$age_str</td>
+    <td>$author</td>
+    <td style="text-align: right;"><a id="$lineno" href="#$lineno" class="linenr">$lineno</a></td>
+    <td style="font-family: monospace;">$data</td>
+  </tr>
+HTML
+	} # while (my $line = <$fd>)
+	print "</table>\n\n";
+	close $fd or print "Reading blob failed.\n";
+	print "</div>";
+	git_footer_html();
+}
+
 sub git_tags {
 	my $head = git_read_head($project);
 	git_header_html();
@@ -1375,7 +1479,8 @@ sub git_blob {
 		      " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commitdiff;h=$hash_base")}, "commitdiff") .
 		      " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=tree;h=$co{'tree'};hb=$hash_base")}, "tree") . "<br/>\n";
 		if (defined $file_name) {
-			print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob_plain;h=$hash;f=$file_name")}, "plain") .
+			print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blame;h=$hash;hb=$hash_base;f=$file_name")}, "blame") .
+			" | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob_plain;h=$hash;f=$file_name")}, "plain") .
 			" | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;hb=HEAD;f=$file_name")}, "head") . "<br/>\n";
 		} else {
 			print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob_plain;h=$hash")}, "plain") . "<br/>\n";
@@ -1496,6 +1601,7 @@ sub git_tree {
 			      "</td>\n" .
 			      "<td class=\"link\">" .
 			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$t_hash$base_key;f=$base$t_name")}, "blob") .
+#			      " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blame;h=$t_hash$base_key;f=$base$t_name")}, "blame") .
 			      " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=history;h=$hash_base;f=$base$t_name")}, "history") .
 			      "</td>\n";
 		} elsif ($t_type eq "tree") {
-- 
1.3.3

Re: [PATCH] gitweb: Adding a `blame' interface.

[Martin Langhoff]

Florian,

Looks good! git-blame/git-annotate are quite expensive to run. Do you
think it would make sense making it conditional on a git-repo-config
option (gitweb.blame=1)?

kernel.org is the flagship user for gitweb, so expensive options
should default to off :-/

cheers,



martin

Re: [PATCH] gitweb: Adding a `blame' interface.

[Florian Forster]

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

Hi Martin,

On Mon, Jun 12, 2006 at 10:02:05AM +1200, Martin Langhoff wrote:
> good! git-blame/git-annotate are quite expensive to run. Do you think
> it would make sense making it conditional on a git-repo-config option
> (gitweb.blame=1)?

sure, that it's a big change and if it helps the kernel.org folks ;)
I'll follow-up with a patch for this in a second..

Would it help to cache `git-annotate's output, e.g. using one of the
`Cache::Cache' modules? Or is browsing of blobs too sparse for this to
result in a performance gain? I'm sure the modules could be integrated
as a weak precondition.

I have two more points regarding gitweb's configuration:
- IMHO it would make sense to move the general gitweb-configuration
  (where are the repositories, where are the binaries, etc) out of the
  script.  As far as I know the Debian maintainer of the `gitweb'
  package has asked for this before but was refused for some reason..
  Possibly a file `gitweb.conf' in the same directory as the script
  could be read and overwrite the builtin defaults..?
- If `GIT_DIR/description' is only used by gitweb it may be more
  consistent to use the git-repo-config option `gitweb.description' in
  the future.

Regards,
-octo
-- 
Florian octo Forster
Hacker in training
GnuPG: 0x91523C3D
http://verplant.org/

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

[PATCH] gitweb: Make the availability of the `blame' interface in gitweb configurable.

[Florian Forster]

Since `git-annotate' is an expensive operation to run it may be desirable to
deactivate this functionality. This patch introduces the `gitweb.blame' option
to git-repo-config and disables the blame support by default.

Signed-off-by: Florian Forster <octo@verplant.org>


---

 gitweb/gitweb.cgi |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)

3eea23e8d8a13579455cdf8d5088794d33bdcba2
diff --git a/gitweb/gitweb.cgi b/gitweb/gitweb.cgi
index 91c075d..5eabe06 100755
--- a/gitweb/gitweb.cgi
+++ b/gitweb/gitweb.cgi
@@ -837,6 +837,25 @@ sub git_read_projects {
 	return @list;
 }
 
+sub git_get_project_config {
+	my $key = shift;
+
+	return unless ($key);
+	$key =~ s/^gitweb\.//;
+	return if ($key =~ m/\W/);
+
+	my $val = qx(git-repo-config --get gitweb.$key);
+	return ($val);
+}
+
+sub git_get_project_config_bool {
+	my $val = git_get_project_config (@_);
+	if ($val and $val =~ m/true|yes|on/) {
+		return (1);
+	}
+	return; # implicit false
+}
+
 sub git_project_list {
 	my @list = git_read_projects();
 	my @projects;
@@ -1233,6 +1252,7 @@ sub git_tag {
 
 sub git_blame {
 	my $fd;
+	die_error('403 Permission denied', "Permission denied.") if (!git_get_project_config_bool ('blame'));
 	die_error('404 Not Found', "What file will it be, master?") if (!$file_name);
 	$hash_base ||= git_read_head($project);
 	die_error(undef, "Reading commit failed.") unless ($hash_base);
@@ -1468,6 +1488,7 @@ sub git_blob {
 		my $base = $hash_base || git_read_head($project);
 		$hash = git_get_hash_by_path($base, $file_name, "blob") || die_error(undef, "Error lookup file.");
 	}
+	my $have_blame = git_get_project_config_bool ('blame');
 	open my $fd, "-|", "$gitbin/git-cat-file blob $hash" or die_error(undef, "Open failed.");
 	git_header_html();
 	if (defined $hash_base && (my %co = git_read_commit($hash_base))) {
@@ -1479,8 +1500,10 @@ sub git_blob {
 		      " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commitdiff;h=$hash_base")}, "commitdiff") .
 		      " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=tree;h=$co{'tree'};hb=$hash_base")}, "tree") . "<br/>\n";
 		if (defined $file_name) {
-			print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blame;h=$hash;hb=$hash_base;f=$file_name")}, "blame") .
-			" | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob_plain;h=$hash;f=$file_name")}, "plain") .
+			if ($have_blame) {
+				print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blame;h=$hash;hb=$hash_base;f=$file_name")}, "blame") .  " | ";
+			}
+			print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob_plain;h=$hash;f=$file_name")}, "plain") .
 			" | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;hb=HEAD;f=$file_name")}, "head") . "<br/>\n";
 		} else {
 			print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob_plain;h=$hash")}, "plain") . "<br/>\n";
-- 
1.3.3

Re: [PATCH] gitweb: Adding a `blame' interface.

[Martin Langhoff]

On 6/12/06, Florian Forster <octo@verplant.org> wrote:
> On Mon, Jun 12, 2006 at 10:02:05AM +1200, Martin Langhoff wrote:
> > good! git-blame/git-annotate are quite expensive to run. Do you think
> > it would make sense making it conditional on a git-repo-config option
> > (gitweb.blame=1)?
>
> sure, that it's a big change and if it helps the kernel.org folks ;)
> I'll follow-up with a patch for this in a second..

That'd be great. I am looking into integrating other feature patches
too (like tarball downloads) that are useful but costly, making them
conditional too...

> Would it help to cache `git-annotate's output, e.g. using one of the

I think we can rely on proxies doing good caching -- a busy host like
kernel.org will have big reverse proxies in front. A git-blame for a
given file+commitsha doesn't change, so we can give it a long cache
time, like... forever ;-)

> I have two more points regarding gitweb's configuration:
> - IMHO it would make sense to move the general gitweb-configuration
>   (where are the repositories, where are the binaries, etc) out of the
>   script.  As far as I know the Debian maintainer of the `gitweb'
>   package has asked for this before but was refused for some reason..

Sounds like a reasonable request. I would make it rely on env vars,
$ENV{GITWEB_CONFIG} can generally point to /etc/gitweb.conf, and that
would override the config values we have.

This is trivial, and it means we buy a lot of flexibility from
apache's httpd.conf being able to point to different config files
depending on arbitrarty conditions.

BTW, I haven't seen the debian maintainer's request, was that on the list?

> - If `GIT_DIR/description' is only used by gitweb it may be more
>   consistent to use the git-repo-config option `gitweb.description' in
>   the future.

Not sure how git-repo configurations deal with long entries. Right now
the description may contain html for instance.



martin

Re: [PATCH] gitweb: Adding a `blame' interface.

[Shawn Pearce]

Martin Langhoff <martin.langhoff@gmail.com> wrote:
> >- If `GIT_DIR/description' is only used by gitweb it may be more
> >  consistent to use the git-repo-config option `gitweb.description' in
> >  the future.
> 
> Not sure how git-repo configurations deal with long entries. Right now
> the description may contain html for instance.

It has to be escaped, which could be ugly with HTML.  For example:

  [gitweb]
    description=<div class=\"description\">\n\
This is a chunk of text which describes this repository.  Some\n\
of this text might be rather long, and might need many lines to\n\
really be able to describe the repository in a nice editor such as\n\
vi running in an 80 character wide xterm.\n\
</div>

Forget a \ in front of a double quote (") or an LF and the entry is
corrupt.  So as nice as it sounds it might not be the best way to
obtain a description for gitweb.

-- 
Shawn.

Re: [PATCH] gitweb: Adding a `blame' interface.

[Johannes Schindelin]

Hi,

On Mon, 12 Jun 2006, Shawn Pearce wrote:

>   [gitweb]
>     description=<div class=\"description\">\n\
> This is a chunk of text which describes this repository.  Some\n\
> of this text might be rather long, and might need many lines to\n\
> really be able to describe the repository in a nice editor such as\n\
> vi running in an 80 character wide xterm.\n\
> </div>

AFAIK the trailing "\" will not work.

Ciao,
Dscho

Re: [PATCH] gitweb: Adding a `blame' interface.

[Shawn Pearce]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
> 
> On Mon, 12 Jun 2006, Shawn Pearce wrote:
> 
> >   [gitweb]
> >     description=<div class=\"description\">\n\
> > This is a chunk of text which describes this repository.  Some\n\
> > of this text might be rather long, and might need many lines to\n\
> > really be able to describe the repository in a nice editor such as\n\
> > vi running in an 80 character wide xterm.\n\
> > </div>
> 
> AFAIK the trailing "\" will not work.

Actually it does.  I figured out that it works (and why it works)
when I implemented the GIT repository parser in Java for my pure
Java version of GIT...

For example:

  [spearce@spearce-pb15 bob]$ cat .git/config 
  [core]
          repositoryformatversion = 0
          filemode = true
  [gitweb]
          description = This is a very\nlong line to put into GIT\n\
  repo config.\n\
  I hope it works.
          on = true
  [spearce@spearce-pb15 bob]$ git repo-config gitweb.description
  This is a very
  long line to put into GIT
  repo config.
  I hope it works.
  [spearce@spearce-pb15 bob]$ git repo-config gitweb.on
  true

The use of a trailing \ makes sense; the collapsing of multiple
spaces into one space unless quoted inside of "" doesn't.
But whatever...

-- 
Shawn.

Re: [PATCH] gitweb: Adding a `blame' interface.

[Linus Torvalds]

On Mon, 12 Jun 2006, Florian Forster wrote:
> 
> Would it help to cache `git-annotate's output, e.g. using one of the
> `Cache::Cache' modules? Or is browsing of blobs too sparse for this to
> result in a performance gain? I'm sure the modules could be integrated
> as a weak precondition.

The apache setup at least on kernel.org is already set up to do caching, 
as long as the generated headers for the page allow it in the first place.

So caching inside gitweb is generally pointless, at least when it's at the 
level of one result page. At a higher level, if the internal caching might 
improve performance of _other_ pages because it caches the result of some 
intermediate important thing, it might be a different issue.

		Linus

Re: [PATCH] gitweb: Supporting caches (was: Adding a `blame' interface.)

[Florian Forster]

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

On Mon, Jun 12, 2006 at 07:59:39AM -0700, Linus Torvalds wrote:
> The apache setup at least on kernel.org is already set up to do
> caching, as long as the generated headers for the page allow it in the
> first place.

I've actually looked into improving native HTTP caching (mostly for
small site without revers proxying) by providing a `Last-Modified'
header where possible and sending a `304 Not Modified' whenever
appropriate.

While it doesn't sound hard it's next to impossible: A commit's
timestamp doesn't change when head a points to it (or does not longer
point to it). Also displaying the timestamps as `Modified xy
{seconds,minutes, hours,...} ago' possess a big problem.

(I guess the webserver could use the `If-Modified-Since' header to check
if the displayed time needs to be updated, but if you ask me it's not
worth the effort.)

In short, the `blob', `blob_plain', and `blobdiff' pages could profit
from that because they don't display the head(s) pointing to the current
commit. On the other hand, this is a little inconsistent and could be
considered a bug. So I'll give up on that unless someone has a great
idea how to handle this.

Regards,
-octo
-- 
Florian octo Forster
Hacker in training
GnuPG: 0x91523C3D
http://verplant.org/

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

Re: gitweb: Config file support (was: Adding a `blame' interface.)

[Florian Forster]

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

Hi Martin,

On Mon, Jun 12, 2006 at 08:34:43PM +1200, Martin Langhoff wrote:
> > As far as I know the Debian maintainer of the `gitweb' package has
> > asked for this before but was refused for some reason..
> BTW, I haven't seen the debian maintainer's request, was that on the list?

Yes, it was a mail by Andres Salomon on May 20th, 2005 with the subject
`add conf file support to gitweb'. A friend of mine asked him if he had
sent the patch upstream and he pointed to this message and explained he
had gotten a private reply saying that gitweb `only covers the special
needs on kernel.org'.

Regards,
-octo
-- 
Florian octo Forster
Hacker in training
GnuPG: 0x91523C3D
http://verplant.org/

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

Re: [PATCH] gitweb: Adding a `blame' interface.

[Jon Loeliger]

On Mon, 2006-06-12 at 03:24, Florian Forster wrote:

> I have two more points regarding gitweb's configuration:
> - IMHO it would make sense to move the general gitweb-configuration
>   (where are the repositories, where are the binaries, etc) out of the
>   script.  As far as I know the Debian maintainer of the `gitweb'
>   package has asked for this before but was refused for some reason..
>   Possibly a file `gitweb.conf' in the same directory as the script
>   could be read and overwrite the builtin defaults..?

I already submitted a patch down this line on 22-Mar-2006:

    http://marc.theaimsgroup.com/?l=git&m=114308224922372&w=2

Thanks,
jdl

Re: [PATCH] gitweb: Adding a `blame' interface.

[Fredrik Kuivinen]

On Sun, Jun 11, 2006 at 05:45:19PM +0200, Florian Forster wrote:
> This patch adds an interface for `git-blame' to `gitweb.cgi'. Links to it are
> place in `git_blob'.
> Internally the code uses `git-annotate' because `git-blame's output differs for
> files that have been renamed in the past. However, I like the term `blame'
> better.
> 

You can pass "--compatibility" to git-blame to get output which is
identical to git-annotates output. However, "--time" is not
implemented in git-blame yet. I will send a patch in a separate mail.

git-blame is a bit faster than git-annotate and, as far as I know, it
produces output which is correct.

- Fredrik

Re: [PATCH] gitweb: Adding a `blame' interface.

[Johannes Schindelin]

Hi,

On Mon, 12 Jun 2006, Fredrik Kuivinen wrote:

> git-blame is a bit faster than git-annotate and, as far as I know, it 
> produces output which is correct.

Yeah: Bring It On(tm)! I already waited for ages for this war to begin!

Ciao,
Dscho

Re: [PATCH] gitweb: Adding a `blame' interface.

[Linus Torvalds]

On Tue, 13 Jun 2006, Johannes Schindelin wrote:
> 
> On Mon, 12 Jun 2006, Fredrik Kuivinen wrote:
> 
> > git-blame is a bit faster than git-annotate and, as far as I know, it 
> > produces output which is correct.
> 
> Yeah: Bring It On(tm)! I already waited for ages for this war to begin!

Sadly, I don't think either of you can really do much about the fact that 
annotate/blame is simply the wrong model for git.

The war _I_d like to see is the GUI thing which does the "show when this 
section changed last" by following the history down only so far that the 
selected section shows up in the diff against the most current thing.

THAT is what I want. It also fits the git model much better, since you 
generally don't have to go back all the way.

Somebody? "Here's a nickel saying you can't do it!" (if somebody needs the 
motivation ;)

		Linus

Re: [PATCH] gitweb: Adding a `blame' interface.

[Johannes Schindelin]

Hi,

On Mon, 12 Jun 2006, Linus Torvalds wrote:

> On Tue, 13 Jun 2006, Johannes Schindelin wrote:
> > 
> > On Mon, 12 Jun 2006, Fredrik Kuivinen wrote:
> > 
> > > git-blame is a bit faster than git-annotate and, as far as I know, it 
> > > produces output which is correct.
> > 
> > Yeah: Bring It On(tm)! I already waited for ages for this war to begin!
> 
> Sadly, I don't think either of you can really do much about the fact that 
> annotate/blame is simply the wrong model for git.
> 
> The war _I_d like to see is the GUI thing which does the "show when this 
> section changed last" by following the history down only so far that the 
> selected section shows up in the diff against the most current thing.

But this is just the next step! Nothing prevents you -- once everybody 
agrees that blame/annotate does the right thing -- to restrict the lines 
of interest. And AFAICT both blame and annotate are good at stopping when 
all lines are accounted for.

Ciao,
Dscho

Re: [PATCH] gitweb: Adding a `blame' interface.

[Linus Torvalds]

On Tue, 13 Jun 2006, Johannes Schindelin wrote:
> > 
> > The war _I_d like to see is the GUI thing which does the "show when this 
> > section changed last" by following the history down only so far that the 
> > selected section shows up in the diff against the most current thing.
> 
> But this is just the next step! Nothing prevents you -- once everybody 
> agrees that blame/annotate does the right thing -- to restrict the lines 
> of interest. And AFAICT both blame and annotate are good at stopping when 
> all lines are accounted for.

You misunderstand. It's not "all", it's "any" (in fact, it would be even 
better if it would be on a byte-range basis, not on a line-based diff 
basis).

And it needs the GUI to make it useful, because nobody in their right mind 
will say "git showchange fs/inode.c 77-89" (and then do it iteratively if 
the first one wasn't actually the interesting case).

		Linus

Re: [PATCH] gitweb: Adding a `blame' interface.

[Junio C Hamano]

"Martin Langhoff" <martin.langhoff@gmail.com> writes:

> Florian,
>
> Looks good! git-blame/git-annotate are quite expensive to run. Do you
> think it would make sense making it conditional on a git-repo-config
> option (gitweb.blame=1)?
>
> kernel.org is the flagship user for gitweb, so expensive options
> should default to off :-/

Seconded.  Thanks Florian and Martin.

Re: [PATCH] gitweb: Adding a `blame' interface.

[Marco Costalba]

On 6/13/06, Linus Torvalds <torvalds@osdl.org> wrote:
>
>
> On Tue, 13 Jun 2006, Johannes Schindelin wrote:
> >
> > On Mon, 12 Jun 2006, Fredrik Kuivinen wrote:
> >
> > > git-blame is a bit faster than git-annotate and, as far as I know, it
> > > produces output which is correct.
> >
> > Yeah: Bring It On(tm)! I already waited for ages for this war to begin!
>
> Sadly, I don't think either of you can really do much about the fact that
> annotate/blame is simply the wrong model for git.
>
> The war _I_d like to see is the GUI thing which does the "show when this
> section changed last" by following the history down only so far that the
> selected section shows up in the diff against the most current thing.
>

Probably I have misunderstood the request, but in qgit you can (from
about one month ago) mouse select some lines in file content then
press the filter button and see only the revisions that modify the
selected text.

Also selected text is highlighted so to better view differences among revisions.

It is not clear to me in what the requested feature differs from this
implementation.

    Marco