From: Sam Vilain <sam.vilain@catalyst.net.nz>
To: Jakub Narebski <jnareb@gmail.com>
Cc: Frank Lichtenheld <frank@lichtenheld.de>,
git@vger.kernel.org, Petr Baudis <pasky@suse.cz>
Subject: Re: [PATCH] perl: add new module Git::Config for cached 'git config' access
Date: Wed, 08 Apr 2009 17:49:17 +1200 [thread overview]
Message-ID: <49DC3ADD.5000902@catalyst.net.nz> (raw)
In-Reply-To: <m3prfo1xh6.fsf@localhost.localdomain>
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
next prev parent reply other threads:[~2009-04-08 6:33 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-04-05 23:46 [PATCH] perl: add new module Git::Config for cached 'git config' access Sam Vilain
2009-04-05 23:46 ` [PATCH] perl: make Git.pm use new Git::Config module Sam Vilain
2009-04-06 9:29 ` [PATCH] perl: add new module Git::Config for cached 'git config' access Frank Lichtenheld
2009-04-06 22:50 ` Sam Vilain
2009-04-07 12:01 ` Jakub Narebski
2009-04-08 5:49 ` Sam Vilain [this message]
2009-04-08 10:18 ` Jakub Narebski
2009-04-08 10:44 ` Sam Vilain
2009-04-08 23:13 ` Jakub Narebski
2009-04-08 6:29 ` Junio C Hamano
2009-04-08 9:50 ` Sam Vilain
2009-04-08 18:51 ` Junio C Hamano
2009-04-08 8:12 ` Petr Baudis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=49DC3ADD.5000902@catalyst.net.nz \
--to=sam.vilain@catalyst.net.nz \
--cc=frank@lichtenheld.de \
--cc=git@vger.kernel.org \
--cc=jnareb@gmail.com \
--cc=pasky@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).