Re: [PATCH] perl: add new module Git::Config for cached 'git config' access
- From
Sam Vilain <sam.vilain@catalyst.net.nz>
- Date
- Apr 8, 2009, 05:49 UTC
- Message-ID
- <49DC3ADD.5000902@catalyst.net.nz>
- In-Reply-To
- <m3prfo1xh6.fsf@localhost.localdomain>
Jakub Narebski wrote:
Show 9 quoted lines
>> - 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.
Show 16 quoted lines
> 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