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

Frank Lichtenheld [off-list ref] writes:

On Sat, May 12, 2007 at 12:59:49PM -0700, Junio C Hamano wrote:

quoted

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>

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?

Change the configuration parser so that it ignores
everything except for ^gitcvs.((ext|pserver).)?
This greatly reduces the risk of failing while
parsing some unknown and irrelevant config option.

The bug that triggered this change was that the
parsing doesn't handle sections that have a
subsection and a variable with the same name.

While this bug still remains, all remaining
causes can be attributed to user error, since
there are no defined variables gitcvs.ext and
gitcvs.pserver.

Signed-off-by: Frank Lichtenheld <redacted>
---
 git-cvsserver.perl |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 3e7bf5b..a07c725 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl

@@ -183,9 +183,9 @@ sub req_Root
     }
     foreach my $line ( @gitvars )
     {
-        next unless ( $line =~ /^(.*?)\.(.*?)(?:\.(.*?))?=(.*)$/ );
-        unless ($3) {
-            $cfg->{$1}{$2} = $4;
+        next unless ( $line =~ /^(gitcvs)\.(?:(ext|pserver)\.)?([\w-]+)=(.*)$/ );
+        unless ($2) {
+            $cfg->{$1}{$3} = $4;
         } else {
             $cfg->{$1}{$2}{$3} = $4;
         }

-- 
1.5.1.4