Re: [PATCH] gitweb: make leftmost column of blame less cluttered.

[Luben Tuikov <ltuikov@yahoo.com>]

--- Junio C Hamano <junkio@cox.net> wrote:
> Perhaps like this.  This has hardcoded chunk-cap of 20 lines,
> which means we never buffer more than 40 lines and never emit
> more than 20 lines at a time.  The reason we do not immediately
> flush after getting 20 lines is if we have a block of 21 lines
> we would end up giving 20 line chunk and 1 line chunk and then a
> chunk for the different revision (orphaned line).  In such a
> case we are better off giving two evenly divided chunks and that
> is why we buffer up to twice the chunk-cap size.
> 
> If people are interested we probably would want to make 20
> configurable.  Maybe not.  I personally suspect it does not
> matter much.

I think this would get us into too much "special cases".  The colored
chunks of commits were added with the intention of "single commit",
so I think we should be ok, to have a single row at the top for
the initial commit when the file had not been changed since.

Initial commits are normally "well known commits", so I think it should
be ok to not have to print every 20 or so lines as this breaks the 
color-block-per-commit intention.

   Luben


> 
> -- >8 --
> [PATCH] gitweb: make leftmost column of blame less cluttered.
> 
> Instead of labelling each and every line with clickable commit
> object name, this makes the blame output to show them only on
> the first line of each group of lines from the same revision.
> 
> Also it makes mouse-over to show the minimum authorship and
> authordate information for extra cuteness ;-).
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>
> ---
>  gitweb/gitweb.perl |   99 +++++++++++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 86 insertions(+), 13 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> old mode 100755
> new mode 100644
> index 3e9d4a0..55d1b2c
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2430,9 +2430,64 @@ sub git_tag {
>  	git_footer_html();
>  }
>  
> +sub git_blame_flush_chunk {
> +	my ($name, $revdata, $color, $rev, @line) = @_;
> +	my $label = substr($rev, 0, 8);
> +	my $line = scalar(@line);
> +	my $cnt = 0;
> +	my $pop = '';
> +
> +	if ($revdata->{$rev} ne '') {
> +		$pop = ' title="' . esc_html($revdata->{$rev}) . '"';
> +	}
> +
> +	for (@line) {
> +		my ($lineno, $data) = @$_;
> +		$cnt++;
> +		print "<tr class=\"$color\">\n";
> +		if ($cnt == 1) {
> +			print "<td class=\"sha1\"$pop";
> +			if ($line > 1) {
> +				print " rowspan=\"$line\"";
> +			}
> +			print ">";
> +			print $cgi->a({-href => href(action=>"commit",
> +						     hash=>$rev,
> +						     file_name=>$name)},
> +				      $label);
> +			print "</td>\n";
> +		}
> +		print "<td class=\"linenr\">".
> +		    "<a id=\"l$lineno\" href=\"#l$lineno\" class=\"linenr\">" .
> +		    esc_html($lineno) . "</a></td>\n";
> +		print "<td class=\"pre\">" . esc_html($data) . "</td>\n";
> +		print "</tr>\n";
> +	}
> +}
> +
> +# We can have up to N*2 lines.  If it is more than N lines, split it
> +# into two to avoid orphans.
> +sub git_blame_flush_chunk_1 {
> +	my ($chunk_cap, $name, $revdata, $color, $rev, @chunk) = @_;
> +	if ($chunk_cap < @chunk) {
> +		my @first = splice(@chunk, 0, @chunk/2);
> +		git_blame_flush_chunk($name,
> +				      $revdata,
> +				      $color,
> +				      $rev,
> +				      @first);
> +	}
> +	git_blame_flush_chunk($name,
> +			      $revdata,
> +			      $color,
> +			      $rev,
> +			      @chunk);
> +}
> +
>  sub git_blame2 {
>  	my $fd;
>  	my $ftype;
> +	my $chunk_cap = 20;
>  
>  	my ($have_blame) = gitweb_check_feature('blame');
>  	if (!$have_blame) {
> @@ -2475,27 +2530,45 @@ sub git_blame2 {
>  <table class="blame">
>  <tr><th>Commit</th><th>Line</th><th>Data</th></tr>
>  HTML
> +	my @chunk = ();
> +	my %revdata = ();
>  	while (<$fd>) {
>  		/^([0-9a-fA-F]{40}).*?(\d+)\)\s{1}(\s*.*)/;
> -		my $full_rev = $1;
> -		my $rev = substr($full_rev, 0, 8);
> -		my $lineno = $2;
> -		my $data = $3;
> -
> +		my ($full_rev, $author, $date, $lineno, $data) =
> +		    /^([0-9a-f]{40}).*?\s\((.*?)\s+([-\d]+ [:\d]+ [-+\d]+)\s+(\d+)\)\s(.*)/;
> +		if (!exists $revdata{$full_rev}) {
> +			$revdata{$full_rev} = "$author, $date";
> +		}
>  		if (!defined $last_rev) {
>  			$last_rev = $full_rev;
>  		} elsif ($last_rev ne $full_rev) {
> +			git_blame_flush_chunk_1($chunk_cap,
> +						$file_name,
> +						\%revdata,
> +						$rev_color[$current_color],
> +						$last_rev, @chunk);
> +			@chunk = ();
>  			$last_rev = $full_rev;
>  			$current_color = ++$current_color % $num_colors;
>  		}
> -		print "<tr class=\"$rev_color[$current_color]\">\n";
> -		print "<td class=\"sha1\">" .
> -			$cgi->a({-href => href(action=>"commit", hash=>$full_rev, file_name=>$file_name)},
> -			        esc_html($rev)) . "</td>\n";
> -		print "<td class=\"linenr\"><a id=\"l$lineno\" href=\"#l$lineno\" class=\"linenr\">" .
> -		      esc_html($lineno) . "</a></td>\n";
> -		print "<td class=\"pre\">" . esc_html($data) . "</td>\n";
> -		print "</tr>\n";
> +		elsif ($chunk_cap * 2 < @chunk) {
> +			# We have more than N*2 lines from the same
> +			# revision.  Flush N lines and leave N lines
> +			# in @chunk to avoid orphaned lines.
> +			my @first = splice(@chunk, 0, $chunk_cap);
> +			git_blame_flush_chunk($file_name,
> +					      \%revdata,
> +					      $rev_color[$current_color],
> +					      $last_rev, @first);
> +		}
> +		push @chunk, [$lineno, $data];
> +	}
> +	if (@chunk) {
> +		git_blame_flush_chunk_1($chunk_cap,
> +					$file_name,
> +					\%revdata,
> +					$rev_color[$current_color],
> +					$last_rev, @chunk);
>  	}
>  	print "</table>\n";
>  	print "</div>";
> -- 
> 1.4.2.3.gbe06
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>