{"thread":{"id":"19888","subject":"[PATCHv3 0/2] gitweb: gravatar support","startedAt":"2009-06-21T17:57:03Z","lastAt":"2009-06-21T22:34:11Z","messageCount":4,"participants":["Giuseppe Bilotta","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"116718","messageId":"1245607025-19296-1-git-send-email-giuseppe.bilotta@gmail.com","threadId":"19888","inReplyTo":null,"subject":"[PATCHv3 0/2] gitweb: gravatar support","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-06-21T17:57:03Z","receivedAt":"2009-06-21T17:57:03Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"I've done the refactoring in a separate patch, to have a better (IMO)\nview of what I was doing. I can squash them together if needed. There\nhave also been a couple of other changes such as the use of 'require'\ninstead of 'use' to access Digest::MD5, and a better call pattern for\ngit_get_gravatar.\n\nOne thing that is missing but could be done is an email -> MD5 hash\nmapping to cache multiple occurences of the same email (not unusual\nin (short)log view).\n\nGiuseppe Bilotta (2):\n gitweb: gravatar support\n gitweb: refactor author+gravatar insertion\n\n gitweb/gitweb.css | 9 +++++-\n gitweb/gitweb.perl | 83 ++++++++++++++++++++++++++++++++++++++++++---------\n 2 files changed, 76 insertions(+), 16 deletions(-)\n"},{"id":"116720","messageId":"1245607025-19296-2-git-send-email-giuseppe.bilotta@gmail.com","threadId":"19888","inReplyTo":"1245607025-19296-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv3 1/2] gitweb: gravatar support","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-06-21T17:57:04Z","receivedAt":"2009-06-21T17:57:04Z","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 | 60 +++++++++++++++++++++++++++++++++++++++++++++------\n 2 files changed, 57 insertions(+), 7 deletions(-)\n\ndiff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\nindex a01eac8..eaf74c3 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 1e7e2d8..f5654d7 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@@ -3214,12 +3241,28 @@ 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 sub git_print_authorship {\n \tmy $co = shift;\n \n \tmy %ad = parse_date($co->{'author_epoch'}, $co->{'author_tz'});\n \tprint \"
\" .\n \t esc_html($co->{'author_name'}) .\n+\t git_get_gravatar($co->{'author_email'}, 'space_before' => 1) .\n \t \" [$ad{'rfc2822'}\";\n \tif ($ad{'hour_local'} < 6) {\n \t\tprintf(\" (%02d:%02d %s)\",\n@@ -4145,7 +4188,7 @@ sub git_shortlog_body {\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 \"\" . git_get_gravatar($co{'author_email'}, 'space_after' => 1) . \"\" . $author . \"\\n\" .\n \t\t \"\";\n \t\tprint format_subject_html($co{'title'}, $co{'title_short'},\n \t\t href(action=>\"commit\", hash=>$commit), $ref);\n@@ -4196,7 +4239,7 @@ sub git_history_body {\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 \"\" . git_get_gravatar($co{'author_email'}, 'space_after' => 1) . \"\" . $author . \"\\n\" .\n \t\t \"\";\n \t\t# originally git_history used chop_str($co{'title'}, 50)\n \t\tprint format_subject_html($co{'title'}, $co{'title_short'},\n@@ -4352,7 +4395,7 @@ sub git_search_grep_body {\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 \"\" . git_get_gravatar($co{'author_email'}, 'space_after' => 1) . \"\" . $author . \"\\n\" .\n \t\t \"\" .\n \t\t $cgi->a({-href => href(action=>\"commit\", hash=>$co{'id'}),\n \t\t -class => \"list subject\"},\n@@ -5095,8 +5138,9 @@ sub git_log {\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 \"\" . esc_html($co{'author_name'}) . \" [$ad{'rfc2822'}] \" .\n+\t\t git_get_gravatar($co{'author_email'}) .\n+\t\t \"
\\n\\n\";\n \n \t\tprint \"
\\n\";\n \t\tgit_print_log($co{'comment'}, -final_empty_line=> 1);\n@@ -5183,7 +5227,8 @@ sub git_commit {\n \t}\n \tprint \"
\\n\" .\n \t \"\\n\";\n-\tprint \"\\n\".\n+\tprint \"\".\n+\t \"\\n\" .\n \t \"\" .\n \t \"\" .\n \t \"\\n\";\n-\tprint \"\\n\";\n+\tprint \"\".\n+\t \"\\n\";\n \tprint \"\\n\";\n-- \n1.6.3.rc1.192.gdbfcb\n"},{"id":"116719","messageId":"1245607025-19296-3-git-send-email-giuseppe.bilotta@gmail.com","threadId":"19888","inReplyTo":"1245607025-19296-2-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv3 2/2] gitweb: refactor author+gravatar insertion","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-06-21T17:57:05Z","receivedAt":"2009-06-21T17:57:05Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"---\n gitweb/gitweb.css | 5 +++-\n gitweb/gitweb.perl | 69 ++++++++++++++++++++++++++++-----------------------\n 2 files changed, 42 insertions(+), 32 deletions(-)\n\ndiff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\nindex eaf74c3..ddf982b 100644\n--- a/gitweb/gitweb.css\n+++ b/gitweb/gitweb.css\n@@ -136,11 +136,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 f5654d7..cb4ecc3 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1496,6 +1496,31 @@ sub format_subject_html {\n \t}\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+# format, perhaps shortened and preceded by the gravatar, the author name\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\\\">\" .\n+\t\tgit_get_gravatar($co->{'author_email'}, 'space_after' => 1) .\n+\t\t$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@@ -3241,28 +3266,14 @@ 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, followed by the gravatar\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 git_get_gravatar($co->{'author_email'}, 'space_before' => 1) .\n \t \" [$ad{'rfc2822'}\";\n \tif ($ad{'hour_local'} < 6) {\n \t\tprintf(\" (%02d:%02d %s)\",\n@@ -3271,7 +3282,9 @@ 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+\tgit_get_gravatar($co->{'author_email'}, 'space_before' => 1) .\n+\t\"\\n\";\n }\n \n sub git_print_page_path {\n@@ -4185,11 +4198,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 \"\\n\" .\n \t\tprint \"\\n\" .\n-\t\t \"\\n\" .\n-\t\t \"\\n\" .\n@@ -4236,11 +4247,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 \"\\n\" .\n-\t\t \"\\n\" .\n-\t\t \"\\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 \"\\n\" .\n-\t\t \"\\n\" .\n+\t\t format_author_html('td', \\%co, 15, 5) .\n \t\t \"
author\" . esc_html($co{'author'}) . \"
author\" . esc_html($co{'author'}) . \"\" .git_get_gravatar($co{'author_email'}, 'size' => 'double') . \"
$ad{'rfc2822'}\";\n \tif ($ad{'hour_local'} < 6) {\n@@ -5195,7 +5240,8 @@ sub git_commit {\n \t}\n \tprint \"
committer\" . esc_html($co{'committer'}) . \"
committer\" . esc_html($co{'committer'}) . \"\" .git_get_gravatar($co{'committer_email'}, 'size' => 'double') . \"
$cd{'rfc2822'}\" .\n \t sprintf(\" (%02d:%02d %s)\", $cd{'hour_local'}, $cd{'minute_local'}, $cd{'tz_local'}) .\n \t \"
$co{'age_string'}$co{'age_string_date'}\" . git_get_gravatar($co{'author_email'}, 'space_after' => 1) . \"\" . $author . \"\";\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 \"
$co{'age_string_date'}\" . git_get_gravatar($co{'author_email'}, 'space_after' => 1) . \"\" . $author . \"\";\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@@ -4393,9 +4402,8 @@ sub git_search_grep_body {\n \t\t\tprint \"
$co{'age_string_date'}\" . git_get_gravatar($co{'author_email'}, 'space_after' => 1) . \"\" . $author . \"\" .\n \t\t $cgi->a({-href => href(action=>\"commit\", hash=>$co{'id'}),\n \t\t -class => \"list subject\"},\n@@ -5137,10 +5145,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-\t\t git_get_gravatar($co{'author_email'}) .\n-\t\t \"
\\n\\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-- \n1.6.3.rc1.192.gdbfcb\n"},{"id":"116724","messageId":"7vab41qkrg.fsf@alter.siamese.dyndns.org","threadId":"19888","inReplyTo":"1245607025-19296-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv3 0/2] gitweb: gravatar support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-21T22:34:11Z","receivedAt":"2009-06-21T22:34:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Giuseppe Bilotta writes:\n\n> I've done the refactoring in a separate patch, to have a better (IMO)\n> view of what I was doing. I can squash them together if needed.\n\nI personally think squashing won't add much value in this case, but I\nthink swapping would.\n\nThe recommended order is that refactor and clean-up come first and then a\nnew feature is built on the base that is made more solid, thanks to the\nrefactoring. That allows the reviewers to see and judge the extent of\ndamage to the codebase caused by the new feature more fairly.\n\nFor example, adding a new feature to an unclean codebase may have to touch\nten different places to add the same code, which would make the feature\nlook expensive and intrusive.\n\nBut if you first refactor the codebase cleanly, these ten places will be\ncalling a single helper function, and you may be able to add the new\nfeature by touching only one place, the helper function. It would be\nclear that the change is not intrusive if presented in such an order.\n\nThis is a tangent, but the key to a better design is to try to resist the\ntemplation to think that yours will be the last cool feature. If you\nthink \"with addition of gravatar, the authorship display is complete\",\nthen there is no reason to name the subroutine that is called by\n\"git_print_authorship\" anything but \"get_gravatar\". If you resist the\ntemptation, however, you would realize that other people may want to add\nsupport for different sources of person-label sources, and try to come up\nwith a more generic name to call it [*1*].\n\nI would not change the name of variable $git_gravatar_enabled, though.\nThat way, people who would want other kinds of personal-icons can add an\nelse-if to the sub, without changing other things.\n\n\n[Footnote]\n\n*1* To put it differently, you can tell where one's imagination ends by\nlooking at the way that person refactors a piece of code and gives names.\nA concrete name (e.g. \"gravatar\") shows where the boundary of abstraction\nlies in that person's imagination. The patch shows that it was done by\nsomebody who never thought of the possibility that git_print_authorship\nmight be able to use service other than gravatar to show small icons\nassociated with an email if other people build on top of his work.\n"}]}