Re: [PATCHv7 8/9] gitweb: use picon for gravatar fallback

From: Jakub Narebski <jnareb@gmail.com>
Date: Jun 28, 2009, 14:42 UTC
Message-ID: <200906281642.36410.jnareb@gmail.com>
In-Reply-To: <1246104305-15191-9-git-send-email-giuseppe.bilotta@gmail.com>

On Sat, 27 June 2009, Giuseppe Bilotta wrote:

I think this issue is complicated enough to do it well and configurable that it is out of scope of current patch series, and it would be better to be left for next series.

.......................................................................

There are few issues connected with multiple avatar providers:

1. Avatar provider side fallback, i.e. what to do if given person
(usually given email address) is not registered with used avatar 
provider.

Gravatar provider by default falls back to [G] gravatar default image, and you can specify fallback using 'd'/'default' parameter, either to one of defined providers: special values "identicon", "monsterid" and "wavatar", or to specified image using URI-encoded URL as value of 'default' parameter. From the point of view of gitweb, you either pass provider or URL literally (encoding), or you resolve fallback provider avatar and pass encoded URI.

Picon provider has built-in many level fallback; current gitweb implementation searches in 'users' database, then in 'domains' database (there e.g. gmail.com domain addresses receive GMail avatar), and last in 'unknown' database which would always provide some default avatar for "person". But in general one can use cs.indiana.edu database as fallback, and try personal / local site database first.

2. Gitweb side fallback; for some kinds of avatars one can think about,
for example 'local' avatars (example URL: "avatar/user.gif"), or 'inrepo'
avatar provider (example URL: "?p=$project;a=blob_plain;f=avatar/user.gif"),
one can easily detect in gitweb if avatar for given email (given person)
exists or not (is available or not).

3. Multiple avatars.  We can use here something like what Gmane does,
which shows gravatar if it is available (and nothing if it is not), 
picon if it is available (and nothing if it is not), and both picon
and gravatar if both are available.  See example in:
  http://permalink.gmane.org/gmane.comp.version-control.git/122394
  http://permalink.gmane.org/gmane.comp.version-control.git/118058

This patch tries to address issue 1.) for gravatar provider (which allow for specifying default avatar via its URL API). I think however that this issue should be configurable; unconditional using picon provider with very long URI (making gravatar-with-picon-fallback provider URI very, very long) is not a good idea.

By configurable I mean that user should be able to specify e.g. gravatar with wavatar fallback, gravatar with static image fallback, and gravatar with arbitrary provider (resolved) fallback.

Unless you wanted to provide this patch as an example of having multiple providers / providing avatar fallback (default value). But then it would be better to have it marked as RFC explicitly...

Show 13 quoted lines

> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index ae73d45..e2638cb 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1546,9 +1546,13 @@ sub picon_url {
>  sub gravatar_url {
>  	my $email = lc shift;
>  	my $size = shift;
> -	$avatar_cache{$email} ||=
> -		"http://www.gravatar.com/avatar.php?gravatar_id=" .
> -			Digest::MD5::md5_hex($email) . "&amp;size=";
> +	if (!$avatar_cache{$email}) {
> +		my $picon = CGI::escape(picon_url($email));

Sidenote: if we end requiring 'escape', we can simply import it.

Show 7 quoted lines

> +		$avatar_cache{$email} = "http://www.gravatar.com/avatar.php?gravatar_id=" .
> +			Digest::MD5::md5_hex($email) .
> +			"&amp;default=$picon" .
> +			"&amp;size=";
> +	}
>  	return $avatar_cache{$email} . $size;
>  }

Nevertheless this is nice example that infrastructure created in previous patches leads to adding such feature quite simple.

-- 
Jakub Narebski
Poland