From: Junio C Hamano <junkio@cox.net>
Date: Wed, 21 Jun 2006 23:10:41 GMT
Subject: Re: [PATCH] gitweb: Add title attribute with full first line of tag comment if it is needed
Message-ID: <7vu06exfym.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <11509297422173-git-send-email-jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> For commits in oneline view (summary, shortlog, history) commit title
> (first line of commit message) is link to commit itself. If commit title
> (commit description) is shortened in output, hyperlink has title attribute
> with full title.

There are pure fixes and this "-title" enhancement intermixed.
Can we have fixes-only first?

> diff --git a/gitweb/gitweb.cgi b/gitweb/gitweb.cgi
> index e044c61..8f8ae4a 100755
> --- a/gitweb/gitweb.cgi
> +++ b/gitweb/gitweb.cgi
> @@ -326,7 +326,7 @@ EOF
>  		} elsif (defined $hash) {
>  			$search_hash = $hash;
>  		} else {
> -			$search_hash  = "HEAD";
> +			$search_hash = "HEAD";
>  		}
>  		$cgi->param("a", "search");
>  		$cgi->param("h", $search_hash);

Is a fix.

> @@ -1107,7 +1107,8 @@ sub git_summary {
>  		foreach my $entry (@$taglist) {
>  			my %tag = %$entry;
>  			my $comment_lines = $tag{'comment'};
> -			my $comment = shift @$comment_lines;
> +			my $comment_long = shift @$comment_lines;
> +			my $comment = $comment_long;
>  			if (defined($comment)) {
>  				$comment = chop_str($comment, 30, 5);
>  			}

Is an enhancement.

> @@ -1119,13 +1120,17 @@ sub git_summary {
>  			$alternate ^= 1;
>  			if ($i-- > 0) {
>  				print "<td><i>$tag{'age'}</i></td>\n" .
> -				      "<td>" .
> +				      "<td>";
>  				      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=$tag{'reftype'};h=$tag{'refid'}"), -class => "list"},
>  				      "<b>" . esc_html($tag{'name'}) . "</b>") .
>  				      "</td>\n" .
>  				      "<td>";

Is a fix.  How could we have missed this?

>  				if (defined($comment)) {
> -				      print $cgi->a({-class => "list", -href => "$my_uri?" . esc_param("p=$project;a=tag;h=$tag{'id'}")}, $comment);
> +					if (length($comment) < length($comment_long)) {
> +						print $cgi->a({-class => "list", -href => "$my_uri?" . esc_param("p=$project;a=tag;h=$tag{'id'}"), -title => "$comment_long"}, $comment);
> +					} else {
> +						print $cgi->a({-class => "list", -href => "$my_uri?" . esc_param("p=$project;a=tag;h=$tag{'id'}")}, $comment);
> +					}
>  				}
>  				print "</td>\n" .
>  				      "<td class=\"link\">";

Is an enhancement -- I suspect it is not worth to switch
depending on the length of $comment_long.  Does it clutter the
output too much if you did -title to everybody?

> @@ -1346,7 +1351,8 @@ sub git_tags {
>  		foreach my $entry (@$taglist) {
>  			my %tag = %$entry;
>  			my $comment_lines = $tag{'comment'};
> -			my $comment = shift @$comment_lines;
> +			my $comment_long = shift @$comment_lines;
> +			my $comment = $comment_long;
>  			if (defined($comment)) {
>  				$comment = chop_str($comment, 30, 5);
>  			}

Is an enhancement.  I think the matching hunk to fix the same
"$cgi->a() output concatenated as print argument" problem is
missing here.  And the next hunk the same comment as the one for
git_summary applies.

And as you say, git_summary and git_tags do look similar.  Maybe
we would want to refactor them first to clean things up before
piling up more features?