Re: [PATCH] cvsserver: Complete rewrite of the configuration parser

From: Junio C Hamano <junkio@cox.net>
Date: May 12, 2007, 19:59 UTC
Message-ID: <7v4pmhyfre.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <11789982521112-git-send-email-frank@lichtenheld.de>

Frank Lichtenheld <frank@lichtenheld.de> writes:

Show 8 quoted lines

>  Maybe a bit overkill if one only wants to solve the problem Junio discovered
>  but I believe it's still worthwile.
>
>  Has a lot of overlap with perl/Git.pm though...
>
>  Not extensively tested but it at least passes the test cases and creates a useful
>  log which should take care of the two main code paths (get_gitcvs and
>  get_gitcvs_bool).

I agree that the general direction should be to do something like this in perl/Git.pm (Pasky CC'ed). As there are some things that current Git.pm config interface does not offer an easy access to what you would want to do, we need to enumerate what you need, decide if they are of general interest and design what to put in Git.pm and what to implement in GITCVS::config as a special-purpose feature.

perl/Git.pm currently gives us only this:

 - grab all values for a named variable, in an array;

 - return canonicalized value for a named boolean variable;

GITCVS::config wants to read *everything* from config and returns a 'config' instance you can:

 - enumerate keys (you do not have this, but it is easy to add);

 - retrieve a value for a key (either one- or two-level);

 - retrieve a canonicalized bool value for a key (either one- or
   two-level);

 - treat a request for "gitcvs.method.option" variable to fall
   back on "gitcvs.option" if the former is not given;

 - the same for boolean variant.

I think the best abstraction is to have the "read everything" interface in perl/Git.pm side, make the current Git::config() and Git::config_bool() interface to use it (without issuing extra 'git config --get-all'). I am not sure it is common for Git.pm users to want the behaviour of "section.method.option" falling back to "section.option", but if it is common enough, it probably is a good idea to have:

	sub config_fallback {
        	my ($self, $section, $specific, $var) = @_;
                my $cfg = $self->config();

                if (exists $cfg{"$section.$specific.$var"}) {
                	return $cfg{"$section.$specific.$var"};
		}
                if (exists $cfg{"$section.$var"}) {
                	return $cfg{"$section.$var"};
		}
                return undef;
	}

on perl/Git.pm side.

But all of this is post 1.5.2 material; we would want to have a minimal fixup on 'master' before 1.5.2, independent of this rewrite.