Re: [PATCHv8 4/7] gitweb: (gr)avatar support
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Jun 30, 2009, 20:16 UTC
- Message-ID
- <200906302216.48872.jnareb@gmail.com>
- In-Reply-To
- <1246312854-3365-5-git-send-email-giuseppe.bilotta@gmail.com>
On Tue, 30 Jun 2009, Giuseppe Bilotta wrote:
Show 12 quoted lines
> Introduce avatar support: the feature adds the appropriate img tag next > to author and committer in commit(diff), history, shortlog, log and tag > views. Multiple avatar providers are possible, but only gravatar is > implemented at the moment. > > Gravatar support depends on Digest::MD5, which is a core package since > Perl 5.8. If gravatars are activated but Digest::MD5 cannot be found, > the feature will be automatically disabled. > > No avatar provider is selected by default, except in the t9500 test. > > Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
I like both commit message and comprehensive comments.
Acked-by: Jakub Narebski <jnareb@gmail.com>
[...]
Show 13 quoted lines
> +# Insert an avatar for the given $email at the given $size if the feature
> +# is enabled.
> +sub git_get_avatar {
> + my ($email, %opts) = @_;
> + my $pre_white = ($opts{-pad_before} ? " " : "");
> + my $post_white = ($opts{-pad_after} ? " " : "");
> + $opts{-size} ||= 'default';
> + my $size = $avatar_size{$opts{-size}} || $avatar_size{'default'};
> + my $url = "";
> + if ($git_avatar eq 'gravatar') {
> + $url = "http://www.gravatar.com/avatar/" .
> + Digest::MD5::md5_hex(lc $email) . "?s=$size";
> + }I guess that you use short form 's' instead of 'size' to make URL (and therefore gitweb pages) shorter, isn't it?
Show 10 quoted lines
> + # Currently only gravatars are supported, but other forms such as
> + # picons can be added by putting an else up here and defining $url
> + # as needed. If no variant puts something in $url, we assume avatars
> + # are completely disabled/unavailable.
> + if ($url) {
> + return $pre_white .
> + "<img width=\"$size\" " .
> + "class=\"avatar\" " .
> + "src=\"$url\" " .
> + "/>" . $post_white;Nitpicky sidenote: it might be more readable to use qq( ... ) quote-like operator to avoid need to escape \" while providing variable interpolation. But on the other hand it might not: this is more obscure Perl operator.
Nevertheless it is not something to worry about (and certainly not something that would require resend).
Show 65 quoted lines
> + } else {
> + return "";
> + }
> +}
> +
> # format the author name of the given commit with the given tag
> # the author name is chopped and escaped according to the other
> # optional parameters (see chop_str).
> @@ -1476,7 +1547,9 @@ sub format_author_html {
> my $tag = shift;
> my $co = shift;
> my $author = chop_and_escape_str($co->{'author_name'}, @_);
> - return "<$tag class=\"author\">" . $author . "</$tag>";
> + return "<$tag class=\"author\">" .
> + git_get_avatar($co->{'author_email'}, -pad_after => 1) .
> + $author . "</$tag>";
> }
>
> # format git diff header line, i.e. "diff --(git|combined|cc) ..."
> @@ -3252,7 +3325,8 @@ sub git_print_authorship {
> esc_html($co->{'author_name'}) .
> " [$ad{'rfc2822'}";
> print_local_time(%ad) if ($opts{-localtime});
> - print "]</$tag>\n";
> + print "]" . git_get_avatar($co->{'author_email'}, -pad_before => 1)
> + . "</$tag>\n";
> }
>
> # Outputs table rows containing the full author or committer information,
> @@ -3267,7 +3341,10 @@ sub git_print_authorship_rows {
> @people = ('author', 'committer') unless @people;
> foreach my $who (@people) {
> my %wd = parse_date($co->{"${who}_epoch"}, $co->{"${who}_tz"});
> - print "<tr><td>$who</td><td>" . esc_html($co->{$who}) . "</td></tr>\n".
> + print "<tr><td>$who</td><td>" . esc_html($co->{$who}) . "</td>" .
> + "<td rowspan=\"2\">" .
> + git_get_avatar($co->{"${who}_email"}, -size => 'double') .
> + "</td></tr>\n" .
> "<tr>" .
> "<td></td><td> $wd{'rfc2822'}";
> print_local_time(%wd);
> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
> index d539619..6275181 100755
> --- a/t/t9500-gitweb-standalone-no-errors.sh
> +++ b/t/t9500-gitweb-standalone-no-errors.sh
> @@ -660,6 +660,7 @@ cat >>gitweb_config.perl <<EOF
>
> \$feature{'blame'}{'override'} = 1;
> \$feature{'snapshot'}{'override'} = 1;
> +\$feature{'avatar'}{'override'} = 1;
> EOF
>
> test_expect_success \
> @@ -671,6 +672,7 @@ test_expect_success \
> 'config override: tree view, features disabled in repo config' \
> 'git config gitweb.blame no &&
> git config gitweb.snapshot none &&
> + git config gitweb.avatar gravatar &&
> gitweb_run "p=.git;a=tree"'
> test_debug 'cat gitweb.log'
>
> --
> 1.6.3.rc1.192.gdbfcb
>
> -- Jakub Narebski Poland