git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Some further gitweb patches
@ 2006-08-04 22:36 Jakub Narebski
  2006-08-04 22:38 ` [PATCH 1/5] gitweb: Cleanup input validation and error messages Jakub Narebski
                   ` (5 more replies)
  0 siblings, 6 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-04 22:36 UTC (permalink / raw)
  To: git

Based on 'next' branch, v1.4.2-rc3-g1c4b267

This is also kind of cleanup series of patches, although it introduces 
minor improvements (patches 1 and 5).

This series does not include latest Luben Tuikov series...
-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/5] gitweb: Cleanup input validation and error messages
  2006-08-04 22:36 [PATCH 0/5] Some further gitweb patches Jakub Narebski
@ 2006-08-04 22:38 ` Jakub Narebski
  2006-08-04 23:54   ` Luben Tuikov
                     ` (2 more replies)
  2006-08-04 22:39 ` [PATCH 2/5] gitweb: Great subroutines renaming Jakub Narebski
                   ` (4 subsequent siblings)
  5 siblings, 3 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-04 22:38 UTC (permalink / raw)
  To: git

Clean up input validation, including removing $rss_link variable and
making error messages more explicit.  Expand and uniquify other error
messages.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This probably conflicts "[PATCH 4/4] gitweb: No periods for error messages".
It uses periods for error messages which does not end in with some 
value of some variable.

 gitweb/gitweb.perl |   88 ++++++++++++++++++++++++----------------------------
 1 files changed, 40 insertions(+), 48 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 58eb5b1..dfc2d09 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -71,13 +71,15 @@ if (! -d $git_temp) {
 	mkdir($git_temp, 0700) || die_error("Couldn't mkdir $git_temp");
 }
 
+
+# ======================================================================
 # input validation and dispatch
 our $action = $cgi->param('a');
 if (defined $action) {
 	if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
-		undef $action;
-		die_error(undef, "Invalid action parameter.");
+		die_error(undef, "Invalid action parameter $action");
 	}
+	# action which does not check rest of parameters
 	if ($action eq "opml") {
 		git_opml();
 		exit;
@@ -85,22 +87,17 @@ if (defined $action) {
 }
 
 our $project = ($cgi->param('p') || $ENV{'PATH_INFO'});
-if (defined $project) {
-	$project =~ s|^/||; $project =~ s|/$||;
-	$project = validate_input($project);
-	if (!defined($project)) {
-		die_error(undef, "Invalid project parameter.");
+$project =~ s|^/||; $project =~ s|/$||;
+if (defined $project || $project) {
+	if (!validate_input($project)) {
+		die_error(undef, "Invalid project parameter $project");
 	}
 	if (!(-d "$projectroot/$project")) {
-		undef $project;
-		die_error(undef, "No such directory.");
+		die_error(undef, "No such directory $project");
 	}
 	if (!(-e "$projectroot/$project/HEAD")) {
-		undef $project;
-		die_error(undef, "No such project.");
+		die_error(undef, "No such project $project");
 	}
-	$rss_link = "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\"" .
-	            "$my_uri?" . esc_param("p=$project;a=rss") . "\" type=\"application/rss+xml\"/>";
 	$ENV{'GIT_DIR'} = "$projectroot/$project";
 } else {
 	git_project_list();
@@ -109,49 +106,43 @@ if (defined $project) {
 
 our $file_name = $cgi->param('f');
 if (defined $file_name) {
-	$file_name = validate_input($file_name);
-	if (!defined($file_name)) {
-		die_error(undef, "Invalid file parameter.");
+	if (!validate_input($file_name)) {
+		die_error(undef, "Invalid file parameter $file_name");
 	}
 }
 
 our $hash = $cgi->param('h');
 if (defined $hash) {
-	$hash = validate_input($hash);
-	if (!defined($hash)) {
-		die_error(undef, "Invalid hash parameter.");
+	if (!validate_input($hash)) {
+		die_error(undef, "Invalid hash parameter $hash");
 	}
 }
 
 our $hash_parent = $cgi->param('hp');
 if (defined $hash_parent) {
-	$hash_parent = validate_input($hash_parent);
-	if (!defined($hash_parent)) {
-		die_error(undef, "Invalid hash parent parameter.");
+	if (!validate_input($hash_parent)) {
+		die_error(undef, "Invalid hash parent parameter $hash_parent");
 	}
 }
 
 our $hash_base = $cgi->param('hb');
 if (defined $hash_base) {
-	$hash_base = validate_input($hash_base);
-	if (!defined($hash_base)) {
-		die_error(undef, "Invalid hash base parameter.");
+	if (!validate_input($hash_base)) {
+		die_error(undef, "Invalid hash base parameter $hash_base");
 	}
 }
 
 our $page = $cgi->param('pg');
 if (defined $page) {
 	if ($page =~ m/[^0-9]$/) {
-		undef $page;
-		die_error(undef, "Invalid page parameter.");
+		die_error(undef, "Invalid page parameter $page");
 	}
 }
 
 our $searchtext = $cgi->param('s');
 if (defined $searchtext) {
 	if ($searchtext =~ m/[^a-zA-Z0-9_\.\/\-\+\:\@ ]/) {
-		undef $searchtext;
-		die_error(undef, "Invalid search parameter.");
+		die_error(undef, "Invalid search parameter $searchtext");
 	}
 	$searchtext = quotemeta $searchtext;
 }
@@ -180,8 +171,7 @@ my %actions = (
 
 $action = 'summary' if (!defined($action));
 if (!defined($actions{$action})) {
-	undef $action;
-	die_error(undef, "Unknown action.");
+	die_error(undef, "Unknown action $action");
 }
 $actions{$action}->();
 exit;
@@ -871,11 +861,13 @@ sub git_header_html {
 <meta name="robots" content="index, nofollow"/>
 <title>$title</title>
 <link rel="stylesheet" type="text/css" href="$stylesheet"/>
-$rss_link
-</head>
-<body>
 EOF
-	print "<div class=\"page_header\">\n" .
+	print "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\"" .
+	      "$my_uri?" . esc_param("p=$project;a=rss") . "\" type=\"application/rss+xml\"/>\n" .
+	      "</head>\n";
+
+	print "<body>\n" .
+	      "<div class=\"page_header\">\n" .
 	      "<a href=\"http://www.kernel.org/pub/software/scm/git/docs/\" title=\"git documentation\">" .
 	      "<img src=\"$logo\" width=\"72\" height=\"27\" alt=\"git\" style=\"float:right; border-width:0px;\"/>" .
 	      "</a>\n";
@@ -1471,18 +1463,18 @@ sub git_blame2 {
 	my $fd;
 	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);
+	die_error('404 Not Found', "File name not defined.") if (!$file_name);
 	$hash_base ||= git_read_head($project);
-	die_error(undef, "Reading commit failed") unless ($hash_base);
+	die_error(undef, "Couldn't find base commit.") unless ($hash_base);
 	my %co = git_read_commit($hash_base)
-		or die_error(undef, "Reading commit failed");
+		or die_error(undef, "Reading commit failed.");
 	if (!defined $hash) {
 		$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
-			or die_error(undef, "Error looking up file");
+			or die_error(undef, "Error looking up file $file_name");
 	}
 	$ftype = git_get_type($hash);
 	if ($ftype !~ "blob") {
-		die_error("400 Bad Request", "object is not a blob");
+		die_error("400 Bad Request", "Object is not a blob.");
 	}
 	open ($fd, "-|", $GIT, "blame", '-l', $file_name, $hash_base)
 		or die_error(undef, "Open git-blame failed.");
@@ -1529,14 +1521,14 @@ sub git_blame2 {
 sub git_blame {
 	my $fd;
 	die_error('403 Permission denied', "Permission denied.") if (!git_get_project_config_bool ('blame'));
-	die_error('404 Not Found', "What file will it be, master?") if (!$file_name);
+	die_error('404 Not Found', "File name not defined.") if (!$file_name);
 	$hash_base ||= git_read_head($project);
-	die_error(undef, "Reading commit failed.") unless ($hash_base);
+	die_error(undef, "Couldn't find base commit.") unless ($hash_base);
 	my %co = git_read_commit($hash_base)
 		or die_error(undef, "Reading commit failed.");
 	if (!defined $hash) {
 		$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
-			or die_error(undef, "Error lookup file.");
+			or die_error(undef, "Error lookup file $file_name");
 	}
 	open ($fd, "-|", $GIT, "annotate", '-l', '-t', '-r', $file_name, $hash_base)
 		or die_error(undef, "Open git-annotate failed.");
@@ -1649,7 +1641,7 @@ sub git_blob_plain {
 		if (defined $file_name) {
 			my $base = $hash_base || git_read_head($project);
 			$hash = git_get_hash_by_path($base, $file_name, "blob")
-				or die_error(undef, "Error lookup file.");
+				or die_error(undef, "Error lookup file $file_name");
 		} else {
 			die_error(undef, "No file name defined.");
 		}
@@ -1682,7 +1674,7 @@ sub git_blob {
 		if (defined $file_name) {
 			my $base = $hash_base || git_read_head($project);
 			$hash = git_get_hash_by_path($base, $file_name, "blob")
-				or die_error(undef, "Error lookup file.");
+				or die_error(undef, "Error lookup file $file_name");
 		} else {
 			die_error(undef, "No file name defined.");
 		}
@@ -2122,7 +2114,7 @@ sub git_commitdiff {
 	open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_parent, $hash
 		or die_error(undef, "Open git-diff-tree failed.");
 	my @difftree = map { chomp; $_ } <$fd>;
-	close $fd or die_error(undef, "Reading diff-tree failed.");
+	close $fd or die_error(undef, "Reading git-diff-tree failed.");
 
 	# non-textual hash id's can be cached
 	my $expires;
@@ -2202,7 +2194,7 @@ sub git_commitdiff_plain {
 	open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_parent, $hash
 		or die_error(undef, "Open git-diff-tree failed.");
 	my @difftree = map { chomp; $_ } <$fd>;
-	close $fd or die_error(undef, "Reading diff-tree failed.");
+	close $fd or die_error(undef, "Reading git-diff-tree failed.");
 
 	# try to figure out the next tag after this commit
 	my $tagname;
@@ -2493,7 +2485,7 @@ sub git_rss {
 	open my $fd, "-|", $GIT, "rev-list", "--max-count=150", git_read_head($project)
 		or die_error(undef, "Open git-rev-list failed.");
 	my @revlist = map { chomp; $_ } <$fd>;
-	close $fd or die_error(undef, "Reading rev-list failed.");
+	close $fd or die_error(undef, "Reading git-rev-list failed.");
 	print $cgi->header(-type => 'text/xml', -charset => 'utf-8');
 	print "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n".
 	      "<rss version=\"2.0\" xmlns:content=\"http://purl.org/rss/1.0/modules/content/\">\n";
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/5] gitweb: Great subroutines renaming
  2006-08-04 22:36 [PATCH 0/5] Some further gitweb patches Jakub Narebski
  2006-08-04 22:38 ` [PATCH 1/5] gitweb: Cleanup input validation and error messages Jakub Narebski
@ 2006-08-04 22:39 ` Jakub Narebski
  2006-08-04 22:40 ` [PATCH 3/5] gitweb: Separate ref parsing in git_read_refs into parse_ref Jakub Narebski
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-04 22:39 UTC (permalink / raw)
  To: git

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,
  or to gitweb actions
* git_get_ prefix for subroutines calling git command
  and returning some output
* git_read_ prefix for subroutines directly accessing git
  repository files
* parse_ prefix for subroutines parsing some text, or reading and
  parsing some text into hash or list
* format_ prefix for subroutines formatting/post-processing 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
* _nav suffix for subroutines related to navigation bars
* _div suffix for subroutines returning or ptinting div element

Renames:
- git_get_referencing => format_mark_referencing
- git_read_head => git_get_head
- read_info_ref => git_read_info_refs
- date_str => parse_date
- git_read_tag => parse_tag
- git_read_commit => parse_commit
- git_blob_plain_mimetype => blob_plain_mimetype
- git_page_nav => git_print_page_nav
- git_header_div => git_print_header_div

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
I don't follow my own guidelines, not completely.
I should probably post this patch earlier.

 gitweb/gitweb.perl |  232 ++++++++++++++++++++++++++--------------------------
 1 files changed, 117 insertions(+), 115 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index dfc2d09..d440546 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -21,7 +21,6 @@ our $cgi = new CGI;
 our $version = "++GIT_VERSION++";
 our $my_url = $cgi->url();
 our $my_uri = $cgi->url(-absolute => 1);
-our $rss_link = "";
 
 # core git executable to use
 # this can just be "git" if your webserver has a sensible PATH
@@ -347,7 +346,7 @@ sub format_log_line_html {
 }
 
 # format marker of refs pointing to given object
-sub git_get_referencing {
+sub format_mark_referencing {
 	my ($refs, $id) = @_;
 
 	if (defined $refs->{$id}) {
@@ -361,7 +360,7 @@ ## -------------------------------------
 ## git utility subroutines, invoking git commands
 
 # get HEAD ref of given project as hash
-sub git_read_head {
+sub git_get_head {
 	my $project = shift;
 	my $oENV = $ENV{'GIT_DIR'};
 	my $retval = undef;
@@ -496,7 +495,7 @@ sub git_read_projects {
 	return @list;
 }
 
-sub read_info_ref {
+sub git_read_info_refs {
 	my $type = shift || "";
 	my %refs;
 	# 5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c	refs/tags/v2.6.11
@@ -521,7 +520,8 @@ sub read_info_ref {
 ## ----------------------------------------------------------------------
 ## parse to hash functions
 
-sub date_str {
+# parse date from epoch and timezone
+sub parse_date {
 	my $epoch = shift;
 	my $tz = shift || "-0000";
 
@@ -546,7 +546,8 @@ sub date_str {
 	return %date;
 }
 
-sub git_read_tag {
+# parse tag given by tag_id
+sub parse_tag {
 	my $tag_id = shift;
 	my %tag;
 	my @comment;
@@ -581,7 +582,8 @@ sub git_read_tag {
 	return %tag
 }
 
-sub git_read_commit {
+# parse commit given by commit_id, with optionally given @commit_lines
+sub parse_commit {
 	my $commit_id = shift;
 	my $commit_text = shift;
 
@@ -701,10 +703,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'})) {
@@ -716,7 +718,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'};
@@ -791,7 +793,7 @@ sub mimetype_guess {
 	return $mime;
 }
 
-sub git_blob_plain_mimetype {
+sub blob_plain_mimetype {
 	my $fd = shift;
 	my $filename = shift;
 
@@ -936,7 +938,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
 
@@ -1003,7 +1005,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 = '';
 
@@ -1043,10 +1045,10 @@ 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 %ad = date_str($co{'author_epoch'});
+		#my $ref = defined $refs ? format_mark_referencing($refs, $commit) : '';
+		my $ref = format_mark_referencing($refs, $commit);
+		my %co = parse_commit($commit);
+		my %ad = parse_date($co{'author_epoch'});
 		if ($alternate) {
 			print "<tr class=\"dark\">\n";
 		} else {
@@ -1275,12 +1277,12 @@ sub git_project_list {
 		die_error(undef, "No projects found.");
 	}
 	foreach my $pr (@list) {
-		my $head = git_read_head($pr->{'path'});
+		my $head = git_get_head($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;
 		}
@@ -1371,9 +1373,9 @@ 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 $head = git_get_head($project);
+	my %co = parse_commit($head);
+	my %cd = parse_date($co{'committer_epoch'}, $co{'committer_tz'});
 
 	my $owner;
 	if (-f $projects_list) {
@@ -1394,9 +1396,9 @@ sub git_summary {
 		$owner = get_file_owner("$projectroot/$project");
 	}
 
-	my $refs = read_info_ref();
+	my $refs = git_read_info_refs();
 	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" .
@@ -1405,24 +1407,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($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");
 	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");
 	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")}, "..."));
 	}
@@ -1431,11 +1433,11 @@ sub git_summary {
 }
 
 sub git_tag {
-	my $head = git_read_head($project);
+	my $head = git_get_head($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" .
@@ -1444,7 +1446,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";
 	}
@@ -1464,9 +1466,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($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")
@@ -1482,8 +1484,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(light dark));
 	my $num_colors = scalar(@rev_color);
@@ -1522,9 +1524,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($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")
@@ -1536,8 +1538,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;
@@ -1610,10 +1612,10 @@ HTML
 }
 
 sub git_tags {
-	my $head = git_read_head($project);
+	my $head = git_get_head($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");
 	if (defined @$taglist) {
@@ -1623,10 +1625,10 @@ sub git_tags {
 }
 
 sub git_heads {
-	my $head = git_read_head($project);
+	my $head = git_get_head($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 $alternate = 0;
@@ -1639,7 +1641,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($project);
 			$hash = git_get_hash_by_path($base, $file_name, "blob")
 				or die_error(undef, "Error lookup file $file_name");
 		} else {
@@ -1650,7 +1652,7 @@ sub git_blob_plain {
 	open my $fd, "-|", $GIT, "cat-file", "blob", $hash
 		or die_error("Couldn't cat $file_name, $hash");
 
-	$type ||= git_blob_plain_mimetype($fd, $file_name);
+	$type ||= blob_plain_mimetype($fd, $file_name);
 
 	# save as filename, even when no $file_name is given
 	my $save_as = "$hash";
@@ -1672,7 +1674,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($project);
 			$hash = git_get_hash_by_path($base, $file_name, "blob")
 				or die_error(undef, "Error lookup file $file_name");
 		} else {
@@ -1682,14 +1684,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_plain_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") . " | ";
@@ -1700,8 +1702,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" .
@@ -1728,7 +1730,7 @@ sub git_blob {
 
 sub git_tree {
 	if (!defined $hash) {
-		$hash = git_read_head($project);
+		$hash = git_get_head($project);
 		if (defined $file_name) {
 			my $base = $hash_base || $hash;
 			$hash = git_get_hash_by_path($base, $file_name, "tree");
@@ -1744,15 +1746,15 @@ 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_read_info_refs();
+	my $ref = format_mark_referencing($refs, $hash_base);
 	git_header_html();
 	my $base_key = "";
 	my $base = "";
-	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";
@@ -1806,14 +1808,14 @@ #			      " | " . $cgi->a({-href => "$my
 }
 
 sub git_log {
-	my $head = git_read_head($project);
+	my $head = git_get_head($project);
 	if (!defined $hash) {
 		$hash = $head;
 	}
 	if (!defined $page) {
 		$page = 0;
 	}
-	my $refs = read_info_ref();
+	my $refs = git_read_info_refs();
 
 	my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
 	open my $fd, "-|", $GIT, "rev-list", $limit, $hash
@@ -1824,24 +1826,24 @@ sub git_log {
 	my $paging_nav = git_get_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_mark_referencing($refs, $commit);
+		my %co = parse_commit($commit);
 		next if !%co;
-		my %ad = date_str($co{'author_epoch'});
-		git_header_div('commit',
-									 "<span class=\"age\">$co{'age_string'}</span>" .
-									 esc_html($co{'title'}) . $ref,
-									 $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);
 		print "<div class=\"title_text\">\n" .
 		      "<div class=\"log_link\">\n" .
 		      $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=commit;h=$commit")}, "commit") .
@@ -1876,12 +1878,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) {
@@ -1897,22 +1899,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_read_info_refs();
+	my $ref = format_mark_referencing($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',
-							 $hash, $co{'tree'}, $hash,
-							 $formats_nav);
+	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";
@@ -2074,11 +2076,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" .
@@ -2104,7 +2106,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.");
 	}
@@ -2121,13 +2123,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_read_info_refs();
+	my $ref = format_mark_referencing($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;
@@ -2198,7 +2200,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_read_info_refs("tags");
 	open $fd, "-|", $GIT, "rev-list", "HEAD";
 	my @commits = map { chomp; $_ } <$fd>;
 	close $fd;
@@ -2212,8 +2214,8 @@ sub git_commitdiff_plain {
 	}
 
 	print $cgi->header(-type => "text/plain", -charset => 'utf-8', '-content-disposition' => "inline; filename=\"git-$hash.patch\"");
-	my %co = git_read_commit($hash);
-	my %ad = date_str($co{'author_epoch'}, $co{'author_tz'});
+	my %co = parse_commit($hash);
+	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".
@@ -2247,17 +2249,17 @@ sub git_commitdiff_plain {
 
 sub git_history {
 	if (!defined $hash_base) {
-		$hash_base = git_read_head($project);
+		$hash_base = git_get_head($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_read_info_refs();
 	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);
 	}
@@ -2273,11 +2275,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_mark_referencing($refs, $commit);
 			if ($alternate) {
 				print "<tr class=\"dark\">\n";
 			} else {
@@ -2313,9 +2315,9 @@ sub git_search {
 		die_error("", "Text field empty.");
 	}
 	if (!defined $hash) {
-		$hash = git_read_head($project);
+		$hash = git_get_head($project);
 	}
-	my %co = git_read_commit($hash);
+	my %co = parse_commit($hash);
 	if (!%co) {
 		die_error(undef, "Unknown commit object.");
 	}
@@ -2334,8 +2336,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;
@@ -2353,7 +2355,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;
 			}
@@ -2434,7 +2436,7 @@ sub git_search {
 					print "</td>\n" .
 					      "</tr>\n";
 				}
-				%co = git_read_commit($1);
+				%co = parse_commit($1);
 			}
 		}
 		close $fd;
@@ -2444,14 +2446,14 @@ sub git_search {
 }
 
 sub git_shortlog {
-	my $head = git_read_head($project);
+	my $head = git_get_head($project);
 	if (!defined $hash) {
 		$hash = $head;
 	}
 	if (!defined $page) {
 		$page = 0;
 	}
-	my $refs = read_info_ref();
+	my $refs = git_read_info_refs();
 
 	my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
 	open my $fd, "-|", $GIT, "rev-list", $limit, $hash
@@ -2469,8 +2471,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);
 
@@ -2482,7 +2484,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($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.");
@@ -2497,12 +2499,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;
@@ -2552,12 +2554,12 @@ sub git_opml {
 
 	foreach my $pr (@list) {
 		my %proj = %$pr;
-		my $head = git_read_head($proj{'path'});
+		my $head = git_get_head($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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/5] gitweb: Separate ref parsing in git_read_refs into parse_ref
  2006-08-04 22:36 [PATCH 0/5] Some further gitweb patches Jakub Narebski
  2006-08-04 22:38 ` [PATCH 1/5] gitweb: Cleanup input validation and error messages Jakub Narebski
  2006-08-04 22:39 ` [PATCH 2/5] gitweb: Great subroutines renaming Jakub Narebski
@ 2006-08-04 22:40 ` Jakub Narebski
  2006-08-04 22:42 ` [PATCH 4/5] gitweb: git_heads cleanup Jakub Narebski
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-04 22:40 UTC (permalink / raw)
  To: git

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Pure coding style patch.

 gitweb/gitweb.perl |   80 +++++++++++++++++++++++++++++-----------------------
 1 files changed, 45 insertions(+), 35 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index d440546..5b30654 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -677,6 +677,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
 
@@ -696,44 +739,11 @@ sub git_read_refs {
 	foreach my $ref_file (@refs) {
 		my $ref_id = git_read_hash("$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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 4/5] gitweb: git_heads cleanup
  2006-08-04 22:36 [PATCH 0/5] Some further gitweb patches Jakub Narebski
                   ` (2 preceding siblings ...)
  2006-08-04 22:40 ` [PATCH 3/5] gitweb: Separate ref parsing in git_read_refs into parse_ref Jakub Narebski
@ 2006-08-04 22:42 ` Jakub Narebski
  2006-08-04 22:43 ` [PATCH 5/5] gitweb: Change appereance of marker of refs pointing to given object Jakub Narebski
  2006-08-05 11:42 ` [PATCH 7/5] Merge changes in "split patch 1" series Jakub Narebski
  5 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-04 22:42 UTC (permalink / raw)
  To: git

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Leftover from refactor generation of shortlog, tags and heads body.

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 5b30654..9e4822b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1641,7 +1641,6 @@ sub git_heads {
 	git_print_header_div('summary', $project);
 
 	my $taglist = git_read_refs("refs/heads");
-	my $alternate = 0;
 	if (defined @$taglist) {
 		git_heads_body($taglist, $head);
 	}
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 5/5] gitweb: Change appereance of marker of refs pointing to given object
  2006-08-04 22:36 [PATCH 0/5] Some further gitweb patches Jakub Narebski
                   ` (3 preceding siblings ...)
  2006-08-04 22:42 ` [PATCH 4/5] gitweb: git_heads cleanup Jakub Narebski
@ 2006-08-04 22:43 ` Jakub Narebski
  2006-08-05 11:42 ` [PATCH 7/5] Merge changes in "split patch 1" series Jakub Narebski
  5 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-04 22:43 UTC (permalink / raw)
  To: git

Change default value of $type in git_read_info_refs to correctly show
full name of ref (and not only last part) if $type was not set.

Refs are now in separate span elements.  Class was changed from "tag"
to "ref", as ref can be head, not only tag.  Currently there is no way
to distinguish between tags and heads.

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

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 460e728..921b339 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -317,7 +317,7 @@ a.rss_logo:hover {
 	background-color: #ee5500;
 }
 
-span.tag {
+span.ref {
 	padding: 0px 4px;
 	font-size: 10px;
 	font-weight: normal;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 9e4822b..e983452 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -350,9 +350,13 @@ sub format_mark_referencing {
 	my ($refs, $id) = @_;
 
 	if (defined $refs->{$id}) {
-		return ' <span class="tag">' . esc_html($refs->{$id}) . '</span>';
+		my $markers = '';
+		foreach my $ref (split /:/, $refs->{$id}) {
+			$markers .= ' <span class="ref">' . esc_html($ref) . '</span>';
+		}
+		return $markers;
 	} else {
-		return "";
+		return '';
 	}
 }
 
@@ -496,7 +500,7 @@ sub git_read_projects {
 }
 
 sub git_read_info_refs {
-	my $type = shift || "";
+	my $type = shift || "(?:heads|tags)";
 	my %refs;
 	# 5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c	refs/tags/v2.6.11
 	# c39ae07f393806ccf406ef966e9a15afc43cc36a	refs/tags/v2.6.11^{}
@@ -507,7 +511,7 @@ sub git_read_info_refs {
 		# e.g. from 'refs/heads/jn/gitweb' it would leave only 'gitweb'
 		if ($line =~ m/^([0-9a-fA-F]{40})\t.*$type\/([^\^]+)/) {
 			if (defined $refs{$1}) {
-				$refs{$1} .= " / $2";
+				$refs{$1} .= ':' . $2;
 			} else {
 				$refs{$1} = $2;
 			}
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/5] gitweb: Cleanup input validation and error messages
  2006-08-04 22:38 ` [PATCH 1/5] gitweb: Cleanup input validation and error messages Jakub Narebski
@ 2006-08-04 23:54   ` Luben Tuikov
  2006-08-05  0:02     ` [PATCH 6/5] gitweb: No periods for " Jakub Narebski
  2006-08-04 23:54   ` [PATCH 1/5] gitweb: Cleanup input validation and " Luben Tuikov
  2006-08-05  0:15   ` Junio C Hamano
  2 siblings, 1 reply; 26+ messages in thread
From: Luben Tuikov @ 2006-08-04 23:54 UTC (permalink / raw)
  To: Jakub Narebski, git

--- Jakub Narebski <jnareb@gmail.com> wrote:
> Clean up input validation, including removing $rss_link variable and
> making error messages more explicit.  Expand and uniquify other error
> messages.

Can you fix your patch then?

Error messages in general do not end with a period.

    Luben


> 
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> This probably conflicts "[PATCH 4/4] gitweb: No periods for error messages".
> It uses periods for error messages which does not end in with some 
> value of some variable.
> 
>  gitweb/gitweb.perl |   88 ++++++++++++++++++++++++----------------------------
>  1 files changed, 40 insertions(+), 48 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 58eb5b1..dfc2d09 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -71,13 +71,15 @@ if (! -d $git_temp) {
>  	mkdir($git_temp, 0700) || die_error("Couldn't mkdir $git_temp");
>  }
>  
> +
> +# ======================================================================
>  # input validation and dispatch
>  our $action = $cgi->param('a');
>  if (defined $action) {
>  	if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
> -		undef $action;
> -		die_error(undef, "Invalid action parameter.");
> +		die_error(undef, "Invalid action parameter $action");
>  	}
> +	# action which does not check rest of parameters
>  	if ($action eq "opml") {
>  		git_opml();
>  		exit;
> @@ -85,22 +87,17 @@ if (defined $action) {
>  }
>  
>  our $project = ($cgi->param('p') || $ENV{'PATH_INFO'});
> -if (defined $project) {
> -	$project =~ s|^/||; $project =~ s|/$||;
> -	$project = validate_input($project);
> -	if (!defined($project)) {
> -		die_error(undef, "Invalid project parameter.");
> +$project =~ s|^/||; $project =~ s|/$||;
> +if (defined $project || $project) {
> +	if (!validate_input($project)) {
> +		die_error(undef, "Invalid project parameter $project");
>  	}
>  	if (!(-d "$projectroot/$project")) {
> -		undef $project;
> -		die_error(undef, "No such directory.");
> +		die_error(undef, "No such directory $project");
>  	}
>  	if (!(-e "$projectroot/$project/HEAD")) {
> -		undef $project;
> -		die_error(undef, "No such project.");
> +		die_error(undef, "No such project $project");
>  	}
> -	$rss_link = "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\"" .
> -	            "$my_uri?" . esc_param("p=$project;a=rss") . "\" type=\"application/rss+xml\"/>";
>  	$ENV{'GIT_DIR'} = "$projectroot/$project";
>  } else {
>  	git_project_list();
> @@ -109,49 +106,43 @@ if (defined $project) {
>  
>  our $file_name = $cgi->param('f');
>  if (defined $file_name) {
> -	$file_name = validate_input($file_name);
> -	if (!defined($file_name)) {
> -		die_error(undef, "Invalid file parameter.");
> +	if (!validate_input($file_name)) {
> +		die_error(undef, "Invalid file parameter $file_name");
>  	}
>  }
>  
>  our $hash = $cgi->param('h');
>  if (defined $hash) {
> -	$hash = validate_input($hash);
> -	if (!defined($hash)) {
> -		die_error(undef, "Invalid hash parameter.");
> +	if (!validate_input($hash)) {
> +		die_error(undef, "Invalid hash parameter $hash");
>  	}
>  }
>  
>  our $hash_parent = $cgi->param('hp');
>  if (defined $hash_parent) {
> -	$hash_parent = validate_input($hash_parent);
> -	if (!defined($hash_parent)) {
> -		die_error(undef, "Invalid hash parent parameter.");
> +	if (!validate_input($hash_parent)) {
> +		die_error(undef, "Invalid hash parent parameter $hash_parent");
>  	}
>  }
>  
>  our $hash_base = $cgi->param('hb');
>  if (defined $hash_base) {
> -	$hash_base = validate_input($hash_base);
> -	if (!defined($hash_base)) {
> -		die_error(undef, "Invalid hash base parameter.");
> +	if (!validate_input($hash_base)) {
> +		die_error(undef, "Invalid hash base parameter $hash_base");
>  	}
>  }
>  
>  our $page = $cgi->param('pg');
>  if (defined $page) {
>  	if ($page =~ m/[^0-9]$/) {
> -		undef $page;
> -		die_error(undef, "Invalid page parameter.");
> +		die_error(undef, "Invalid page parameter $page");
>  	}
>  }
>  
>  our $searchtext = $cgi->param('s');
>  if (defined $searchtext) {
>  	if ($searchtext =~ m/[^a-zA-Z0-9_\.\/\-\+\:\@ ]/) {
> -		undef $searchtext;
> -		die_error(undef, "Invalid search parameter.");
> +		die_error(undef, "Invalid search parameter $searchtext");
>  	}
>  	$searchtext = quotemeta $searchtext;
>  }
> @@ -180,8 +171,7 @@ my %actions = (
>  
>  $action = 'summary' if (!defined($action));
>  if (!defined($actions{$action})) {
> -	undef $action;
> -	die_error(undef, "Unknown action.");
> +	die_error(undef, "Unknown action $action");
>  }
>  $actions{$action}->();
>  exit;
> @@ -871,11 +861,13 @@ sub git_header_html {
>  <meta name="robots" content="index, nofollow"/>
>  <title>$title</title>
>  <link rel="stylesheet" type="text/css" href="$stylesheet"/>
> -$rss_link
> -</head>
> -<body>
>  EOF
> -	print "<div class=\"page_header\">\n" .
> +	print "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\"" .
> +	      "$my_uri?" . esc_param("p=$project;a=rss") . "\" type=\"application/rss+xml\"/>\n" .
> +	      "</head>\n";
> +
> +	print "<body>\n" .
> +	      "<div class=\"page_header\">\n" .
>  	      "<a href=\"http://www.kernel.org/pub/software/scm/git/docs/\" title=\"git
> documentation\">" .
>  	      "<img src=\"$logo\" width=\"72\" height=\"27\" alt=\"git\" style=\"float:right;
> border-width:0px;\"/>" .
>  	      "</a>\n";
> @@ -1471,18 +1463,18 @@ sub git_blame2 {
>  	my $fd;
>  	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);
> +	die_error('404 Not Found', "File name not defined.") if (!$file_name);
>  	$hash_base ||= git_read_head($project);
> -	die_error(undef, "Reading commit failed") unless ($hash_base);
> +	die_error(undef, "Couldn't find base commit.") unless ($hash_base);
>  	my %co = git_read_commit($hash_base)
> -		or die_error(undef, "Reading commit failed");
> +		or die_error(undef, "Reading commit failed.");
>  	if (!defined $hash) {
>  		$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
> -			or die_error(undef, "Error looking up file");
> +			or die_error(undef, "Error looking up file $file_name");
>  	}
>  	$ftype = git_get_type($hash);
>  	if ($ftype !~ "blob") {
> -		die_error("400 Bad Request", "object is not a blob");
> +		die_error("400 Bad Request", "Object is not a blob.");
>  	}
>  	open ($fd, "-|", $GIT, "blame", '-l', $file_name, $hash_base)
>  		or die_error(undef, "Open git-blame failed.");
> @@ -1529,14 +1521,14 @@ sub git_blame2 {
>  sub git_blame {
>  	my $fd;
>  	die_error('403 Permission denied', "Permission denied.") if (!git_get_project_config_bool
> ('blame'));
> -	die_error('404 Not Found', "What file will it be, master?") if (!$file_name);
> +	die_error('404 Not Found', "File name not defined.") if (!$file_name);
>  	$hash_base ||= git_read_head($project);
> -	die_error(undef, "Reading commit failed.") unless ($hash_base);
> +	die_error(undef, "Couldn't find base commit.") unless ($hash_base);
>  	my %co = git_read_commit($hash_base)
>  		or die_error(undef, "Reading commit failed.");
>  	if (!defined $hash) {
>  		$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
> -			or die_error(undef, "Error lookup file.");
> +			or die_error(undef, "Error lookup file $file_name");
>  	}
>  	open ($fd, "-|", $GIT, "annotate", '-l', '-t', '-r', $file_name, $hash_base)
>  		or die_error(undef, "Open git-annotate failed.");
> @@ -1649,7 +1641,7 @@ sub git_blob_plain {
>  		if (defined $file_name) {
>  			my $base = $hash_base || git_read_head($project);
>  			$hash = git_get_hash_by_path($base, $file_name, "blob")
> -				or die_error(undef, "Error lookup file.");
> +				or die_error(undef, "Error lookup file $file_name");
>  		} else {
>  			die_error(undef, "No file name defined.");
>  		}
> @@ -1682,7 +1674,7 @@ sub git_blob {
>  		if (defined $file_name) {
>  			my $base = $hash_base || git_read_head($project);
>  			$hash = git_get_hash_by_path($base, $file_name, "blob")
> -				or die_error(undef, "Error lookup file.");
> +				or die_error(undef, "Error lookup file $file_name");
>  		} else {
>  			die_error(undef, "No file name defined.");
>  		}
> @@ -2122,7 +2114,7 @@ sub git_commitdiff {
>  	open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_parent, $hash
>  		or die_error(undef, "Open git-diff-tree failed.");
>  	my @difftree = map { chomp; $_ } <$fd>;
> -	close $fd or die_error(undef, "Reading diff-tree failed.");
> +	close $fd or die_error(undef, "Reading git-diff-tree failed.");
>  
>  	# non-textual hash id's can be cached
>  	my $expires;
> @@ -2202,7 +2194,7 @@ sub git_commitdiff_plain {
>  	open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_parent, $hash
>  		or die_error(undef, "Open git-diff-tree failed.");
>  	my @difftree = map { chomp; $_ } <$fd>;
> -	close $fd or die_error(undef, "Reading diff-tree failed.");
> +	close $fd or die_error(undef, "Reading git-diff-tree failed.");
>  
>  	# try to figure out the next tag after this commit
>  	my $tagname;
> @@ -2493,7 +2485,7 @@ sub git_rss {
>  	open my $fd, "-|", $GIT, "rev-list", "--max-count=150", git_read_head($project)
>  		or die_error(undef, "Open git-rev-list failed.");
>  	my @revlist = map { chomp; $_ } <$fd>;
> -	close $fd or die_error(undef, "Reading rev-list failed.");
> +	close $fd or die_error(undef, "Reading git-rev-list failed.");
>  	print $cgi->header(-type => 'text/xml', -charset => 'utf-8');
>  	print "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n".
>  	      "<rss version=\"2.0\" xmlns:content=\"http://purl.org/rss/1.0/modules/content/\">\n";
> -- 
> 1.4.1.1
> 
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/5] gitweb: Cleanup input validation and error messages
  2006-08-04 22:38 ` [PATCH 1/5] gitweb: Cleanup input validation and error messages Jakub Narebski
  2006-08-04 23:54   ` Luben Tuikov
@ 2006-08-04 23:54   ` Luben Tuikov
  2006-08-05  0:15   ` Junio C Hamano
  2 siblings, 0 replies; 26+ messages in thread
From: Luben Tuikov @ 2006-08-04 23:54 UTC (permalink / raw)
  To: Jakub Narebski, git

--- Jakub Narebski <jnareb@gmail.com> wrote:
> Clean up input validation, including removing $rss_link variable and
> making error messages more explicit.  Expand and uniquify other error
> messages.
> 
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> This probably conflicts "[PATCH 4/4] gitweb: No periods for error messages".
> It uses periods for error messages which does not end in with some 
> value of some variable.

Can you fix your patch then?
Error messages in general do not end with a period.

    Luben

> 
>  gitweb/gitweb.perl |   88 ++++++++++++++++++++++++----------------------------
>  1 files changed, 40 insertions(+), 48 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 58eb5b1..dfc2d09 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -71,13 +71,15 @@ if (! -d $git_temp) {
>  	mkdir($git_temp, 0700) || die_error("Couldn't mkdir $git_temp");
>  }
>  
> +
> +# ======================================================================
>  # input validation and dispatch
>  our $action = $cgi->param('a');
>  if (defined $action) {
>  	if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
> -		undef $action;
> -		die_error(undef, "Invalid action parameter.");
> +		die_error(undef, "Invalid action parameter $action");
>  	}
> +	# action which does not check rest of parameters
>  	if ($action eq "opml") {
>  		git_opml();
>  		exit;
> @@ -85,22 +87,17 @@ if (defined $action) {
>  }
>  
>  our $project = ($cgi->param('p') || $ENV{'PATH_INFO'});
> -if (defined $project) {
> -	$project =~ s|^/||; $project =~ s|/$||;
> -	$project = validate_input($project);
> -	if (!defined($project)) {
> -		die_error(undef, "Invalid project parameter.");
> +$project =~ s|^/||; $project =~ s|/$||;
> +if (defined $project || $project) {
> +	if (!validate_input($project)) {
> +		die_error(undef, "Invalid project parameter $project");
>  	}
>  	if (!(-d "$projectroot/$project")) {
> -		undef $project;
> -		die_error(undef, "No such directory.");
> +		die_error(undef, "No such directory $project");
>  	}
>  	if (!(-e "$projectroot/$project/HEAD")) {
> -		undef $project;
> -		die_error(undef, "No such project.");
> +		die_error(undef, "No such project $project");
>  	}
> -	$rss_link = "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\"" .
> -	            "$my_uri?" . esc_param("p=$project;a=rss") . "\" type=\"application/rss+xml\"/>";
>  	$ENV{'GIT_DIR'} = "$projectroot/$project";
>  } else {
>  	git_project_list();
> @@ -109,49 +106,43 @@ if (defined $project) {
>  
>  our $file_name = $cgi->param('f');
>  if (defined $file_name) {
> -	$file_name = validate_input($file_name);
> -	if (!defined($file_name)) {
> -		die_error(undef, "Invalid file parameter.");
> +	if (!validate_input($file_name)) {
> +		die_error(undef, "Invalid file parameter $file_name");
>  	}
>  }
>  
>  our $hash = $cgi->param('h');
>  if (defined $hash) {
> -	$hash = validate_input($hash);
> -	if (!defined($hash)) {
> -		die_error(undef, "Invalid hash parameter.");
> +	if (!validate_input($hash)) {
> +		die_error(undef, "Invalid hash parameter $hash");
>  	}
>  }
>  
>  our $hash_parent = $cgi->param('hp');
>  if (defined $hash_parent) {
> -	$hash_parent = validate_input($hash_parent);
> -	if (!defined($hash_parent)) {
> -		die_error(undef, "Invalid hash parent parameter.");
> +	if (!validate_input($hash_parent)) {
> +		die_error(undef, "Invalid hash parent parameter $hash_parent");
>  	}
>  }
>  
>  our $hash_base = $cgi->param('hb');
>  if (defined $hash_base) {
> -	$hash_base = validate_input($hash_base);
> -	if (!defined($hash_base)) {
> -		die_error(undef, "Invalid hash base parameter.");
> +	if (!validate_input($hash_base)) {
> +		die_error(undef, "Invalid hash base parameter $hash_base");
>  	}
>  }
>  
>  our $page = $cgi->param('pg');
>  if (defined $page) {
>  	if ($page =~ m/[^0-9]$/) {
> -		undef $page;
> -		die_error(undef, "Invalid page parameter.");
> +		die_error(undef, "Invalid page parameter $page");
>  	}
>  }
>  
>  our $searchtext = $cgi->param('s');
>  if (defined $searchtext) {
>  	if ($searchtext =~ m/[^a-zA-Z0-9_\.\/\-\+\:\@ ]/) {
> -		undef $searchtext;
> -		die_error(undef, "Invalid search parameter.");
> +		die_error(undef, "Invalid search parameter $searchtext");
>  	}
>  	$searchtext = quotemeta $searchtext;
>  }
> @@ -180,8 +171,7 @@ my %actions = (
>  
>  $action = 'summary' if (!defined($action));
>  if (!defined($actions{$action})) {
> -	undef $action;
> -	die_error(undef, "Unknown action.");
> +	die_error(undef, "Unknown action $action");
>  }
>  $actions{$action}->();
>  exit;
> @@ -871,11 +861,13 @@ sub git_header_html {
>  <meta name="robots" content="index, nofollow"/>
>  <title>$title</title>
>  <link rel="stylesheet" type="text/css" href="$stylesheet"/>
> -$rss_link
> -</head>
> -<body>
>  EOF
> -	print "<div class=\"page_header\">\n" .
> +	print "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\"" .
> +	      "$my_uri?" . esc_param("p=$project;a=rss") . "\" type=\"application/rss+xml\"/>\n" .
> +	      "</head>\n";
> +
> +	print "<body>\n" .
> +	      "<div class=\"page_header\">\n" .
>  	      "<a href=\"http://www.kernel.org/pub/software/scm/git/docs/\" title=\"git
> documentation\">" .
>  	      "<img src=\"$logo\" width=\"72\" height=\"27\" alt=\"git\" style=\"float:right;
> border-width:0px;\"/>" .
>  	      "</a>\n";
> @@ -1471,18 +1463,18 @@ sub git_blame2 {
>  	my $fd;
>  	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);
> +	die_error('404 Not Found', "File name not defined.") if (!$file_name);
>  	$hash_base ||= git_read_head($project);
> -	die_error(undef, "Reading commit failed") unless ($hash_base);
> +	die_error(undef, "Couldn't find base commit.") unless ($hash_base);
>  	my %co = git_read_commit($hash_base)
> -		or die_error(undef, "Reading commit failed");
> +		or die_error(undef, "Reading commit failed.");
>  	if (!defined $hash) {
>  		$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
> -			or die_error(undef, "Error looking up file");
> +			or die_error(undef, "Error looking up file $file_name");
>  	}
>  	$ftype = git_get_type($hash);
>  	if ($ftype !~ "blob") {
> -		die_error("400 Bad Request", "object is not a blob");
> +		die_error("400 Bad Request", "Object is not a blob.");
>  	}
>  	open ($fd, "-|", $GIT, "blame", '-l', $file_name, $hash_base)
>  		or die_error(undef, "Open git-blame failed.");
> @@ -1529,14 +1521,14 @@ sub git_blame2 {
>  sub git_blame {
>  	my $fd;
>  	die_error('403 Permission denied', "Permission denied.") if (!git_get_project_config_bool
> ('blame'));
> -	die_error('404 Not Found', "What file will it be, master?") if (!$file_name);
> +	die_error('404 Not Found', "File name not defined.") if (!$file_name);
>  	$hash_base ||= git_read_head($project);
> -	die_error(undef, "Reading commit failed.") unless ($hash_base);
> +	die_error(undef, "Couldn't find base commit.") unless ($hash_base);
>  	my %co = git_read_commit($hash_base)
>  		or die_error(undef, "Reading commit failed.");
>  	if (!defined $hash) {
>  		$hash = git_get_hash_by_path($hash_base, $file_name, "blob")
> -			or die_error(undef, "Error lookup file.");
> +			or die_error(undef, "Error lookup file $file_name");
>  	}
>  	open ($fd, "-|", $GIT, "annotate", '-l', '-t', '-r', $file_name, $hash_base)
>  		or die_error(undef, "Open git-annotate failed.");
> @@ -1649,7 +1641,7 @@ sub git_blob_plain {
>  		if (defined $file_name) {
>  			my $base = $hash_base || git_read_head($project);
>  			$hash = git_get_hash_by_path($base, $file_name, "blob")
> -				or die_error(undef, "Error lookup file.");
> +				or die_error(undef, "Error lookup file $file_name");
>  		} else {
>  			die_error(undef, "No file name defined.");
>  		}
> @@ -1682,7 +1674,7 @@ sub git_blob {
>  		if (defined $file_name) {
>  			my $base = $hash_base || git_read_head($project);
>  			$hash = git_get_hash_by_path($base, $file_name, "blob")
> -				or die_error(undef, "Error lookup file.");
> +				or die_error(undef, "Error lookup file $file_name");
>  		} else {
>  			die_error(undef, "No file name defined.");
>  		}
> @@ -2122,7 +2114,7 @@ sub git_commitdiff {
>  	open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_parent, $hash
>  		or die_error(undef, "Open git-diff-tree failed.");
>  	my @difftree = map { chomp; $_ } <$fd>;
> -	close $fd or die_error(undef, "Reading diff-tree failed.");
> +	close $fd or die_error(undef, "Reading git-diff-tree failed.");
>  
>  	# non-textual hash id's can be cached
>  	my $expires;
> @@ -2202,7 +2194,7 @@ sub git_commitdiff_plain {
>  	open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_parent, $hash
>  		or die_error(undef, "Open git-diff-tree failed.");
>  	my @difftree = map { chomp; $_ } <$fd>;
> -	close $fd or die_error(undef, "Reading diff-tree failed.");
> +	close $fd or die_error(undef, "Reading git-diff-tree failed.");
>  
>  	# try to figure out the next tag after this commit
>  	my $tagname;
> @@ -2493,7 +2485,7 @@ sub git_rss {
>  	open my $fd, "-|", $GIT, "rev-list", "--max-count=150", git_read_head($project)
>  		or die_error(undef, "Open git-rev-list failed.");
>  	my @revlist = map { chomp; $_ } <$fd>;
> -	close $fd or die_error(undef, "Reading rev-list failed.");
> +	close $fd or die_error(undef, "Reading git-rev-list failed.");
>  	print $cgi->header(-type => 'text/xml', -charset => 'utf-8');
>  	print "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n".
>  	      "<rss version=\"2.0\" xmlns:content=\"http://purl.org/rss/1.0/modules/content/\">\n";
> -- 
> 1.4.1.1
> 
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 6/5] gitweb: No periods for error messages
  2006-08-04 23:54   ` Luben Tuikov
@ 2006-08-05  0:02     ` Jakub Narebski
  0 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05  0:02 UTC (permalink / raw)
  To: git

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Luben Tuikov wrote:
> --- Jakub Narebski <jnareb@gmail.com> wrote:
> > Clean up input validation, including removing $rss_link variable and
> > making error messages more explicit.  Expand and uniquify other error
> > messages.
> 
> Can you fix your patch then?
> 
> Error messages in general do not end with a period.

Here you have the patch, on top of series.

 gitweb/gitweb.perl |   66 ++++++++++++++++++++++++++--------------------------
 1 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e983452..4316bd0 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -420,7 +420,7 @@ sub git_get_hash_by_path {
        my $tree = $base;
 
        open my $fd, "-|", $GIT, "ls-tree", $base, "--", $path
-               or die_error(undef, "Open git-ls-tree failed.");
+               or die_error(undef, "Open git-ls-tree failed");
        my $line = <$fd>;
        close $fd or return undef;
 
@@ -1282,13 +1282,13 @@ ## actions
 sub git_project_list {
        my $order = $cgi->param('o');
        if (defined $order && $order !~ m/project|descr|owner|age/) {
-               die_error(undef, "Invalid order parameter '$order'.");
+               die_error(undef, "Invalid order parameter $order");
        }
 
        my @list = git_read_projects();
        my @projects;
        if (!@list) {
-               die_error(undef, "No projects found.");
+               die_error(undef, "No projects found");
        }
        foreach my $pr (@list) {
                my $head = git_get_head($pr->{'path'});
@@ -1422,7 +1422,7 @@ sub git_summary {
              "</table>\n";
 
        open my $fd, "-|", $GIT, "rev-list", "--max-count=17", git_get_head($project)
-               or die_error(undef, "Open git-rev-list failed.");
+               or die_error(undef, "Open git-rev-list failed");
        my @revlist = map { chomp; $_ } <$fd>;
        close $fd;
        git_print_header_div('shortlog');
@@ -1478,22 +1478,22 @@ sub git_tag {
 sub git_blame2 {
        my $fd;
        my $ftype;
-       die_error(undef, "Permission denied.") if (!git_get_project_config_bool ('blame'));
+       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_get_head($project);
-       die_error(undef, "Couldn't find base commit.") unless ($hash_base);
+       die_error(undef, "Couldn't find base commit") unless ($hash_base);
        my %co = parse_commit($hash_base)
-               or die_error(undef, "Reading commit failed.");
+               or die_error(undef, "Reading commit failed");
        if (!defined $hash) {
                $hash = git_get_hash_by_path($hash_base, $file_name, "blob")
                        or die_error(undef, "Error looking up file $file_name");
        }
        $ftype = git_get_type($hash);
        if ($ftype !~ "blob") {
-               die_error("400 Bad Request", "Object is not a blob.");
+               die_error("400 Bad Request", "Object is not a blob");
        }
        open ($fd, "-|", $GIT, "blame", '-l', $file_name, $hash_base)
-               or die_error(undef, "Open git-blame failed.");
+               or die_error(undef, "Open git-blame failed");
        git_header_html();
        my $formats_nav =
                $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$hash;hb=$hash_base;f=$file_name")}, "blob") .
@@ -1536,18 +1536,18 @@ sub git_blame2 {
 
 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);
+       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_get_head($project);
        die_error(undef, "Couldn't find base commit.") unless ($hash_base);
        my %co = parse_commit($hash_base)
-               or die_error(undef, "Reading commit failed.");
+               or die_error(undef, "Reading commit failed");
        if (!defined $hash) {
                $hash = git_get_hash_by_path($hash_base, $file_name, "blob")
                        or die_error(undef, "Error lookup file $file_name");
        }
        open ($fd, "-|", $GIT, "annotate", '-l', '-t', '-r', $file_name, $hash_base)
-               or die_error(undef, "Open git-annotate failed.");
+               or die_error(undef, "Open git-annotate failed");
        git_header_html();
        my $formats_nav =
                $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$hash;hb=$hash_base;f=$file_name")}, "blob") .
@@ -1658,7 +1658,7 @@ sub git_blob_plain {
                        $hash = git_get_hash_by_path($base, $file_name, "blob")
                                or die_error(undef, "Error lookup file $file_name");
                } else {
-                       die_error(undef, "No file name defined.");
+                       die_error(undef, "No file name defined");
                }
        }
        my $type = shift;
@@ -1691,12 +1691,12 @@ sub git_blob {
                        $hash = git_get_hash_by_path($base, $file_name, "blob")
                                or die_error(undef, "Error lookup file $file_name");
                } else {
-                       die_error(undef, "No file name defined.");
+                       die_error(undef, "No file name defined");
                }
        }
        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.");
+               or die_error(undef, "Couldn't cat $file_name, $hash");
        my $mimetype = blob_plain_mimetype($fd, $file_name);
        if ($mimetype !~ m/^text\//) {
                close $fd;
@@ -1754,9 +1754,9 @@ sub git_tree {
        }
        $/ = "\0";
        open my $fd, "-|", $GIT, "ls-tree", '-z', $hash
-               or die_error(undef, "Open git-ls-tree failed.");
+               or die_error(undef, "Open git-ls-tree failed");
        my @entries = map { chomp; $_ } <$fd>;
-       close $fd or die_error(undef, "Reading tree failed.");
+       close $fd or die_error(undef, "Reading tree failed");
        $/ = "\n";
 
        my $refs = git_read_info_refs();
@@ -1832,7 +1832,7 @@ sub git_log {
 
        my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
        open my $fd, "-|", $GIT, "rev-list", $limit, $hash
-               or die_error(undef, "Open git-rev-list failed.");
+               or die_error(undef, "Open git-rev-list failed");
        my @revlist = map { chomp; $_ } <$fd>;
        close $fd;
 
@@ -1893,7 +1893,7 @@ sub git_log {
 sub git_commit {
        my %co = parse_commit($hash);
        if (!%co) {
-               die_error(undef, "Unknown commit object.");
+               die_error(undef, "Unknown commit object");
        }
        my %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});
        my %cd = parse_date($co{'committer_epoch'}, $co{'committer_tz'});
@@ -1903,9 +1903,9 @@ sub git_commit {
                $parent = "--root";
        }
        open my $fd, "-|", $GIT, "diff-tree", '-r', '-M', $parent, $hash
-               or die_error(undef, "Open git-diff-tree failed.");
+               or die_error(undef, "Open git-diff-tree failed");
        my @difftree = map { chomp; $_ } <$fd>;
-       close $fd or die_error(undef, "Reading git-diff-tree failed.");
+       close $fd or die_error(undef, "Reading git-diff-tree failed");
 
        # non-textual hash id's can be cached
        my $expires;
@@ -2121,15 +2121,15 @@ sub git_commitdiff {
        mkdir($git_temp, 0700);
        my %co = parse_commit($hash);
        if (!%co) {
-               die_error(undef, "Unknown commit object.");
+               die_error(undef, "Unknown commit object");
        }
        if (!defined $hash_parent) {
                $hash_parent = $co{'parent'};
        }
        open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_parent, $hash
-               or die_error(undef, "Open git-diff-tree failed.");
+               or die_error(undef, "Open git-diff-tree failed");
        my @difftree = map { chomp; $_ } <$fd>;
-       close $fd or die_error(undef, "Reading git-diff-tree failed.");
+       close $fd or die_error(undef, "Reading git-diff-tree failed");
 
        # non-textual hash id's can be cached
        my $expires;
@@ -2207,9 +2207,9 @@ sub git_commitdiff {
 sub git_commitdiff_plain {
        mkdir($git_temp, 0700);
        open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_parent, $hash
-               or die_error(undef, "Open git-diff-tree failed.");
+               or die_error(undef, "Open git-diff-tree failed");
        my @difftree = map { chomp; $_ } <$fd>;
-       close $fd or die_error(undef, "Reading git-diff-tree failed.");
+       close $fd or die_error(undef, "Reading git-diff-tree failed");
 
        # try to figure out the next tag after this commit
        my $tagname;
@@ -2267,7 +2267,7 @@ sub git_history {
        my $ftype;
        my %co = parse_commit($hash_base);
        if (!%co) {
-               die_error(undef, "Unknown commit object.");
+               die_error(undef, "Unknown commit object");
        }
        my $refs = git_read_info_refs();
        git_header_html();
@@ -2325,14 +2325,14 @@ sub git_history {
 
 sub git_search {
        if (!defined $searchtext) {
-               die_error("", "Text field empty.");
+               die_error(undef, "Text field empty");
        }
        if (!defined $hash) {
                $hash = git_get_head($project);
        }
        my %co = parse_commit($hash);
        if (!%co) {
-               die_error(undef, "Unknown commit object.");
+               die_error(undef, "Unknown commit object");
        }
        # pickaxe may take all resources of your box and run for several minutes
        # with every query - so decide by yourself how public you make this feature :)
@@ -2470,7 +2470,7 @@ sub git_shortlog {
 
        my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
        open my $fd, "-|", $GIT, "rev-list", $limit, $hash
-               or die_error(undef, "Open git-rev-list failed.");
+               or die_error(undef, "Open git-rev-list failed");
        my @revlist = map { chomp; $_ } <$fd>;
        close $fd;
 
@@ -2498,9 +2498,9 @@ ## feeds (RSS, OPML)
 sub git_rss {
        # http://www.notestips.com/80256B3A007F2692/1/NAMO5P9UPQ
        open my $fd, "-|", $GIT, "rev-list", "--max-count=150", git_get_head($project)
-               or die_error(undef, "Open git-rev-list failed.");
+               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.");
+       close $fd or die_error(undef, "Reading git-rev-list failed");
        print $cgi->header(-type => 'text/xml', -charset => 'utf-8');
        print "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n".
              "<rss version=\"2.0\" xmlns:content=\"http://purl.org/rss/1.0/modules/content/\">\n";
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/5] gitweb: Cleanup input validation and error messages
  2006-08-04 22:38 ` [PATCH 1/5] gitweb: Cleanup input validation and error messages Jakub Narebski
  2006-08-04 23:54   ` Luben Tuikov
  2006-08-04 23:54   ` [PATCH 1/5] gitweb: Cleanup input validation and " Luben Tuikov
@ 2006-08-05  0:15   ` Junio C Hamano
  2006-08-05  0:26     ` Jakub Narebski
  2 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2006-08-05  0:15 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

Jakub Narebski <jnareb@gmail.com> writes:

>  our $action = $cgi->param('a');
>  if (defined $action) {
>  	if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
> -		undef $action;
> -		die_error(undef, "Invalid action parameter.");
> +		die_error(undef, "Invalid action parameter $action");
>  	}

Doesn't this make us parrot what the browser threw at us without
escaping back for HTML (iow, die_error does not seem to escape
$error)?

>  our $project = ($cgi->param('p') || $ENV{'PATH_INFO'});
> -if (defined $project) {
> -	$project =~ s|^/||; $project =~ s|/$||;
> -	$project = validate_input($project);
> -	if (!defined($project)) {
> -		die_error(undef, "Invalid project parameter.");
> +$project =~ s|^/||; $project =~ s|/$||;

Unrelated change but it is probably safe.

> +if (defined $project || $project) {
> +	if (!validate_input($project)) {
> +		die_error(undef, "Invalid project parameter $project");
>  	}

Because undef is not true, "|| $project" here does not make much
sense to me.  Even if you meant to say "&&" to exclude empty
string or "0", wouldn't validate_input() take care of them?
Same input parrotting comment applies here and everywhere.

> -	$rss_link = "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\"" .
> -	            "$my_uri?" . esc_param("p=$project;a=rss") . "\" type=\"application/rss+xml\"/>";

The reason of removal is...?  Ah, you inlined it.  It was not
clear from the proposed commit log message.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/5] gitweb: Cleanup input validation and error messages
  2006-08-05  0:15   ` Junio C Hamano
@ 2006-08-05  0:26     ` Jakub Narebski
  2006-08-05 10:51       ` [PATCH 0/9] gitweb: First patch corrected and split into separate patches Jakub Narebski
  0 siblings, 1 reply; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05  0:26 UTC (permalink / raw)
  To: git

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>>  our $action = $cgi->param('a');
>>  if (defined $action) {
>>      if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
>> -            undef $action;
>> -            die_error(undef, "Invalid action parameter.");
>> +            die_error(undef, "Invalid action parameter $action");
>>      }
> 
> Doesn't this make us parrot what the browser threw at us without
> escaping back for HTML (iow, die_error does not seem to escape
> $error)?

I wanted to know what is the parameter gitweb considers invalid.
Perhaps the execution wasn't the best...

[...]
>> -    $rss_link = "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\"" .
>> -                "$my_uri?" . esc_param("p=$project;a=rss") . "\" type=\"application/rss+xml\"/>";
> 
> The reason of removal is...?  Ah, you inlined it.  It was not
> clear from the proposed commit log message.

I'm sorry for unrelated changes (the commit could be probably split 
into four).

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 0/9] gitweb: First patch corrected and split into separate patches
  2006-08-05  0:26     ` Jakub Narebski
@ 2006-08-05 10:51       ` Jakub Narebski
  2006-08-05 10:55         ` [PATCH 1/9] gitweb: Separate input validation and dispatch, add comment about opml action Jakub Narebski
                           ` (8 more replies)
  0 siblings, 9 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05 10:51 UTC (permalink / raw)
  To: Jakub Narebski, git

Jakub Narebski wrote:

> I'm sorry for unrelated changes (the commit could be probably split 
> into four).

This series splits unrelated changes into separate patches, does not add
controversial adding (unescaped) value of invalid/errorneous parameter to
error message, and correct errors noticed during creation of this series.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/9] gitweb: Separate input validation and dispatch, add comment about opml action
  2006-08-05 10:51       ` [PATCH 0/9] gitweb: First patch corrected and split into separate patches Jakub Narebski
@ 2006-08-05 10:55         ` Jakub Narebski
  2006-08-05 10:56         ` [PATCH 2/9] gitweb: die_error first (optional) parameter is HTTP status Jakub Narebski
                           ` (7 subsequent siblings)
  8 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05 10:55 UTC (permalink / raw)
  To: git

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 58eb5b1..8b53fe6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -71,6 +71,7 @@ if (! -d $git_temp) {
        mkdir($git_temp, 0700) || die_error("Couldn't mkdir $git_temp");
 }
 
+# ======================================================================
 # input validation and dispatch
 our $action = $cgi->param('a');
 if (defined $action) {
@@ -78,6 +79,7 @@ if (defined $action) {
                undef $action;
                die_error(undef, "Invalid action parameter.");
        }
+       # action which does not check rest of parameters
        if ($action eq "opml") {
                git_opml();
                exit;
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/9] gitweb: die_error first (optional) parameter is HTTP status
  2006-08-05 10:51       ` [PATCH 0/9] gitweb: First patch corrected and split into separate patches Jakub Narebski
  2006-08-05 10:55         ` [PATCH 1/9] gitweb: Separate input validation and dispatch, add comment about opml action Jakub Narebski
@ 2006-08-05 10:56         ` Jakub Narebski
  2006-08-05 10:56         ` [PATCH 3/9] gitweb: Use undef for die_error to use default first (status) parameter value Jakub Narebski
                           ` (6 subsequent siblings)
  8 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05 10:56 UTC (permalink / raw)
  To: git

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8b53fe6..6feec28 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -68,7 +68,7 @@ our $git_version = qx($GIT --version) =~
 
 $projects_list ||= $projectroot;
 if (! -d $git_temp) {
-       mkdir($git_temp, 0700) || die_error("Couldn't mkdir $git_temp");
+       mkdir($git_temp, 0700) || die_error(undef, "Couldn't mkdir $git_temp");
 }
 
 # ======================================================================
@@ -1658,7 +1658,7 @@ sub git_blob_plain {
        }
        my $type = shift;
        open my $fd, "-|", $GIT, "cat-file", "blob", $hash
-               or die_error("Couldn't cat $file_name, $hash");
+               or die_error(undef, "Couldn't cat $file_name, $hash");
 
        $type ||= git_blob_plain_mimetype($fd, $file_name);
 
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/9] gitweb: Use undef for die_error to use default first (status) parameter value
  2006-08-05 10:51       ` [PATCH 0/9] gitweb: First patch corrected and split into separate patches Jakub Narebski
  2006-08-05 10:55         ` [PATCH 1/9] gitweb: Separate input validation and dispatch, add comment about opml action Jakub Narebski
  2006-08-05 10:56         ` [PATCH 2/9] gitweb: die_error first (optional) parameter is HTTP status Jakub Narebski
@ 2006-08-05 10:56         ` Jakub Narebski
  2006-08-05 10:58         ` [PATCH 4/9] gitweb: Don't undefine query parameter related variables before die_error Jakub Narebski
                           ` (5 subsequent siblings)
  8 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05 10:56 UTC (permalink / raw)
  To: git

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
For consistency.

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 6feec28..9b9bf37 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2320,7 +2320,7 @@ sub git_history {
 
 sub git_search {
        if (!defined $searchtext) {
-               die_error("", "Text field empty.");
+               die_error(undef, "Text field empty.");
        }
        if (!defined $hash) {
                $hash = git_read_head($project);
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 4/9] gitweb: Don't undefine query parameter related variables before die_error
  2006-08-05 10:51       ` [PATCH 0/9] gitweb: First patch corrected and split into separate patches Jakub Narebski
                           ` (2 preceding siblings ...)
  2006-08-05 10:56         ` [PATCH 3/9] gitweb: Use undef for die_error to use default first (status) parameter value Jakub Narebski
@ 2006-08-05 10:58         ` Jakub Narebski
  2006-08-05 11:12         ` [PATCH 5/9] gitweb: Cleanup and uniquify error messages Jakub Narebski
                           ` (4 subsequent siblings)
  8 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05 10:58 UTC (permalink / raw)
  To: git

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
It would allow to include value of invalid parameter in error message

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 9b9bf37..6f3f465 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -76,7 +76,6 @@ # input validation and dispatch
 our $action = $cgi->param('a');
 if (defined $action) {
        if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
-               undef $action;
                die_error(undef, "Invalid action parameter.");
        }
        # action which does not check rest of parameters
@@ -89,16 +88,13 @@ if (defined $action) {
 our $project = ($cgi->param('p') || $ENV{'PATH_INFO'});
 if (defined $project) {
        $project =~ s|^/||; $project =~ s|/$||;
-       $project = validate_input($project);
-       if (!defined($project)) {
+       if (!validate_input($project)) {
                die_error(undef, "Invalid project parameter.");
        }
        if (!(-d "$projectroot/$project")) {
-               undef $project;
                die_error(undef, "No such directory.");
        }
        if (!(-e "$projectroot/$project/HEAD")) {
-               undef $project;
                die_error(undef, "No such project.");
        }
        $rss_link = "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\"" .
@@ -111,32 +107,28 @@ if (defined $project) {
 
 our $file_name = $cgi->param('f');
 if (defined $file_name) {
-       $file_name = validate_input($file_name);
-       if (!defined($file_name)) {
+       if (!validate_input($file_name)) {
                die_error(undef, "Invalid file parameter.");
        }
 }
 
 our $hash = $cgi->param('h');
 if (defined $hash) {
-       $hash = validate_input($hash);
-       if (!defined($hash)) {
+       if (!validate_input($hash)) {
                die_error(undef, "Invalid hash parameter.");
        }
 }
 
 our $hash_parent = $cgi->param('hp');
 if (defined $hash_parent) {
-       $hash_parent = validate_input($hash_parent);
-       if (!defined($hash_parent)) {
+       if (!validate_input($hash_parent)) {
                die_error(undef, "Invalid hash parent parameter.");
        }
 }
 
 our $hash_base = $cgi->param('hb');
 if (defined $hash_base) {
-       $hash_base = validate_input($hash_base);
-       if (!defined($hash_base)) {
+       if (!validate_input($hash_base)) {
                die_error(undef, "Invalid hash base parameter.");
        }
 }
@@ -144,7 +136,6 @@ if (defined $hash_base) {
 our $page = $cgi->param('pg');
 if (defined $page) {
        if ($page =~ m/[^0-9]$/) {
-               undef $page;
                die_error(undef, "Invalid page parameter.");
        }
 }
@@ -152,7 +143,6 @@ if (defined $page) {
 our $searchtext = $cgi->param('s');
 if (defined $searchtext) {
        if ($searchtext =~ m/[^a-zA-Z0-9_\.\/\-\+\:\@ ]/) {
-               undef $searchtext;
                die_error(undef, "Invalid search parameter.");
        }
        $searchtext = quotemeta $searchtext;
@@ -182,7 +172,6 @@ my %actions = (
 
 $action = 'summary' if (!defined($action));
 if (!defined($actions{$action})) {
-       undef $action;
        die_error(undef, "Unknown action.");
 }
 $actions{$action}->();
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 5/9] gitweb: Cleanup and uniquify error messages
  2006-08-05 10:51       ` [PATCH 0/9] gitweb: First patch corrected and split into separate patches Jakub Narebski
                           ` (3 preceding siblings ...)
  2006-08-05 10:58         ` [PATCH 4/9] gitweb: Don't undefine query parameter related variables before die_error Jakub Narebski
@ 2006-08-05 11:12         ` Jakub Narebski
  2006-08-05 11:13         ` [PATCH 6/9] gitweb: No periods for " Jakub Narebski
                           ` (3 subsequent siblings)
  8 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05 11:12 UTC (permalink / raw)
  To: git

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 6f3f465..8773a8d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1464,7 +1464,7 @@ sub git_blame2 {
        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);
-       die_error(undef, "Reading commit failed") unless ($hash_base);
+       die_error(undef, "Couldn't find base commit.") unless ($hash_base);
        my %co = git_read_commit($hash_base)
                or die_error(undef, "Reading commit failed");
        if (!defined $hash) {
@@ -1473,7 +1473,7 @@ sub git_blame2 {
        }
        $ftype = git_get_type($hash);
        if ($ftype !~ "blob") {
-               die_error("400 Bad Request", "object is not a blob");
+               die_error("400 Bad Request", "Object is not a blob");
        }
        open ($fd, "-|", $GIT, "blame", '-l', $file_name, $hash_base)
                or die_error(undef, "Open git-blame failed.");
@@ -1520,9 +1520,9 @@ sub git_blame2 {
 sub git_blame {
        my $fd;
        die_error('403 Permission denied', "Permission denied.") if (!git_get_project_config_bool ('blame'));
-       die_error('404 Not Found', "What file will it be, master?") if (!$file_name);
+       die_error('404 Not Found', "File name not defined.") if (!$file_name);
        $hash_base ||= git_read_head($project);
-       die_error(undef, "Reading commit failed.") unless ($hash_base);
+       die_error(undef, "Couldn't find base commit.") unless ($hash_base);
        my %co = git_read_commit($hash_base)
                or die_error(undef, "Reading commit failed.");
        if (!defined $hash) {
@@ -2113,7 +2113,7 @@ sub git_commitdiff {
        open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_parent, $hash
                or die_error(undef, "Open git-diff-tree failed.");
        my @difftree = map { chomp; $_ } <$fd>;
-       close $fd or die_error(undef, "Reading diff-tree failed.");
+       close $fd or die_error(undef, "Reading git-diff-tree failed.");
 
        # non-textual hash id's can be cached
        my $expires;
@@ -2484,7 +2484,7 @@ sub git_rss {
        open my $fd, "-|", $GIT, "rev-list", "--max-count=150", git_read_head($project)
                or die_error(undef, "Open git-rev-list failed.");
        my @revlist = map { chomp; $_ } <$fd>;
-       close $fd or die_error(undef, "Reading rev-list failed.");
+       close $fd or die_error(undef, "Reading git-rev-list failed.");
        print $cgi->header(-type => 'text/xml', -charset => 'utf-8');
        print "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n".
              "<rss version=\"2.0\" xmlns:content=\"http://purl.org/rss/1.0/modules/content/\">\n";
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 6/9] gitweb: No periods for error messages
  2006-08-05 10:51       ` [PATCH 0/9] gitweb: First patch corrected and split into separate patches Jakub Narebski
                           ` (4 preceding siblings ...)
  2006-08-05 11:12         ` [PATCH 5/9] gitweb: Cleanup and uniquify error messages Jakub Narebski
@ 2006-08-05 11:13         ` Jakub Narebski
  2006-08-05 15:55           ` Luben Tuikov
  2006-08-05 11:15         ` [PATCH 7/9] gitweb: No error messages with unescaped/unprotected user input Jakub Narebski
                           ` (2 subsequent siblings)
  8 siblings, 1 reply; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05 11:13 UTC (permalink / raw)
  To: git

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Like in Luben Tuikov patch

 gitweb/gitweb.perl |   92 ++++++++++++++++++++++++++--------------------------
 1 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8773a8d..d8ba016 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -76,7 +76,7 @@ # input validation and dispatch
 our $action = $cgi->param('a');
 if (defined $action) {
        if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
-               die_error(undef, "Invalid action parameter.");
+               die_error(undef, "Invalid action parameter");
        }
        # action which does not check rest of parameters
        if ($action eq "opml") {
@@ -89,13 +89,13 @@ our $project = ($cgi->param('p') || $ENV
 if (defined $project) {
        $project =~ s|^/||; $project =~ s|/$||;
        if (!validate_input($project)) {
-               die_error(undef, "Invalid project parameter.");
+               die_error(undef, "Invalid project parameter");
        }
        if (!(-d "$projectroot/$project")) {
-               die_error(undef, "No such directory.");
+               die_error(undef, "No such directory");
        }
        if (!(-e "$projectroot/$project/HEAD")) {
-               die_error(undef, "No such project.");
+               die_error(undef, "No such project");
        }
        $rss_link = "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\"" .
                    "$my_uri?" . esc_param("p=$project;a=rss") . "\" type=\"application/rss+xml\"/>";
@@ -108,42 +108,42 @@ if (defined $project) {
 our $file_name = $cgi->param('f');
 if (defined $file_name) {
        if (!validate_input($file_name)) {
-               die_error(undef, "Invalid file parameter.");
+               die_error(undef, "Invalid file parameter");
        }
 }
 
 our $hash = $cgi->param('h');
 if (defined $hash) {
        if (!validate_input($hash)) {
-               die_error(undef, "Invalid hash parameter.");
+               die_error(undef, "Invalid hash parameter");
        }
 }
 
 our $hash_parent = $cgi->param('hp');
 if (defined $hash_parent) {
        if (!validate_input($hash_parent)) {
-               die_error(undef, "Invalid hash parent parameter.");
+               die_error(undef, "Invalid hash parent parameter");
        }
 }
 
 our $hash_base = $cgi->param('hb');
 if (defined $hash_base) {
        if (!validate_input($hash_base)) {
-               die_error(undef, "Invalid hash base parameter.");
+               die_error(undef, "Invalid hash base parameter");
        }
 }
 
 our $page = $cgi->param('pg');
 if (defined $page) {
        if ($page =~ m/[^0-9]$/) {
-               die_error(undef, "Invalid page parameter.");
+               die_error(undef, "Invalid page parameter");
        }
 }
 
 our $searchtext = $cgi->param('s');
 if (defined $searchtext) {
        if ($searchtext =~ m/[^a-zA-Z0-9_\.\/\-\+\:\@ ]/) {
-               die_error(undef, "Invalid search parameter.");
+               die_error(undef, "Invalid search parameter");
        }
        $searchtext = quotemeta $searchtext;
 }
@@ -172,7 +172,7 @@ my %actions = (
 
 $action = 'summary' if (!defined($action));
 if (!defined($actions{$action})) {
-       die_error(undef, "Unknown action.");
+       die_error(undef, "Unknown action");
 }
 $actions{$action}->();
 exit;
@@ -418,7 +418,7 @@ sub git_get_hash_by_path {
        my $tree = $base;
 
        open my $fd, "-|", $GIT, "ls-tree", $base, "--", $path
-               or die_error(undef, "Open git-ls-tree failed.");
+               or die_error(undef, "Open git-ls-tree failed");
        my $line = <$fd>;
        close $fd or return undef;
 
@@ -1265,13 +1265,13 @@ ## actions
 sub git_project_list {
        my $order = $cgi->param('o');
        if (defined $order && $order !~ m/project|descr|owner|age/) {
-               die_error(undef, "Invalid order parameter '$order'.");
+               die_error(undef, "Invalid order parameter '$order'");
        }
 
        my @list = git_read_projects();
        my @projects;
        if (!@list) {
-               die_error(undef, "No projects found.");
+               die_error(undef, "No projects found");
        }
        foreach my $pr (@list) {
                my $head = git_read_head($pr->{'path'});
@@ -1405,7 +1405,7 @@ sub git_summary {
              "</table>\n";
 
        open my $fd, "-|", $GIT, "rev-list", "--max-count=17", git_read_head($project)
-               or die_error(undef, "Open git-rev-list failed.");
+               or die_error(undef, "Open git-rev-list failed");
        my @revlist = map { chomp; $_ } <$fd>;
        close $fd;
        git_header_div('shortlog');
@@ -1461,10 +1461,10 @@ sub git_tag {
 sub git_blame2 {
        my $fd;
        my $ftype;
-       die_error(undef, "Permission denied.") if (!git_get_project_config_bool ('blame'));
+       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);
-       die_error(undef, "Couldn't find base commit.") unless ($hash_base);
+       die_error(undef, "Couldn't find base commit") unless ($hash_base);
        my %co = git_read_commit($hash_base)
                or die_error(undef, "Reading commit failed");
        if (!defined $hash) {
@@ -1476,7 +1476,7 @@ sub git_blame2 {
                die_error("400 Bad Request", "Object is not a blob");
        }
        open ($fd, "-|", $GIT, "blame", '-l', $file_name, $hash_base)
-               or die_error(undef, "Open git-blame failed.");
+               or die_error(undef, "Open git-blame failed");
        git_header_html();
        my $formats_nav =
                $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$hash;hb=$hash_base;f=$file_name")}, "blob") .
@@ -1519,18 +1519,18 @@ sub git_blame2 {
 
 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);
+       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);
-       die_error(undef, "Couldn't find base commit.") unless ($hash_base);
+       die_error(undef, "Couldn't find base commit") unless ($hash_base);
        my %co = git_read_commit($hash_base)
-               or die_error(undef, "Reading commit failed.");
+               or die_error(undef, "Reading commit failed");
        if (!defined $hash) {
                $hash = git_get_hash_by_path($hash_base, $file_name, "blob")
-                       or die_error(undef, "Error lookup file.");
+                       or die_error(undef, "Error lookup file");
        }
        open ($fd, "-|", $GIT, "annotate", '-l', '-t', '-r', $file_name, $hash_base)
-               or die_error(undef, "Open git-annotate failed.");
+               or die_error(undef, "Open git-annotate failed");
        git_header_html();
        my $formats_nav =
                $cgi->a({-href => "$my_uri?" . esc_param("p=$project;a=blob;h=$hash;hb=$hash_base;f=$file_name")}, "blob") .
@@ -1640,9 +1640,9 @@ sub git_blob_plain {
                if (defined $file_name) {
                        my $base = $hash_base || git_read_head($project);
                        $hash = git_get_hash_by_path($base, $file_name, "blob")
-                               or die_error(undef, "Error lookup file.");
+                               or die_error(undef, "Error lookup file");
                } else {
-                       die_error(undef, "No file name defined.");
+                       die_error(undef, "No file name defined");
                }
        }
        my $type = shift;
@@ -1673,14 +1673,14 @@ sub git_blob {
                if (defined $file_name) {
                        my $base = $hash_base || git_read_head($project);
                        $hash = git_get_hash_by_path($base, $file_name, "blob")
-                               or die_error(undef, "Error lookup file.");
+                               or die_error(undef, "Error lookup file");
                } else {
-                       die_error(undef, "No file name defined.");
+                       die_error(undef, "No file name defined");
                }
        }
        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.");
+               or die_error(undef, "Couldn't cat $file_name, $hash");
        my $mimetype = git_blob_plain_mimetype($fd, $file_name);
        if ($mimetype !~ m/^text\//) {
                close $fd;
@@ -1738,9 +1738,9 @@ sub git_tree {
        }
        $/ = "\0";
        open my $fd, "-|", $GIT, "ls-tree", '-z', $hash
-               or die_error(undef, "Open git-ls-tree failed.");
+               or die_error(undef, "Open git-ls-tree failed");
        my @entries = map { chomp; $_ } <$fd>;
-       close $fd or die_error(undef, "Reading tree failed.");
+       close $fd or die_error(undef, "Reading tree failed");
        $/ = "\n";
 
        my $refs = read_info_ref();
@@ -1816,7 +1816,7 @@ sub git_log {
 
        my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
        open my $fd, "-|", $GIT, "rev-list", $limit, $hash
-               or die_error(undef, "Open git-rev-list failed.");
+               or die_error(undef, "Open git-rev-list failed");
        my @revlist = map { chomp; $_ } <$fd>;
        close $fd;
 
@@ -1877,7 +1877,7 @@ sub git_log {
 sub git_commit {
        my %co = git_read_commit($hash);
        if (!%co) {
-               die_error(undef, "Unknown commit object.");
+               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'});
@@ -1887,9 +1887,9 @@ sub git_commit {
                $parent = "--root";
        }
        open my $fd, "-|", $GIT, "diff-tree", '-r', '-M', $parent, $hash
-               or die_error(undef, "Open git-diff-tree failed.");
+               or die_error(undef, "Open git-diff-tree failed");
        my @difftree = map { chomp; $_ } <$fd>;
-       close $fd or die_error(undef, "Reading git-diff-tree failed.");
+       close $fd or die_error(undef, "Reading git-diff-tree failed");
 
        # non-textual hash id's can be cached
        my $expires;
@@ -2105,15 +2105,15 @@ sub git_commitdiff {
        mkdir($git_temp, 0700);
        my %co = git_read_commit($hash);
        if (!%co) {
-               die_error(undef, "Unknown commit object.");
+               die_error(undef, "Unknown commit object");
        }
        if (!defined $hash_parent) {
                $hash_parent = $co{'parent'};
        }
        open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_parent, $hash
-               or die_error(undef, "Open git-diff-tree failed.");
+               or die_error(undef, "Open git-diff-tree failed");
        my @difftree = map { chomp; $_ } <$fd>;
-       close $fd or die_error(undef, "Reading git-diff-tree failed.");
+       close $fd or die_error(undef, "Reading git-diff-tree failed");
 
        # non-textual hash id's can be cached
        my $expires;
@@ -2191,9 +2191,9 @@ sub git_commitdiff {
 sub git_commitdiff_plain {
        mkdir($git_temp, 0700);
        open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_parent, $hash
-               or die_error(undef, "Open git-diff-tree failed.");
+               or die_error(undef, "Open git-diff-tree failed");
        my @difftree = map { chomp; $_ } <$fd>;
-       close $fd or die_error(undef, "Reading diff-tree failed.");
+       close $fd or die_error(undef, "Reading diff-tree failed");
 
        # try to figure out the next tag after this commit
        my $tagname;
@@ -2251,7 +2251,7 @@ sub git_history {
        my $ftype;
        my %co = git_read_commit($hash_base);
        if (!%co) {
-               die_error(undef, "Unknown commit object.");
+               die_error(undef, "Unknown commit object");
        }
        my $refs = read_info_ref();
        git_header_html();
@@ -2309,14 +2309,14 @@ sub git_history {
 
 sub git_search {
        if (!defined $searchtext) {
-               die_error(undef, "Text field empty.");
+               die_error(undef, "Text field empty");
        }
        if (!defined $hash) {
                $hash = git_read_head($project);
        }
        my %co = git_read_commit($hash);
        if (!%co) {
-               die_error(undef, "Unknown commit object.");
+               die_error(undef, "Unknown commit object");
        }
        # pickaxe may take all resources of your box and run for several minutes
        # with every query - so decide by yourself how public you make this feature :)
@@ -2454,7 +2454,7 @@ sub git_shortlog {
 
        my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
        open my $fd, "-|", $GIT, "rev-list", $limit, $hash
-               or die_error(undef, "Open git-rev-list failed.");
+               or die_error(undef, "Open git-rev-list failed");
        my @revlist = map { chomp; $_ } <$fd>;
        close $fd;
 
@@ -2482,9 +2482,9 @@ ## 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)
-               or die_error(undef, "Open git-rev-list failed.");
+               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.");
+       close $fd or die_error(undef, "Reading git-rev-list failed");
        print $cgi->header(-type => 'text/xml', -charset => 'utf-8');
        print "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n".
              "<rss version=\"2.0\" xmlns:content=\"http://purl.org/rss/1.0/modules/content/\">\n";
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 7/9] gitweb: No error messages with unescaped/unprotected user input
  2006-08-05 10:51       ` [PATCH 0/9] gitweb: First patch corrected and split into separate patches Jakub Narebski
                           ` (5 preceding siblings ...)
  2006-08-05 11:13         ` [PATCH 6/9] gitweb: No periods for " Jakub Narebski
@ 2006-08-05 11:15         ` Jakub Narebski
  2006-08-05 11:16         ` [PATCH 8/9] gitweb: PATH_INFO=/ means no project Jakub Narebski
  2006-08-05 11:18         ` [PATCH 9/9] gitweb: Inline $rss_link Jakub Narebski
  8 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05 11:15 UTC (permalink / raw)
  To: git

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Probably some error messages with unescaped user input are left...

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index d8ba016..013bfe7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1265,7 +1265,7 @@ ## actions
 sub git_project_list {
        my $order = $cgi->param('o');
        if (defined $order && $order !~ m/project|descr|owner|age/) {
-               die_error(undef, "Invalid order parameter '$order'");
+               die_error(undef, "Unknown order parameter");
        }
 
        my @list = git_read_projects();
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 8/9] gitweb: PATH_INFO=/ means no project
  2006-08-05 10:51       ` [PATCH 0/9] gitweb: First patch corrected and split into separate patches Jakub Narebski
                           ` (6 preceding siblings ...)
  2006-08-05 11:15         ` [PATCH 7/9] gitweb: No error messages with unescaped/unprotected user input Jakub Narebski
@ 2006-08-05 11:16         ` Jakub Narebski
  2006-08-05 11:18         ` [PATCH 9/9] gitweb: Inline $rss_link Jakub Narebski
  8 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05 11:16 UTC (permalink / raw)
  To: git

Prepared for refactoring input validation.

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

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 013bfe7..2e1a2ba 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -86,8 +86,8 @@ if (defined $action) {
 }
 
 our $project = ($cgi->param('p') || $ENV{'PATH_INFO'});
-if (defined $project) {
-       $project =~ s|^/||; $project =~ s|/$||;
+$project =~ s|^/||; $project =~ s|/$||;
+if (defined $project && $project) {
        if (!validate_input($project)) {
                die_error(undef, "Invalid project parameter");
        }
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 9/9] gitweb: Inline $rss_link
  2006-08-05 10:51       ` [PATCH 0/9] gitweb: First patch corrected and split into separate patches Jakub Narebski
                           ` (7 preceding siblings ...)
  2006-08-05 11:16         ` [PATCH 8/9] gitweb: PATH_INFO=/ means no project Jakub Narebski
@ 2006-08-05 11:18         ` Jakub Narebski
  8 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05 11:18 UTC (permalink / raw)
  To: git

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Does anyone set $rss_link variable?

 gitweb/gitweb.perl |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 2e1a2ba..57f7198 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -21,7 +21,6 @@ our $cgi = new CGI;
 our $version = "++GIT_VERSION++";
 our $my_url = $cgi->url();
 our $my_uri = $cgi->url(-absolute => 1);
-our $rss_link = "";
 
 # core git executable to use
 # this can just be "git" if your webserver has a sensible PATH
@@ -97,8 +96,6 @@ if (defined $project && $project) {
        if (!(-e "$projectroot/$project/HEAD")) {
                die_error(undef, "No such project");
        }
-       $rss_link = "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\"" .
-                   "$my_uri?" . esc_param("p=$project;a=rss") . "\" type=\"application/rss+xml\"/>";
        $ENV{'GIT_DIR'} = "$projectroot/$project";
 } else {
        git_project_list();
@@ -862,11 +859,13 @@ sub git_header_html {
 <meta name="robots" content="index, nofollow"/>
 <title>$title</title>
 <link rel="stylesheet" type="text/css" href="$stylesheet"/>
-$rss_link
-</head>
-<body>
 EOF
-       print "<div class=\"page_header\">\n" .
+       print "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\"" .
+             "$my_uri?" . esc_param("p=$project;a=rss") . "\" type=\"application/rss+xml\"/>\n" .
+             "</head>\n";
+
+       print "<body>\n" .
+             "<div class=\"page_header\">\n" .
              "<a href=\"http://www.kernel.org/pub/software/scm/git/docs/\" title=\"git documentation\">" .
              "<img src=\"$logo\" width=\"72\" height=\"27\" alt=\"git\" style=\"float:right; border-width:0px;\"/>" .
              "</a>\n";
-- 
1.4.1.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 7/5] Merge changes in "split patch 1" series
  2006-08-04 22:36 [PATCH 0/5] Some further gitweb patches Jakub Narebski
                   ` (4 preceding siblings ...)
  2006-08-04 22:43 ` [PATCH 5/5] gitweb: Change appereance of marker of refs pointing to given object Jakub Narebski
@ 2006-08-05 11:42 ` Jakub Narebski
  2006-08-05 14:55   ` Johannes Schindelin
  5 siblings, 1 reply; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05 11:42 UTC (permalink / raw)
  To: git

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Strange that git-format-patch does not output merges...

 gitweb/gitweb.perl |   44 ++++++++++++++++++++++----------------------
 1 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4316bd0..bf1b10f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -67,16 +67,16 @@ our $git_version = qx($GIT --version) =~
 
 $projects_list ||= $projectroot;
 if (! -d $git_temp) {
-       mkdir($git_temp, 0700) || die_error("Couldn't mkdir $git_temp");
+       mkdir($git_temp, 0700) || die_error(undef, "Couldn't mkdir $git_temp");
 }
 
 
-# ======================================================================
-# input validation and dispatch
+## ======================================================================
+## input validation and dispatch
 our $action = $cgi->param('a');
 if (defined $action) {
        if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
-               die_error(undef, "Invalid action parameter $action");
+               die_error(undef, "Invalid action parameter");
        }
        # action which does not check rest of parameters
        if ($action eq "opml") {
@@ -87,15 +87,15 @@ if (defined $action) {
 
 our $project = ($cgi->param('p') || $ENV{'PATH_INFO'});
 $project =~ s|^/||; $project =~ s|/$||;
-if (defined $project || $project) {
+if (defined $project && $project) {
        if (!validate_input($project)) {
-               die_error(undef, "Invalid project parameter $project");
+               die_error(undef, "Invalid project parameter");
        }
        if (!(-d "$projectroot/$project")) {
-               die_error(undef, "No such directory $project");
+               die_error(undef, "No such directory");
        }
        if (!(-e "$projectroot/$project/HEAD")) {
-               die_error(undef, "No such project $project");
+               die_error(undef, "No such project");
        }
        $ENV{'GIT_DIR'} = "$projectroot/$project";
 } else {
@@ -106,42 +106,42 @@ if (defined $project || $project) {
 our $file_name = $cgi->param('f');
 if (defined $file_name) {
        if (!validate_input($file_name)) {
-               die_error(undef, "Invalid file parameter $file_name");
+               die_error(undef, "Invalid file parameter");
        }
 }
 
 our $hash = $cgi->param('h');
 if (defined $hash) {
        if (!validate_input($hash)) {
-               die_error(undef, "Invalid hash parameter $hash");
+               die_error(undef, "Invalid hash parameter");
        }
 }
 
 our $hash_parent = $cgi->param('hp');
 if (defined $hash_parent) {
        if (!validate_input($hash_parent)) {
-               die_error(undef, "Invalid hash parent parameter $hash_parent");
+               die_error(undef, "Invalid hash parent parameter");
        }
 }
 
 our $hash_base = $cgi->param('hb');
 if (defined $hash_base) {
        if (!validate_input($hash_base)) {
-               die_error(undef, "Invalid hash base parameter $hash_base");
+               die_error(undef, "Invalid hash base parameter");
        }
 }
 
 our $page = $cgi->param('pg');
 if (defined $page) {
        if ($page =~ m/[^0-9]$/) {
-               die_error(undef, "Invalid page parameter $page");
+               die_error(undef, "Invalid page parameter");
        }
 }
 
 our $searchtext = $cgi->param('s');
 if (defined $searchtext) {
        if ($searchtext =~ m/[^a-zA-Z0-9_\.\/\-\+\:\@ ]/) {
-               die_error(undef, "Invalid search parameter $searchtext");
+               die_error(undef, "Invalid search parameter");
        }
        $searchtext = quotemeta $searchtext;
 }
@@ -170,7 +170,7 @@ my %actions = (
 
 $action = 'summary' if (!defined($action));
 if (!defined($actions{$action})) {
-       die_error(undef, "Unknown action $action");
+       die_error(undef, "Unknown action");
 }
 $actions{$action}->();
 exit;
@@ -1282,7 +1282,7 @@ ## actions
 sub git_project_list {
        my $order = $cgi->param('o');
        if (defined $order && $order !~ m/project|descr|owner|age/) {
-               die_error(undef, "Invalid order parameter $order");
+               die_error(undef, "Unknown order parameter");
        }
 
        my @list = git_read_projects();
@@ -1479,7 +1479,7 @@ sub git_blame2 {
        my $fd;
        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);
+       die_error('404 Not Found', "File name not defined") if (!$file_name);
        $hash_base ||= git_get_head($project);
        die_error(undef, "Couldn't find base commit") unless ($hash_base);
        my %co = parse_commit($hash_base)
@@ -1539,12 +1539,12 @@ sub git_blame {
        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_get_head($project);
-       die_error(undef, "Couldn't find base commit.") unless ($hash_base);
+       die_error(undef, "Couldn't find base commit") unless ($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")
-                       or die_error(undef, "Error lookup file $file_name");
+                       or die_error(undef, "Error lookup file");
        }
        open ($fd, "-|", $GIT, "annotate", '-l', '-t', '-r', $file_name, $hash_base)
                or die_error(undef, "Open git-annotate failed");
@@ -1656,14 +1656,14 @@ sub git_blob_plain {
                if (defined $file_name) {
                        my $base = $hash_base || git_get_head($project);
                        $hash = git_get_hash_by_path($base, $file_name, "blob")
-                               or die_error(undef, "Error lookup file $file_name");
+                               or die_error(undef, "Error lookup file");
                } else {
                        die_error(undef, "No file name defined");
                }
        }
        my $type = shift;
        open my $fd, "-|", $GIT, "cat-file", "blob", $hash
-               or die_error("Couldn't cat $file_name, $hash");
+               or die_error(undef, "Couldn't cat $file_name, $hash");
 
        $type ||= blob_plain_mimetype($fd, $file_name);
 
@@ -1689,7 +1689,7 @@ sub git_blob {
                if (defined $file_name) {
                        my $base = $hash_base || git_get_head($project);
                        $hash = git_get_hash_by_path($base, $file_name, "blob")
-                               or die_error(undef, "Error lookup file $file_name");
+                               or die_error(undef, "Error lookup file");
                } else {
                        die_error(undef, "No file name defined");
                }
-- 
1.4.1.1
git diff --patch-with-stat

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 7/5] Merge changes in "split patch 1" series
  2006-08-05 11:42 ` [PATCH 7/5] Merge changes in "split patch 1" series Jakub Narebski
@ 2006-08-05 14:55   ` Johannes Schindelin
  2006-08-05 15:05     ` Jakub Narebski
  0 siblings, 1 reply; 26+ messages in thread
From: Johannes Schindelin @ 2006-08-05 14:55 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

Hi,

On Sat, 5 Aug 2006, Jakub Narebski wrote:

> Strange that git-format-patch does not output merges...

And what exactly should it output? patch against commit^1 or commit^2, 
etc.? Both? Combined diff?

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 7/5] Merge changes in "split patch 1" series
  2006-08-05 14:55   ` Johannes Schindelin
@ 2006-08-05 15:05     ` Jakub Narebski
  0 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05 15:05 UTC (permalink / raw)
  To: git

Johannes Schindelin wrote:
 
> On Sat, 5 Aug 2006, Jakub Narebski wrote:
> 
>> Strange that git-format-patch does not output merges...
> 
> And what exactly should it output? patch against commit^1 or commit^2, 
> etc.? Both? Combined diff?

In my case, commit^1. But I'd be satisfied if it would warn me about missing
commit (the merge was topmost commit).

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 6/9] gitweb: No periods for error messages
  2006-08-05 11:13         ` [PATCH 6/9] gitweb: No periods for " Jakub Narebski
@ 2006-08-05 15:55           ` Luben Tuikov
  2006-08-05 16:15             ` Jakub Narebski
  0 siblings, 1 reply; 26+ messages in thread
From: Luben Tuikov @ 2006-08-05 15:55 UTC (permalink / raw)
  To: Jakub Narebski, git

--- Jakub Narebski <jnareb@gmail.com> wrote:

> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> Like in Luben Tuikov patch

Good work!

I see this patch bears identical Subject line and
is logically identical as the one I posted.

Maybe you ment to include the above line above your
Signed-off-by: line?

    Luben


> 
>  gitweb/gitweb.perl |   92 ++++++++++++++++++++++++++--------------------------
>  1 files changed, 46 insertions(+), 46 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 8773a8d..d8ba016 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -76,7 +76,7 @@ # input validation and dispatch
>  our $action = $cgi->param('a');
>  if (defined $action) {
>         if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
> -               die_error(undef, "Invalid action parameter.");
> +               die_error(undef, "Invalid action parameter");
>         }
>         # action which does not check rest of parameters
>         if ($action eq "opml") {
> @@ -89,13 +89,13 @@ our $project = ($cgi->param('p') || $ENV
>  if (defined $project) {
>         $project =~ s|^/||; $project =~ s|/$||;
>         if (!validate_input($project)) {
> -               die_error(undef, "Invalid project parameter.");
> +               die_error(undef, "Invalid project parameter");
>         }
>         if (!(-d "$projectroot/$project")) {
> -               die_error(undef, "No such directory.");
> +               die_error(undef, "No such directory");
>         }
>         if (!(-e "$projectroot/$project/HEAD")) {
> -               die_error(undef, "No such project.");
> +               die_error(undef, "No such project");
>         }
>         $rss_link = "<link rel=\"alternate\" title=\"" . esc_param($project) . " log\" href=\""
> .
>                     "$my_uri?" . esc_param("p=$project;a=rss") . "\"
> type=\"application/rss+xml\"/>";
> @@ -108,42 +108,42 @@ if (defined $project) {
>  our $file_name = $cgi->param('f');
>  if (defined $file_name) {
>         if (!validate_input($file_name)) {
> -               die_error(undef, "Invalid file parameter.");
> +               die_error(undef, "Invalid file parameter");
>         }
>  }
>  
>  our $hash = $cgi->param('h');
>  if (defined $hash) {
>         if (!validate_input($hash)) {
> -               die_error(undef, "Invalid hash parameter.");
> +               die_error(undef, "Invalid hash parameter");
>         }
>  }
>  
>  our $hash_parent = $cgi->param('hp');
>  if (defined $hash_parent) {
>         if (!validate_input($hash_parent)) {
> -               die_error(undef, "Invalid hash parent parameter.");
> +               die_error(undef, "Invalid hash parent parameter");
>         }
>  }
>  
>  our $hash_base = $cgi->param('hb');
>  if (defined $hash_base) {
>         if (!validate_input($hash_base)) {
> -               die_error(undef, "Invalid hash base parameter.");
> +               die_error(undef, "Invalid hash base parameter");
>         }
>  }
>  
>  our $page = $cgi->param('pg');
>  if (defined $page) {
>         if ($page =~ m/[^0-9]$/) {
> -               die_error(undef, "Invalid page parameter.");
> +               die_error(undef, "Invalid page parameter");
>         }
>  }
>  
>  our $searchtext = $cgi->param('s');
>  if (defined $searchtext) {
>         if ($searchtext =~ m/[^a-zA-Z0-9_\.\/\-\+\:\@ ]/) {
> -               die_error(undef, "Invalid search parameter.");
> +               die_error(undef, "Invalid search parameter");
>         }
>         $searchtext = quotemeta $searchtext;
>  }
> @@ -172,7 +172,7 @@ my %actions = (
>  
>  $action = 'summary' if (!defined($action));
>  if (!defined($actions{$action})) {
> -       die_error(undef, "Unknown action.");
> +       die_error(undef, "Unknown action");
>  }
>  $actions{$action}->();
>  exit;
> @@ -418,7 +418,7 @@ sub git_get_hash_by_path {
>         my $tree = $base;
>  
>         open my $fd, "-|", $GIT, "ls-tree", $base, "--", $path
> -               or die_error(undef, "Open git-ls-tree failed.");
> +               or die_error(undef, "Open git-ls-tree failed");
>         my $line = <$fd>;
>         close $fd or return undef;
>  
> @@ -1265,13 +1265,13 @@ ## actions
>  sub git_project_list {
>         my $order = $cgi->param('o');
>         if (defined $order && $order !~ m/project|descr|owner|age/) {
> -               die_error(undef, "Invalid order parameter '$order'.");
> +               die_error(undef, "Invalid order parameter '$order'");
>         }
>  
>         my @list = git_read_projects();
>         my @projects;
>         if (!@list) {
> -               die_error(undef, "No projects found.");
> +               die_error(undef, "No projects found");
>         }
>         foreach my $pr (@list) {
>                 my $head = git_read_head($pr->{'path'});
> @@ -1405,7 +1405,7 @@ sub git_summary {
>               "</table>\n";
>  
>         open my $fd, "-|", $GIT, "rev-list", "--max-count=17", git_read_head($project)
> -               or die_error(undef, "Open git-rev-list failed.");
> +               or die_error(undef, "Open git-rev-list failed");
>         my @revlist = map { chomp; $_ } <$fd>;
>         close $fd;
>         git_header_div('shortlog');
> @@ -1461,10 +1461,10 @@ sub git_tag {
>  sub git_blame2 {
>         my $fd;
>         my $ftype;
> -       die_error(undef, "Permission denied.") if (!git_get_project_config_bool ('blame'));
> +       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);
> -       die_error(undef, "Couldn't find base commit.") unless ($hash_base);
> +       die_error(undef, "Couldn't find base commit") unless ($hash_base);
>         my %co = git_read_commit($hash_base)
>                 or die_error(undef, "Reading commit failed");
>         if (!defined $hash) {
> @@ -1476,7 +1476,7 @@ sub git_blame2 {
>                 die_error("400 Bad Request", "Object is not a blob");
>         }
>         open ($fd, "-|", $GIT, "blame", '-l', $file_name, $hash_base)
> -               or die_error(undef, "Open git-blame failed.");
> +               or die_error(undef, "Open git-blame failed");
>         git_header_html();
>         my $formats_nav =
>                 $cgi->a({-href => "$my_uri?" .
> esc_param("p=$project;a=blob;h=$hash;hb=$hash_base;f=$file_name")}, "blob") .
> @@ -1519,18 +1519,18 @@ sub git_blame2 {
>  
>  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);
> +       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);
> -       die_error(undef, "Couldn't find base commit.") unless ($hash_base);
> +       die_error(undef, "Couldn't find base commit") unless ($hash_base);
>         my %co = git_read_commit($hash_base)
> -               or die_error(undef, "Reading commit failed.");
> +               or die_error(undef, "Reading commit failed");
>         if (!defined $hash) {
>                 $hash = git_get_hash_by_path($hash_base, $file_name, "blob")
> -                       or die_error(undef, "Error lookup file.");
> +                       or die_error(undef, "Error lookup file");
>         }
>         open ($fd, "-|", $GIT, "annotate", '-l', '-t', '-r', $file_name, $hash_base)
> -               or die_error(undef, "Open git-annotate failed.");
> +               or die_error(undef, "Open git-annotate failed");
>         git_header_html();
>         my $formats_nav =
>                 $cgi->a({-href => "$my_uri?" .
> esc_param("p=$project;a=blob;h=$hash;hb=$hash_base;f=$file_name")}, "blob") .
> @@ -1640,9 +1640,9 @@ sub git_blob_plain {
>                 if (defined $file_name) {
>                         my $base = $hash_base || git_read_head($project);
>                         $hash = git_get_hash_by_path($base, $file_name, "blob")
> -                               or die_error(undef, "Error lookup file.");
> +                               or die_error(undef, "Error lookup file");
>                 } else {
> -                       die_error(undef, "No file name defined.");
> +                       die_error(undef, "No file name defined");
>                 }
>         }
>         my $type = shift;
> @@ -1673,14 +1673,14 @@ sub git_blob {
>                 if (defined $file_name) {
>                         my $base = $hash_base || git_read_head($project);
>                         $hash = git_get_hash_by_path($base, $file_name, "blob")
> -                               or die_error(undef, "Error lookup file.");
> +                               or die_error(undef, "Error lookup file");
>                 } else {
> -                       die_error(undef, "No file name defined.");
> +                       die_error(undef, "No file name defined");
>                 }
>         }
>         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.");
> +               or die_error(undef, "Couldn't cat $file_name, $hash");
>         my $mimetype = git_blob_plain_mimetype($fd, $file_name);
>         if ($mimetype !~ m/^text\//) {
>                 close $fd;
> @@ -1738,9 +1738,9 @@ sub git_tree {
>         }
>         $/ = "\0";
>         open my $fd, "-|", $GIT, "ls-tree", '-z', $hash
> -               or die_error(undef, "Open git-ls-tree failed.");
> +               or die_error(undef, "Open git-ls-tree failed");
>         my @entries = map { chomp; $_ } <$fd>;
> -       close $fd or die_error(undef, "Reading tree failed.");
> +       close $fd or die_error(undef, "Reading tree failed");
>         $/ = "\n";
>  
>         my $refs = read_info_ref();
> @@ -1816,7 +1816,7 @@ sub git_log {
>  
>         my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
>         open my $fd, "-|", $GIT, "rev-list", $limit, $hash
> -               or die_error(undef, "Open git-rev-list failed.");
> +               or die_error(undef, "Open git-rev-list failed");
>         my @revlist = map { chomp; $_ } <$fd>;
>         close $fd;
>  
> @@ -1877,7 +1877,7 @@ sub git_log {
>  sub git_commit {
>         my %co = git_read_commit($hash);
>         if (!%co) {
> -               die_error(undef, "Unknown commit object.");
> +               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'});
> @@ -1887,9 +1887,9 @@ sub git_commit {
>                 $parent = "--root";
>         }
>         open my $fd, "-|", $GIT, "diff-tree", '-r', '-M', $parent, $hash
> -               or die_error(undef, "Open git-diff-tree failed.");
> +               or die_error(undef, "Open git-diff-tree failed");
>         my @difftree = map { chomp; $_ } <$fd>;
> -       close $fd or die_error(undef, "Reading git-diff-tree failed.");
> +       close $fd or die_error(undef, "Reading git-diff-tree failed");
>  
>         # non-textual hash id's can be cached
>         my $expires;
> @@ -2105,15 +2105,15 @@ sub git_commitdiff {
>         mkdir($git_temp, 0700);
>         my %co = git_read_commit($hash);
>         if (!%co) {
> -               die_error(undef, "Unknown commit object.");
> +               die_error(undef, "Unknown commit object");
>         }
>         if (!defined $hash_parent) {
>                 $hash_parent = $co{'parent'};
>         }
>         open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_parent, $hash
> -               or die_error(undef, "Open git-diff-tree failed.");
> +               or die_error(undef, "Open git-diff-tree failed");
>         my @difftree = map { chomp; $_ } <$fd>;
> -       close $fd or die_error(undef, "Reading git-diff-tree failed.");
> +       close $fd or die_error(undef, "Reading git-diff-tree failed");
>  
>         # non-textual hash id's can be cached
>         my $expires;
> @@ -2191,9 +2191,9 @@ sub git_commitdiff {
>  sub git_commitdiff_plain {
>         mkdir($git_temp, 0700);
>         open my $fd, "-|", $GIT, "diff-tree", '-r', $hash_parent, $hash
> -               or die_error(undef, "Open git-diff-tree failed.");
> +               or die_error(undef, "Open git-diff-tree failed");
>         my @difftree = map { chomp; $_ } <$fd>;
> -       close $fd or die_error(undef, "Reading diff-tree failed.");
> +       close $fd or die_error(undef, "Reading diff-tree failed");
>  
>         # try to figure out the next tag after this commit
>         my $tagname;
> @@ -2251,7 +2251,7 @@ sub git_history {
>         my $ftype;
>         my %co = git_read_commit($hash_base);
>         if (!%co) {
> -               die_error(undef, "Unknown commit object.");
> +               die_error(undef, "Unknown commit object");
>         }
>         my $refs = read_info_ref();
>         git_header_html();
> @@ -2309,14 +2309,14 @@ sub git_history {
>  
>  sub git_search {
>         if (!defined $searchtext) {
> -               die_error(undef, "Text field empty.");
> +               die_error(undef, "Text field empty");
>         }
>         if (!defined $hash) {
>                 $hash = git_read_head($project);
>         }
>         my %co = git_read_commit($hash);
>         if (!%co) {
> -               die_error(undef, "Unknown commit object.");
> +               die_error(undef, "Unknown commit object");
>         }
>         # pickaxe may take all resources of your box and run for several minutes
>         # with every query - so decide by yourself how public you make this feature :)
> @@ -2454,7 +2454,7 @@ sub git_shortlog {
>  
>         my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
>         open my $fd, "-|", $GIT, "rev-list", $limit, $hash
> -               or die_error(undef, "Open git-rev-list failed.");
> +               or die_error(undef, "Open git-rev-list failed");
>         my @revlist = map { chomp; $_ } <$fd>;
>         close $fd;
>  
> @@ -2482,9 +2482,9 @@ ## 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)
> -               or die_error(undef, "Open git-rev-list failed.");
> +               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.");
> +       close $fd or die_error(undef, "Reading git-rev-list failed");
>         print $cgi->header(-type => 'text/xml', -charset => 'utf-8');
>         print "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n".
>               "<rss version=\"2.0\"
> xmlns:content=\"http://purl.org/rss/1.0/modules/content/\">\n";
> -- 
> 1.4.1.1
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 6/9] gitweb: No periods for error messages
  2006-08-05 15:55           ` Luben Tuikov
@ 2006-08-05 16:15             ` Jakub Narebski
  0 siblings, 0 replies; 26+ messages in thread
From: Jakub Narebski @ 2006-08-05 16:15 UTC (permalink / raw)
  To: git

Luben Tuikov wrote:

> --- Jakub Narebski <jnareb@gmail.com> wrote:
> 
>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
>> ---
>> Like in Luben Tuikov patch
> 
> Good work!
> 
> I see this patch bears identical Subject line and
> is logically identical as the one I posted.
> 
> Maybe you ment to include the above line above your
> Signed-off-by: line?

Probably should have just added you to Signed-off-by: line.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2006-08-05 16:15 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-08-04 22:36 [PATCH 0/5] Some further gitweb patches Jakub Narebski
2006-08-04 22:38 ` [PATCH 1/5] gitweb: Cleanup input validation and error messages Jakub Narebski
2006-08-04 23:54   ` Luben Tuikov
2006-08-05  0:02     ` [PATCH 6/5] gitweb: No periods for " Jakub Narebski
2006-08-04 23:54   ` [PATCH 1/5] gitweb: Cleanup input validation and " Luben Tuikov
2006-08-05  0:15   ` Junio C Hamano
2006-08-05  0:26     ` Jakub Narebski
2006-08-05 10:51       ` [PATCH 0/9] gitweb: First patch corrected and split into separate patches Jakub Narebski
2006-08-05 10:55         ` [PATCH 1/9] gitweb: Separate input validation and dispatch, add comment about opml action Jakub Narebski
2006-08-05 10:56         ` [PATCH 2/9] gitweb: die_error first (optional) parameter is HTTP status Jakub Narebski
2006-08-05 10:56         ` [PATCH 3/9] gitweb: Use undef for die_error to use default first (status) parameter value Jakub Narebski
2006-08-05 10:58         ` [PATCH 4/9] gitweb: Don't undefine query parameter related variables before die_error Jakub Narebski
2006-08-05 11:12         ` [PATCH 5/9] gitweb: Cleanup and uniquify error messages Jakub Narebski
2006-08-05 11:13         ` [PATCH 6/9] gitweb: No periods for " Jakub Narebski
2006-08-05 15:55           ` Luben Tuikov
2006-08-05 16:15             ` Jakub Narebski
2006-08-05 11:15         ` [PATCH 7/9] gitweb: No error messages with unescaped/unprotected user input Jakub Narebski
2006-08-05 11:16         ` [PATCH 8/9] gitweb: PATH_INFO=/ means no project Jakub Narebski
2006-08-05 11:18         ` [PATCH 9/9] gitweb: Inline $rss_link Jakub Narebski
2006-08-04 22:39 ` [PATCH 2/5] gitweb: Great subroutines renaming Jakub Narebski
2006-08-04 22:40 ` [PATCH 3/5] gitweb: Separate ref parsing in git_read_refs into parse_ref Jakub Narebski
2006-08-04 22:42 ` [PATCH 4/5] gitweb: git_heads cleanup Jakub Narebski
2006-08-04 22:43 ` [PATCH 5/5] gitweb: Change appereance of marker of refs pointing to given object Jakub Narebski
2006-08-05 11:42 ` [PATCH 7/5] Merge changes in "split patch 1" series Jakub Narebski
2006-08-05 14:55   ` Johannes Schindelin
2006-08-05 15:05     ` Jakub Narebski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).