git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCHv7 8/9] gitweb: use picon for gravatar fallback
Date: Sun, 28 Jun 2009 16:42:36 +0200	[thread overview]
Message-ID: <200906281642.36410.jnareb@gmail.com> (raw)
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...

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

> +		$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

  parent reply	other threads:[~2009-06-28 14:37 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-27 12:04 [PATCHv7 0/9] gitweb: avatar support Giuseppe Bilotta
2009-06-27 12:04 ` [PATCHv7 1/9] gitweb: refactor author name insertion Giuseppe Bilotta
2009-06-27 12:04   ` [PATCHv7 2/9] gitweb: uniform author info for commit and commitdiff Giuseppe Bilotta
2009-06-27 12:04     ` [PATCHv7 3/9] gitweb: use git_print_authorship_rows in 'tag' view too Giuseppe Bilotta
2009-06-27 12:05       ` [PATCHv7 4/9] gitweb: right-align date cell in shortlog Giuseppe Bilotta
2009-06-27 12:05         ` [PATCHv7 5/9] gitweb: (gr)avatar support Giuseppe Bilotta
2009-06-27 12:05           ` [PATCHv7 6/9] gitweb: gravatar url cache Giuseppe Bilotta
2009-06-27 12:05             ` [PATCHv7 7/9] gitweb: picon avatar provider Giuseppe Bilotta
2009-06-27 12:05               ` [PATCHv7 8/9] gitweb: use picon for gravatar fallback Giuseppe Bilotta
2009-06-27 12:05                 ` [PATCHv7 9/9] gitweb: add alt text to avatar img Giuseppe Bilotta
2009-06-28 15:43                   ` Jakub Narebski
2009-06-28 16:10                     ` Giuseppe Bilotta
2009-06-28 14:42                 ` Jakub Narebski [this message]
2009-06-28 11:35               ` [PATCHv7 7/9] gitweb: picon avatar provider Jakub Narebski
2009-06-28 16:03                 ` Giuseppe Bilotta
2009-06-27 22:15             ` [PATCHv7 6/9] gitweb: gravatar url cache Jakub Narebski
2009-06-27 19:45           ` [PATCHv7 5/9] gitweb: (gr)avatar support Jakub Narebski
2009-06-27 22:45             ` Giuseppe Bilotta
2009-06-27 23:20               ` Jakub Narebski
2009-06-27 18:28         ` [PATCHv7 4/9] gitweb: right-align date cell in shortlog Jakub Narebski
2009-06-27 22:27           ` Giuseppe Bilotta
2009-06-27 18:10       ` [PATCHv7 3/9] gitweb: use git_print_authorship_rows in 'tag' view too Jakub Narebski
2009-06-27 22:24         ` Giuseppe Bilotta
2009-06-27 16:10     ` [PATCHv7 2/9] gitweb: uniform author info for commit and commitdiff Jakub Narebski
2009-06-27 18:38       ` Junio C Hamano
2009-06-27 22:33         ` Giuseppe Bilotta
2009-06-27 14:24   ` [PATCHv7 1/9] gitweb: refactor author name insertion Jakub Narebski
2009-06-27 15:26     ` Giuseppe Bilotta
2009-06-27 15:48       ` Giuseppe Bilotta
2009-06-29 21:37 ` [PATCHv7 0/9] gitweb: avatar support Jakub Narebski
2009-06-29 21:55   ` Giuseppe Bilotta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200906281642.36410.jnareb@gmail.com \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=giuseppe.bilotta@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).