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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]