Re: [PATCHv4 2/2] gitweb: gravatar support
- From
Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
- Date
- Jun 24, 2009, 20:00 UTC
- Message-ID
- <cb7bb73a0906241300mf10e073j99ad2b09c806d0d7@mail.gmail.com>
- In-Reply-To
- <20090624224430.6117@nanako3.lavabit.com>
On Wed, Jun 24, 2009 at 3:44 PM, Nanako Shiraishi<nanako3@lavabit.com> wrote:
Show 21 quoted lines
> Quoting Giuseppe Bilotta <giuseppe.bilotta@gmail.com>:
>
>> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
>> index 68b22ff..ddf982b 100644
>> ........
>> +img.avatar {
>> + vertical-align:middle;
>> +}
>> +
>> .........
>> +# Pixel sizes for avatars. If the default font sizes or lineheights
>> +# are changed, it may be appropriate to change these values too via
>> +# $GITWEB_CONFIG.
>> +our %avatar_size = (
>> + 'default' => 16,
>> + 'double' => 32
>> +) ;
>> ........
>
> Early parts of the patch talk about "avatars"; compared with "icons" Junio
> suggested, I think that is a better generic word to use for this purpose.Well, I would argue that 'avatar' is pretty commonly understood to be the name of the picture identifying an individual online. OTOH, I've been thinking that there are other kind of icons that might be used across gitweb in the future, and they are likely to use the same sizes, so I'll go for icon_size here.
Show 28 quoted lines
>> @@ -1474,7 +1501,7 @@ 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>\n";
>> + return "<$tag class=\"author\">" . git_get_gravatar($co->{'author_email'}, 'space_after' => 1) . $author . "</$tag>\n";
>> }
>
> But the function that returns a string suitable for embedding in the HTML
> page, given an e-mail address, is called get_gravatar(), not get_avatar()?
>
> I expected from an earlier review message by Junio that get_avatar() will
> look like this:
>
> sub git_get_avatar {
> my $url = undef;
> if ($git_gravatar_enabled) {
> my $md5 = .......;
> $url = "http://www.gravatar.com/avatar.php?gravatar_id=$md5";
> } else if ($git_picons_enabled) {
> my $userpath = .......;
> $url = "http://www.cs.indiana.edu/picons/db/users/$userpath/face.xpm";
> }
> return "" unless (defined $url);
> return $pre_white . "<img ... src=\"$url\" size=$size />" . $post_white;
> }
>
> (without "picons" part, of course).Right. I was wondering if it would make sense to factor each avatar block separately (i.e. have a git_get_avatar that calls git_get_whatever) but it probably doesn't make sense.
I'll wait for Jakub's (and whoever else wants to chip in) comments, and then I'll send an update to this patch following your recommendations.
-- Giuseppe "Oblomov" Bilotta