Re: [PATCH] gitweb: make remote_heads config setting work.

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 9, 2012, 04:40 UTC
Message-ID: <7vk3tvqthw.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20121105235047.GA78156@redoubt.spodhuis.org>

Phil Pennock <phil@apcera.com> writes:

Show 8 quoted lines

> @@ -2702,6 +2702,7 @@ sub git_get_project_config {
>  		$key = lc($key);
>  	}
>  	$key =~ s/^gitweb\.//;
> +	$key =~ s/_//g;
>  	return if ($key =~ m/\W/);
>  
>  	# type sanity check

The idea to strip "_" from "remote_heads" to create "remoteheads" is fine, but I am not sure if the implementation is good.

Looking at the code before this part:

	if (my ($hi, $mi, $lo) = ($key =~ /^([^.]*)\.(.*)\.([^.]*)$/)) {
		$key = join(".", lc($hi), $mi, lc($lo));
	} else {
		$key = lc($key);
	}
	$key =~ s/^gitweb\.//;
	return if ($key =~ m/\W/);

the new code is munding the $hi and $mi parts, while the mistaken configuration this patch is trying to correct is about the $lo part, and possibly the $hi part, but never the $mi part.

It is expected that $hi will always be gitweb, and I suspect that there may not be any "per something" configuration variable (e.g. "gitweb.master.blame" may be used to override the default value given by "gitweb.blame" only for the master branch), that uses $mi part, so it might not matter to munge the entire $key like this patch does with the current code.

But that makes it even more important to get this part right _now_; otherwise, it is likely that this new code will introduce a bug to a future patch that adds "per something" configuration support.