Re: [PATCH 2/4] gitweb: show notes in shortlog view
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Feb 6, 2010, 00:18 UTC
- Message-ID
- <201002060118.21137.jnareb@gmail.com>
- In-Reply-To
- <1265300338-25021-3-git-send-email-giuseppe.bilotta@gmail.com>
On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:
> Subject: [PATCH 2/4] gitweb: show notes in shortlog view
Is it RFC?
Why it is only for 'shortlog' view, and not also for 'history' which is also shortlog-like view? Or is there reason why it is not present for 'history' view?
> The presence of the note is shown by a small icon, hovering on which > reveals the actual note content.
Signoff?
Show 22 quoted lines
> ---
> gitweb/gitweb.css | 29 +++++++++++++++++++++++++++++
> gitweb/gitweb.perl | 30 +++++++++++++++++++++++++++++-
> 2 files changed, 58 insertions(+), 1 deletions(-)
>
> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
> index 50067f2..7d1836b 100644
> --- a/gitweb/gitweb.css
> +++ b/gitweb/gitweb.css
> @@ -572,3 +572,32 @@ span.match {
> div.binary {
> font-style: italic;
> }
> +
> +span.notes {
> + float:right;
> + position:relative;
> +}
> +
> +span.notes span.note-container:before {
> + content: url('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAgAAAAIAgMAAAC5YVYYAAAAAXNSR0IArs4c6QAAAAlQTFRFAABnybuD//+t5rXizQAAAAF0Uk5TAEDm2GYAAAABYktHRACIBR1IAAAAGElEQVQI12MIDWXIWglDQHYIQ1YAQ6gDAFWPBrAKFe0fAAAAAElFTkSuQmCC');
> +}Not all web browsers support ':before' pseudo-element, and 'content' (pseudo-)property.
Not all web browsers support 'data:' URI schema in CSS; also such image cannot be cached (on the other hand it doesn't require extra TCP connection on first access, and CSS file is cached anyway).
On the other hand adding extra images to gitweb would probably require additional (yet another) build time parameter to tell where static images are (besides logo and favicon).
So perhaps it is good solution, at least for a first attempt.
[...]
Show 11 quoted lines
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 9ba5815..1701ed1 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1628,6 +1628,33 @@ sub format_subject_html {
> }
> }
>
> +# display notes next to a commit
> +sub format_notes_html {
> + my %notes = %{$_[0]};Why not use 'my $notes = shift;', and later '%$notes'?
> + my $ret = "";
Perhaps $return or $result would be a better name, to better distinguish it from visually similar $ref (see $ref vs $res);
> + while (my ($ref, $text) = each %notes) {
> + # remove 'refs/notes/' and an optional final s
> + $ref =~ s/^refs\/notes\///;You can use different delimiter than / to avoid 'leaning toothpick' syndrome, e.g.: $ref =~ s!^refs/notes/!!;
Show 8 quoted lines
> + $ref =~ s/s$//;
> +
> + # double markup is needed to allow pure CSS cross-browser 'popup'
> + # of the note
> + $ret .= "<span title='$ref' class='note-container $ref'>";
> + $ret .= "<span title='$ref' class='note $ref'>";
> + foreach my $line (split /\n/, $text) {
> + $ret .= esc_html($line) . "<br/>";Probably would want
$ret .= esc_html($line) . "<br/>\n";
here. Or do we want single string here?
Also, do you want/need final <br>? If not, perhaps
join("<br/>", map { esc_html($_) } split(/\n/, $text);would be a better solution (you can always add final "<br/>" later)?
Show 32 quoted lines
> + }
> + $ret .= "</span></span>";
> + }
> + if ($ret) {
> + return "<span class='notes'>$ret</span>";
> + } else {
> + return $ret;
> + }
> +
> +
> +}
> +
> # Rather than recomputing the url for an email multiple times, we cache it
> # after the first hit. This gives a visible benefit in views where the avatar
> # for the same email is used repeatedly (e.g. shortlog).
> @@ -4595,6 +4622,7 @@ sub git_shortlog_body {
> my %co = %{$commitlist->[$i]};
> my $commit = $co{'id'};
> my $ref = format_ref_marker($refs, $commit);
> + my $notes = format_notes_html($co{'notes'});
> if ($alternate) {
> print "<tr class=\"dark\">\n";
> } else {
> @@ -4605,7 +4633,7 @@ sub git_shortlog_body {
> print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" .
> format_author_html('td', \%co, 10) . "<td>";
> print format_subject_html($co{'title'}, $co{'title_short'},
> - href(action=>"commit", hash=>$commit), $ref);
> + href(action=>"commit", hash=>$commit), $ref . $notes);
> print "</td>\n" .
> "<td class=\"link\">" .
> $cgi->a({-href => href(action=>"commit", hash=>$commit)}, "commit") . " | " .Nice.
-- Jakub Narebski Poland