[PATCH 0/9] gitweb: Great subroutine renaming + task separation into subroutines + improvements

[PATCH 0/9] gitweb: Great subroutine renaming + task separation into subroutines + improvements

[Jakub Narebski]

This series of patches is based on the 'next' branch, commit
10a6653c818f78c6032d45e1d4da544085e1f28b (v1.4.2-g10a6653)

It begins with:
* [PATCH 1/9] gitweb: Great subroutines renaming

Then there is series of patches which separate task into subroutine; 
'refactor' in patch subject here means that we use new subroutine in 
more than one place, 'separate' here means that new subroutine is (for 
now) used only in one place
* [PATCH 2/9] gitweb: Separate ref parsing in git_get_refs_list 
  into parse_ref
* [PATCH 3/9] gitweb: Refactor printing shortened title 
  in git_shortlog_body and git_tags_body
* [PATCH 4/9] gitweb: Separate main part of git_history 
  into git_history_body
* [PATCH 5/9] gitweb: Separate finding project owner 
  into git_get_project_owner

Then there is couple of improvements
* [PATCH 6/9] gitweb: Change appereance of marker of refs pointing 
  to given object
* [PATCH 7/9] gitweb: Skip comments in mime.types like file
* [PATCH 8/9] gitweb: True fix: Support for the standard mime.types map
  in gitweb

And finally yet another separation of task into subroutine, with small 
improvements (and perhaps to be reused)
* [PATCH 9/9] gitweb: Separate printing difftree in git_commit into
  git_difftree_body


-- 
Jakub Narebski
Poland

[PATCH 1/9] gitweb: Great subroutines renaming

[Jakub Narebski]

Rename some of subroutines to better reflect what they do.
Some renames were not performed because subroutine name
reflects hash key.

Subroutines name guideline:
* git_ prefix for subroutines related to git commands,
  git repository, or to gitweb actions
* git_get_ prefix for inner subroutines calling git command
  or reading some file in the repository and returning some output
* parse_ prefix for subroutines parsing some text (or reading and
  parsing some text) into hash or list
* format_ prefix for subroutines formatting, post-processing
  or generating some HTML/text fragment
* _get_ infix for subroutines which return result
* _print_ infix for subroutines which print fragment of output
* _body suffix for subroutines which outputs main part (body)
  of related action (usually table)
* _nav suffix for subroutines related to navigation bars
* _div suffix for subroutines returning or printing div element
* subroutine names should not be based on how the result is obtained,
  as this might change easily

Renames performed:
- git_get_referencing => format_ref_marker
- git_get_paging_nav => format_paging_nav
- git_read_head => git_get_head_hash
- git_read_hash => git_get_hash_by_ref
- git_read_description => git_get_project_description
- git_read_projects => git_get_projects_list
- read_info_ref => git_get_references
- git_read_refs => git_get_refs_list
- date_str => parse_date
- git_read_tag => parse_tag
- git_read_commit => parse_commit
- git_blob_plain_mimetype => blob_mimetype
- git_page_nav => git_print_page_nav
- git_header_div => git_print_header_div

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
I'm not sure if to add script used to create this commit, and to
port changes to be "post-rename". Here it is:

#!/bin/bash
if test -z "$1"; then
	FILE=`git rev-parse --show-cdup`gitweb/gitweb.perl
else
	FILE=$1
fi
echo "Great subroutine renaming: $FILE"
perl -s -p -i.re~ \
	-e 's/\bgit_get_referencing\b/format_ref_marker/;' \
	-e 's/\bgit_get_paging_nav\b/format_paging_nav/;' \
	\
	-e 's/\bgit_read_head\b/git_get_head_hash/;' \
	-e 's/\bgit_read_hash\b/git_get_hash_by_ref/;' \
	-e 's/\bgit_read_description\b/git_get_project_description/;' \
	-e 's/\bgit_read_projects\b/git_get_projects_list/;' \
	-e 's/\bread_info_ref\b/git_get_references/;' \
	-e 's/\bgit_read_refs\b/git_get_refs_list/;' \
	\
	-e 's/\bdate_str\b/parse_date/;' \
	-e 's/\bgit_read_tag\b/parse_tag/;' \
	-e 's/\bgit_read_commit\b/parse_commit/;' \
	\
	-e 's/\bgit_blob_plain_mimetype\b/blob_mimetype/;' \
	\
	-e 's/\bgit_page_nav\b/git_print_page_nav/;' \
	-e 's/\bgit_header_div\b/git_print_header_div/;' \
	"$FILE"
# end of gitweb-rename.sh


 gitweb/gitweb.perl |  250 ++++++++++++++++++++++++++--------------------------
 1 files changed, 125 insertions(+), 125 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 626fcc9..28df59e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -362,7 +362,7 @@ sub format_log_line_html {
 }
 
 # format marker of refs pointing to given object
-sub git_get_referencing {
+sub format_ref_marker {
 	my ($refs, $id) = @_;
 
 	if (defined $refs->{$id}) {
@@ -376,7 +376,7 @@ ## -------------------------------------
 ## git utility subroutines, invoking git commands
 
 # get HEAD ref of given project as hash
-sub git_read_head {
+sub git_get_head_hash {
 	my $project = shift;
 	my $oENV = $ENV{'GIT_DIR'};
 	my $retval = undef;
@@ -445,7 +445,7 @@ ## .....................................
 ## git utility functions, directly accessing git repository
 
 # assumes that PATH is not symref
-sub git_read_hash {
+sub git_get_hash_by_ref {
 	my $path = shift;
 
 	open my $fd, "$projectroot/$path" or return undef;
@@ -457,7 +457,7 @@ sub git_read_hash {
 	}
 }
 
-sub git_read_description {
+sub git_get_project_description {
 	my $path = shift;
 
 	open my $fd, "$projectroot/$path/description" or return undef;
@@ -467,7 +467,7 @@ sub git_read_description {
 	return $descr;
 }
 
-sub git_read_projects {
+sub git_get_projects_list {
 	my @list;
 
 	if (-d $projects_list) {
@@ -511,7 +511,7 @@ sub git_read_projects {
 	return @list;
 }
 
-sub read_info_ref {
+sub git_get_references {
 	my $type = shift || "";
 	my %refs;
 	# 5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c	refs/tags/v2.6.11
@@ -536,7 +536,7 @@ sub read_info_ref {
 ## ----------------------------------------------------------------------
 ## parse to hash functions
 
-sub date_str {
+sub parse_date {
 	my $epoch = shift;
 	my $tz = shift || "-0000";
 
@@ -561,7 +561,7 @@ sub date_str {
 	return %date;
 }
 
-sub git_read_tag {
+sub parse_tag {
 	my $tag_id = shift;
 	my %tag;
 	my @comment;
@@ -596,7 +596,7 @@ sub git_read_tag {
 	return %tag
 }
 
-sub git_read_commit {
+sub parse_commit {
 	my $commit_id = shift;
 	my $commit_text = shift;
 
@@ -693,7 +693,7 @@ sub git_read_commit {
 ## ......................................................................
 ## parse to array of hashes functions
 
-sub git_read_refs {
+sub git_get_refs_list {
 	my $ref_dir = shift;
 	my @reflist;
 
@@ -707,7 +707,7 @@ sub git_read_refs {
 	}, "$projectroot/$project/$ref_dir");
 
 	foreach my $ref_file (@refs) {
-		my $ref_id = git_read_hash("$project/$ref_dir/$ref_file");
+		my $ref_id = git_get_hash_by_ref("$project/$ref_dir/$ref_file");
 		my $type = git_get_type($ref_id) || next;
 		my %ref_item;
 		my %co;
@@ -716,10 +716,10 @@ sub git_read_refs {
 		$ref_item{'epoch'} = 0;
 		$ref_item{'age'} = "unknown";
 		if ($type eq "tag") {
-			my %tag = git_read_tag($ref_id);
+			my %tag = parse_tag($ref_id);
 			$ref_item{'comment'} = $tag{'comment'};
 			if ($tag{'type'} eq "commit") {
-				%co = git_read_commit($tag{'object'});
+				%co = parse_commit($tag{'object'});
 				$ref_item{'epoch'} = $co{'committer_epoch'};
 				$ref_item{'age'} = $co{'age_string'};
 			} elsif (defined($tag{'epoch'})) {
@@ -731,7 +731,7 @@ sub git_read_refs {
 			$ref_item{'name'} = $tag{'name'};
 			$ref_item{'refid'} = $tag{'object'};
 		} elsif ($type eq "commit"){
-			%co = git_read_commit($ref_id);
+			%co = parse_commit($ref_id);
 			$ref_item{'reftype'} = "commit";
 			$ref_item{'name'} = $ref_file;
 			$ref_item{'title'} = $co{'title'};
@@ -806,7 +806,7 @@ sub mimetype_guess {
 	return $mime;
 }
 
-sub git_blob_plain_mimetype {
+sub blob_mimetype {
 	my $fd = shift;
 	my $filename = shift;
 
@@ -925,7 +925,7 @@ EOF
 sub git_footer_html {
 	print "<div class=\"page_footer\">\n";
 	if (defined $project) {
-		my $descr = git_read_description($project);
+		my $descr = git_get_project_description($project);
 		if (defined $descr) {
 			print "<div class=\"page_footer_text\">" . esc_html($descr) . "</div>\n";
 		}
@@ -955,7 +955,7 @@ sub die_error {
 ## ----------------------------------------------------------------------
 ## functions printing or outputting HTML: navigation
 
-sub git_page_nav {
+sub git_print_page_nav {
 	my ($current, $suppress, $head, $treehead, $treebase, $extra) = @_;
 	$extra = '' if !defined $extra; # pager or formats
 
@@ -989,7 +989,7 @@ sub git_page_nav {
 	      "</div>\n";
 }
 
-sub git_get_paging_nav {
+sub format_paging_nav {
 	my ($action, $hash, $head, $page, $nrevs) = @_;
 	my $paging_nav;
 
@@ -1022,7 +1022,7 @@ sub git_get_paging_nav {
 ## ......................................................................
 ## functions printing or outputting HTML: div
 
-sub git_header_div {
+sub git_print_header_div {
 	my ($action, $title, $hash, $hash_base) = @_;
 	my $rest = '';
 
@@ -1062,9 +1062,9 @@ sub git_shortlog_body {
 	my $alternate = 0;
 	for (my $i = $from; $i <= $to; $i++) {
 		my $commit = $revlist->[$i];
-		#my $ref = defined $refs ? git_get_referencing($refs, $commit) : '';
-		my $ref = git_get_referencing($refs, $commit);
-		my %co = git_read_commit($commit);
+		#my $ref = defined $refs ? format_ref_marker($refs, $commit) : '';
+		my $ref = format_ref_marker($refs, $commit);
+		my %co = parse_commit($commit);
 		if ($alternate) {
 			print "<tr class=\"dark\">\n";
 		} else {
@@ -1282,24 +1282,24 @@ sub git_project_list {
 		die_error(undef, "Unknown order parameter");
 	}
 
-	my @list = git_read_projects();
+	my @list = git_get_projects_list();
 	my @projects;
 	if (!@list) {
 		die_error(undef, "No projects found");
 	}
 	foreach my $pr (@list) {
-		my $head = git_read_head($pr->{'path'});
+		my $head = git_get_head_hash($pr->{'path'});
 		if (!defined $head) {
 			next;
 		}
 		$ENV{'GIT_DIR'} = "$projectroot/$pr->{'path'}";
-		my %co = git_read_commit($head);
+		my %co = parse_commit($head);
 		if (!%co) {
 			next;
 		}
 		$pr->{'commit'} = \%co;
 		if (!defined $pr->{'descr'}) {
-			my $descr = git_read_description($pr->{'path'}) || "";
+			my $descr = git_get_project_description($pr->{'path'}) || "";
 			$pr->{'descr'} = chop_str($descr, 25, 5);
 		}
 		if (!defined $pr->{'owner'}) {
@@ -1383,10 +1383,10 @@ sub git_project_list {
 }
 
 sub git_summary {
-	my $descr = git_read_description($project) || "none";
-	my $head = git_read_head($project);
-	my %co = git_read_commit($head);
-	my %cd = date_str($co{'committer_epoch'}, $co{'committer_tz'});
+	my $descr = git_get_project_description($project) || "none";
+	my $head = git_get_head_hash($project);
+	my %co = parse_commit($head);
+	my %cd = parse_date($co{'committer_epoch'}, $co{'committer_tz'});
 
 	my $owner;
 	if (-f $projects_list) {
@@ -1407,9 +1407,9 @@ sub git_summary {
 		$owner = get_file_owner("$projectroot/$project");
 	}
 
-	my $refs = read_info_ref();
+	my $refs = git_get_references();
 	git_header_html();
-	git_page_nav('summary','', $head);
+	git_print_page_nav('summary','', $head);
 
 	print "<div class=\"title\">&nbsp;</div>\n";
 	print "<table cellspacing=\"0\">\n" .
@@ -1418,24 +1418,24 @@ sub git_summary {
 	      "<tr><td>last change</td><td>$cd{'rfc2822'}</td></tr>\n" .
 	      "</table>\n";
 
-	open my $fd, "-|", $GIT, "rev-list", "--max-count=17", git_read_head($project)
+	open my $fd, "-|", $GIT, "rev-list", "--max-count=17", git_get_head_hash($project)
 		or die_error(undef, "Open git-rev-list failed");
 	my @revlist = map { chomp; $_ } <$fd>;
 	close $fd;
-	git_header_div('shortlog');
+	git_print_header_div('shortlog');
 	git_shortlog_body(\@revlist, 0, 15, $refs,
 	                  $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=shortlog")}, "..."));
 
-	my $taglist = git_read_refs("refs/tags");
+	my $taglist = git_get_refs_list("refs/tags");
 	if (defined @$taglist) {
-		git_header_div('tags');
+		git_print_header_div('tags');
 		git_tags_body($taglist, 0, 15,
 		              $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=tags")}, "..."));
 	}
 
-	my $headlist = git_read_refs("refs/heads");
+	my $headlist = git_get_refs_list("refs/heads");
 	if (defined @$headlist) {
-		git_header_div('heads');
+		git_print_header_div('heads');
 		git_heads_body($headlist, $head, 0, 15,
 		               $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=heads")}, "..."));
 	}
@@ -1444,11 +1444,11 @@ sub git_summary {
 }
 
 sub git_tag {
-	my $head = git_read_head($project);
+	my $head = git_get_head_hash($project);
 	git_header_html();
-	git_page_nav('','', $head,undef,$head);
-	my %tag = git_read_tag($hash);
-	git_header_div('commit', esc_html($tag{'name'}), $hash);
+	git_print_page_nav('','', $head,undef,$head);
+	my %tag = parse_tag($hash);
+	git_print_header_div('commit', esc_html($tag{'name'}), $hash);
 	print "<div class=\"title_text\">\n" .
 	      "<table cellspacing=\"0\">\n" .
 	      "<tr>\n" .
@@ -1457,7 +1457,7 @@ sub git_tag {
 	      "<td class=\"link\">" . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=$tag{'type'};h=$tag{'object'}")}, $tag{'type'}) . "</td>\n" .
 	      "</tr>\n";
 	if (defined($tag{'author'})) {
-		my %ad = date_str($tag{'epoch'}, $tag{'tz'});
+		my %ad = parse_date($tag{'epoch'}, $tag{'tz'});
 		print "<tr><td>author</td><td>" . esc_html($tag{'author'}) . "</td></tr>\n";
 		print "<tr><td></td><td>" . $ad{'rfc2822'} . sprintf(" (%02d:%02d %s)", $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'}) . "</td></tr>\n";
 	}
@@ -1477,9 +1477,9 @@ sub git_blame2 {
 	my $ftype;
 	die_error(undef, "Permission denied") if (!git_get_project_config_bool ('blame'));
 	die_error('404 Not Found', "File name not defined") if (!$file_name);
-	$hash_base ||= git_read_head($project);
+	$hash_base ||= git_get_head_hash($project);
 	die_error(undef, "Couldn't find base commit") unless ($hash_base);
-	my %co = git_read_commit($hash_base)
+	my %co = parse_commit($hash_base)
 		or die_error(undef, "Reading commit failed");
 	if (!defined $hash) {
 		$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
@@ -1495,8 +1495,8 @@ sub git_blame2 {
 	my $formats_nav =
 		$cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$hash;hb=$hash_base;f=$file_name")}, "blob") .
 		" | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blame;f=$file_name")}, "head");
-	git_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
-	git_header_div('commit', esc_html($co{'title'}), $hash_base);
+	git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
+	git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
 	git_print_page_path($file_name, $ftype);
 	my @rev_color = (qw(light2 dark2));
 	my $num_colors = scalar(@rev_color);
@@ -1535,9 +1535,9 @@ sub git_blame {
 	my $fd;
 	die_error('403 Permission denied', "Permission denied") if (!git_get_project_config_bool ('blame'));
 	die_error('404 Not Found', "File name not defined") if (!$file_name);
-	$hash_base ||= git_read_head($project);
+	$hash_base ||= git_get_head_hash($project);
 	die_error(undef, "Couldn't find base commit") unless ($hash_base);
-	my %co = git_read_commit($hash_base)
+	my %co = parse_commit($hash_base)
 		or die_error(undef, "Reading commit failed");
 	if (!defined $hash) {
 		$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
@@ -1549,8 +1549,8 @@ sub git_blame {
 	my $formats_nav =
 		$cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$hash;hb=$hash_base;f=$file_name")}, "blob") .
 		" | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blame;f=$file_name")}, "head");
-	git_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
-	git_header_div('commit', esc_html($co{'title'}), $hash_base);
+	git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
+	git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
 	git_print_page_path($file_name, 'blob');
 	print "<div class=\"page_body\">\n";
 	print <<HTML;
@@ -1618,12 +1618,12 @@ HTML
 }
 
 sub git_tags {
-	my $head = git_read_head($project);
+	my $head = git_get_head_hash($project);
 	git_header_html();
-	git_page_nav('','', $head,undef,$head);
-	git_header_div('summary', $project);
+	git_print_page_nav('','', $head,undef,$head);
+	git_print_header_div('summary', $project);
 
-	my $taglist = git_read_refs("refs/tags");
+	my $taglist = git_get_refs_list("refs/tags");
 	if (defined @$taglist) {
 		git_tags_body($taglist);
 	}
@@ -1631,12 +1631,12 @@ sub git_tags {
 }
 
 sub git_heads {
-	my $head = git_read_head($project);
+	my $head = git_get_head_hash($project);
 	git_header_html();
-	git_page_nav('','', $head,undef,$head);
-	git_header_div('summary', $project);
+	git_print_page_nav('','', $head,undef,$head);
+	git_print_header_div('summary', $project);
 
-	my $taglist = git_read_refs("refs/heads");
+	my $taglist = git_get_refs_list("refs/heads");
 	if (defined @$taglist) {
 		git_heads_body($taglist, $head);
 	}
@@ -1646,7 +1646,7 @@ sub git_heads {
 sub git_blob_plain {
 	if (!defined $hash) {
 		if (defined $file_name) {
-			my $base = $hash_base || git_read_head($project);
+			my $base = $hash_base || git_get_head_hash($project);
 			$hash = git_get_hash_by_path($base, $file_name, "blob")
 				or die_error(undef, "Error lookup file");
 		} else {
@@ -1657,7 +1657,7 @@ sub git_blob_plain {
 	open my $fd, "-|", $GIT, "cat-file", "blob", $hash
 		or die_error(undef, "Couldn't cat $file_name, $hash");
 
-	$type ||= git_blob_plain_mimetype($fd, $file_name);
+	$type ||= blob_mimetype($fd, $file_name);
 
 	# save as filename, even when no $file_name is given
 	my $save_as = "$hash";
@@ -1679,7 +1679,7 @@ sub git_blob_plain {
 sub git_blob {
 	if (!defined $hash) {
 		if (defined $file_name) {
-			my $base = $hash_base || git_read_head($project);
+			my $base = $hash_base || git_get_head_hash($project);
 			$hash = git_get_hash_by_path($base, $file_name, "blob")
 				or die_error(undef, "Error lookup file");
 		} else {
@@ -1689,14 +1689,14 @@ sub git_blob {
 	my $have_blame = git_get_project_config_bool ('blame');
 	open my $fd, "-|", $GIT, "cat-file", "blob", $hash
 		or die_error(undef, "Couldn't cat $file_name, $hash");
-	my $mimetype = git_blob_plain_mimetype($fd, $file_name);
+	my $mimetype = blob_mimetype($fd, $file_name);
 	if ($mimetype !~ m/^text\//) {
 		close $fd;
 		return git_blob_plain($mimetype);
 	}
 	git_header_html();
 	my $formats_nav = '';
-	if (defined $hash_base && (my %co = git_read_commit($hash_base))) {
+	if (defined $hash_base && (my %co = parse_commit($hash_base))) {
 		if (defined $file_name) {
 			if ($have_blame) {
 				$formats_nav .= $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blame;h=$hash;hb=$hash_base;f=$file_name")}, "blame") . " | ";
@@ -1707,8 +1707,8 @@ sub git_blob {
 		} else {
 			$formats_nav .= $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob_plain;h=$hash")}, "plain");
 		}
-		git_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
-		git_header_div('commit', esc_html($co{'title'}), $hash_base);
+		git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
+		git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
 	} else {
 		print "<div class=\"page_nav\">\n" .
 		      "<br/><br/></div>\n" .
@@ -1730,7 +1730,7 @@ sub git_blob {
 
 sub git_tree {
 	if (!defined $hash) {
-		$hash = git_read_head($project);
+		$hash = git_get_head_hash($project);
 		if (defined $file_name) {
 			my $base = $hash_base || $hash;
 			$hash = git_get_hash_by_path($base, $file_name, "tree");
@@ -1746,16 +1746,16 @@ sub git_tree {
 	close $fd or die_error(undef, "Reading tree failed");
 	$/ = "\n";
 
-	my $refs = read_info_ref();
-	my $ref = git_get_referencing($refs, $hash_base);
+	my $refs = git_get_references();
+	my $ref = format_ref_marker($refs, $hash_base);
 	git_header_html();
 	my $base_key = "";
 	my $base = "";
 	my $have_blame = git_get_project_config_bool ('blame');
-	if (defined $hash_base && (my %co = git_read_commit($hash_base))) {
+	if (defined $hash_base && (my %co = parse_commit($hash_base))) {
 		$base_key = ";hb=$hash_base";
-		git_page_nav('tree','', $hash_base);
-		git_header_div('commit', esc_html($co{'title'}) . $ref, $hash_base);
+		git_print_page_nav('tree','', $hash_base);
+		git_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash_base);
 	} else {
 		print "<div class=\"page_nav\">\n";
 		print "<br/><br/></div>\n";
@@ -1811,14 +1811,14 @@ sub git_tree {
 }
 
 sub git_log {
-	my $head = git_read_head($project);
+	my $head = git_get_head_hash($project);
 	if (!defined $hash) {
 		$hash = $head;
 	}
 	if (!defined $page) {
 		$page = 0;
 	}
-	my $refs = read_info_ref();
+	my $refs = git_get_references();
 
 	my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
 	open my $fd, "-|", $GIT, "rev-list", $limit, $hash
@@ -1826,24 +1826,24 @@ sub git_log {
 	my @revlist = map { chomp; $_ } <$fd>;
 	close $fd;
 
-	my $paging_nav = git_get_paging_nav('log', $hash, $head, $page, $#revlist);
+	my $paging_nav = format_paging_nav('log', $hash, $head, $page, $#revlist);
 
 	git_header_html();
-	git_page_nav('log','', $hash,undef,undef, $paging_nav);
+	git_print_page_nav('log','', $hash,undef,undef, $paging_nav);
 
 	if (!@revlist) {
-		my %co = git_read_commit($hash);
+		my %co = parse_commit($hash);
 
-		git_header_div('summary', $project);
+		git_print_header_div('summary', $project);
 		print "<div class=\"page_body\"> Last change $co{'age_string'}.<br/><br/></div>\n";
 	}
 	for (my $i = ($page * 100); $i <= $#revlist; $i++) {
 		my $commit = $revlist[$i];
-		my $ref = git_get_referencing($refs, $commit);
-		my %co = git_read_commit($commit);
+		my $ref = format_ref_marker($refs, $commit);
+		my %co = parse_commit($commit);
 		next if !%co;
-		my %ad = date_str($co{'author_epoch'});
-		git_header_div('commit',
+		my %ad = parse_date($co{'author_epoch'});
+		git_print_header_div('commit',
 		               "<span class=\"age\">$co{'age_string'}</span>" .
 		               esc_html($co{'title'}) . $ref,
 		               $commit);
@@ -1881,12 +1881,12 @@ sub git_log {
 }
 
 sub git_commit {
-	my %co = git_read_commit($hash);
+	my %co = parse_commit($hash);
 	if (!%co) {
 		die_error(undef, "Unknown commit object");
 	}
-	my %ad = date_str($co{'author_epoch'}, $co{'author_tz'});
-	my %cd = date_str($co{'committer_epoch'}, $co{'committer_tz'});
+	my %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});
+	my %cd = parse_date($co{'committer_epoch'}, $co{'committer_tz'});
 
 	my $parent = $co{'parent'};
 	if (!defined $parent) {
@@ -1902,22 +1902,22 @@ sub git_commit {
 	if ($hash =~ m/^[0-9a-fA-F]{40}$/) {
 		$expires = "+1d";
 	}
-	my $refs = read_info_ref();
-	my $ref = git_get_referencing($refs, $co{'id'});
+	my $refs = git_get_references();
+	my $ref = format_ref_marker($refs, $co{'id'});
 	my $formats_nav = '';
 	if (defined $file_name && defined $co{'parent'}) {
 		my $parent = $co{'parent'};
 		$formats_nav .= $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blame;hb=$parent;f=$file_name")}, "blame");
 	}
 	git_header_html(undef, $expires);
-	git_page_nav('commit', defined $co{'parent'} ? '' : 'commitdiff',
+	git_print_page_nav('commit', defined $co{'parent'} ? '' : 'commitdiff',
 	             $hash, $co{'tree'}, $hash,
 	             $formats_nav);
 
 	if (defined $co{'parent'}) {
-		git_header_div('commitdiff', esc_html($co{'title'}) . $ref, $hash);
+		git_print_header_div('commitdiff', esc_html($co{'title'}) . $ref, $hash);
 	} else {
-		git_header_div('tree', esc_html($co{'title'}) . $ref, $co{'tree'}, $hash);
+		git_print_header_div('tree', esc_html($co{'title'}) . $ref, $co{'tree'}, $hash);
 	}
 	print "<div class=\"title_text\">\n" .
 	      "<table cellspacing=\"0\">\n";
@@ -2079,11 +2079,11 @@ sub git_commit {
 sub git_blobdiff {
 	mkdir($git_temp, 0700);
 	git_header_html();
-	if (defined $hash_base && (my %co = git_read_commit($hash_base))) {
+	if (defined $hash_base && (my %co = parse_commit($hash_base))) {
 		my $formats_nav =
 			$cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blobdiff_plain;h=$hash;hp=$hash_parent")}, "plain");
-		git_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
-		git_header_div('commit', esc_html($co{'title'}), $hash_base);
+		git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
+		git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
 	} else {
 		print "<div class=\"page_nav\">\n" .
 		      "<br/><br/></div>\n" .
@@ -2109,7 +2109,7 @@ sub git_blobdiff_plain {
 
 sub git_commitdiff {
 	mkdir($git_temp, 0700);
-	my %co = git_read_commit($hash);
+	my %co = parse_commit($hash);
 	if (!%co) {
 		die_error(undef, "Unknown commit object");
 	}
@@ -2126,13 +2126,13 @@ sub git_commitdiff {
 	if ($hash =~ m/^[0-9a-fA-F]{40}$/) {
 		$expires = "+1d";
 	}
-	my $refs = read_info_ref();
-	my $ref = git_get_referencing($refs, $co{'id'});
+	my $refs = git_get_references();
+	my $ref = format_ref_marker($refs, $co{'id'});
 	my $formats_nav =
 		$cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commitdiff_plain;h=$hash;hp=$hash_parent")}, "plain");
 	git_header_html(undef, $expires);
-	git_page_nav('commitdiff','', $hash,$co{'tree'},$hash, $formats_nav);
-	git_header_div('commit', esc_html($co{'title'}) . $ref, $hash);
+	git_print_page_nav('commitdiff','', $hash,$co{'tree'},$hash, $formats_nav);
+	git_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash);
 	print "<div class=\"page_body\">\n";
 	my $comment = $co{'comment'};
 	my $empty = 0;
@@ -2200,7 +2200,7 @@ sub git_commitdiff {
 
 sub git_commitdiff_plain {
 	mkdir($git_temp, 0700);
-	my %co = git_read_commit($hash);
+	my %co = parse_commit($hash);
 	if (!%co) {
 		die_error(undef, "Unknown commit object");
 	}
@@ -2214,7 +2214,7 @@ sub git_commitdiff_plain {
 
 	# try to figure out the next tag after this commit
 	my $tagname;
-	my $refs = read_info_ref("tags");
+	my $refs = git_get_references("tags");
 	open $fd, "-|", $GIT, "rev-list", "HEAD";
 	my @commits = map { chomp; $_ } <$fd>;
 	close $fd;
@@ -2228,7 +2228,7 @@ sub git_commitdiff_plain {
 	}
 
 	print $cgi->header(-type => "text/plain", -charset => 'utf-8', '-content-disposition' => "inline; filename=\"git-$hash.patch\"");
-	my %ad = date_str($co{'author_epoch'}, $co{'author_tz'});
+	my %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});
 	my $comment = $co{'comment'};
 	print "From: $co{'author'}\n" .
 	      "Date: $ad{'rfc2822'} ($ad{'tz_local'})\n".
@@ -2264,17 +2264,17 @@ sub git_commitdiff_plain {
 
 sub git_history {
 	if (!defined $hash_base) {
-		$hash_base = git_read_head($project);
+		$hash_base = git_get_head_hash($project);
 	}
 	my $ftype;
-	my %co = git_read_commit($hash_base);
+	my %co = parse_commit($hash_base);
 	if (!%co) {
 		die_error(undef, "Unknown commit object");
 	}
-	my $refs = read_info_ref();
+	my $refs = git_get_references();
 	git_header_html();
-	git_page_nav('','', $hash_base,$co{'tree'},$hash_base);
-	git_header_div('commit', esc_html($co{'title'}), $hash_base);
+	git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base);
+	git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
 	if (!defined $hash && defined $file_name) {
 		$hash = git_get_hash_by_path($hash_base, $file_name);
 	}
@@ -2290,11 +2290,11 @@ sub git_history {
 	while (my $line = <$fd>) {
 		if ($line =~ m/^([0-9a-fA-F]{40})/){
 			my $commit = $1;
-			my %co = git_read_commit($commit);
+			my %co = parse_commit($commit);
 			if (!%co) {
 				next;
 			}
-			my $ref = git_get_referencing($refs, $commit);
+			my $ref = format_ref_marker($refs, $commit);
 			if ($alternate) {
 				print "<tr class=\"dark\">\n";
 			} else {
@@ -2330,9 +2330,9 @@ sub git_search {
 		die_error(undef, "Text field empty");
 	}
 	if (!defined $hash) {
-		$hash = git_read_head($project);
+		$hash = git_get_head_hash($project);
 	}
-	my %co = git_read_commit($hash);
+	my %co = parse_commit($hash);
 	if (!%co) {
 		die_error(undef, "Unknown commit object");
 	}
@@ -2351,8 +2351,8 @@ sub git_search {
 		$pickaxe_search = 1;
 	}
 	git_header_html();
-	git_page_nav('','', $hash,$co{'tree'},$hash);
-	git_header_div('commit', esc_html($co{'title'}), $hash);
+	git_print_page_nav('','', $hash,$co{'tree'},$hash);
+	git_print_header_div('commit', esc_html($co{'title'}), $hash);
 
 	print "<table cellspacing=\"0\">\n";
 	my $alternate = 0;
@@ -2370,7 +2370,7 @@ sub git_search {
 				next;
 			}
 			my @commit_lines = split "\n", $commit_text;
-			my %co = git_read_commit(undef, \@commit_lines);
+			my %co = parse_commit(undef, \@commit_lines);
 			if (!%co) {
 				next;
 			}
@@ -2451,7 +2451,7 @@ sub git_search {
 					print "</td>\n" .
 					      "</tr>\n";
 				}
-				%co = git_read_commit($1);
+				%co = parse_commit($1);
 			}
 		}
 		close $fd;
@@ -2461,14 +2461,14 @@ sub git_search {
 }
 
 sub git_shortlog {
-	my $head = git_read_head($project);
+	my $head = git_get_head_hash($project);
 	if (!defined $hash) {
 		$hash = $head;
 	}
 	if (!defined $page) {
 		$page = 0;
 	}
-	my $refs = read_info_ref();
+	my $refs = git_get_references();
 
 	my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
 	open my $fd, "-|", $GIT, "rev-list", $limit, $hash
@@ -2476,7 +2476,7 @@ sub git_shortlog {
 	my @revlist = map { chomp; $_ } <$fd>;
 	close $fd;
 
-	my $paging_nav = git_get_paging_nav('shortlog', $hash, $head, $page, $#revlist);
+	my $paging_nav = format_paging_nav('shortlog', $hash, $head, $page, $#revlist);
 	my $next_link = '';
 	if ($#revlist >= (100 * ($page+1)-1)) {
 		$next_link =
@@ -2486,8 +2486,8 @@ sub git_shortlog {
 
 
 	git_header_html();
-	git_page_nav('shortlog','', $hash,$hash,$hash, $paging_nav);
-	git_header_div('summary', $project);
+	git_print_page_nav('shortlog','', $hash,$hash,$hash, $paging_nav);
+	git_print_header_div('summary', $project);
 
 	git_shortlog_body(\@revlist, ($page * 100), $#revlist, $refs, $next_link);
 
@@ -2499,7 +2499,7 @@ ## feeds (RSS, OPML)
 
 sub git_rss {
 	# http://www.notestips.com/80256B3A007F2692/1/NAMO5P9UPQ
-	open my $fd, "-|", $GIT, "rev-list", "--max-count=150", git_read_head($project)
+	open my $fd, "-|", $GIT, "rev-list", "--max-count=150", git_get_head_hash($project)
 		or die_error(undef, "Open git-rev-list failed");
 	my @revlist = map { chomp; $_ } <$fd>;
 	close $fd or die_error(undef, "Reading git-rev-list failed");
@@ -2514,12 +2514,12 @@ sub git_rss {
 
 	for (my $i = 0; $i <= $#revlist; $i++) {
 		my $commit = $revlist[$i];
-		my %co = git_read_commit($commit);
+		my %co = parse_commit($commit);
 		# we read 150, we always show 30 and the ones more recent than 48 hours
 		if (($i >= 20) && ((time - $co{'committer_epoch'}) > 48*60*60)) {
 			last;
 		}
-		my %cd = date_str($co{'committer_epoch'});
+		my %cd = parse_date($co{'committer_epoch'});
 		open $fd, "-|", $GIT, "diff-tree", '-r', $co{'parent'}, $co{'id'} or next;
 		my @difftree = map { chomp; $_ } <$fd>;
 		close $fd or next;
@@ -2556,7 +2556,7 @@ sub git_rss {
 }
 
 sub git_opml {
-	my @list = git_read_projects();
+	my @list = git_get_projects_list();
 
 	print $cgi->header(-type => 'text/xml', -charset => 'utf-8');
 	print "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n".
@@ -2569,12 +2569,12 @@ sub git_opml {
 
 	foreach my $pr (@list) {
 		my %proj = %$pr;
-		my $head = git_read_head($proj{'path'});
+		my $head = git_get_head_hash($proj{'path'});
 		if (!defined $head) {
 			next;
 		}
 		$ENV{'GIT_DIR'} = "$projectroot/$proj{'path'}";
-		my %co = git_read_commit($head);
+		my %co = parse_commit($head);
 		if (!%co) {
 			next;
 		}
-- 
1.4.1.1

Re: [PATCH 1/9] gitweb: Great subroutines renaming

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> I'm not sure if to add script used to create this commit, and to
> port changes to be "post-rename". Here it is:
>
> #!/bin/bash
> if test -z "$1"; then
> 	FILE=`git rev-parse --show-cdup`gitweb/gitweb.perl
> else
> 	FILE=$1
> fi
> echo "Great subroutine renaming: $FILE"
> perl -s -p -i.re~ \
> 	-e 's/\bgit_get_referencing\b/format_ref_marker/;' \

Will apply, but I suspect all of these need g at the end as in
"s/foo/bar/g;".

[PATCH 2/9] gitweb: Separate ref parsing in git_get_refs_list into parse_ref

[Jakub Narebski]

Note that for each ref there are usually two calls to git subroutines:
first to get the type of ref, second to parse ref if ref is of commit
or tag type.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   80 +++++++++++++++++++++++++++++-----------------------
 1 files changed, 45 insertions(+), 35 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 28df59e..0c4ec92 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -690,6 +690,49 @@ sub parse_commit {
 	return %co;
 }
 
+# parse ref from ref_file, given by ref_id, with given type
+sub parse_ref {
+	my $ref_file = shift;
+	my $ref_id = shift;
+	my $type = shift || git_get_type($ref_id);
+	my %ref_item;
+
+	$ref_item{'type'} = $type;
+	$ref_item{'id'} = $ref_id;
+	$ref_item{'epoch'} = 0;
+	$ref_item{'age'} = "unknown";
+	if ($type eq "tag") {
+		my %tag = parse_tag($ref_id);
+		$ref_item{'comment'} = $tag{'comment'};
+		if ($tag{'type'} eq "commit") {
+			my %co = parse_commit($tag{'object'});
+			$ref_item{'epoch'} = $co{'committer_epoch'};
+			$ref_item{'age'} = $co{'age_string'};
+		} elsif (defined($tag{'epoch'})) {
+			my $age = time - $tag{'epoch'};
+			$ref_item{'epoch'} = $tag{'epoch'};
+			$ref_item{'age'} = age_string($age);
+		}
+		$ref_item{'reftype'} = $tag{'type'};
+		$ref_item{'name'} = $tag{'name'};
+		$ref_item{'refid'} = $tag{'object'};
+	} elsif ($type eq "commit"){
+		my %co = parse_commit($ref_id);
+		$ref_item{'reftype'} = "commit";
+		$ref_item{'name'} = $ref_file;
+		$ref_item{'title'} = $co{'title'};
+		$ref_item{'refid'} = $ref_id;
+		$ref_item{'epoch'} = $co{'committer_epoch'};
+		$ref_item{'age'} = $co{'age_string'};
+	} else {
+		$ref_item{'reftype'} = $type;
+		$ref_item{'name'} = $ref_file;
+		$ref_item{'refid'} = $ref_id;
+	}
+
+	return %ref_item;
+}
+
 ## ......................................................................
 ## parse to array of hashes functions
 
@@ -709,44 +752,11 @@ sub git_get_refs_list {
 	foreach my $ref_file (@refs) {
 		my $ref_id = git_get_hash_by_ref("$project/$ref_dir/$ref_file");
 		my $type = git_get_type($ref_id) || next;
-		my %ref_item;
-		my %co;
-		$ref_item{'type'} = $type;
-		$ref_item{'id'} = $ref_id;
-		$ref_item{'epoch'} = 0;
-		$ref_item{'age'} = "unknown";
-		if ($type eq "tag") {
-			my %tag = parse_tag($ref_id);
-			$ref_item{'comment'} = $tag{'comment'};
-			if ($tag{'type'} eq "commit") {
-				%co = parse_commit($tag{'object'});
-				$ref_item{'epoch'} = $co{'committer_epoch'};
-				$ref_item{'age'} = $co{'age_string'};
-			} elsif (defined($tag{'epoch'})) {
-				my $age = time - $tag{'epoch'};
-				$ref_item{'epoch'} = $tag{'epoch'};
-				$ref_item{'age'} = age_string($age);
-			}
-			$ref_item{'reftype'} = $tag{'type'};
-			$ref_item{'name'} = $tag{'name'};
-			$ref_item{'refid'} = $tag{'object'};
-		} elsif ($type eq "commit"){
-			%co = parse_commit($ref_id);
-			$ref_item{'reftype'} = "commit";
-			$ref_item{'name'} = $ref_file;
-			$ref_item{'title'} = $co{'title'};
-			$ref_item{'refid'} = $ref_id;
-			$ref_item{'epoch'} = $co{'committer_epoch'};
-			$ref_item{'age'} = $co{'age_string'};
-		} else {
-			$ref_item{'reftype'} = $type;
-			$ref_item{'name'} = $ref_file;
-			$ref_item{'refid'} = $ref_id;
-		}
+		my %ref_item = parse_ref($ref_file, $ref_id, $type);
 
 		push @reflist, \%ref_item;
 	}
-	# sort tags by age
+	# sort refs by age
 	@reflist = sort {$b->{'epoch'} <=> $a->{'epoch'}} @reflist;
 	return \@reflist;
 }
-- 
1.4.1.1

Re: [PATCH 2/9] gitweb: Separate ref parsing in git_get_refs_list into parse_ref

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Note that for each ref there are usually two calls to git subroutines:
> first to get the type of ref, second to parse ref if ref is of commit
> or tag type.
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
>  gitweb/gitweb.perl |   80 +++++++++++++++++++++++++++++-----------------------
>  1 files changed, 45 insertions(+), 35 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 28df59e..0c4ec92 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -690,6 +690,49 @@ sub parse_commit {
>  	return %co;
>  }
>  
> +# parse ref from ref_file, given by ref_id, with given type
> +sub parse_ref {
> +	my $ref_file = shift;
> +	my $ref_id = shift;
> +	my $type = shift || git_get_type($ref_id);

This git_get_type() is additional from the original.
Future-proofing yourself for other new callers?

Re: [PATCH 2/9] gitweb: Separate ref parsing in git_get_refs_list into parse_ref

[Jakub Narebski]

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Note that for each ref there are usually two calls to git subroutines:
>> first to get the type of ref, second to parse ref if ref is of commit
>> or tag type.

[...]  
>> +# parse ref from ref_file, given by ref_id, with given type
>> +sub parse_ref {
>> +    my $ref_file = shift;
>> +    my $ref_id = shift;
>> +    my $type = shift || git_get_type($ref_id);
> 
> This git_get_type() is additional from the original.
> Future-proofing yourself for other new callers?

Yes, just in case.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

[PATCH 3/9] gitweb: Refactor printing shortened title in git_shortlog_body and git_tags_body

[Jakub Narebski]

Separate printing of perhaps shortened title (subject) in
git_shortlog_body and git_tags_body into format_subject_html.

While at it, remove presentation element <b>...</b> used to format
title (subject) and move formatting to CSS.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
CSS is used in such a way as to not change output

 gitweb/gitweb.css  |    5 +++++
 gitweb/gitweb.perl |   34 ++++++++++++++++++----------------
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 47c1ade..f58a418 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -117,9 +117,14 @@ div.list_head {
 
 a.list {
 	text-decoration: none;
+	font-weight: bold;
 	color: #000000;
 }
 
+table.tags a.list {
+	font-weight: normal;
+}
+
 a.list:hover {
 	text-decoration: underline;
 	color: #880000;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 0c4ec92..c4d6eab 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -372,6 +372,22 @@ sub format_ref_marker {
 	}
 }
 
+# format, perhaps shortened and with markers, title line
+sub format_subject_html {
+	my ($long, $short, $query, $extra) = @_;
+	$extra = '' unless defined($extra);
+
+	if (length($short) < length($long)) {
+		return $cgi->a({-href => "$my_uri?" . esc_param($query),
+		               -class => "list", -title => $long},
+		       esc_html($short) . $extra);
+	} else {
+		return $cgi->a({-href => "$my_uri?" . esc_param($query),
+		               -class => "list"},
+		       esc_html($long)  . $extra);
+	}
+}
+
 ## ----------------------------------------------------------------------
 ## git utility subroutines, invoking git commands
 
@@ -1085,15 +1101,7 @@ sub git_shortlog_body {
 		print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" .
 		      "<td><i>" . esc_html(chop_str($co{'author_name'}, 10)) . "</i></td>\n" .
 		      "<td>";
-		if (length($co{'title_short'}) < length($co{'title'})) {
-			print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commit;h=$commit"),
-			               -class => "list", -title => "$co{'title'}"},
-			      "<b>" . esc_html($co{'title_short'}) . "$ref</b>");
-		} else {
-			print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commit;h=$commit"),
-			               -class => "list"},
-			      "<b>" . esc_html($co{'title'}) . "$ref</b>");
-		}
+		print format_subject_html($co{'title'}, $co{'title_short'}, "p=$project;a=commit;h=$commit", $ref);
 		print "</td>\n" .
 		      "<td class=\"link\">" .
 		      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commit;h=$commit")}, "commit") . " | " .
@@ -1139,13 +1147,7 @@ sub git_tags_body {
 		      "</td>\n" .
 		      "<td>";
 		if (defined $comment) {
-			if (length($comment_short) < length($comment)) {
-				print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=tag;h=$tag{'id'}"),
-				               -class => "list", -title => $comment}, $comment_short);
-			} else {
-				print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=tag;h=$tag{'id'}"),
-				               -class => "list"}, $comment);
-			}
+			print format_subject_html($comment, $comment_short, "p=$project;a=tag;h=$tag{'id'}");
 		}
 		print "</td>\n" .
 		      "<td class=\"selflink\">";
-- 
1.4.1.1

Re: [PATCH 3/9] gitweb: Refactor printing shortened title in git_shortlog_body and git_tags_body

[Junio C Hamano]

Beautiful.

[PATCH 4/9] gitweb: Separate main part of git_history into git_history_body

[Jakub Narebski]

Separates main part of git_history into git_history_body subroutine,
and makes output more similar to git_shortlog.  Adds "diff to current"
link only for history of regular file (blob).

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   96 +++++++++++++++++++++++++++++++++-------------------
 1 files changed, 60 insertions(+), 36 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c4d6eab..5af6e77 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1117,6 +1117,64 @@ sub git_shortlog_body {
 	print "</table>\n";
 }
 
+sub git_history_body {
+	# Warning: assumes constant type (blob or tree) during history
+	my ($fd, $refs, $hash_base, $ftype, $extra) = @_;
+
+	print "<table class=\"history\" cellspacing=\"0\">\n";
+	my $alternate = 0;
+	while (my $line = <$fd>) {
+		if ($line !~ m/^([0-9a-fA-F]{40})/) {
+			next;
+		}
+
+		my $commit = $1;
+		my %co = parse_commit($commit);
+		if (!%co) {
+			next;
+		}
+
+		my $ref = format_ref_marker($refs, $commit);
+
+		if ($alternate) {
+			print "<tr class=\"dark\">\n";
+		} else {
+			print "<tr class=\"light\">\n";
+		}
+		$alternate ^= 1;
+		print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" .
+		      # shortlog uses      chop_str($co{'author_name'}, 10)
+		      "<td><i>" . esc_html(chop_str($co{'author_name'}, 15, 3)) . "</i></td>\n" .
+		      "<td>";
+		# originally git_history used chop_str($co{'title'}, 50)
+		print format_subject_html($co{'title'}, $co{'title_short'}, "p=$project;a=commit;h=$commit", $ref);
+		print "</td>\n" .
+		      "<td class=\"link\">" .
+		      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commit;h=$commit")}, "commit") . " | " .
+		      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commitdiff;h=$commit")}, "commitdiff") . " | " .
+		      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=$ftype;hb=$commit;f=$file_name")}, $ftype);
+
+		if ($ftype eq 'blob') {
+			my $blob_current = git_get_hash_by_path($hash_base, $file_name);
+			my $blob_parent  = git_get_hash_by_path($commit, $file_name);
+			if (defined $blob_current && defined $blob_parent &&
+					$blob_current ne $blob_parent) {
+				print " | " .
+					$cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blobdiff;h=$blob_current;hp=$blob_parent;hb=$commit;f=$file_name")},
+					        "diff to current");
+			}
+		}
+		print "</td>\n" .
+		      "</tr>\n";
+	}
+	if (defined $extra) {
+		print "<tr>\n" .
+		      "<td colspan=\"4\">$extra</td>\n" .
+		      "</tr>\n";
+	}
+	print "</table>\n";
+}
+
 sub git_tags_body {
 	# uses global variable $project
 	my ($taglist, $from, $to, $extra) = @_;
@@ -2297,42 +2355,8 @@ sub git_history {
 
 	open my $fd, "-|",
 		$GIT, "rev-list", "--full-history", $hash_base, "--", $file_name;
-	print "<table cellspacing=\"0\">\n";
-	my $alternate = 0;
-	while (my $line = <$fd>) {
-		if ($line =~ m/^([0-9a-fA-F]{40})/){
-			my $commit = $1;
-			my %co = parse_commit($commit);
-			if (!%co) {
-				next;
-			}
-			my $ref = format_ref_marker($refs, $commit);
-			if ($alternate) {
-				print "<tr class=\"dark\">\n";
-			} else {
-				print "<tr class=\"light\">\n";
-			}
-			$alternate ^= 1;
-			print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" .
-			      "<td><i>" . esc_html(chop_str($co{'author_name'}, 15, 3)) . "</i></td>\n" .
-			      "<td>" . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commit;h=$commit"), -class => "list"}, "<b>" .
-			      esc_html(chop_str($co{'title'}, 50)) . "$ref</b>") . "</td>\n" .
-			      "<td class=\"link\">" .
-			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commit;h=$commit")}, "commit") .
-			      " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commitdiff;h=$commit")}, "commitdiff") .
-			      " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=$ftype;hb=$commit;f=$file_name")}, $ftype);
-			my $blob = git_get_hash_by_path($hash_base, $file_name);
-			my $blob_parent = git_get_hash_by_path($commit, $file_name);
-			if (defined $blob && defined $blob_parent && $blob ne $blob_parent) {
-				print " | " .
-				$cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blobdiff;h=$blob;hp=$blob_parent;hb=$commit;f=$file_name")},
-				"diff to current");
-			}
-			print "</td>\n" .
-			      "</tr>\n";
-		}
-	}
-	print "</table>\n";
+	git_history_body($fd, $refs, $hash_base, $ftype);
+
 	close $fd;
 	git_footer_html();
 }
-- 
1.4.1.1

[PATCH 5/9] gitweb: Separate finding project owner into git_get_project_owner

[Jakub Narebski]

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
And added some comments, while at it.

 gitweb/gitweb.perl |   50 ++++++++++++++++++++++++++++++++------------------
 1 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 5af6e77..6be6c55 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -527,6 +527,37 @@ sub git_get_projects_list {
 	return @list;
 }
 
+sub git_get_project_owner {
+	my $project = shift;
+	my $owner;
+
+	return undef unless $project;
+
+	# read from file (url-encoded):
+	# 'git%2Fgit.git Linus+Torvalds'
+	# 'libs%2Fklibc%2Fklibc.git H.+Peter+Anvin'
+	# 'linux%2Fhotplug%2Fudev.git Greg+Kroah-Hartman'
+	if (-f $projects_list) {
+		open (my $fd , $projects_list);
+		while (my $line = <$fd>) {
+			chomp $line;
+			my ($pr, $ow) = split ' ', $line;
+			$pr = unescape($pr);
+			$ow = unescape($ow);
+			if ($pr eq $project) {
+				$owner = decode("utf8", $ow, Encode::FB_DEFAULT);
+				last;
+			}
+		}
+		close $fd;
+	}
+	if (!defined $owner) {
+		$owner = get_file_owner("$projectroot/$project");
+	}
+
+	return $owner;
+}
+
 sub git_get_references {
 	my $type = shift || "";
 	my %refs;
@@ -1458,24 +1489,7 @@ sub git_summary {
 	my %co = parse_commit($head);
 	my %cd = parse_date($co{'committer_epoch'}, $co{'committer_tz'});
 
-	my $owner;
-	if (-f $projects_list) {
-		open (my $fd , $projects_list);
-		while (my $line = <$fd>) {
-			chomp $line;
-			my ($pr, $ow) = split ' ', $line;
-			$pr = unescape($pr);
-			$ow = unescape($ow);
-			if ($pr eq $project) {
-				$owner = decode("utf8", $ow, Encode::FB_DEFAULT);
-				last;
-			}
-		}
-		close $fd;
-	}
-	if (!defined $owner) {
-		$owner = get_file_owner("$projectroot/$project");
-	}
+	my $owner = git_get_project_owner($project);
 
 	my $refs = git_get_references();
 	git_header_html();
-- 
1.4.1.1

[PATCH 6/9] gitweb: Change appereance of marker of refs pointing to given object

[Jakub Narebski]

Change git_get_references to include type of ref in the %refs value, which
means putting everything after 'refs/' as a ref name, not only last
part of the name.  Instead of separating refs pointing to the same
object by " / " separator, use anonymous array reference to store all
refs pointing to given object.

Use 'git-ls-remote .' if $projectroot/$project/info/refs does not
exist.  (Perhaps it should be used always.)

Refs are now in separate span elements.  Class is dependent on the ref
type: currently known classes are 'tag', 'head', 'remote', and 'ref'
(last one for HEAD and other refs in the main directory).  There is
encompassing span element of class refs, just in case of unknown ref
type.

This might be considered cleaner separating of git_get_references into
filling %refs hash only, and not taking part in formatting ref marker.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Alternate solution to the one proposed by Thomas Kolejka in
 "[PATCH] gitweb: Different colours for tags and heads"
  Message-Id: <20060811151224.177110@gmx.net>

CSS and format_ref_marker is conservative: as of now it should not be
"ref" (generic) type of references. 

 gitweb/gitweb.css  |   19 +++++++++++++++++--
 gitweb/gitweb.perl |   37 ++++++++++++++++++++++++++++++-------
 2 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index f58a418..21ce99c 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -326,15 +326,30 @@ a.rss_logo:hover {
 	background-color: #ee5500;
 }
 
-span.tag {
+span.refs span {
 	padding: 0px 4px;
 	font-size: 10px;
 	font-weight: normal;
-	background-color: #ffffaa;
 	border: 1px solid;
+	background-color: #ffaaff;
+	border-color: #ffccff #ff00ee #ff00ee #ffccff;
+}
+
+span.refs span.ref {
+	background-color: #aaaaff;
+	border-color: #ccccff #0033cc #0033cc #ccccff;
+}
+
+span.refs span.tag {
+	background-color: #ffffaa;
 	border-color: #ffffcc #ffee00 #ffee00 #ffffcc;
 }
 
+span.refs span.head {
+	background-color: #aaffaa;
+	border-color: #ccffcc #00cc33 #00cc33 #ccffcc;
+}
+
 span.atnight {
 	color: #cc0000;
 }
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 6be6c55..4fe3fc7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -364,9 +364,26 @@ sub format_log_line_html {
 # format marker of refs pointing to given object
 sub format_ref_marker {
 	my ($refs, $id) = @_;
+	my $markers = '';
 
 	if (defined $refs->{$id}) {
-		return ' <span class="tag">' . esc_html($refs->{$id}) . '</span>';
+		foreach my $ref (@{$refs->{$id}}) {
+			my ($type, $name) = qw();
+			# e.g. tags/v2.6.11 or heads/next
+			if ($ref =~ m!^(.*?)s?/(.*)$!) {
+				$type = $1;
+				$name = $2;
+			} else {
+				$type = "ref";
+				$name = $ref;
+			}
+
+			$markers .= " <span class=\"$type\">" . esc_html($name) . "</span>";
+		}
+	}
+
+	if ($markers) {
+		return ' <span class="refs">'. $markers . '</span>';
 	} else {
 		return "";
 	}
@@ -561,18 +578,24 @@ sub git_get_project_owner {
 sub git_get_references {
 	my $type = shift || "";
 	my %refs;
+	my $fd;
 	# 5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c	refs/tags/v2.6.11
 	# c39ae07f393806ccf406ef966e9a15afc43cc36a	refs/tags/v2.6.11^{}
-	open my $fd, "$projectroot/$project/info/refs" or return;
+	if (-f "$projectroot/$project/info/refs") {
+		open $fd, "$projectroot/$project/info/refs"
+			or return;
+	} else {
+		open $fd, "-|", $GIT, "ls-remote", "."
+			or return;
+	}
+
 	while (my $line = <$fd>) {
 		chomp $line;
-		# attention: for $type == "" it saves only last path part of ref name
-		# e.g. from 'refs/heads/jn/gitweb' it would leave only 'gitweb'
-		if ($line =~ m/^([0-9a-fA-F]{40})\t.*$type\/([^\^]+)/) {
+		if ($line =~ m/^([0-9a-fA-F]{40})\trefs\/($type\/?[^\^]+)/) {
 			if (defined $refs{$1}) {
-				$refs{$1} .= " / $2";
+				push @{$refs{$1}}, $2;
 			} else {
-				$refs{$1} = $2;
+				$refs{$1} = [ $2 ];
 			}
 		}
 	}
-- 
1.4.1.1

Re: [PATCH 6/9] gitweb: Change appereance of marker of refs pointing to given object

[Jakub Narebski]

Jakub Narebski wrote:
> Change git_get_references to include type of ref in the %refs value, which
> means putting everything after 'refs/' as a ref name, not only last
> part of the name.  Instead of separating refs pointing to the same
> object by " / " separator, use anonymous array reference to store all
> refs pointing to given object.

Earlier version of the patch used ':' (instead of ' / ') to concatenate
multiple refs pointing to the same object, then split on ':'. Character
':' was used because it cannot be part of valid ref name (as of
git-check-ref-name).

Note that usually there is at most one ref pointing to given object;
multiple refs are quite rare (usually when starting new branch, or just
tagged a branch).

-- 
Jakub Narebski
Poland

Re: [PATCH 6/9] gitweb: Change appereance of marker of refs pointing to given object

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Change git_get_references to include type of ref in the %refs value, which
> means putting everything after 'refs/' as a ref name, not only last
> part of the name.  Instead of separating refs pointing to the same
> object by " / " separator, use anonymous array reference to store all
> refs pointing to given object.
>
> Use 'git-ls-remote .' if $projectroot/$project/info/refs does not
> exist.  (Perhaps it should be used always.)
>
> Refs are now in separate span elements.  Class is dependent on the ref
> type: currently known classes are 'tag', 'head', 'remote', and 'ref'
> (last one for HEAD and other refs in the main directory).  There is
> encompassing span element of class refs, just in case of unknown ref
> type.

I do not see definition that matches "span.refs span.remote" in
the CSS, though.

> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 6be6c55..4fe3fc7 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -364,9 +364,26 @@ sub format_log_line_html {
>  # format marker of refs pointing to given object
>  sub format_ref_marker {
>  	my ($refs, $id) = @_;
> +	my $markers = '';
>  
>  	if (defined $refs->{$id}) {
> -		return ' <span class="tag">' . esc_html($refs->{$id}) . '</span>';
> +		foreach my $ref (@{$refs->{$id}}) {
> +			my ($type, $name) = qw();

Just () -- qw() is not needed.

> +			# e.g. tags/v2.6.11 or heads/next
> +			if ($ref =~ m!^(.*?)s?/(.*)$!) {
> +				$type = $1;
> +				$name = $2;
> +			} else {
> +				$type = "ref";
> +				$name = $ref;
> +			}

Hmph.  Maybe have a hash that defines the ones you know how to
handle, and do something like:

        if ($ref =~ m|^([^/]+)/(.*)$| &&  exists $i_know_this_class{$1}) {
        	$type = $1;
                $name = $2;
	}
        else {
        	$type = 'ref';
                $name = $ref;
	}

> @@ -561,18 +578,24 @@ sub git_get_project_owner {
>...
> +		if ($line =~ m/^([0-9a-fA-F]{40})\trefs\/($type\/?[^\^]+)/) {
>  			if (defined $refs{$1}) {
> -				$refs{$1} .= " / $2";
> +				push @{$refs{$1}}, $2;
>  			} else {
> -				$refs{$1} = $2;
> +				$refs{$1} = [ $2 ];
>  			}
>  		}

It would work as in your patch, but I would have preferred to
see "exists $refs{$1}" there instead of "defined".

Or lose the if() and do it like this, which would be cleaner?

	if ($line =~ ...) {
        	push @{$refs{$1}}, $2;
	}

Re: [PATCH 6/9] gitweb: Change appereance of marker of refs pointing to given object

[Jakub Narebski]

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Change git_get_references to include type of ref in the %refs value, which
>> means putting everything after 'refs/' as a ref name, not only last
>> part of the name.  Instead of separating refs pointing to the same
>> object by " / " separator, use anonymous array reference to store all
>> refs pointing to given object.
>>
>> Use 'git-ls-remote .' if $projectroot/$project/info/refs does not
>> exist.  (Perhaps it should be used always.)
>>
>> Refs are now in separate span elements.  Class is dependent on the ref
>> type: currently known classes are 'tag', 'head', 'remote', and 'ref'
>> (last one for HEAD and other refs in the main directory).  There is
>> encompassing span element of class refs, just in case of unknown ref
>> type.
> 
> I do not see definition that matches "span.refs span.remote" in
> the CSS, though.

It get's default style "span.refs span" (and fefault colors).

>> +                    # e.g. tags/v2.6.11 or heads/next
>> +                    if ($ref =~ m!^(.*?)s?/(.*)$!) {
>> +                            $type = $1;
>> +                            $name = $2;
>> +                    } else {
>> +                            $type = "ref";
>> +                            $name = $ref;
>> +                    }
> 
> Hmph.  Maybe have a hash that defines the ones you know how to
> handle, and do something like:
> 
>         if ($ref =~ m|^([^/]+)/(.*)$| &&  exists $i_know_this_class{$1}) {
>               $type = $1;
>               $name = $2;
>       }
>         else {
>               $type = 'ref';
>               $name = $ref;
>       }

But what types are known is defined in CSS, not in gitweb.perl. The rule
is that type is defined by first component of the ref name, in singular form
(so with optional final s removed, refs -> ref, tags -> tag, remotes -> remote).
All unknown types get default "span.refs span" format, thanks to encompassing
span element.

"ref" class is for refs directly in $GIT_DIR, like HEAD or ORIG_HEAD (as of now
you cannot get such a ref due to the regexp used in git_get_references, and
the fact that you have no such refs in $GIT_DIR/info/refs file, and 
'git-ls-remote .' gives only HEAD; but we could add them 'by hand').

>> @@ -561,18 +578,24 @@ sub git_get_project_owner {
>>...
>> +            if ($line =~ m/^([0-9a-fA-F]{40})\trefs\/($type\/?[^\^]+)/) {
>>                      if (defined $refs{$1}) {
>> -                            $refs{$1} .= " / $2";
>> +                            push @{$refs{$1}}, $2;
>>                      } else {
>> -                            $refs{$1} = $2;
>> +                            $refs{$1} = [ $2 ];
>>                      }
>>              }
> 
> It would work as in your patch, but I would have preferred to
> see "exists $refs{$1}" there instead of "defined".
> 
> Or lose the if() and do it like this, which would be cleaner?
> 
>       if ($line =~ ...) {
>               push @{$refs{$1}}, $2;
>       }

That's even better.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

[PATCH 7/9] gitweb: Skip comments in mime.types like file

[Jakub Narebski]

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4fe3fc7..52ae2aa 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -858,6 +858,7 @@ sub mimetype_guess_file {
 	my %mimemap;
 	open(MIME, $mimemap) or return undef;
 	while (<MIME>) {
+		next if m/^#/; # skip comments
 		my ($mime, $exts) = split(/\t+/);
 		if (defined $exts) {
 			my @exts = split(/\s+/, $exts);
-- 
1.4.1.1

[PATCH 8/9] gitweb: True fix: Support for the standard mime.types map in gitweb

[Jakub Narebski]

True fix for error in mimetype_guess, error introduced in original commit
2d00737489b8c61ed616b261c7c9bd314e2b0b41 and later fixed temporarily
by commenting out the line that caused error in commit
57bd4d3523efecf60197040cad34154aff4ddf80.

Gitweb now supports mime.types map $mimetypes_file relative to project.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Perhaps commit message should consist only of:

gitweb: Gitweb now supports mime.types map $mimetypes_file relative to project.

 gitweb/gitweb.perl |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 52ae2aa..15875a8 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -880,7 +880,10 @@ sub mimetype_guess {
 
 	if ($mimetypes_file) {
 		my $file = $mimetypes_file;
-		#$file =~ m#^/# or $file = "$projectroot/$path/$file";
+		if ($file !~ m!^/!) { # if it is relative path
+			# it is relative to project
+			$file = "$projectroot/$project/$file";
+		}
 		$mime = mimetype_guess_file($filename, $file);
 	}
 	$mime ||= mimetype_guess_file($filename, '/etc/mime.types');
-- 
1.4.1.1

Re: [PATCH 8/9] gitweb: True fix: Support for the standard mime.types map in gitweb

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> True fix for error in mimetype_guess, error introduced in original commit
> 2d00737489b8c61ed616b261c7c9bd314e2b0b41 and later fixed temporarily
> by commenting out the line that caused error in commit
> 57bd4d3523efecf60197040cad34154aff4ddf80.
>
> Gitweb now supports mime.types map $mimetypes_file relative to project.

Care to explain what that commenting out was about?

>  	if ($mimetypes_file) {
>  		my $file = $mimetypes_file;
> -		#$file =~ m#^/# or $file = "$projectroot/$path/$file";
> +		if ($file !~ m!^/!) { # if it is relative path
> +			# it is relative to project
> +			$file = "$projectroot/$project/$file";
> +		}
>  		$mime = mimetype_guess_file($filename, $file);

Are $project and $file always defined when we take this new
codepath?

Re: [PATCH 8/9] gitweb: True fix: Support for the standard mime.types map in gitweb

[Jakub Narebski]

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> True fix for error in mimetype_guess, error introduced in original commit
>> 2d00737489b8c61ed616b261c7c9bd314e2b0b41 and later fixed temporarily
>> by commenting out the line that caused error in commit
>> 57bd4d3523efecf60197040cad34154aff4ddf80.
>>
>> Gitweb now supports mime.types map $mimetypes_file relative to project.
> 
> Care to explain what that commenting out was about?
> 
>>      if ($mimetypes_file) {
>>              my $file = $mimetypes_file;
>> -            #$file =~ m#^/# or $file = "$projectroot/$path/$file";
>> +            if ($file !~ m!^/!) { # if it is relative path
>> +                    # it is relative to project
>> +                    $file = "$projectroot/$project/$file";
>> +            }
>>              $mime = mimetype_guess_file($filename, $file);
> 
> Are $project and $file always defined when we take this new
> codepath?
 
$project is defined always in the "input validation and dispatch" part.

"my $file = $mimetypes_file;", and only if $mimetypes_file is true
(so it must be defined). $mimetypes_file is undef by default, and it
is one of the configuration variables which are not set from Makefile
in "make gitweb/gitweb.cgi".

$path was never defined; I guess that was copy'n'paste error, copying
from other subroutine that used $path as one of arguments.


I don't know why I didn't fix this error correctly, only patched it up
the first time...

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

[PATCH 9/9] gitweb: Separate printing difftree in git_commit into git_difftree_body

[Jakub Narebski]

Separate printing difftree in git_commit into separate
git_difftree_body subroutine. Add support for "C" (copied) status. For
"M" and "C" add parameter 'fp' (filename parent) to the "diff" link;
currently not supported by git_blobdiff ("blobdiff" action).

Reindented, realigned, added comments.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.css  |    4 +
 gitweb/gitweb.perl |  237 +++++++++++++++++++++++++++++++---------------------
 2 files changed, 146 insertions(+), 95 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 21ce99c..9013895 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -226,6 +226,10 @@ table.diff_tree span.file_status.mode_ch
 	color: #777777;
 }
 
+table.diff_tree span.file_status.copied {
+  color: #70a070;
+}
+
 /* age2: 60*60*24*2 <= age */
 table.project_list td.age2, table.blame td.age2 {
 	font-style: italic;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 15875a8..ab28caa 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1136,6 +1136,145 @@ sub git_print_page_path {
 ## ......................................................................
 ## functions printing large fragments of HTML
 
+sub git_difftree_body {
+	my ($difftree, $parent) = @_;
+
+	print "<div class=\"list_head\">\n";
+	if ($#{$difftree} > 10) {
+		print(($#{$difftree} + 1) . " files changed:\n");
+	}
+	print "</div>\n";
+
+	print "<table class=\"diff_tree\">\n";
+	my $alternate = 0;
+	foreach my $line (@{$difftree}) {
+		# ':100644 100644 03b218260e99b78c6df0ed378e59ed9205ccc96d 3b93d5e7cc7f7dd4ebed13a5cc1a4ad976fc94d8 M	ls-files.c'
+		# ':100644 100644 7f9281985086971d3877aca27704f2aaf9c448ce bc190ebc71bbd923f2b728e505408f5e54bd073a M	rev-tree.c'
+		if ($line !~ m/^:([0-7]{6}) ([0-7]{6}) ([0-9a-fA-F]{40}) ([0-9a-fA-F]{40}) (.)([0-9]{0,3})\t(.*)$/) {
+			next;
+		}
+		my $from_mode = $1;
+		my $to_mode = $2;
+		my $from_id = $3;
+		my $to_id = $4;
+		my $status = $5;
+		my $similarity = $6; # score
+		my $file = validate_input(unquote($7));
+
+		if ($alternate) {
+			print "<tr class=\"dark\">\n";
+		} else {
+			print "<tr class=\"light\">\n";
+		}
+		$alternate ^= 1;
+
+		if ($status eq "A") { # created
+			my $mode_chng = "";
+			if (S_ISREG(oct $to_mode)) {
+				$mode_chng = sprintf(" with mode: %04o", (oct $to_mode) & 0777);
+			}
+			print "<td>" .
+			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$to_id;hb=$hash;f=$file"),
+			              -class => "list"}, esc_html($file)) .
+			      "</td>\n" .
+			      "<td><span class=\"file_status new\">[new " . file_type($to_mode) . "$mode_chng]</span></td>\n" .
+			      "<td class=\"link\">" .
+			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$to_id;hb=$hash;f=$file")}, "blob") .
+			      "</td>\n";
+
+		} elsif ($status eq "D") { # deleted
+			print "<td>" .
+			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$from_id;hb=$parent;f=$file"),
+			               -class => "list"}, esc_html($file)) . "</td>\n" .
+			      "<td><span class=\"file_status deleted\">[deleted " . file_type($from_mode). "]</span></td>\n" .
+			      "<td class=\"link\">" .
+			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$from_id;hb=$parent;f=$file")}, "blob") . " | " .
+			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=history;hb=$parent;f=$file")}, "history") .
+			      "</td>\n"
+
+		} elsif ($status eq "M" || $status eq "T") { # modified, or type changed
+			my $mode_chnge = "";
+			if ($from_mode != $to_mode) {
+				$mode_chnge = " <span class=\"file_status mode_chnge\">[changed";
+				if (((oct $from_mode) & S_IFMT) != ((oct $to_mode) & S_IFMT)) {
+					$mode_chnge .= " from " . file_type($from_mode) . " to " . file_type($to_mode);
+				}
+				if (((oct $from_mode) & 0777) != ((oct $to_mode) & 0777)) {
+					if (S_ISREG($from_mode) && S_ISREG($to_mode)) {
+						$mode_chnge .= sprintf(" mode: %04o->%04o", (oct $from_mode) & 0777, (oct $to_mode) & 0777);
+					} elsif (S_ISREG($to_mode)) {
+						$mode_chnge .= sprintf(" mode: %04o", (oct $to_mode) & 0777);
+					}
+				}
+				$mode_chnge .= "]</span>\n";
+			}
+			print "<td>";
+			if ($to_id ne $from_id) { # modified
+				print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blobdiff;h=$to_id;hp=$from_id;hb=$hash;f=$file"),
+				              -class => "list"}, esc_html($file));
+			} else { # mode changed
+				print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$to_id;hb=$hash;f=$file"),
+				              -class => "list"}, esc_html($file));
+			}
+			print "</td>\n" .
+			      "<td>$mode_chnge</td>\n" .
+			      "<td class=\"link\">" .
+			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$to_id;hb=$hash;f=$file")}, "blob");
+			if ($to_id ne $from_id) { # modified
+				print " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blobdiff;h=$to_id;hp=$from_id;hb=$hash;f=$file")}, "diff");
+			}
+			print " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=history;hb=$hash;f=$file")}, "history") . "\n";
+			print "</td>\n";
+
+		} elsif ($status eq "R") { # renamed
+			my ($from_file, $to_file) = split "\t", $file;
+			my $mode_chng = "";
+			if ($from_mode != $to_mode) {
+				$mode_chng = sprintf(", mode: %04o", (oct $to_mode) & 0777);
+			}
+			print "<td>" .
+			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$to_id;hb=$hash;f=$to_file"),
+			              -class => "list"}, esc_html($to_file)) . "</td>\n" .
+			      "<td><span class=\"file_status moved\">[moved from " .
+			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$from_id;hb=$parent;f=$from_file"),
+			              -class => "list"}, esc_html($from_file)) .
+			      " with " . (int $similarity) . "% similarity$mode_chng]</span></td>\n" .
+			      "<td class=\"link\">" .
+			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$to_id;hb=$hash;f=$to_file")}, "blob");
+			if ($to_id ne $from_id) {
+				print " | " .
+				      $cgi->a({-href => "$my_uri?" .
+				              esc_param("p=$project;a=blobdiff;h=$to_id;hp=$from_id;hb=$hash;f=$to_file;fp=$from_file")}, "diff");
+			}
+			print "</td>\n";
+
+		} elsif ($status eq "C") { # copied
+			my ($from_file, $to_file) = split "\t", $file;
+			my $mode_chng = "";
+			if ($from_mode != $to_mode) {
+				$mode_chng = sprintf(", mode: %04o", (oct $to_mode) & 0777);
+			}
+			print "<td>" .
+			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$to_id;hb=$hash;f=$to_file"),
+			              -class => "list"}, esc_html($to_file)) . "</td>\n" .
+			      "<td><span class=\"file_status copied\">[copied from " .
+			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$from_id;hb=$parent;f=$from_file"),
+			              -class => "list"}, esc_html($from_file)) .
+			      " with " . (int $similarity) . "% similarity$mode_chng]</span></td>\n" .
+			      "<td class=\"link\">" .
+			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$to_id;hb=$hash;f=$to_file")}, "blob");
+			if ($to_id ne $from_id) {
+				print " | " .
+				      $cgi->a({-href => "$my_uri?" .
+				              esc_param("p=$project;a=blobdiff;h=$to_id;hp=$from_id;hb=$hash;f=$to_file;fp=$from_file")}, "diff");
+			}
+			print "</td>\n";
+		} # we should not encounter Unmerged (U) or Unknown (X) status
+		print "</tr>\n";
+	}
+	print "</table>\n";
+}
+
 sub git_shortlog_body {
 	# uses global variable $project
 	my ($revlist, $from, $to, $refs, $extra) = @_;
@@ -2089,101 +2228,9 @@ sub git_commit {
 		}
 	}
 	print "</div>\n";
-	print "<div class=\"list_head\">\n";
-	if ($#difftree > 10) {
-		print(($#difftree + 1) . " files changed:\n");
-	}
-	print "</div>\n";
-	print "<table class=\"diff_tree\">\n";
-	my $alternate = 0;
-	foreach my $line (@difftree) {
-		# ':100644 100644 03b218260e99b78c6df0ed378e59ed9205ccc96d 3b93d5e7cc7f7dd4ebed13a5cc1a4ad976fc94d8 M      ls-files.c'
-		# ':100644 100644 7f9281985086971d3877aca27704f2aaf9c448ce bc190ebc71bbd923f2b728e505408f5e54bd073a M      rev-tree.c'
-		if ($line !~ m/^:([0-7]{6}) ([0-7]{6}) ([0-9a-fA-F]{40}) ([0-9a-fA-F]{40}) (.)([0-9]{0,3})\t(.*)$/) {
-			next;
-		}
-		my $from_mode = $1;
-		my $to_mode = $2;
-		my $from_id = $3;
-		my $to_id = $4;
-		my $status = $5;
-		my $similarity = $6;
-		my $file = validate_input(unquote($7));
-		if ($alternate) {
-			print "<tr class=\"dark\">\n";
-		} else {
-			print "<tr class=\"light\">\n";
-		}
-		$alternate ^= 1;
-		if ($status eq "A") {
-			my $mode_chng = "";
-			if (S_ISREG(oct $to_mode)) {
-				$mode_chng = sprintf(" with mode: %04o", (oct $to_mode) & 0777);
-			}
-			print "<td>" .
-			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$to_id;hb=$hash;f=$file"), -class => "list"}, esc_html($file)) . "</td>\n" .
-			      "<td><span class=\"file_status new\">[new " . file_type($to_mode) . "$mode_chng]</span></td>\n" .
-			      "<td class=\"link\">" . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$to_id;hb=$hash;f=$file")}, "blob") . "</td>\n";
-		} elsif ($status eq "D") {
-			print "<td>" .
-			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$from_id;hb=$parent;f=$file"), -class => "list"}, esc_html($file)) . "</td>\n" .
-			      "<td><span class=\"file_status deleted\">[deleted " . file_type($from_mode). "]</span></td>\n" .
-			      "<td class=\"link\">" .
-			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$from_id;hb=$parent;f=$file")}, "blob") .
-			      " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=history;hb=$parent;f=$file")}, "history") .
-			      "</td>\n"
-		} elsif ($status eq "M" || $status eq "T") {
-			my $mode_chnge = "";
-			if ($from_mode != $to_mode) {
-				$mode_chnge = " <span class=\"file_status mode_chnge\">[changed";
-				if (((oct $from_mode) & S_IFMT) != ((oct $to_mode) & S_IFMT)) {
-					$mode_chnge .= " from " . file_type($from_mode) . " to " . file_type($to_mode);
-				}
-				if (((oct $from_mode) & 0777) != ((oct $to_mode) & 0777)) {
-					if (S_ISREG($from_mode) && S_ISREG($to_mode)) {
-						$mode_chnge .= sprintf(" mode: %04o->%04o", (oct $from_mode) & 0777, (oct $to_mode) & 0777);
-					} elsif (S_ISREG($to_mode)) {
-						$mode_chnge .= sprintf(" mode: %04o", (oct $to_mode) & 0777);
-					}
-				}
-				$mode_chnge .= "]</span>\n";
-			}
-			print "<td>";
-			if ($to_id ne $from_id) {
-				print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blobdiff;h=$to_id;hp=$from_id;hb=$hash;f=$file"), -class => "list"}, esc_html($file));
-			} else {
-				print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$to_id;hb=$hash;f=$file"), -class => "list"}, esc_html($file));
-			}
-			print "</td>\n" .
-			      "<td>$mode_chnge</td>\n" .
-			      "<td class=\"link\">";
-			print $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$to_id;hb=$hash;f=$file")}, "blob");
-			if ($to_id ne $from_id) {
-				print " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blobdiff;h=$to_id;hp=$from_id;hb=$hash;f=$file")}, "diff");
-			}
-			print " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=history;hb=$hash;f=$file")}, "history") . "\n";
-			print "</td>\n";
-		} elsif ($status eq "R") {
-			my ($from_file, $to_file) = split "\t", $file;
-			my $mode_chng = "";
-			if ($from_mode != $to_mode) {
-				$mode_chng = sprintf(", mode: %04o", (oct $to_mode) & 0777);
-			}
-			print "<td>" .
-			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$to_id;hb=$hash;f=$to_file"), -class => "list"}, esc_html($to_file)) . "</td>\n" .
-			      "<td><span class=\"file_status moved\">[moved from " .
-			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$from_id;hb=$parent;f=$from_file"), -class => "list"}, esc_html($from_file)) .
-			      " with " . (int $similarity) . "% similarity$mode_chng]</span></td>\n" .
-			      "<td class=\"link\">" .
-			      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$to_id;hb=$hash;f=$to_file")}, "blob");
-			if ($to_id ne $from_id) {
-				print " | " . $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blobdiff;h=$to_id;hp=$from_id;hb=$hash;f=$to_file")}, "diff");
-			}
-			print "</td>\n";
-		}
-		print "</tr>\n";
-	}
-	print "</table>\n";
+
+	git_difftree_body(\@difftree, $parent);
+
 	git_footer_html();
 }
 
-- 
1.4.1.1

Re: [PATCH 9/9] gitweb: Separate printing difftree in git_commit into git_difftree_body

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> +		if ($status eq "A") { # created
>...
> +		} elsif ($status eq "D") { # deleted
>...
> +		} elsif ($status eq "M" || $status eq "T") { # modified, or type changed
>...
> +		} elsif ($status eq "R") { # renamed
>...
> +		} elsif ($status eq "C") { # copied
>...
> +		} # we should not encounter Unmerged (U) or Unknown (X) status

The body of these still seem to have quite a lot of
similarities.  Maybe further refactoring is in the works?

Re: [PATCH 9/9] gitweb: Separate printing difftree in git_commit into git_difftree_body

[Jakub Narebski]

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> +            if ($status eq "A") { # created
>>...
>> +            } elsif ($status eq "D") { # deleted
>>...
>> +            } elsif ($status eq "M" || $status eq "T") { # modified, or type changed
>>...
>> +            } elsif ($status eq "R") { # renamed
>>...
>> +            } elsif ($status eq "C") { # copied
>>...
>> +            } # we should not encounter Unmerged (U) or Unknown (X) status
> 
> The body of these still seem to have quite a lot of
> similarities.  Maybe further refactoring is in the works?

Probably. For now I only separated the difftree printing part into separate 
subroutine, and added $status eq "C" case (I don't know if we might encounter
it at all, due to current set of options to diff-tree).

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH 0/9] gitweb: Great subroutine renaming + task separation into subroutines + improvements

[Jakub Narebski]

Jakub Narebski wrote:

> This series of patches is based on the 'next' branch, commit
> 10a6653c818f78c6032d45e1d4da544085e1f28b (v1.4.2-g10a6653)

But it rebases without problem on top of current master
  460cccd3ba1d38fd64c9e83e40d58bcf3e9d7d2c (v1.4.2-g460cccd)
('git rebase --onto master next' rebases without errors)

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH 0/9] gitweb: Great subroutine renaming + task separation into subroutines + improvements

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Jakub Narebski wrote:
>
>> This series of patches is based on the 'next' branch, commit
>> 10a6653c818f78c6032d45e1d4da544085e1f28b (v1.4.2-g10a6653)
>
> But it rebases without problem on top of current master
>   460cccd3ba1d38fd64c9e83e40d58bcf3e9d7d2c (v1.4.2-g460cccd)
> ('git rebase --onto master next' rebases without errors)

I sent a few comments, some suggesting further improvements and
others expressing my puzzlement, but I'll be applying all of
them, so please base your further work on all of the 9 series.