{"thread":{"id":"19905","subject":"[PATCHv4 1/2] gitweb: refactor author name insertion","startedAt":"2009-06-22T22:49:57Z","lastAt":"2009-06-25T01:35:27Z","messageCount":10,"participants":["Giuseppe Bilotta","Junio C Hamano","Jakub Narebski","Nanako Shiraishi"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"116794","messageId":"1245710999-17763-1-git-send-email-giuseppe.bilotta@gmail.com","threadId":"19905","inReplyTo":null,"subject":"[PATCHv4 0/2] gitweb: gravatar support","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-06-22T22:49:57Z","receivedAt":"2009-06-22T22:49:57Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"As suggested by Junio, the author display refactoring now comes before\nthe actual gravatar feature implementation. The refactoring also goes a\nlittle bit deeper, changing the 'commitdiff' view to match 'commit' for\nthe author and committer layout.\n\nGiuseppe Bilotta (2):\n gitweb: refactor author name insertion\n gitweb: gravatar support\n\n gitweb/gitweb.css | 9 +++-\n gitweb/gitweb.perl | 123 ++++++++++++++++++++++++++++++++++++++-------------\n 2 files changed, 99 insertions(+), 33 deletions(-)\n"},{"id":"116793","messageId":"1245710999-17763-2-git-send-email-giuseppe.bilotta@gmail.com","threadId":"19905","inReplyTo":"1245710999-17763-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv4 1/2] gitweb: refactor author name insertion","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-06-22T22:49:58Z","receivedAt":"2009-06-22T22:49:58Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"The refactoring allows easier customization of the output by means of\nCSS and improves extensibility on the CGI side too.\n\nLayout is preserved for all views except for 'commitdiff', which now\nuses the same format as 'commit'.\n\nSigned-off-by: Giuseppe Bilotta \n---\n gitweb/gitweb.css | 5 ++-\n gitweb/gitweb.perl | 79 +++++++++++++++++++++++++++++++---------------------\n 2 files changed, 51 insertions(+), 33 deletions(-)\n\ndiff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\nindex a01eac8..68b22ff 100644\n--- a/gitweb/gitweb.css\n+++ b/gitweb/gitweb.css\n@@ -132,11 +132,14 @@ div.list_head {\n \tfont-style: italic;\n }\n \n+.author_date, .author {\n+\tfont-style: italic;\n+}\n+\n div.author_date {\n \tpadding: 8px;\n \tborder: solid #d9d8d1;\n \tborder-width: 0px 0px 1px 0px;\n-\tfont-style: italic;\n }\n \n a.list {\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 1e7e2d8..223648f 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1469,6 +1469,14 @@ sub format_subject_html {\n \t}\n }\n \n+# format the author name with the given tag and optionally shortening it\n+sub format_author_html {\n+\tmy $tag = shift;\n+\tmy $co = shift;\n+\tmy $author = chop_and_escape_str($co->{'author_name'}, @_);\n+\treturn \"<$tag class=\\\"author\\\">\" . $author . \"\\n\";\n+}\n+\n # format git diff header line, i.e. \"diff --(git|combined|cc) ...\"\n sub format_git_diff_header_line {\n \tmy $line = shift;\n@@ -3214,11 +3222,13 @@ sub git_print_header_div {\n \t \"\\n\\n\";\n }\n \n+# Outputs the author name and date in long form\n sub git_print_authorship {\n \tmy $co = shift;\n+\tmy $tag = shift || 'div';\n \n \tmy %ad = parse_date($co->{'author_epoch'}, $co->{'author_tz'});\n-\tprint \"
\" .\n+\tprint \"<$tag class=\\\"author_date\\\">\" .\n \t esc_html($co->{'author_name'}) .\n \t \" [$ad{'rfc2822'}\";\n \tif ($ad{'hour_local'} < 6) {\n@@ -3228,7 +3238,30 @@ sub git_print_authorship {\n \t\tprintf(\" (%02d:%02d %s)\",\n \t\t $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});\n \t}\n-\tprint \"]
\\n\";\n+\tprint \"]\\n\";\n+}\n+\n+# Outputs the author and commiter name and date in long form\n+sub git_print_who {\n+\tmy $co = shift;\n+\tmy %ad = parse_date($co->{'author_epoch'}, $co->{'author_tz'});\n+\tmy %cd = parse_date($co->{'committer_epoch'}, $co->{'committer_tz'});\n+\tprint \"author\" . esc_html($co->{'author'}) . \"\\n\".\n+\t \"\" .\n+\t \" $ad{'rfc2822'}\";\n+\tif ($ad{'hour_local'} < 6) {\n+\t\tprintf(\" (%02d:%02d %s)\",\n+\t\t $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});\n+\t} else {\n+\t\tprintf(\" (%02d:%02d %s)\",\n+\t\t $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});\n+\t}\n+\tprint \"\" .\n+\t \"\\n\";\n+\tprint \"committer\" . esc_html($co->{'committer'}) . \"\\n\";\n+\tprint \" $cd{'rfc2822'}\" .\n+\t sprintf(\" (%02d:%02d %s)\", $cd{'hour_local'}, $cd{'minute_local'}, $cd{'tz_local'}) .\n+\t \"\\n\";\n }\n \n sub git_print_page_path {\n@@ -4142,11 +4175,9 @@ sub git_shortlog_body {\n \t\t\tprint \"\\n\";\n \t\t}\n \t\t$alternate ^= 1;\n-\t\tmy $author = chop_and_escape_str($co{'author_name'}, 10);\n \t\t# git_summary() used print \"$co{'age_string'}\\n\" .\n \t\tprint \"$co{'age_string_date'}\\n\" .\n-\t\t \"\" . $author . \"\\n\" .\n-\t\t \"\";\n+\t\t\tformat_author_html('td', \\%co, 10) . \"\";\n \t\tprint format_subject_html($co{'title'}, $co{'title_short'},\n \t\t href(action=>\"commit\", hash=>$commit), $ref);\n \t\tprint \"\\n\" .\n@@ -4193,11 +4224,9 @@ sub git_history_body {\n \t\t\tprint \"\\n\";\n \t\t}\n \t\t$alternate ^= 1;\n-\t# shortlog uses chop_str($co{'author_name'}, 10)\n-\t\tmy $author = chop_and_escape_str($co{'author_name'}, 15, 3);\n \t\tprint \"$co{'age_string_date'}\\n\" .\n-\t\t \"\" . $author . \"\\n\" .\n-\t\t \"\";\n+\t# shortlog uses format_author_html('td', \\%co, 10)\n+\t\t\tformat_author_html('td', \\%co, 15, 3) . \"\";\n \t\t# originally git_history used chop_str($co{'title'}, 50)\n \t\tprint format_subject_html($co{'title'}, $co{'title_short'},\n \t\t href(action=>\"commit\", hash=>$commit), $ref);\n@@ -4350,9 +4379,8 @@ sub git_search_grep_body {\n \t\t\tprint \"\\n\";\n \t\t}\n \t\t$alternate ^= 1;\n-\t\tmy $author = chop_and_escape_str($co{'author_name'}, 15, 5);\n \t\tprint \"$co{'age_string_date'}\\n\" .\n-\t\t \"\" . $author . \"\\n\" .\n+\t\t format_author_html('td', \\%co, 15, 5) .\n \t\t \"\" .\n \t\t $cgi->a({-href => href(action=>\"commit\", hash=>$co{'id'}),\n \t\t -class => \"list subject\"},\n@@ -5094,9 +5122,9 @@ sub git_log {\n \t\t \" | \" .\n \t\t $cgi->a({-href => href(action=>\"tree\", hash=>$commit, hash_base=>$commit)}, \"tree\") .\n \t\t \"
\\n\" .\n-\t\t \"\\n\" .\n-\t\t \"\" . esc_html($co{'author_name'}) . \" [$ad{'rfc2822'}]
\\n\" .\n \t\t \"\\n\";\n+\t\t git_print_authorship(\\%co);\n+\t\t print \"
\\n\\n\";\n \n \t\tprint \"
\\n\";\n \t\tgit_print_log($co{'comment'}, -final_empty_line=> 1);\n@@ -5115,8 +5143,6 @@ sub git_commit {\n \t$hash ||= $hash_base || \"HEAD\";\n \tmy %co = parse_commit($hash)\n \t or die_error(404, \"Unknown commit object\");\n-\tmy %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});\n-\tmy %cd = parse_date($co{'committer_epoch'}, $co{'committer_tz'});\n \n \tmy $parent = $co{'parent'};\n \tmy $parents = $co{'parents'}; # listref\n@@ -5183,22 +5209,7 @@ sub git_commit {\n \t}\n \tprint \"
\\n\" .\n \t \"\\n\";\n-\tprint \"\\n\".\n-\t \"\" .\n-\t \"\" .\n-\t \"\\n\";\n-\tprint \"\\n\";\n-\tprint \"\\n\";\n+\tgit_print_who(\\%co);\n \tprint \"\\n\";\n \tprint \"\" .\n \t \"\" .\n@@ -5579,7 +5590,11 @@ sub git_commitdiff {\n \t\tgit_header_html(undef, $expires);\n \t\tgit_print_page_nav('commitdiff','', $hash,$co{'tree'},$hash, $formats_nav);\n \t\tgit_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash);\n-\t\tgit_print_authorship(\\%co);\n+\t\tprint \"
\\n\" .\n+\t\t \"
author\" . esc_html($co{'author'}) . \"
$ad{'rfc2822'}\";\n-\tif ($ad{'hour_local'} < 6) {\n-\t\tprintf(\" (%02d:%02d %s)\",\n-\t\t $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});\n-\t} else {\n-\t\tprintf(\" (%02d:%02d %s)\",\n-\t\t $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});\n-\t}\n-\tprint \"
committer\" . esc_html($co{'committer'}) . \"
$cd{'rfc2822'}\" .\n-\t sprintf(\" (%02d:%02d %s)\", $cd{'hour_local'}, $cd{'minute_local'}, $cd{'tz_local'}) .\n-\t \"
commit$co{'id'}
tree
\\n\";\n+\t\tgit_print_who(\\%co);\n+\t\tprint \"
\".\n+\t\t \"
\\n\";\n \t\tprint \"
\\n\";\n \t\tif (@{$co{'comment'}} > 1) {\n \t\t\tprint \"
\\n\";\n-- \n1.6.3.rc1.192.gdbfcb\n"},{"id":"116795","messageId":"1245710999-17763-3-git-send-email-giuseppe.bilotta@gmail.com","threadId":"19905","inReplyTo":"1245710999-17763-2-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv4 2/2] gitweb: gravatar support","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-06-22T22:49:59Z","receivedAt":"2009-06-22T22:49:59Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Introduce gravatar support by adding the appropriate img tag next to\nauthor and committer in commit(diff), history, shortlog and log view.\n\nThe feature is disabled by default, and depends on Digest::MD5, which\nis a core package since Perl 5.8. If Digest::MD5 cannot be found,\nenabling this feature results in a no-op.\n\nSigned-off-by: Giuseppe Bilotta \n---\n gitweb/gitweb.css | 4 +++\n gitweb/gitweb.perl | 56 ++++++++++++++++++++++++++++++++++++++++++++++-----\n 2 files changed, 54 insertions(+), 6 deletions(-)\n\ndiff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\nindex 68b22ff..ddf982b 100644\n--- a/gitweb/gitweb.css\n+++ b/gitweb/gitweb.css\n@@ -28,6 +28,10 @@ img.logo {\n \tborder-width: 0px;\n }\n \n+img.avatar {\n+\tvertical-align:middle;\n+}\n+\n div.page_header {\n \theight: 25px;\n \tpadding: 8px;\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 223648f..531d589 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -195,6 +195,14 @@ our %known_snapshot_format_aliases = (\n \t'x-zip' => undef, '' => undef,\n );\n \n+# Pixel sizes for avatars. If the default font sizes or lineheights\n+# are changed, it may be appropriate to change these values too via\n+# $GITWEB_CONFIG.\n+our %avatar_size = (\n+\t'default' => 16,\n+\t'double' => 32\n+) ;\n+\n # You define site-wide feature defaults here; override them with\n # $GITWEB_CONFIG as necessary.\n our %feature = (\n@@ -365,6 +373,21 @@ our %feature = (\n \t\t'sub' => \\&feature_patches,\n \t\t'override' => 0,\n \t\t'default' => [16]},\n+\n+\t# Gravatar support. When this feature is enabled, views such as\n+\t# shortlog or commit will display the gravatar associated with\n+\t# the email of the committer(s) and/or author(s). Please note that\n+\t# the feature depends on Digest::MD5.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'gravatar'}{'default'} = [1];\n+\t# To have project specific config enable override in $GITWEB_CONFIG\n+\t# $feature{'gravatar'}{'override'} = 1;\n+\t# and in project config gitweb.gravatar = 0|1;\n+\t'gravatar' => {\n+\t\t'sub' => sub { feature_bool('gravatar', @_) },\n+\t\t'override' => 0,\n+\t\t'default' => [0]},\n );\n \n sub gitweb_get_feature {\n@@ -814,6 +837,10 @@ $git_dir = \"$projectroot/$project\" if $project;\n our @snapshot_fmts = gitweb_get_feature('snapshot');\n @snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);\n \n+# check if gravatars are enabled and dependencies are satisfied\n+our $git_gravatar_enabled = gitweb_check_feature('gravatar') &&\n+\t(eval { require Digest::MD5; 1; });\n+\n # dispatch\n if (!defined $action) {\n \tif (defined $hash) {\n@@ -1474,7 +1501,7 @@ sub format_author_html {\n \tmy $tag = shift;\n \tmy $co = shift;\n \tmy $author = chop_and_escape_str($co->{'author_name'}, @_);\n-\treturn \"<$tag class=\\\"author\\\">\" . $author . \"\\n\";\n+\treturn \"<$tag class=\\\"author\\\">\" . git_get_gravatar($co->{'author_email'}, 'space_after' => 1) . $author . \"\\n\";\n }\n \n # format git diff header line, i.e. \"diff --(git|combined|cc) ...\"\n@@ -3222,6 +3249,21 @@ sub git_print_header_div {\n \t \"\\n
\\n\";\n }\n \n+# insert a gravatar for the given $email at the given $size if the feature\n+# is enabled\n+sub git_get_gravatar {\n+\tif ($git_gravatar_enabled) {\n+\t\tmy ($email, %params) = @_;\n+\t\tmy $pre_white = ($params{'space_before'} ? \" \" : \"\");\n+\t\tmy $post_white = ($params{'space_after'} ? \" \" : \"\");\n+\t\tmy $size = $avatar_size{$params{'size'}} || $avatar_size{'default'};\n+\t\treturn $pre_white . \"\" . $post_white;\n+\t} else {\n+\t\treturn \"\";\n+\t}\n+}\n+\n # Outputs the author name and date in long form\n sub git_print_authorship {\n \tmy $co = shift;\n@@ -3238,7 +3280,8 @@ sub git_print_authorship {\n \t\tprintf(\" (%02d:%02d %s)\",\n \t\t $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});\n \t}\n-\tprint \"]\\n\";\n+\tprint \"]\" . git_get_gravatar($co->{'author_email'}, 'space_before' => 1)\n+\t\t . \"\\n\";\n }\n \n # Outputs the author and commiter name and date in long form\n@@ -3246,9 +3289,9 @@ sub git_print_who {\n \tmy $co = shift;\n \tmy %ad = parse_date($co->{'author_epoch'}, $co->{'author_tz'});\n \tmy %cd = parse_date($co->{'committer_epoch'}, $co->{'committer_tz'});\n-\tprint \"author\" . esc_html($co->{'author'}) . \"\\n\".\n-\t \"\" .\n-\t \" $ad{'rfc2822'}\";\n+\tprint \"author\" . esc_html($co->{'author'}) . \"\".\n+\t \"\" .git_get_gravatar($co->{'author_email'}, 'size' => 'double') . \"\\n\" .\n+\t \" $ad{'rfc2822'}\";\n \tif ($ad{'hour_local'} < 6) {\n \t\tprintf(\" (%02d:%02d %s)\",\n \t\t $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});\n@@ -3258,7 +3301,8 @@ sub git_print_who {\n \t}\n \tprint \"\" .\n \t \"\\n\";\n-\tprint \"committer\" . esc_html($co->{'committer'}) . \"\\n\";\n+\tprint \"committer\" . esc_html($co->{'committer'}) . \"\".\n+\t \"\" .git_get_gravatar($co->{'committer_email'}, 'size' => 'double') . \"\\n\";\n \tprint \" $cd{'rfc2822'}\" .\n \t sprintf(\" (%02d:%02d %s)\", $cd{'hour_local'}, $cd{'minute_local'}, $cd{'tz_local'}) .\n \t \"\\n\";\n-- \n1.6.3.rc1.192.gdbfcb\n"},{"id":"116852","messageId":"7vy6riyu5m.fsf@alter.siamese.dyndns.org","threadId":"19905","inReplyTo":"1245710999-17763-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv4 0/2] gitweb: gravatar support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-24T07:20:53Z","receivedAt":"2009-06-24T07:20:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Giuseppe Bilotta writes:\n\n> As suggested by Junio, the author display refactoring now comes before\n> the actual gravatar feature implementation. The refactoring also goes a\n> little bit deeper, changing the 'commitdiff' view to match 'commit' for\n> the author and committer layout.\n>\n> Giuseppe Bilotta (2):\n> gitweb: refactor author name insertion\n> gitweb: gravatar support\n>\n> gitweb/gitweb.css | 9 +++-\n> gitweb/gitweb.perl | 123 ++++++++++++++++++++++++++++++++++++++-------------\n> 2 files changed, 99 insertions(+), 33 deletions(-)\n\nThanks. It looks much better for reviewing, now the order of the patches\nare sane.\n\nDoes anybody have comments on the patches?\n"},{"id":"116853","messageId":"200906240945.48691.jnareb@gmail.com","threadId":"19905","inReplyTo":"7vy6riyu5m.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv4 0/2] gitweb: gravatar support","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-06-24T07:45:45Z","receivedAt":"2009-06-24T07:45:45Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Giuseppe Bilotta writes:\n> \n> > As suggested by Junio, the author display refactoring now comes before\n> > the actual gravatar feature implementation. The refactoring also goes a\n> > little bit deeper, changing the 'commitdiff' view to match 'commit' for\n> > the author and committer layout.\n> >\n> > Giuseppe Bilotta (2):\n> > gitweb: refactor author name insertion\n> > gitweb: gravatar support\n> >\n> > gitweb/gitweb.css | 9 +++-\n> > gitweb/gitweb.perl | 123 ++++++++++++++++++++++++++++++++++++++-------------\n> > 2 files changed, 99 insertions(+), 33 deletions(-)\n> \n> Thanks. It looks much better for reviewing, now the order of the patches\n> are sane.\n> \n> Does anybody have comments on the patches?\n\nI'll send my comments today.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"116854","messageId":"20090624171522.6117@nanako3.lavabit.com","threadId":"19905","inReplyTo":"7vy6riyu5m.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv4 0/2] gitweb: gravatar support","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-06-24T08:15:22Z","receivedAt":"2009-06-24T08:15:22Z","isPatch":false,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano :\n\n> Thanks. It looks much better for reviewing, now the order of the patches\n> are sane.\n>\n> Does anybody have comments on the patches?\n\nI'll send my comments later today.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"116863","messageId":"20090624224430.6117@nanako3.lavabit.com","threadId":"19905","inReplyTo":"1245710999-17763-3-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv4 2/2] gitweb: gravatar support","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-06-24T13:44:30Z","receivedAt":"2009-06-24T13:44:30Z","isPatch":false,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Giuseppe Bilotta :\n\n> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\n> index 68b22ff..ddf982b 100644\n> ........\n> +img.avatar {\n> +\tvertical-align:middle;\n> +}\n> +\n> .........\n> +# Pixel sizes for avatars. If the default font sizes or lineheights\n> +# are changed, it may be appropriate to change these values too via\n> +# $GITWEB_CONFIG.\n> +our %avatar_size = (\n> +\t'default' => 16,\n> +\t'double' => 32\n> +) ;\n> ........\n\nEarly parts of the patch talk about \"avatars\"; compared with \"icons\" Junio\nsuggested, I think that is a better generic word to use for this purpose.\n\n> +\t# Gravatar support. When this feature is enabled, views such as\n> ........\n> +\t'gravatar' => {\n> +\t\t'sub' => sub { feature_bool('gravatar', @_) },\n> +\t\t'override' => 0,\n> +\t\t'default' => [0]},\n> );\n\nAnd this \"feature\" is about getting such avatars from \"gravatar\" service;\nit is good to use such a specific word here.\n\n> +# check if gravatars are enabled and dependencies are satisfied\n> +our $git_gravatar_enabled = gitweb_check_feature('gravatar') &&\n> +\t(eval { require Digest::MD5; 1; });\n\nThe same.\n\n> @@ -1474,7 +1501,7 @@ sub format_author_html {\n> \tmy $tag = shift;\n> \tmy $co = shift;\n> \tmy $author = chop_and_escape_str($co->{'author_name'}, @_);\n> -\treturn \"<$tag class=\\\"author\\\">\" . $author . \"\\n\";\n> +\treturn \"<$tag class=\\\"author\\\">\" . git_get_gravatar($co->{'author_email'}, 'space_after' => 1) . $author . \"\\n\";\n> }\n\nBut the function that returns a string suitable for embedding in the HTML\npage, given an e-mail address, is called get_gravatar(), not get_avatar()? \n\nI expected from an earlier review message by Junio that get_avatar() will\nlook like this:\n\nsub git_get_avatar {\n\tmy $url = undef;\n\tif ($git_gravatar_enabled) {\n\t\tmy $md5 = .......;\n\t\t$url = \"http://www.gravatar.com/avatar.php?gravatar_id=$md5\";\n\t} else if ($git_picons_enabled) {\n\t\tmy $userpath = .......;\n\t\t$url = \"http://www.cs.indiana.edu/picons/db/users/$userpath/face.xpm\";\n\t}\n\treturn \"\" unless (defined $url);\n\treturn $pre_white . \"\" . $post_white;\n}\n\n(without \"picons\" part, of course).\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"116879","messageId":"cb7bb73a0906241300mf10e073j99ad2b09c806d0d7@mail.gmail.com","threadId":"19905","inReplyTo":"20090624224430.6117@nanako3.lavabit.com","subject":"Re: [PATCHv4 2/2] gitweb: gravatar support","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-06-24T20:00:36Z","receivedAt":"2009-06-24T20:00:36Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Wed, Jun 24, 2009 at 3:44 PM, Nanako Shiraishi wrote:\n> Quoting Giuseppe Bilotta :\n>\n>> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\n>> index 68b22ff..ddf982b 100644\n>> ........\n>> +img.avatar {\n>> +     vertical-align:middle;\n>> +}\n>> +\n>> .........\n>> +# Pixel sizes for avatars. If the default font sizes or lineheights\n>> +# are changed, it may be appropriate to change these values too via\n>> +# $GITWEB_CONFIG.\n>> +our %avatar_size = (\n>> +     'default' => 16,\n>> +     'double'  => 32\n>> +) ;\n>> ........\n>\n> Early parts of the patch talk about \"avatars\"; compared with \"icons\" Junio\n> suggested, I think that is a better generic word to use for this purpose.\n\nWell, I would argue that 'avatar' is pretty commonly understood to be\nthe name of the picture identifying an individual online. OTOH, I've\nbeen thinking that there are other kind of icons that might be used\nacross gitweb in the future, and they are likely to use the same\nsizes, so I'll go for icon_size here.\n\n>> @@ -1474,7 +1501,7 @@ sub format_author_html {\n>>       my $tag = shift;\n>>       my $co = shift;\n>>       my $author = chop_and_escape_str($co->{'author_name'}, @_);\n>> -     return \"<$tag class=\\\"author\\\">\" . $author . \"\\n\";\n>> +     return \"<$tag class=\\\"author\\\">\" . git_get_gravatar($co->{'author_email'}, 'space_after' => 1) . $author . \"\\n\";\n>>  }\n>\n> But the function that returns a string suitable for embedding in the HTML\n> page, given an e-mail address, is called get_gravatar(), not get_avatar()?\n>\n> I expected from an earlier review message by Junio that get_avatar() will\n> look like this:\n>\n> sub git_get_avatar {\n>        my $url = undef;\n>        if ($git_gravatar_enabled) {\n>                my $md5 = .......;\n>                $url = \"http://www.gravatar.com/avatar.php?gravatar_id=$md5\";\n>        } else if ($git_picons_enabled) {\n>                my $userpath = .......;\n>                $url = \"http://www.cs.indiana.edu/picons/db/users/$userpath/face.xpm\";\n>        }\n>        return \"\" unless (defined $url);\n>        return $pre_white . \"\" . $post_white;\n> }\n>\n> (without \"picons\" part, of course).\n\nRight. I was wondering if it would make sense to factor each avatar\nblock separately (i.e. have a git_get_avatar that calls\ngit_get_whatever) but it probably doesn't make sense.\n\nI'll wait for Jakub's (and whoever else wants to chip in) comments,\nand then I'll send an update to this patch following your\nrecommendations.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"116898","messageId":"200906250323.24868.jnareb@gmail.com","threadId":"19905","inReplyTo":"1245710999-17763-2-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv4 1/2] gitweb: refactor author name insertion","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-06-25T01:23:24Z","receivedAt":"2009-06-25T01:23:24Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 23 June 2009, Giuseppe Bilotta wrote:\n\n> The refactoring allows easier customization of the output by means of\n> CSS, and improves extensibility on the CGI side too.\n\nHow? The above description is slightly lacking in details. Does it\nmean that we replaced remains of presentational HTML ( and tags)\nby CSS styling using classes? What about support for text browsers \nsuch as lynx, links/elinks, w3m? (I'm not saying here that this should\nblock moving from presentational HTML to CSS, just that it should be\nconsidered).\n\nHow it improves extensibility on the CGI side? Does it mean that it\nwould be easier to add new features or otherwise extend gitweb, because\ncommon parts are factored out?\n\n> \n> Layout is preserved for all views except for 'commitdiff', which now\n> uses the same format as 'commit'.\n\nCurrently (i.e. without this patch) gitweb uses the following forms of\nshowing authorship, from shortest to longest:\n\n * \"Giuseppe Bilotta\"\n \n This is form used in 'shortlog' and 'history' views.\n\n * \"Giuseppe Bilotta [Mon, 22 Jun 2009 22:49:58 +0000]\"\n\n This is form used by 'log' view.\n\n * \"Giuseppe Bilotta [Mon, 22 Jun 2009 22:49:58 +0000 (00:49 +0200)]\"\n\n This is form used by 'commitdiff' view.\n\n * \"author Giuseppe Bilotta \n Mon, 22 Jun 2009 22:49:58 +0000 (00:49 +0200)\n committer Jakub Narebski \n Tue, 23 Jun 2009 18:02:21 +0000 (20:02 +0200)\"\n\n This is form used by 'commit' view.\n\nI can understand factoring out code dealing with authorship in 'commit'\nview, because it is a bit different from other headers; on the other\nhand it is slightly similar to dealing other headers.\n\nI think that changing 'commitdiff' view to use more detailed author\ninformation might be a good idea. But I do not think that having\nit bundled together with this refactoring patch is a good idea. It\nreally should be a separate patch, and refactoring shouldn't change\noutput, if possible (and I think here is very much possible, see below).\n\nIf you wanted to reduce number of places where you need to add later\ngravatar support, it would be better to factor out formatting authorship\ninformation in 'log' and 'commitdiff' view, either by having 'log' view\nuse 'commitdiff' authorship subroutine (and not 'commitdiff' use new\n'commit' authorship subroutine), or make this subroutine produce two\nslightly different outputs.\n\n> \n> Signed-off-by: Giuseppe Bilotta \n> ---\n> gitweb/gitweb.css | 5 ++-\n> gitweb/gitweb.perl | 79 +++++++++++++++++++++++++++++++---------------------\n> 2 files changed, 51 insertions(+), 33 deletions(-)\n> \n> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\n> index a01eac8..68b22ff 100644\n> --- a/gitweb/gitweb.css\n> +++ b/gitweb/gitweb.css\n> @@ -132,11 +132,14 @@ div.list_head {\n> \tfont-style: italic;\n> }\n> \n> +.author_date, .author {\n> +\tfont-style: italic;\n> +}\n> +\n> div.author_date {\n> \tpadding: 8px;\n> \tborder: solid #d9d8d1;\n> \tborder-width: 0px 0px 1px 0px;\n> -\tfont-style: italic;\n> }\n\nGood.\n\n> \n> a.list {\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 1e7e2d8..223648f 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -1469,6 +1469,14 @@ sub format_subject_html {\n> \t}\n> }\n> \n> +# format the author name with the given tag and optionally shortening it\n\nI would like to see calling convention for this subroutine described, \nbecause it is not entirely obvious; see passing of (misnamed)\nchop_and_escape_str subroutine parameters.\n\n> +sub format_author_html {\n> +\tmy $tag = shift;\n> +\tmy $co = shift;\n> +\tmy $author = chop_and_escape_str($co->{'author_name'}, @_);\n> +\treturn \"<$tag class=\\\"author\\\">\" . $author . \"\\n\";\n> +}\n> +\n> # format git diff header line, i.e. \"diff --(git|combined|cc) ...\"\n> sub format_git_diff_header_line {\n> \tmy $line = shift;\n> @@ -3214,11 +3222,13 @@ sub git_print_header_div {\n> \t \"\\n
\\n\";\n> }\n> \n> +# Outputs the author name and date in long form\n> sub git_print_authorship {\n> \tmy $co = shift;\n> +\tmy $tag = shift || 'div';\n> \n> \tmy %ad = parse_date($co->{'author_epoch'}, $co->{'author_tz'});\n> -\tprint \"
\" .\n> +\tprint \"<$tag class=\\\"author_date\\\">\" .\n> \t esc_html($co->{'author_name'}) .\n> \t \" [$ad{'rfc2822'}\";\n> \tif ($ad{'hour_local'} < 6) {\n> @@ -3228,7 +3238,30 @@ sub git_print_authorship {\n> \t\tprintf(\" (%02d:%02d %s)\",\n> \t\t $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});\n> \t}\n> -\tprint \"]
\\n\";\n> +\tprint \"]\\n\";\n> +}\n\nGood. You could have add option to print or not print localtime\n(or use version with localtime in 'log' view).\n\n> +\n> +# Outputs the author and commiter name and date in long form\n> +sub git_print_who {\n\nHmmm... perhaps git_print_authorship_long? I am not sure if git_print_who\nis a good name to have. (Yes, I know, naming is hard).\n\n> +\tmy $co = shift;\n> +\tmy %ad = parse_date($co->{'author_epoch'}, $co->{'author_tz'});\n> +\tmy %cd = parse_date($co->{'committer_epoch'}, $co->{'committer_tz'});\n> +\tprint \"author\" . esc_html($co->{'author'}) . \"\\n\".\n> +\t \"\" .\n> +\t \" $ad{'rfc2822'}\";\n> +\tif ($ad{'hour_local'} < 6) {\n> +\t\tprintf(\" (%02d:%02d %s)\",\n> +\t\t $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});\n> +\t} else {\n> +\t\tprintf(\" (%02d:%02d %s)\",\n> +\t\t $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});\n> +\t}\n> +\tprint \"\" .\n> +\t \"\\n\";\n> +\tprint \"committer\" . esc_html($co->{'committer'}) . \"\\n\";\n> +\tprint \" $cd{'rfc2822'}\" .\n> +\t sprintf(\" (%02d:%02d %s)\", $cd{'hour_local'}, $cd{'minute_local'}, $cd{'tz_local'}) .\n> +\t \"\\n\";\n> }\n\nWhile it might be a good idea of factoring out this part into separate\nsubroutine, I don't think that 'commitdiff' should use it... well, at\nleast not in this commit.\n\n> \n> sub git_print_page_path {\n> @@ -4142,11 +4175,9 @@ sub git_shortlog_body {\n> \t\t\tprint \"\\n\";\n> \t\t}\n> \t\t$alternate ^= 1;\n> -\t\tmy $author = chop_and_escape_str($co{'author_name'}, 10);\n> \t\t# git_summary() used print \"$co{'age_string'}\\n\" .\n> \t\tprint \"$co{'age_string_date'}\\n\" .\n> -\t\t \"\" . $author . \"\\n\" .\n> -\t\t \"\";\n> +\t\t\tformat_author_html('td', \\%co, 10) . \"\";\n\n+\t\t format_author_html('td', \\%co, 10) . \"\";\n\nTabs are for indent, spaces are for align, at least for gitweb.perl\n(this unfortunately requires either handcrafting, or using smart-tabs,\nbut it has the advantage of presenting the same layour regardless of\nthe tab size).\n\n> \t\tprint format_subject_html($co{'title'}, $co{'title_short'},\n> \t\t href(action=>\"commit\", hash=>$commit), $ref);\n> \t\tprint \"\\n\" .\n\nVery good.\n\n> @@ -4193,11 +4224,9 @@ sub git_history_body {\n> \t\t\tprint \"\\n\";\n> \t\t}\n> \t\t$alternate ^= 1;\n> -\t# shortlog uses chop_str($co{'author_name'}, 10)\n> -\t\tmy $author = chop_and_escape_str($co{'author_name'}, 15, 3);\n> \t\tprint \"$co{'age_string_date'}\\n\" .\n> -\t\t \"\" . $author . \"\\n\" .\n> -\t\t \"\";\n> +\t# shortlog uses format_author_html('td', \\%co, 10)\n> +\t\t\tformat_author_html('td', \\%co, 15, 3) . \"\";\n\nTabs are for indent, spaces are for align.\nAnd you don't need to try to align in the comment now.\n\n> \t\t# originally git_history used chop_str($co{'title'}, 50)\n> \t\tprint format_subject_html($co{'title'}, $co{'title_short'},\n> \t\t href(action=>\"commit\", hash=>$commit), $ref);\n> @@ -4350,9 +4379,8 @@ sub git_search_grep_body {\n> \t\t\tprint \"\\n\";\n> \t\t}\n> \t\t$alternate ^= 1;\n> -\t\tmy $author = chop_and_escape_str($co{'author_name'}, 15, 5);\n> \t\tprint \"$co{'age_string_date'}\\n\" .\n> -\t\t \"\" . $author . \"\\n\" .\n> +\t\t format_author_html('td', \\%co, 15, 5) .\n> \t\t \"\" .\n> \t\t $cgi->a({-href => href(action=>\"commit\", hash=>$co{'id'}),\n> \t\t -class => \"list subject\"},\n\nGood. \n\nThis one has correct whitespace mixture (tabs for indent, spaces for\nalign).\n\n> @@ -5094,9 +5122,9 @@ sub git_log {\n> \t\t \" | \" .\n> \t\t $cgi->a({-href => href(action=>\"tree\", hash=>$commit, hash_base=>$commit)}, \"tree\") .\n> \t\t \"
\\n\" .\n> -\t\t \"
\\n\" .\n> -\t\t \"\" . esc_html($co{'author_name'}) . \" [$ad{'rfc2822'}]
\\n\" .\n> \t\t \"\\n\";\n> +\t\t git_print_authorship(\\%co);\n> +\t\t print \"
\\n\\n\";\n> \n> \t\tprint \"
\\n\";\n> \t\tgit_print_log($co{'comment'}, -final_empty_line=> 1);\n\nGood.\n\n> @@ -5115,8 +5143,6 @@ sub git_commit {\n> \t$hash ||= $hash_base || \"HEAD\";\n> \tmy %co = parse_commit($hash)\n> \t or die_error(404, \"Unknown commit object\");\n> -\tmy %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});\n> -\tmy %cd = parse_date($co{'committer_epoch'}, $co{'committer_tz'});\n> \n> \tmy $parent = $co{'parent'};\n> \tmy $parents = $co{'parents'}; # listref\n> @@ -5183,22 +5209,7 @@ sub git_commit {\n> \t}\n> \tprint \"
\\n\" .\n> \t \"\\n\";\n> -\tprint \"\\n\".\n> -\t \"\" .\n> -\t \"\" .\n> -\t \"\\n\";\n> -\tprint \"\\n\";\n> -\tprint \"\\n\";\n> +\tgit_print_who(\\%co);\n> \tprint \"\\n\";\n> \tprint \"\" .\n> \t \"\" .\n\nThis makes git_commit subroutine less complex, so it might be good\nidea anyway.\n\n> @@ -5579,7 +5590,11 @@ sub git_commitdiff {\n> \t\tgit_header_html(undef, $expires);\n> \t\tgit_print_page_nav('commitdiff','', $hash,$co{'tree'},$hash, $formats_nav);\n> \t\tgit_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash);\n> -\t\tgit_print_authorship(\\%co);\n> +\t\tprint \"
\\n\" .\n> +\t\t \"
author\" . esc_html($co{'author'}) . \"
$ad{'rfc2822'}\";\n> -\tif ($ad{'hour_local'} < 6) {\n> -\t\tprintf(\" (%02d:%02d %s)\",\n> -\t\t $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});\n> -\t} else {\n> -\t\tprintf(\" (%02d:%02d %s)\",\n> -\t\t $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});\n> -\t}\n> -\tprint \"
committer\" . esc_html($co{'committer'}) . \"
$cd{'rfc2822'}\" .\n> -\t sprintf(\" (%02d:%02d %s)\", $cd{'hour_local'}, $cd{'minute_local'}, $cd{'tz_local'}) .\n> -\t \"
commit$co{'id'}
tree
\\n\";\n> +\t\tgit_print_who(\\%co);\n> +\t\tprint \"
\".\n> +\t\t \"
\\n\";\n> \t\tprint \"
\\n\";\n> \t\tif (@{$co{'comment'}} > 1) {\n> \t\t\tprint \"
\\n\";\n\nBut here we can use git_print_authorship('div', \\%co, undef, undef, -localtime=>1)\nor something like that; no need to change 'commitdiff' view.\n\n> -- \n> 1.6.3.rc1.192.gdbfcb\n> \n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"116899","messageId":"200906250335.28019.jnareb@gmail.com","threadId":"19905","inReplyTo":"1245710999-17763-3-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv4 2/2] gitweb: gravatar support","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-06-25T01:35:27Z","receivedAt":"2009-06-25T01:35:27Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 23 June 2009, Giuseppe Bilotta wrote:\n\n> Introduce gravatar support by adding the appropriate img tag next to\n> author and committer in commit(diff), history, shortlog and log view.\n\nThat reminds me that I have refactoring all log-like views in my TODO\nfile for gitweb for quite long time.\n\n> \n> The feature is disabled by default, and depends on Digest::MD5, which\n> is a core package since Perl 5.8. If Digest::MD5 cannot be found,\n> enabling this feature results in a no-op.\n\nGood description.\n\n> \n> Signed-off-by: Giuseppe Bilotta \n> ---\n> gitweb/gitweb.css | 4 +++\n> gitweb/gitweb.perl | 56 ++++++++++++++++++++++++++++++++++++++++++++++-----\n> 2 files changed, 54 insertions(+), 6 deletions(-)\n> \n> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\n> index 68b22ff..ddf982b 100644\n> --- a/gitweb/gitweb.css\n> +++ b/gitweb/gitweb.css\n> @@ -28,6 +28,10 @@ img.logo {\n> \tborder-width: 0px;\n> }\n> \n> +img.avatar {\n> +\tvertical-align:middle;\n> +}\n> +\n\nNitpick: \"vertical-align: middle;\" (with space separating key from\nvalue).\n\n> div.page_header {\n> \theight: 25px;\n> \tpadding: 8px;\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 223648f..531d589 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -195,6 +195,14 @@ our %known_snapshot_format_aliases = (\n> \t'x-zip' => undef, '' => undef,\n> );\n> \n> +# Pixel sizes for avatars. If the default font sizes or lineheights\n> +# are changed, it may be appropriate to change these values too via\n> +# $GITWEB_CONFIG.\n> +our %avatar_size = (\n> +\t'default' => 16,\n> +\t'double' => 32\n> +) ;\n\nGood idea, good description. I wonder if it would be worth adding\nto gitweb_conf.perl description in gitweb/README and gitweb/INSTALL...\n\nNitpick: \");\"\n\n> +\n> # You define site-wide feature defaults here; override them with\n> # $GITWEB_CONFIG as necessary.\n> our %feature = (\n> @@ -365,6 +373,21 @@ our %feature = (\n> \t\t'sub' => \\&feature_patches,\n> \t\t'override' => 0,\n> \t\t'default' => [16]},\n> +\n> +\t# Gravatar support. When this feature is enabled, views such as\n> +\t# shortlog or commit will display the gravatar associated with\n> +\t# the email of the committer(s) and/or author(s). Please note that\n> +\t# the feature depends on Digest::MD5.\n> +\n> +\t# To enable system wide have in $GITWEB_CONFIG\n> +\t# $feature{'gravatar'}{'default'} = [1];\n> +\t# To have project specific config enable override in $GITWEB_CONFIG\n> +\t# $feature{'gravatar'}{'override'} = 1;\n> +\t# and in project config gitweb.gravatar = 0|1;\n> +\t'gravatar' => {\n> +\t\t'sub' => sub { feature_bool('gravatar', @_) },\n> +\t\t'override' => 0,\n> +\t\t'default' => [0]},\n> );\n\nGood.\n\n> \n> sub gitweb_get_feature {\n> @@ -814,6 +837,10 @@ $git_dir = \"$projectroot/$project\" if $project;\n> our @snapshot_fmts = gitweb_get_feature('snapshot');\n> @snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);\n> \n> +# check if gravatars are enabled and dependencies are satisfied\n> +our $git_gravatar_enabled = gitweb_check_feature('gravatar') &&\n> +\t(eval { require Digest::MD5; 1; });\n> +\n\nI think this is correct.\n\n> # dispatch\n> if (!defined $action) {\n> \tif (defined $hash) {\n> @@ -1474,7 +1501,7 @@ sub format_author_html {\n> \tmy $tag = shift;\n> \tmy $co = shift;\n> \tmy $author = chop_and_escape_str($co->{'author_name'}, @_);\n> -\treturn \"<$tag class=\\\"author\\\">\" . $author . \"\\n\";\n> +\treturn \"<$tag class=\\\"author\\\">\" . git_get_gravatar($co->{'author_email'}, 'space_after' => 1) . $author . \"\\n\";\n> }\n\nLine too long, please break it.\n\n> \n> # format git diff header line, i.e. \"diff --(git|combined|cc) ...\"\n> @@ -3222,6 +3249,21 @@ sub git_print_header_div {\n> \t \"\\n
\\n\";\n> }\n> \n> +# insert a gravatar for the given $email at the given $size if the feature\n> +# is enabled\n> +sub git_get_gravatar {\n> +\tif ($git_gravatar_enabled) {\n> +\t\tmy ($email, %params) = @_;\n> +\t\tmy $pre_white = ($params{'space_before'} ? \" \" : \"\");\n> +\t\tmy $post_white = ($params{'space_after'} ? \" \" : \"\");\n\nThis name of parameter is a bit too low level for my taste. It doesn't\nmatter whether we add ' ' nonbreakable space before or after img,\nbut whethere gravatar image has _padding_ on the left/on the right hand\nside of gravatar image. So 'pad_left' and 'pad_right', or 'pad_before'\nand 'pad_after' rather than 'space_before' and 'space_after'.\n\nBut this is a matter of taste...\n\n> +\t\tmy $size = $avatar_size{$params{'size'}} || $avatar_size{'default'};\n\nNice trick.\n\n> +\t\treturn $pre_white . \" +\t\t Digest::MD5::md5_hex(lc $email) . \"&size=$size\\\" />\" . $post_white;\n> +\t} else {\n> +\t\treturn \"\";\n> +\t}\n> +}\n> +\n> # Outputs the author name and date in long form\n> sub git_print_authorship {\n> \tmy $co = shift;\n> @@ -3238,7 +3280,8 @@ sub git_print_authorship {\n> \t\tprintf(\" (%02d:%02d %s)\",\n> \t\t $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});\n> \t}\n> -\tprint \"]\\n\";\n> +\tprint \"]\" . git_get_gravatar($co->{'author_email'}, 'space_before' => 1)\n> +\t\t . \"\\n\";\n> }\n> \n\nGood.\n\n> # Outputs the author and commiter name and date in long form\n> @@ -3246,9 +3289,9 @@ sub git_print_who {\n> \tmy $co = shift;\n> \tmy %ad = parse_date($co->{'author_epoch'}, $co->{'author_tz'});\n> \tmy %cd = parse_date($co->{'committer_epoch'}, $co->{'committer_tz'});\n> -\tprint \"author\" . esc_html($co->{'author'}) . \"\\n\".\n> -\t \"\" .\n> -\t \" $ad{'rfc2822'}\";\n> +\tprint \"author\" . esc_html($co->{'author'}) . \"\".\n> +\t \"\" .git_get_gravatar($co->{'author_email'}, 'size' => 'double') . \"\\n\" .\n\n . git_get_gravatar($co->{'author_email'}, 'size' => 'double') .\n\n> +\t \" $ad{'rfc2822'}\";\n\nIf you put on separate line, as was before, it would be more \nobvious in this diff that you are only adding single line.\n\n> \tif ($ad{'hour_local'} < 6) {\n> \t\tprintf(\" (%02d:%02d %s)\",\n> \t\t $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});\n> @@ -3258,7 +3301,8 @@ sub git_print_who {\n> \t}\n> \tprint \"\" .\n> \t \"\\n\";\n> -\tprint \"committer\" . esc_html($co->{'committer'}) . \"\\n\";\n> +\tprint \"committer\" . esc_html($co->{'committer'}) . \"\".\n> +\t \"\" .git_get_gravatar($co->{'committer_email'}, 'size' => 'double') . \"\\n\";\n> \tprint \" $cd{'rfc2822'}\" .\n> \t sprintf(\" (%02d:%02d %s)\", $cd{'hour_local'}, $cd{'minute_local'}, $cd{'tz_local'}) .\n> \t \"\\n\";\n\nSidenote: you add here additional column. Which is present only in \nfragment of this table. Doesn't it screw layout of the rest of headers?\n\n> -- \n> 1.6.3.rc1.192.gdbfcb\n> \n> \n\n-- \nJakub Narebski\nPoland\n"}]}