Re: [PATCHv2] gitweb: gravatar support
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:58
Giuseppe Bilotta [off-list ref] writes:
Introduce gravatar support by adding the appropriate img tag next to author and committer in commit, shortlog and log view. The feature is disabled by default, and depends on Digest::MD5, which is available in most Perl installations. Signed-off-by: Giuseppe Bilotta <redacted> --- gitweb/gitweb.css | 4 ++++ gitweb/gitweb.perl | 51 ++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 48 insertions(+), 7 deletions(-) Changes from the previous version include gravatar use in history view, CSS use and the possibility to override the feature on a per-project basis.
I see these repeated patterns in your patch.
quoted hunk ↗ jump to hunk
@@ -4145,7 +4179,7 @@ sub git_shortlog_body { my $author = chop_and_escape_str($co{'author_name'}, 10); # git_summary() used print "<td><i>$co{'age_string'}</i></td>\n" . print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" . - "<td><i>" . $author . "</i></td>\n" . + "<td>" . git_get_gravatar($co{'author_email'}, 16, 1) . "<i>" . $author . "</i></td>\n" .... - "<td><i>" . $author . "</i></td>\n" . + "<td>" . git_get_gravatar($co{'author_email'}, 16, 1) . "<i>" . $author . "</i></td>\n" . ... - "<td><i>" . $author . "</i></td>\n" . + "<td>" . git_get_gravatar($co{'author_email'}, 16, 1) . "<i>" . $author . "</i></td>\n" . ... - print "<tr><td>author</td><td>" . esc_html($co{'author'}) . "</td></tr>\n". + print "<tr><td>author</td><td>" . esc_html($co{'author'}) . "</td>". + "<td rowspan=\"2\">" .git_get_gravatar($co{'author_email'}) . "</td></tr>\n" . ... - print "<tr><td>committer</td><td>" . esc_html($co{'committer'}) . "</td></tr>\n"; + print "<tr><td>committer</td><td>" . esc_html($co{'committer'}) . "</td>". + "<td rowspan=\"2\">" .git_get_gravatar($co{'committer_email'}) . "</td></tr>\n"; ...
Doesn't it strike you as needing a bit more refactoring? The first three
are identical, and you can refactor them to
- "<td><i>" . $author . "</i></td>\n" .
+ "<td>" . oneline_person($author) . "</td>\n" .
where oneline_person is
sub oneline_person {
my ($me) = @_;
return ($me->{'smallicon'} .
"<i>" . $me->{'name_escaped'} . "</i>");
}
And instead of doing "my $author = chop_and_escape_str()", you will
set up richer my $author upfront, like so:
my $author = +{
name_escaped => chop_and_escape_str($co{'author_name'}, 10),
smallicon => git_get_gravatar($co{'author_email'}, 16, 1),
};
and pass it to the helper function.
That way, people who do not like italics, or people who want to have the
icon at the end instead of at the beginning, can touch only one place
(i.e. "sub oneline_person").
By the way, in the above example, I named the field 'smallicon', as use of
gravatar is merely an implementation detail. It is plausible other people
may want to use picons instead.
I do not know about the following hunk (why does it have the icon at the
end, unlike the other ones?), but I think you got the idea.
- "<i>" . esc_html($co{'author_name'}) . " [$ad{'rfc2822'}]</i><br/>\n" .
- "</div>\n";
+ "<i>" . esc_html($co{'author_name'}) . " [$ad{'rfc2822'}]</i> " .
+ git_get_gravatar($co{'author_email'}, 16) .
+ "<br/>\n</div>\n";
In the medium term, we may want to go one step further and do
package person;
and make sets of methods like "oneline_person".