Re: [PATCHv7 2/9] gitweb: uniform author info for commit and commitdiff

From: Jakub Narebski <jnareb@gmail.com>
Date: Jun 27, 2009, 16:10 UTC
Message-ID: <200906271810.23119.jnareb@gmail.com>
In-Reply-To: <1246104305-15191-3-git-send-email-giuseppe.bilotta@gmail.com>

On Sat, 27 June 2009, Giuseppe Bilotta wrote:

Show 10 quoted lines

> Switch from
> 
> A U Thor <email@example.com> [date time]
> 
> to
> 
> author	A U Thor <email@example.com>
> 	date time
> committer	C O Mitter <other.email@example.com>
> 	committer date time

I would use:

  Switch from form similar to the one used by 'log' view

  	A U Thor <email@example.com> [date time]

  to the form used in 'commit' view

	author       A U Thor <email@example.com>
  	 	     date time
  	committer    C O Mitter <other.email@example.com>
  	             date time

(i.e. use spaces and not tabs to align). But this is minor issue, not worth worrying about IMVHO.

> 
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>

I am still not sure about this change. On one hand side it is unifying of 'commit' and 'commitdiff' view; most other web interfaces have single view equivalent to git-show, which displays both commit info, and the diff. (And 'commitdiff' with 'hp' parameter i.e. between two commits has to be redesigned anyway, so this issue doesn't enter this consideration).

On the other hand side IIRC 'commitdiff' uses short (one-line) authorship info because the main point is the diff, and multi-line author and commit info like the one used in 'commit' view takes a bit of vertical space. Also one can use similarity between 'log' and 'commitdiff' views (git-log and git-show) as a counter for argument that 'commitdiff' has to look like 'commit'.

But otherwise I quite like this patch.

Show 21 quoted lines

> ---
>  gitweb/gitweb.perl |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 9be723c..0d8005d 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5599,7 +5599,11 @@ sub git_commitdiff {
>  		git_header_html(undef, $expires);
>  		git_print_page_nav('commitdiff','', $hash,$co{'tree'},$hash, $formats_nav);
>  		git_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash);
> -		git_print_authorship(\%co, 'localtime' => 1);
> +		print "<div class=\"title_text\">\n" .
> +		      "<table class=\"object_header\">\n";
> +		git_print_authorship_rows(\%co);
> +		print "</table>".
> +		      "</div>\n";
>  		print "<div class=\"page_body\">\n";
>  		if (@{$co{'comment'}} > 1) {
>  			print "<div class=\"log\">\n";

Nice and short, thanks to earlier (re)factoring.

BTW. after this change the -localtime part of git_print_authorship() subroutine is unused... just saying ;-) Not something terribly important.

-- 
Jakub Narebski
Poland