Re: [RFC] gitweb: Add committags support (take 2)

From: Junio C Hamano <junkio@cox.net>
Date: Dec 4, 2006, 02:38 UTC
Message-ID: <7virgstmg6.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <200612040001.13640.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

Show 7 quoted lines

> The subroutine should return either reference to scalar which means
> "do not process", scalar which means changed available for further
> processing, or void (undef) which means no change. In [PATCH 2/3] we
> will enable returning also list of elements, each of which could be
> reference to scalar or scalar (for example signoff would return three
> element list: opening span element as ref, signoff text as scalar,
> closing span element as ref).

Personally I think that "list of elements" should be in the first patch to build the foundation.

Show 13 quoted lines

>   our %committags = (
>   	'sha1' => {
>   		'pattern' => qr/[0-9a-fA-F]{40}/,
>   		'sub' => sub {
>   			my $hash_text = shift;
>   			my $type = git_get_type($hash_text);
>   			if ($type) {
>   				return \$cgi->a({-href => href(action=>$type, hash=>$hash_text),
>   				                -class => "text"}, $hash_text);
>   			}
>   			return undef;
>   		},
>   	},

It might make sense to do a /[0-9a-f]{8,40}/ pattern, and make sure that the named object exists in the sub (which you already do). People often write abbreviated commit object name that has a high chance of staying unique during the life of the project.

>   	'mantis' => {
>   		'pattern' => qr/(BUG|FEATURE)\(\d+\)/,
>   	'bugzilla' => {
>   		'pattern' => qr/bug\s+\d+/,

This is not wrong per-se but somehow feels funny. It feels as if there is a convention that bugzilla bugs are usually spelled in lowercase while mantis bugs are in uppercase, but I do not think you are suggesting that.

I do not know how this %committags{} is used per project. With a setting like repo.or.cz, it is likely that one instance of gitweb is serving unrelated projects that have their issue tracker at different locations using different "committags" convention. Is the idea to eventually allow enabling/disabling elements from the global %committags per repository somehow (perhaps not just enable/disable but even overriding patterns or parameters)?

> 3. To not split message into many fragments we concatenate strings
> if possible.

I do not know why "avoiding splits" is needed, if it raises issues that you need to ask the list about in a message like this...