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

From: Junio C Hamano <junkio@cox.net>
Date: May 12, 2007, 22:43 UTC
Message-ID: <7v3b21wtlf.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20070512213153.GC7184@planck.djpig.de>

Frank Lichtenheld <frank@lichtenheld.de> writes:

Show 10 quoted lines

> On Sat, May 12, 2007 at 12:59:49PM -0700, Junio C Hamano wrote:
>> 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.
>
> Fair enough. So far I see three very minimal solutions, but I can't
> decide which one is the least ugly:
>
> (For all we can begin by limiting the used variables to
> ^gitcvs.((ext|pserver).)? )

That sounds sensible.  And ignore anything that do not match.

> 1) Drop variables named gitcvs.ext and gitcvs.pserver manually

I do not see any need for this; gitcvs.ext or gitcvs.pserver as variables do not exist, at least right now. The breakage was purely that the old parser tried to parse things it does not even know about (e.g. diff.color) without knowing the rules there.

> 2) Use the complete variable name as key to the hash instead of
>    using a hash of hashes of hashes
>    { "diff.color => "auto",
>      "diff.color.whitespace" => "blue reverse" }

No need for this nor the next one either. You understand only gitcvs.<option> or gitcvs.<method>.<option>, and you know there is no string that is common in <option> and <method>

Show 7 quoted lines

> 3) Make the second level always a hash, instead of using a string
>    directly, so that Junio's example would look like this
>    { diff => { color => { value => "auto",
>    			  whitespace => "blue reverse" } } }
>
>
> Opinions?