Re: [PATCH 4/4] gitweb: show notes in commit(diff) view

[Jakub Narebski <jnareb@gmail.com>]

On Thu, 4 Jan 2010, Giuseppe Bilotta wrote:

> The notes are shown side-by-side along the bottom of the commit
> message. 

The same question apply as for previous commit.

What happens if screen size is too small to contain both commit message
and notes?  Does it do the sensible thing of putting notes _below_
commit message in such situation?  I do not know CSS+HTML enogh to
answer this question myself.

> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 0d0877e..0d03026 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2837,12 +2837,31 @@ sub parse_commit {
>  	%co = parse_commit_text(<$fd>, 1);
>  	close $fd;
>  
> +	my %notes = ();
> +	foreach my $note_ref (get_note_refs()) {
> +		my $obj = "$note_ref:$co{'id'}";
> +		if (open my $fd, '-|', git_cmd(), 'rev-parse',
> +			'--verify', '-q', $obj) {
> +			my $exists = <$fd>;
> +			close $fd;
> +			if (defined $exists) {
> +				if (open $fd, '-|', git_cmd(), 'show', $obj) {
> +					$notes{$note_ref} = scalar <$fd>;
> +					close $fd;
> +				}
> +			}
> +		}
> +	}
> +	$co{'notes'} = \%notes;
> +
>  	return %co;
>  }

Duplicated code.  Please put this code in a separate subroutine, to be
called in those two places.
  
>  # return all refs matching refs/notes/<globspecs> where the globspecs
>  # are taken from the notes feature content.
>  sub get_note_refs {
> +	local $/ = "";
> +

Why it is needed here?  Why you want to use empty lines as terminator
(which means reading whole paragraphs), while treating two or more
consecutive empty lines as a single empty line (according to
perlvar(1))?

If you want to slurp whole file, this should be

   	local $/;

or more explicit

   	local $/ = undef;

>  	my @globs = gitweb_get_feature('notes');
>  	my @note_refs = ();
>  	foreach my $glob (@globs) {
> @@ -5875,6 +5894,7 @@ sub git_commit {
>  
>  	print "<div class=\"page_body\">\n";
>  	git_print_log($co{'comment'});
> +	print format_notes_html($co{'notes'}, 'div');
>  	print "</div>\n";
>  
>  	git_difftree_body(\@difftree, $hash, @$parents);
> @@ -6230,6 +6250,7 @@ sub git_commitdiff {
>  			git_print_log($co{'comment'}, -final_empty_line=> 1, -remove_title => 1);
>  			print "</div>\n"; # class="log"
>  		}
> +		print format_notes_html($co{'notes'}, 'div');
>  
>  	} elsif ($format eq 'plain') {
>  		my $refs = git_get_references("tags");

This of course assumes that we want notes treated exactly (or almost
exactly) the same way for 'log', 'commit' and 'commitdiff' views.
Perhaps it is a good assumption (at least for first step)...

-- 
Jakub Narebski
Poland