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?
> ---
> 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.
[...]
> 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/!!;
> + $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)?
> + }
> + $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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html