Re: [PATCH] perl: add new module Git::Config for cached 'git config' access

[Sam Vilain <sam.vilain@catalyst.net.nz>]

Jakub Narebski wrote:
>> -		my ($item, $value) = m{(.*?)=(.*)};
>> +		my ($item, $value) = m{(.*?)\n((?s:.*))\0}
>> +			or die "failed to parse it; \$_='$_'";
> 
> Errr... wouldn't it be better to simply use 
> 
> +		my ($item, $value) = split("\n", $_, 2)
> 
> here?

Yeah, I guess that's easier to read and possibly faster; both are
using the regexp engine and using COW strings though, so it's probably
not as bad as one might think.

> Have you tested Git::Config with a "null" value, i.e. something
> like
> 
>     [section]
>         noval
> 
> in the config file (which evaluates to 'true' with '--bool' option)?
> Because from what I remember from the discussion on the 
> "git config --null --list" format the lack of "\n" is used to
> distinguish between noval (which is equivalent to 'true'), and empty
> value (which is equivalent to 'false')
> 
>     [boolean]
>         noval        # equivalent to 'true'
>         empty1 =     # equivalent to 'false'
>         empty2 = ""  # equivalent to 'false'

That I didn't consider.  Below is a patch for this.  Any more gremlins?

Subject: perl: fix no value items in Git::Config

When interpreted as boolean, items in the configuration which do not
have an '=' are interpreted as true.  Parse for this situation, and
represent it with an object in the state hash which works a bit like
undef, but isn't.  Various internal tests that items were multiple
values with ref() must become stricter.  Reported by Jakub Narebski.
Sneak a couple of vim footer changes in too.

Signed-off-by: Sam Vilain <sam@vilain.net>
---
  pull 'perl-Config' from git://github.com/samv/git for a rebased
  version without the vim footer changes, and with cleaner whitespace.

 perl/Git/Config.pm |   36 +++++++++++++++++++++++++++---------
 t/t9700/config.t   |   22 +++++++++++++++++++++-
 2 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/perl/Git/Config.pm b/perl/Git/Config.pm
index a35d9f3..6b4d928 100644
--- a/perl/Git/Config.pm
+++ b/perl/Git/Config.pm
@@ -144,7 +144,7 @@ sub _config {
 	}
 
 	if (defined wantarray) {
-		my @values = ref $state ? @$state :
+		my @values = ref($state) eq "ARRAY" ? @$state :
 			defined $state ? ($state) : ();
 
 		if ( my $type = $self->type( $item ) ) {
@@ -171,6 +171,8 @@ Reads the current state of the configuration file.
 
 =cut
 
+our $NOVALUE = bless [__PACKAGE__."/NOVALUE"], "Git::Config::novalue";
+
 sub read {
 	my $self = shift;
 	my $which = shift;
@@ -185,13 +187,19 @@ sub read {
 
 	local($/)="\0";
 	while (<$fh>) {
-		my ($item, $value) = m{(.*?)\n((?s:.*))\0}
-			or die "failed to parse it; \$_='$_'";
+		my ($item, $value) = split "\n", $_, 2;
+		if (defined $value) {
+			chop($value);
+		} else {
+			chop($item);
+			$value = $NOVALUE;
+		}
+		my $exists = exists $read_state->{$item};
 		my $sl = \( $read_state->{$item} );
-		if (!defined $$sl) {
+		if (!$exists) {
 			$$sl = $value;
 		}
-		elsif (!ref $$sl) {
+		elsif (!ref $$sl or ref $$sl ne "ARRAY") {
 			$$sl = [ $$sl, $value ];
 		}
 		else {
@@ -325,7 +333,7 @@ sub _write {
 		if ($type ne "string") {
 			push @cmd, "--$type";
 		}
-		if (ref $value) {
+		if (ref $value eq "ARRAY") {
 			$git->command_oneline (
 				"config", @cmd, "--replace-all",
 				 $item, $value->[0],
@@ -378,7 +386,7 @@ sub thaw {
 {
 	package Git::Config::string;
 	sub freeze { shift }
-	sub thaw   { shift }
+	sub thaw   { (shift)."" }
 }
 {
 	package Git::Config::integer;
@@ -408,6 +416,15 @@ sub thaw {
 	}
 }
 {
+	package Git::Config::novalue;
+	sub as_string { "" }
+	sub as_num    {  0 }
+	use overload
+		'""' => \&as_string,
+		'0+' => \&as_num,
+		fallback => 1;
+}
+{
 	package Git::Config::boolean;
 	our @true = qw(true yes 1);
 	our @false = qw(false no 0);
@@ -424,7 +441,8 @@ sub thaw {
 	}
 	sub thaw {
 		my $val = shift;
-		if ($val =~ m{$true_re}) {
+		if (eval{$val->isa("Git::Config::novalue")}
+			    or $val =~ m{$true_re}) {
 			1;
 		}
 		elsif ($val =~ m{$false_re}) {
@@ -464,4 +482,4 @@ Perl Artistic License 2.0 or later, or the GPL v2 or later.
 #   cperl-indent-wrt-brace: nil
 # End:
 #
-# vim: vim:tw=78:sts=0:noet
+# vim: tw=78:sts=0:noet
diff --git a/t/t9700/config.t b/t/t9700/config.t
index f0f7d2d..9d7860f 100644
--- a/t/t9700/config.t
+++ b/t/t9700/config.t
@@ -17,6 +17,14 @@ in_empty_repo sub {
 	$git->command_oneline("config", "foo.false.val", "false");
 	$git->command_oneline("config", "foo.true.val", "yes");
 	$git->command_oneline("config", "multiline.val", "hello\nmultiline.val=world");
+	open(CONFIG, ">>.git/config") or die $!;
+	print CONFIG <<CONF;
+[boolean]
+   noval
+   empty1 =
+   empty2 = ""
+CONF
+	close CONFIG;
 
 	my $conf = Git::Config->new();
 	ok($conf, "constructed a new Git::Config");
@@ -100,6 +108,18 @@ in_empty_repo sub {
 	$git->command_oneline("config", "foo.falseval", "false");
 	$git->command_oneline("config", "foo.trueval", "on");
 
+	is($conf->config("boolean.noval"), "", "noval: string");
+	is($conf->config("boolean.empty1"), "", "empty1: string");
+	is($conf->config("boolean.empty2"), "", "empty2: string");
+
+	$conf->type("boolean.*" => "boolean");
+
+	ok($conf->config("boolean.noval"), "noval: boolean");
+	eval{my $x = $conf->config("boolean.empty1")};
+	ok($@, "empty1: boolean");
+	eval{my $x = $conf->config("boolean.empty2")};
+	ok($@, "empty2: boolean");
+
 	SKIP:{
 		if (eval {
 			$git->command(
@@ -128,4 +148,4 @@ in_empty_repo sub {
 #   cperl-indent-wrt-brace: nil
 # End:
 #
-# vim: vim:tw=78:sts=0:noet
+# vim: tw=78:sts=0:noet
-- 
1.6.0