Re: [PATCH 2/3] gitweb: Handle a few other tags in git_print_log

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 3, 2012, 20:05 UTC
Message-ID: <7v3958vcep.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1341295377-22083-3-git-send-email-namhyung@kernel.org>

Namhyung Kim <namhyung@kernel.org> writes:

Show 17 quoted lines

> There are many of tags used in s-o-b area. Add
> support for a few of well-known ones.
>
> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> ---
>  gitweb/gitweb.perl |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 7585e08..e0701af 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -4485,8 +4485,9 @@ sub git_print_log {
>  
>  	# print log
>  	my $empty = 0;
> +	my $tags = "acked|reviewed|reported|tested|suggested"

Missing ';' at the end.

>  	foreach my $line (@$log) {
> -		if ($line =~ m/^ *(signed[ \-]off[ \-]by[ :]|acked[ \-]by[ :]|cc[ :])/i) {
> +		if ($line =~ m/^ *(signed[ \-]off[ \-]by[ :]|($tags)[ \-]by[ :]|cc[ :])/i) {

Is anybody actually helped by these spaces that make the regexp unnecessarily cluttered?

I am very tempted to suggest doing something like this:

	my $tags = join('|', qw(signed-off acked reviewed reported tested suggested));
	for my $line (@$log) {
		if ($line =~ m/^\s*(?:(?:$tags)-by|cc):/i) {
			...

or even this:

	for my $line (@$log) {
		if ($line =~ m/^\s*(?:[a-z][-a-z]*[a-z]): /i) {
			...