Re: [PATCHv4 2/2] gitweb: gravatar support

From: Nanako Shiraishi <nanako3@lavabit.com>
Date: Jun 24, 2009, 13:44 UTC
Message-ID: <20090624224430.6117@nanako3.lavabit.com>
In-Reply-To: <1245710999-17763-3-git-send-email-giuseppe.bilotta@gmail.com>

Quoting Giuseppe Bilotta <giuseppe.bilotta@gmail.com>:

Show 16 quoted lines

> 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.

Show 7 quoted lines

> +	# Gravatar support. When this feature is enabled, views such as
> ........
> +	'gravatar' => {
> +		'sub' => sub { feature_bool('gravatar', @_) },
> +		'override' => 0,
> +		'default' => [0]},
>  );

And this "feature" is about getting such avatars from "gravatar" service; it is good to use such a specific word here.

> +# check if gravatars are enabled and dependencies are satisfied
> +our $git_gravatar_enabled = gitweb_check_feature('gravatar') &&
> +	(eval { require Digest::MD5; 1; });

The same.

Show 7 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).

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/