[PATCH v2 1/3] gitweb: Cleanup git_print_log()

[PATCH v2 1/3] gitweb: Cleanup git_print_log()

[Namhyung Kim]

If $signoff set to 1, the $line would be handled in
the if statement for the both cases. So the outer of
the conditional always sees the $signoff always set
to 0 and no need to check it. Thus we can finally get
rid of it.

Also rename $empty to more clear $skip_blank_line as
suggested by Junio.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 gitweb/gitweb.perl |   21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 55e0e9e..82c5da7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4484,30 +4484,23 @@ sub git_print_log {
 	}
 
 	# print log
-	my $signoff = 0;
-	my $empty = 0;
+	my $skip_blank_line = 0;
 	foreach my $line (@$log) {
 		if ($line =~ m/^ *(signed[ \-]off[ \-]by[ :]|acked[ \-]by[ :]|cc[ :])/i) {
-			$signoff = 1;
-			$empty = 0;
 			if (! $opts{'-remove_signoff'}) {
 				print "<span class=\"signoff\">" . esc_html($line) . "</span><br/>\n";
-				next;
-			} else {
-				# remove signoff lines
-				next;
+				$skip_blank_line = 1;
 			}
-		} else {
-			$signoff = 0;
+			next;
 		}
 
 		# print only one empty line
 		# do not print empty line after signoff
 		if ($line eq "") {
-			next if ($empty || $signoff);
-			$empty = 1;
+			next if ($skip_blank_line);
+			$skip_blank_line = 1;
 		} else {
-			$empty = 0;
+			$skip_blank_line = 0;
 		}
 
 		print format_log_line_html($line) . "<br/>\n";
@@ -4515,7 +4508,7 @@ sub git_print_log {
 
 	if ($opts{'-final_empty_line'}) {
 		# end with single empty line
-		print "<br/>\n" unless $empty;
+		print "<br/>\n" unless $skip_blank_line;
 	}
 }
 
-- 
1.7.10.4

[PATCH v2 2/3] gitweb: Handle other types of tag in git_print_log

[Namhyung Kim]

There are many types of tags used in S-o-b area [1].
Update the regex to handle them properly. It requires
the tag should be started by a capital letter and ended
by '-by: ' or '-By: '. The only exception is 'Cc: '.

[1] http://lwn.net/Articles/503829/

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 gitweb/gitweb.perl |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 82c5da7..362784d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4486,7 +4486,7 @@ sub git_print_log {
 	# print log
 	my $skip_blank_line = 0;
 	foreach my $line (@$log) {
-		if ($line =~ m/^ *(signed[ \-]off[ \-]by[ :]|acked[ \-]by[ :]|cc[ :])/i) {
+		if ($line =~ m/^\s*([A-Z][-A-Za-z]*-[Bb]y|C[Cc]): /) {
 			if (! $opts{'-remove_signoff'}) {
 				print "<span class=\"signoff\">" . esc_html($line) . "</span><br/>\n";
 				$skip_blank_line = 1;
-- 
1.7.10.4

[PATCH v2 3/3] gitweb: Add support to Link: tag

[Namhyung Kim]

The tip tree is the one of major subsystem tree in the
Linux kernel project. On the tip tree, the Link: (or
similar Buglink:) tag is used for tracking the original
discussion or context. Since it's ususally in the S-o-b
area, it'd be better using same style with others.

Also as it tends to contain a message-id sent from git
send-email, a part of the line would set a wrong hyperlink
like [1]. Fix it by not using format_log_line_html().

[1] git.kernel.org/?p=linux/kernel/git/tip/tip.git;a=commit;h=08942f6d5d992e9486b07653fd87ea8182a22fa0

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 gitweb/gitweb.perl |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 362784d..3d6a705 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4494,6 +4494,16 @@ sub git_print_log {
 			next;
 		}
 
+		if ($line =~ m,\s*([a-z]*link): (https?://\S+),i) {
+			if (! $opts{'-remove_signoff'}) {
+				print "<span class=\"signoff\">" . esc_html($1) . ": " .
+					"<a href=\"" . esc_html($2) . "\">" . esc_html($2) . "</a>" .
+					"</span><br/>\n";
+				$skip_blank_line = 1;
+			}
+			next;
+		}
+
 		# print only one empty line
 		# do not print empty line after signoff
 		if ($line eq "") {
-- 
1.7.10.4

Re: [PATCH v2 1/3] gitweb: Cleanup git_print_log()

[Junio C Hamano]

Namhyung Kim <namhyung@kernel.org> writes:

> If $signoff set to 1, the $line would be handled in
> the if statement for the both cases. So the outer of
> the conditional always sees the $signoff always set
> to 0 and no need to check it. Thus we can finally get
> rid of it.
>
> Also rename $empty to more clear $skip_blank_line as
> suggested by Junio.

Thanks.  It does not make it clear that you fixed a bug in the
original, so let me rephrase it like so:

-- >8 --
From: Namhyung Kim <namhyung@kernel.org>
Date: Wed, 4 Jul 2012 11:47:24 +0900
Subject: [PATCH] gitweb: Cleanup git_print_log()

When we see a signed-off-by line (and its friends), we set $signoff
to true, but then we process the next line after we are done without
giving control to the rest of the loop.  And when the line we saw is
not a signed-off-by line, we reset $signoff to false before running
the remainder of the loop.

Hence, the check for $signoff that attempts to remove an extra empty
line between two signed-off-by line was not doing anything useful.

Rename $empty to a more explicit name $skip_blank_line to tell us to
skip a blank line when we see one, set it after we see and emit a
blank line (to avoid showing more than one empty lines in a raw) or
after we handle a signed-off-by line (to avoid empty lines after
such a line), to fix this bug, and get rid of the $signoff variable
that is not useful.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Re: [PATCH v2 3/3] gitweb: Add support to Link: tag

[Junio C Hamano]

Namhyung Kim <namhyung@kernel.org> writes:

> The tip tree is the one of major subsystem tree in the
> Linux kernel project. On the tip tree, the Link: (or
> similar Buglink:) tag is used for tracking the original
> discussion or context. Since it's ususally in the S-o-b
> area, it'd be better using same style with others.
>
> Also as it tends to contain a message-id sent from git
> send-email, a part of the line would set a wrong hyperlink
> like [1]. Fix it by not using format_log_line_html().
>
> [1] git.kernel.org/?p=linux/kernel/git/tip/tip.git;a=commit;h=08942f6d5d992e9486b07653fd87ea8182a22fa0
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  gitweb/gitweb.perl |   10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 362784d..3d6a705 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -4494,6 +4494,16 @@ sub git_print_log {
>  			next;
>  		}
>  
> +		if ($line =~ m,\s*([a-z]*link): (https?://\S+),i) {
> +			if (! $opts{'-remove_signoff'}) {
> +				print "<span class=\"signoff\">" . esc_html($1) . ": " .
> +					"<a href=\"" . esc_html($2) . "\">" . esc_html($2) . "</a>" .
> +					"</span><br/>\n";

Thanks.  Is the first use of esc_html($2) correct (I am always
confused between esc_html, esc_param and esc_url)?

> +				$skip_blank_line = 1;
> +			}
> +			next;
> +		}
> +
>  		# print only one empty line
>  		# do not print empty line after signoff
>  		if ($line eq "") {