{"thread":{"id":"18739","subject":"[PATCH] perl: make Git.pm use new Git::Config module","startedAt":"2009-04-05T23:46:15Z","lastAt":"2009-04-08T23:13:53Z","messageCount":13,"participants":["Sam Vilain","Frank Lichtenheld","Jakub Narebski","Junio C Hamano","Petr Baudis"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"110492","messageId":"1238975176-14354-1-git-send-email-sam.vilain@catalyst.net.nz","threadId":"18739","inReplyTo":null,"subject":"[PATCH] perl: add new module Git::Config for cached 'git config' access","fromName":"Sam Vilain","fromEmail":"sam.vilain@catalyst.net.nz","sentAt":"2009-04-05T23:46:15Z","receivedAt":"2009-04-05T23:46:15Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"Add a new module, Git::Config, for a better Git configuration API.\n\nSigned-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>\n---\n perl/Git/Config.pm   |  465 ++++++++++++++++++++++++++++++++++++++++++++++++++\n perl/Makefile.PL     |    5 +-\n t/t9700-perl-git.sh  |    8 +\n t/t9700/TestUtils.pm |   77 +++++++++\n t/t9700/config.t     |  127 ++++++++++++++\n 5 files changed, 681 insertions(+), 1 deletions(-)\n create mode 100644 perl/Git/Config.pm\n create mode 100644 t/t9700/TestUtils.pm\n create mode 100644 t/t9700/config.t\n\ndiff --git a/perl/Git/Config.pm b/perl/Git/Config.pm\nnew file mode 100644\nindex 0000000..a0a6a41\n--- /dev/null\n+++ b/perl/Git/Config.pm\n@@ -0,0 +1,465 @@\n+\n+package Git::Config;\n+\n+use 5.006;\n+use strict;\n+use warnings;\n+\n+use Git;\n+use Error qw(:try);\n+\n+=head1 NAME\n+\n+Git::Config - caching interface to git-config state\n+\n+=head1 SYNOPSIS\n+\n+  use Git::Config;\n+\n+  my $conf = Git::Config->new( $git );\n+\n+  # return single config items\n+  my $value = $conf->config( VARIABLE );\n+\n+  # return multi-config items\n+  my @values = $conf->config( VARIABLE );\n+\n+  # manipulate type of slot, for special interpretation\n+  $conf->type( VARIABLE => $type );\n+\n+  # change value\n+  $conf->config( VARIABLE => $value );\n+\n+  # read or configure a global or system variable\n+  $conf->global( VARIABLE => $value );\n+  $conf->system( VARIABLE => $value );\n+\n+  # default is to autoflush \n+  $conf->autoflush( 0 );\n+\n+  # if autoflush is disabled, flush explicitly.\n+  $conf->flush;\n+\n+  # update cache\n+  $conf->read;\n+\n+=head1 DESCRIPTION\n+\n+This module provides a cached interface to the B<git config> command\n+in Perl.\n+\n+=head1 CONSTRUCTION\n+\n+=head2 B<Git::Config-E<gt>new( I<$git> )>\n+\n+Creates a new B<Git::Config> object.  Does not read the configuration\n+file yet.\n+\n+=cut\n+\n+sub new {\n+\tmy $class = shift;\n+\tmy $git = shift || Git->repository;\n+\tmy $self = bless { git => $git }, $class;\n+\twhile ( my ($item, $value) = splice @_, 0, 2 ) {\n+\t\ttry {\n+\t\t\t$self->$item($value);\n+\t\t}\n+\t\tcatch {\n+\t\t\tthrow Error::Simple (\n+\t\t\t\t\"Invalid constructor arg '$item'\",\n+\t\t\t       );\n+\t\t};\n+\t}\n+\treturn $self;\n+}\n+\n+=head1 METHODS\n+\n+=head2 B<$conf-E<gt>config( C<item> )>\n+\n+Reads the value of a particular configuration item.\n+When called in B<list context>, returns all values of multiple value\n+items.\n+When called in B<scalar context>, raises an error if the item is\n+specified multiple times.\n+\n+=head2 B<$conf-E<gt>config( C<item> =E<gt> I<$value> )>\n+\n+Sets the value of an item.\n+If an B<array reference> is specified, the entire list of items is\n+replaced with the values specified.\n+If a single item is passed but the item is specified multiple times in\n+the configuration file, raises an error.\n+Returns the old value.\n+\n+=cut\n+\n+sub config {\n+\treturn $_[0]->_config(\"\", @_[1..$#_]);\n+}\n+\n+sub _config {\n+\tmy $self = shift;\n+\tmy $which = shift;\n+\tmy $item = shift;\n+\tmy $value_passed = @_;\n+\tmy $value = shift;\n+\n+\tmy $type = $self->type( $item );\n+\n+\tmy $_which = $which ? \"${which}_\" : \"\";\n+\tmy $_read = \"read_${_which}state\";\n+\tmy $_state = \"${_which}state\";\n+\n+\tif (!$self->{$_read}) {\n+\t\t$self->read($which);\n+\t}\n+\n+\tmy $state;\n+\tif (exists $self->{$_state}{$item}) {\n+\t\t$state = $self->{$_state}{$item};\n+\t}\n+\telse {\n+\t\t$state = $self->{$_read}{$item};\n+\t}\n+\n+\tif ($value_passed) {\n+\t\tif (!ref $value and defined $value and ref $state) {\n+\t\t\tthrow Error::Simple (\n+\t\t\t\t\"'$item' is specified multiple times\",\n+\t\t\t       );\n+\t\t}\n+\t\tif ( $type ) {\n+\t\t\tif (ref $value) {\n+\t\t\t\t$value = [ map { $self->freeze($type, $_) }\n+\t\t\t\t\t\t   @$value ];\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\t$value = $self->freeze($type, $value);\n+\t\t\t}\n+\t\t}\n+\t\t$self->{$_state}{$item} = $value;\n+\t\t$self->flush() if $self->autoflush;\n+\t}\n+\n+\tif (defined wantarray) {\n+\t\tmy @values = ref $state ? @$state :\n+\t\t\tdefined $state ? ($state) : ();\n+\n+\t\tif ( my $type = $self->type( $item ) ) {\n+\t\t\t@values = map { $self->thaw($type, $_) }\n+\t\t\t\t@values;\n+\t\t}\n+\n+\t\tif ( @values > 1 and\n+\t\t\t     ( ($value_passed and not ref $value) or\n+\t\t\t\t       (not $value_passed and not wantarray) )\n+\t\t\t    ) {\n+\t\t\tthrow Error::Simple (\n+\t\t\t\t\"'$item' is specified multiple times\",\n+\t\t\t       );\n+\t\t}\n+\n+\t\treturn (wantarray ? @values : $values[0]);\n+\t}\n+}\n+\n+=head2 B<$conf-E<gt>read()>\n+\n+Reads the current state of the configuration file.\n+\n+=cut\n+\n+sub read {\n+\tmy $self = shift;\n+\tmy $which = shift;\n+\n+\tmy $git = $self->{git};\n+\n+\tmy ($fh, $c) = $git->command_output_pipe(\n+\t\t'config', ( $which ? (\"--$which\") : () ),\n+\t\t'--list',\n+\t       );\n+\tmy $read_state = {};\n+\n+\twhile (<$fh>) {\n+\t\tmy ($item, $value) = m{(.*?)=(.*)};\n+\t\tmy $sl = \\( $read_state->{$item} );\n+\t\tif (!defined $$sl) {\n+\t\t\t$$sl = $value;\n+\t\t}\n+\t\telsif (!ref $$sl) {\n+\t\t\t$$sl = [ $$sl, $value ];\n+\t\t}\n+\t\telse {\n+\t\t\tpush @{ $$sl }, $value;\n+\t\t}\n+\t}\n+\n+\t$git->command_close_pipe($fh, $c);\n+\n+\tif ( $which ) {\n+\t\t$which .= \"_\";\n+\t}\n+\telse {\n+\t\t$which = \"\";\n+\t};\n+\n+\t$self->{\"read_${which}state\"} = $read_state;\n+}\n+\n+=head2 type( VARIABLE => $type )\n+\n+Specifies which set of rules to use when returning to and from the\n+config file.  C<$type> may be C<string>, C<boolean> or C<integer>.\n+Globs such as C<*> are allowed which match everything except C<.>\n+(full stop).\n+\n+=head2 type( VARIABLE )\n+\n+Returns the first matching defined type rule.\n+\n+=cut\n+\n+sub type {\n+\tmy $self = shift;\n+\tmy $item = shift;\n+\tmy $got_type = @_;\n+\tmy $type = shift;\n+\t$type ||= \"string\";\n+\n+\tmy $types = $self->{types} ||= [];\n+\tif ( $got_type ) {\n+\t\t$item =~ s{([\\.(?])}{\\\\$1}g;\n+\t\t$item =~ s{\\*}{[^\\\\.]*}g;\n+\t\t$item = qr{^$item$};\n+\t\t@$types = grep { $_->[0] ne $item }\n+\t\t\t@$types;\n+\t\tpush @$types, [ $item, $type ];\n+\t}\n+\telse {\n+\ttype:\n+\t\tfor (@$types) {\n+\t\t\tif ($item =~ m{$_->[0]}) {\n+\t\t\t\t$type = $_->[1];\n+\t\t\t\tlast type;\n+\t\t\t}\n+\t\t}\n+\n+\t\t$type;\n+\t}\n+}\n+\n+=head2 global( VARIABLE )\n+\n+=head2 system( VARIABLE )\n+\n+Return the value of the given variable in the global or system\n+configuration (F<~/.gitconfig> and F</etc/gitconfig> on Unix).\n+\n+=head2 global( VARIABLE => $value )\n+\n+=head2 system( VARIABLE => $value )\n+\n+Set the value of the given variable in the global or system\n+configuration.\n+\n+=cut\n+\n+sub global { return $_[0]->_config(\"global\", @_[1..$#_]) }\n+sub system { return $_[0]->_config(\"system\", @_[1..$#_]) }\n+\n+=head2 autoflush\n+\n+Returns 0 or 1 depending on whether autoflush is enabled.  Defaults to\n+1.\n+\n+=head2 autoflush ( $value )\n+\n+Set the value of autoflush.  If set to 1, flushes immediately.\n+\n+=cut\n+\n+sub autoflush {\n+\tmy $self = shift;\n+\tif (@_) {\n+\t\t$self->{autoflush} = shift;\n+\t}\n+\tnot (defined $self->{autoflush} and not $self->{autoflush});\n+}\n+\n+=head2 flush\n+\n+Flushes all changed configuration values to the config file(s).\n+\n+=cut\n+\n+sub flush {\n+\tmy $self = shift;\n+\n+\tfor my $which (\"\", \"global\", \"system\") {\n+\t\tmy $st = $which . ($which ? \"_\" : \"\") . \"state\";;\n+\t\tmy $read = \"read_$st\";\n+\t\tif (my $new = delete $self->{$st}) {\n+\t\t\t$self->_write($which, $new, $self->{$read});\n+\t\t\t$self->read($which);\n+\t\t}\n+\t}\n+}\n+\n+sub _write {\n+\tmy $self = shift;\n+\tmy $which = shift;\n+\tmy $state = shift;\n+\tmy $read_state = shift;\n+\n+\tmy $git = $self->{git};\n+\n+\twhile (my ($item, $value) = each %$state) {\n+\t\tmy $old_value = $read_state->{$item};\n+\t\tmy @cmd = ($which ? (\"--$which\") : () );\n+\t\tmy $type = $self->type($item);\n+\t\tif ($type ne \"string\") {\n+\t\t\tpush @cmd, \"--$type\";\n+\t\t}\n+\t\tif (ref $value) {\n+\t\t\t$git->command_oneline (\n+\t\t\t\t\"config\", @cmd, \"--replace-all\",\n+\t\t\t\t $item, $value->[0],\n+\t\t\t       );\n+\t\t\tfor my $i (1..$#$value) {\n+\t\t\t\t$git->command_oneline (\n+\t\t\t\t\t\"config\", @cmd, \"--add\",\n+\t\t\t\t\t$item, $value->[$i],\n+\t\t\t\t       );\n+\t\t\t};\n+\t\t}\n+\t\telsif (defined $value) {\n+\t\t\t$git->command_oneline (\n+\t\t\t\t\"config\", @cmd, $item, $value,\n+\t\t\t       );\n+\t\t}\n+\t\telsif ($read_state->{$item}) {\n+\t\t\t$git->command_oneline (\n+\t\t\t\t\"config\", \"--unset-all\", @cmd,\n+\t\t\t\t$item, $value,\n+\t\t\t       );\n+\t\t}\n+\t\telse {\n+\t\t\t# nothing to do - already not in config\n+\t\t}\n+\t}\n+}\n+\n+sub _dispatch {\n+\tno strict 'refs';\n+\tmy $self = shift;\n+\tmy $func = shift;\n+\tmy $type = shift;\n+\tmy $sym = __PACKAGE__.\"::${type}::$func\";\n+\tdefined &{$sym}\n+\t\tor throw Error::Simple \"Bad type '$type' in $func\";\n+\t&{$sym}(@_, $self);\n+}\n+\n+sub freeze {\n+\tmy $self = shift;\n+\t$self->_dispatch(\"freeze\", @_);\n+}\n+\n+sub thaw {\n+\tmy $self = shift;\n+\t$self->_dispatch(\"thaw\", @_);\n+}\n+\n+{\n+\tpackage Git::Config::string;\n+\tsub freeze { shift }\n+\tsub thaw   { shift }\n+}\n+{\n+\tpackage Git::Config::integer;\n+\tour @mul = (\"\", \"k\", \"M\", \"G\");\n+\tsub freeze {\n+\t\tmy $val = shift;\n+\t\tmy $scale = 0;\n+\t\twhile ( (my $num = int($val/1024))*1024 == $val ) {\n+\t\t\t$scale++;\n+\t\t \t$val = $num;\n+\t\t\tlast if $scale == $#mul;\n+\t\t}\n+\t\t$val.$mul[$scale];\n+\t}\n+\tour $mul_re = qr/^(\\d+)\\s*${\\( \"(\".join(\"|\", @mul).\")\" )}$/i;\n+\tsub thaw {\n+\t\tmy $val = shift;\n+\t\t$val =~ m{$mul_re}\n+\t\t\tor throw Error::Simple \"Bad value for integer: '$val'\";\n+\t\tmy $num = $1;\n+\t\tif ($2) {\n+\t\t\tmy $scale = 0;\n+\t\t\tdo { $num = $num * 1024 }\n+\t\t\t\tuntil (lc($mul[++$scale]) eq lc($2));\n+\t\t}\n+\t\t$num;\n+\t}\n+}\n+{\n+\tpackage Git::Config::boolean;\n+\tour @true = qw(true yes 1);\n+\tour @false = qw(false no 0);\n+\tour $true_re = qr/^${\\( \"(\".join(\"|\", @true).\")\" )}$/i;\n+\tour $false_re = qr/^${\\( \"(\".join(\"|\", @false).\")\" )}$/i;\n+\tsub freeze {\n+\t\tmy $val = shift;\n+\t\tif (!!$val) {\n+\t\t\t$true[0];\n+\t\t}\n+\t\telse {\n+\t\t\t$false[0];\n+\t\t}\n+\t}\n+\tsub thaw {\n+\t\tmy $val = shift;\n+\t\tif ($val =~ m{$true_re}) {\n+\t\t\t1;\n+\t\t}\n+\t\telsif ($val =~ m{$false_re}) {\n+\t\t\t0;\n+\t\t}\n+\t\telse {\n+\t\t\tthrow Error::Simple \"Bad value for boolean: '$val'\";\n+\t\t}\n+\t}\n+}\n+\n+1;\n+\n+__END__\n+\n+=head1 SEE ALSO\n+\n+L<Git>\n+\n+=head1 AUTHOR AND LICENSE\n+\n+Copyright 2009, Sam Vilain, L<sam@vilain.net>.  All Rights Reserved.\n+This program is Free Software; you may use it under the terms of the\n+Perl Artistic License 2.0 or later, or the GPL v2 or later.\n+\n+=cut\n+\n+# Local Variables:\n+#   mode: cperl\n+#   cperl-brace-offset: 0\n+#   cperl-continued-brace-offset: 0\n+#   cperl-indent-level: 8\n+#   cperl-label-offset: -8\n+#   cperl-merge-trailing-else: nil\n+#   cperl-continued-statement-offset: 8\n+#   cperl-indent-parens-as-block: t\n+#   cperl-indent-wrt-brace: nil\n+# End:\n+#\n+# vim: vim:tw=78:sts=0:noet\ndiff --git a/perl/Makefile.PL b/perl/Makefile.PL\nindex 320253e..80df0b0 100644\n--- a/perl/Makefile.PL\n+++ b/perl/Makefile.PL\n@@ -8,7 +8,10 @@ instlibdir:\n MAKE_FRAG\n }\n \n-my %pm = ('Git.pm' => '$(INST_LIBDIR)/Git.pm');\n+my %pm = (map {( \"$_.pm\" => \"\\$(INST_LIBDIR)/$_.pm\" )}\n+\t'Git',\n+\t'Git/Config',\n+\t);\n \n # We come with our own bundled Error.pm. It's not in the set of default\n # Perl modules so install it if it's not available on the system yet.\ndiff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh\nindex b81d5df..2f53d00 100755\n--- a/t/t9700-perl-git.sh\n+++ b/t/t9700-perl-git.sh\n@@ -37,8 +37,16 @@ test_expect_success \\\n      git config --add test.int 2k\n      '\n \n+export PERL5LIB=\"$TEST_DIRECTORY/t9700:$GITPERLLIB:$PERL5LIB\"\n+\n test_external_without_stderr \\\n     'Perl API' \\\n     perl \"$TEST_DIRECTORY\"/t9700/test.pl\n \n+rm -rf .git\n+\n+test_external_without_stderr \\\n+    'config API' \\\n+    perl \"$TEST_DIRECTORY\"/t9700/config.t\n+\n test_done\ndiff --git a/t/t9700/TestUtils.pm b/t/t9700/TestUtils.pm\nnew file mode 100644\nindex 0000000..72a386d\n--- /dev/null\n+++ b/t/t9700/TestUtils.pm\n@@ -0,0 +1,77 @@\n+#!/usr/bin/perl\n+\n+package TestUtils;\n+\n+use Carp;\n+use base qw(Exporter);\n+BEGIN {\n+\tour @EXPORT = qw(mk_tmp_repo in_empty_repo tmp_git random_port_pair);\n+\t$SIG{__WARN__} = sub {\n+\t    my $i = 0;\n+\t    while (my $caller = caller($i)) {\n+\t\t    if ($caller eq \"Test::More\") {\n+\t\t\t    return 0;\n+\t\t    }\n+\t\t    $i++;\n+\t    }\n+\t    my $culprit = caller;\n+\t    if ($culprit =~ m{Class::C3}) {\n+\t\treturn 1;\n+\t    }\n+\t    else {\n+\t\tprint STDERR \"*** WARNING FROM $culprit FOLLOWS ***\\n\";\n+\t    }\n+\t    Carp::cluck(@_);\n+\t    print STDERR \"*** END OF STACK DUMP ***\\n\";\n+\t    1\n+\t}\n+\tunless $ENV{NO_WARNING_TRACES};\n+}\n+\n+use Cwd qw(fast_abs_path);\n+use File::Temp qw(tempdir);\n+\n+sub mk_tmp_repo {\n+\tmy $temp_dir = tempdir( \"tmpXXXXX\", CLEANUP => 1 );\n+\tsystem(\"cd $temp_dir; git-init >/dev/null 2>&1\") == 0\n+\t\tor die \"git-init failed; rc=$?\";\n+\tfast_abs_path($temp_dir);\n+}\n+\n+use Cwd;\n+\n+sub in_empty_repo {\n+\tmy $coderef = shift;\n+\tmy $old_wd = getcwd;\n+\tmy $path = mk_tmp_repo();\n+\tchdir($path);\n+\t$coderef->();\n+\tchdir($old_wd);\n+}\n+\n+use Git;\n+sub tmp_git {\n+\tGit->repository(mk_tmp_repo);\n+}\n+\n+# return an array ref of two unprivileged ports\n+sub random_port_pair {\n+\tmy $port = int(rand(2**16-1024-1)+1024);\n+\t[ $port, $port + 1 ];\n+}\n+\n+1;\n+\n+# Local Variables:\n+#   mode: cperl\n+#   cperl-brace-offset: 0\n+#   cperl-continued-brace-offset: 0\n+#   cperl-indent-level: 8\n+#   cperl-label-offset: -8\n+#   cperl-merge-trailing-else: nil\n+#   cperl-continued-statement-offset: 8\n+#   cperl-indent-parens-as-block: t\n+#   cperl-indent-wrt-brace: nil\n+# End:\n+#\n+# vim: vim:tw=78:sts=0:noet\ndiff --git a/t/t9700/config.t b/t/t9700/config.t\nnew file mode 100644\nindex 0000000..395a5c9\n--- /dev/null\n+++ b/t/t9700/config.t\n@@ -0,0 +1,127 @@\n+#!/usr/bin/env perl -- -w\n+\n+use TestUtils;\n+use Test::More no_plan;\n+use strict;\n+\n+use Error qw(:try);\n+\n+use_ok(\"Git::Config\");\n+\n+in_empty_repo sub {\n+\tmy $git = Git->repository;\n+\t$git->command_oneline(\"config\", \"foo.bar\", \"baz\");\n+\t$git->command_oneline(\"config\", \"list.value\", \"one\");\n+\t$git->command_oneline(\"config\", \"--add\", \"list.value\", \"two\");\n+\t$git->command_oneline(\"config\", \"foo.intval\", \"12g\");\n+\t$git->command_oneline(\"config\", \"foo.false.val\", \"false\");\n+\t$git->command_oneline(\"config\", \"foo.true.val\", \"yes\");\n+\n+\tmy $conf = Git::Config->new();\n+\tok($conf, \"constructed a new Git::Config\");\n+\tisa_ok($conf, \"Git::Config\", \"Git::Config->new()\");\n+\n+\tis($conf->config(\"foo.bar\"), \"baz\", \"read single line\");\n+\t$conf->config(\"foo.bar\", \"frop\");\n+\tlike($git->command_oneline(\"config\", \"foo.bar\"), qr/frop/,\n+\t\t\"->config() has immediate effect\");\n+\t$conf->autoflush(0);\n+\tis_deeply(\n+\t\t[$conf->config(\"list.value\")],\n+\t\t[qw(one two)],\n+\t\t\"read multi-value item\",\n+\t);\n+\n+\teval {\n+\t\tmy $val = $conf->config(\"list.value\");\n+\t};\n+\tlike ($@, qr{multiple}i,\n+\t      \"produced an error reading a list into a scalar\");\n+\n+\teval {\n+\t\t$conf->config(\"list.value\" => \"single\");\n+\t};\n+\tlike($@, qr{multiple}i,\n+\t     \"produced an error replacing a list with a scalar\");\n+\n+\tok(eval { $conf->config(\"foo.bar\", [ \"baz\", \"frop\"]); 1 },\n+\t\t\"no error replacing a scalar with a list\");\n+\n+\tlike($git->command_oneline(\"config\", \"foo.bar\"), qr/frop/,\n+\t\t\"->config() no immediate effect with autoflush = 0\");\n+\n+\t$conf->flush;\n+\n+\tlike($git->command(\"config\", \"--get-all\", \"foo.bar\"),\n+\t\tqr/baz\\s*frop/,\n+\t\t\"->flush()\");\n+\n+\t$conf->config(\"x.y$$\" => undef);\n+\tmy $x = $conf->config(\"x.y$$\");\n+\tok(!defined $x, \"unset items return undef in scalar context\");\n+\tmy @x = $conf->config(\"x.y$$\");\n+\tok(@x == 0, \"unset items return empty list in list context\");\n+\n+\t$conf->config(\"foo.bar\" => undef);\n+\t@x = $conf->config(\"foo.bar\");\n+\tok(@x == 0, \"deleted items appear empty immediately\");\n+\t$conf->flush;\n+\tok(!eval {\n+\t\t$git->command(\"config\", \"--get-all\", \"foo.bar\");\n+\t\t1;\n+\t}, \"deleted items are removed from the config file\");\n+\n+\t$conf->type(\"foo.intval\", \"integer\");\n+\tis($conf->type(\"foo.intval\"), \"integer\",\n+\t   \"->type()\");\n+\tis($conf->config(\"foo.intval\"), 12*1024*1024*1024,\n+\t   \"integer thaw\");\n+\t$conf->config(\"foo.intval\", 1025*1024);\n+\t$conf->type(\"foo.intval\", \"string\");\n+\tis($conf->config(\"foo.intval\"), \"1025k\",\n+\t   \"integer freeze\");\n+\n+\t$conf->type(\"foo.*.val\", \"boolean\");\n+\tis($conf->type(\"foo.this.val\"), \"boolean\",\n+\t   \"wildcard types\");\n+\tis($conf->config(\"foo.true.val\"), 1,\n+\t   \"boolean thaw - 'yes'\");\n+\tis($conf->config(\"foo.false.val\"), 0,\n+\t   \"boolean thaw - 'false'\");\n+\tmy $unset = $conf->config(\"foo.bar.val\");\n+\tis($unset, undef,\n+\t   \"boolean thaw - not present\");\n+\n+\t$git->command_oneline(\"config\", \"foo.intval\", \"12g\");\n+\t$git->command_oneline(\"config\", \"foo.falseval\", \"false\");\n+\t$git->command_oneline(\"config\", \"foo.trueval\", \"on\");\n+\n+\tSKIP:{\n+\t\tif (eval {\n+\t\t\t$git->command(\n+\t\t\t\t\"config\", \"--global\",\n+\t\t\t\t\"--get-all\", \"foo.bar\"\n+\t\t\t       );\n+\t\t\t1;\n+\t\t}) {\n+\t\t\tskip \"someone set foo.bar in global config\", 1;\n+\t\t}\n+\t\tmy @foo_bar = $conf->global(\"foo.bar\");\n+\t\tis_deeply(\\@foo_bar, [], \"->global() reading only\");\n+\t}\n+};\n+\n+\n+# Local Variables:\n+#   mode: cperl\n+#   cperl-brace-offset: 0\n+#   cperl-continued-brace-offset: 0\n+#   cperl-indent-level: 8\n+#   cperl-label-offset: -8\n+#   cperl-merge-trailing-else: nil\n+#   cperl-continued-statement-offset: 8\n+#   cperl-indent-parens-as-block: t\n+#   cperl-indent-wrt-brace: nil\n+# End:\n+#\n+# vim: vim:tw=78:sts=0:noet\n-- \n1.6.0\n"},{"id":"110491","messageId":"1238975176-14354-2-git-send-email-sam.vilain@catalyst.net.nz","threadId":"18739","inReplyTo":"1238975176-14354-1-git-send-email-sam.vilain@catalyst.net.nz","subject":"[PATCH] perl: make Git.pm use new Git::Config module","fromName":"Sam Vilain","fromEmail":"sam.vilain@catalyst.net.nz","sentAt":"2009-04-05T23:46:16Z","receivedAt":"2009-04-05T23:46:16Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"Plug these two modules together.  The only minor API difference is that\nin void context (ie, return value is not being used), the new Git::Config\nfunction does not try to unpack the value.  So, the exist test - which was\ntesting an error condition - must be changed to actually try to retrieve\nthe values so that the exceptions can happen.\n\nSigned-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>\n---\n perl/Git.pm     |  103 +++++++++++++++++++++++++++----------------------------\n t/t9700/test.pl |    4 +-\n 2 files changed, 53 insertions(+), 54 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 7d7f2b1..3141b41 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -550,32 +550,38 @@ does. In scalar context requires the variable to be set only one time\n (exception is thrown otherwise), in array context returns allows the\n variable to be set multiple times and returns all the values.\n \n-This currently wraps command('config') so it is not so fast.\n+This delegates via L<Git::Config> for cached config file access.  To\n+force a re-read of the configuration, call C<$git-E<gt>config->read>\n \n-=cut\n+=item config\n \n-sub config {\n-\tmy ($self, $var) = _maybe_self(@_);\n+With no arguments, the C<config> method returns a L<Git::Config>\n+object.  See L<Git::Config> for the API information.\n \n-\ttry {\n-\t\tmy @cmd = ('config');\n-\t\tunshift @cmd, $self if $self;\n-\t\tif (wantarray) {\n-\t\t\treturn command(@cmd, '--get-all', $var);\n-\t\t} else {\n-\t\t\treturn command_oneline(@cmd, '--get', $var);\n-\t\t}\n-\t} catch Git::Error::Command with {\n-\t\tmy $E = shift;\n-\t\tif ($E->value() == 1) {\n-\t\t\t# Key not found.\n-\t\t\treturn;\n-\t\t} else {\n-\t\t\tthrow $E;\n-\t\t}\n+=item config ( VARIABLE => value )\n+\n+With two arguments, the configuration is updated with the passed\n+value.\n+\n+=cut\n+\n+sub _config {\n+\tmy ($self) = _maybe_self_new(@_);\n+\t$self->{config} ||= do {\n+\t\trequire Git::Config;\n+\t\tGit::Config->new($self);\n \t};\n }\n \n+sub config {\n+\t(my $self, @_) = _maybe_self_new(@_);\n+\tif (@_) {\n+\t\treturn $self->_config->config(@_);\n+\t}\n+\telse {\n+\t\treturn $self->_config;\n+\t}\n+}\n \n =item config_bool ( VARIABLE )\n \n@@ -583,28 +589,21 @@ Retrieve the bool configuration C<VARIABLE>. The return value\n is usable as a boolean in perl (and C<undef> if it's not defined,\n of course).\n \n-This currently wraps command('config') so it is not so fast.\n+This delegates via L<Git::Config> for cached config file access.\n+\n+=item config_bool ( VARIABLE => value )\n+\n+Sets a boolean slot to the given value.  This always writes 'true' or\n+'false' to the configuration file, regardless of the value passed.\n \n =cut\n \n sub config_bool {\n-\tmy ($self, $var) = _maybe_self(@_);\n+\t(my ($self, $var), @_) = _maybe_self_new(@_);\n \n-\ttry {\n-\t\tmy @cmd = ('config', '--bool', '--get', $var);\n-\t\tunshift @cmd, $self if $self;\n-\t\tmy $val = command_oneline(@cmd);\n-\t\treturn undef unless defined $val;\n-\t\treturn $val eq 'true';\n-\t} catch Git::Error::Command with {\n-\t\tmy $E = shift;\n-\t\tif ($E->value() == 1) {\n-\t\t\t# Key not found.\n-\t\t\treturn undef;\n-\t\t} else {\n-\t\t\tthrow $E;\n-\t\t}\n-\t};\n+\tmy $conf = $self->_config;\n+\t$conf->type($var => \"boolean\");\n+\t$conf->config($var, @_);\n }\n \n =item config_int ( VARIABLE )\n@@ -615,26 +614,22 @@ or 'g' in the config file will cause the value to be multiplied\n by 1024, 1048576 (1024^2), or 1073741824 (1024^3) prior to output.\n It would return C<undef> if configuration variable is not defined,\n \n-This currently wraps command('config') so it is not so fast.\n+This delegates via L<Git::Config> for cached config file access.\n+\n+=item config_int ( VARIABLE => value )\n+\n+Sets a integer slot to the given value.  This method will reduce the\n+written value to the shortest way to express the given number; eg\n+1024 is written as C<1k> and 16777216 will be written as C<16M>.\n \n =cut\n \n sub config_int {\n-\tmy ($self, $var) = _maybe_self(@_);\n+\t(my ($self, $var), @_) = _maybe_self_new(@_);\n \n-\ttry {\n-\t\tmy @cmd = ('config', '--int', '--get', $var);\n-\t\tunshift @cmd, $self if $self;\n-\t\treturn command_oneline(@cmd);\n-\t} catch Git::Error::Command with {\n-\t\tmy $E = shift;\n-\t\tif ($E->value() == 1) {\n-\t\t\t# Key not found.\n-\t\t\treturn undef;\n-\t\t} else {\n-\t\t\tthrow $E;\n-\t\t}\n-\t};\n+\tmy $conf = $self->_config;\n+\t$conf->type($var => \"integer\");\n+\t$conf->config($var, @_);\n }\n \n =item get_colorbool ( NAME )\n@@ -1204,6 +1199,7 @@ either version 2, or (at your option) any later version.\n \n =cut\n \n+our $_self;\n \n # Take raw method argument list and return ($obj, @args) in case\n # the method was called upon an instance and (undef, @args) if\n@@ -1211,6 +1207,9 @@ either version 2, or (at your option) any later version.\n sub _maybe_self {\n \tUNIVERSAL::isa($_[0], 'Git') ? @_ : (undef, @_);\n }\n+sub _maybe_self_new {\n+\tUNIVERSAL::isa($_[0], 'Git') ? @_ : ($_self||=Git->repository, @_);\n+}\n \n # Check if the command id is something reasonable.\n sub _check_valid_cmd {\ndiff --git a/t/t9700/test.pl b/t/t9700/test.pl\nindex 697daf3..1b94633 100755\n--- a/t/t9700/test.pl\n+++ b/t/t9700/test.pl\n@@ -35,9 +35,9 @@ is($r->get_color(\"color.test.slot1\", \"red\"), $ansi_green, \"get_color\");\n # Save and restore STDERR; we will probably extract this into a\n # \"dies_ok\" method and possibly move the STDERR handling to Git.pm.\n open our $tmpstderr, \">&STDERR\" or die \"cannot save STDERR\"; close STDERR;\n-eval { $r->config(\"test.dupstring\") };\n+eval { my $x = $r->config(\"test.dupstring\") };\n ok($@, \"config: duplicate entry in scalar context fails\");\n-eval { $r->config_bool(\"test.boolother\") };\n+eval { my $x = $r->config_bool(\"test.boolother\") };\n ok($@, \"config_bool: non-boolean values fail\");\n open STDERR, \">&\", $tmpstderr or die \"cannot restore STDERR\";\n \n-- \n1.6.0\n"},{"id":"110552","messageId":"20090406092942.GW17706@mail-vs.djpig.de","threadId":"18739","inReplyTo":"1238975176-14354-1-git-send-email-sam.vilain@catalyst.net.nz","subject":"Re: [PATCH] perl: add new module Git::Config for cached 'git config' access","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2009-04-06T09:29:42Z","receivedAt":"2009-04-06T09:29:42Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Mon, Apr 06, 2009 at 11:46:15AM +1200, Sam Vilain wrote:\n> +\tmy ($fh, $c) = $git->command_output_pipe(\n> +\t\t'config', ( $which ? (\"--$which\") : () ),\n> +\t\t'--list',\n> +\t       );\n> +\tmy $read_state = {};\n> +\n> +\twhile (<$fh>) {\n> +\t\tmy ($item, $value) = m{(.*?)=(.*)};\n> +\t\tmy $sl = \\( $read_state->{$item} );\n> +\t\tif (!defined $$sl) {\n> +\t\t\t$$sl = $value;\n> +\t\t}\n> +\t\telsif (!ref $$sl) {\n> +\t\t\t$$sl = [ $$sl, $value ];\n> +\t\t}\n> +\t\telse {\n> +\t\t\tpush @{ $$sl }, $value;\n> +\t\t}\n> +\t}\n\nAny reason why you don't use --null here? The output of --list without --null\nis not reliably parsable, since people can put newlines in values.\n\nGruesse,\n-- \nFrank Lichtenheld <frank@lichtenheld.de>\nwww: http://www.djpig.de/\n"},{"id":"110639","messageId":"1239058276.31863.19.camel@maia.lan","threadId":"18739","inReplyTo":"20090406092942.GW17706@mail-vs.djpig.de","subject":"Re: [PATCH] perl: add new module Git::Config for cached 'git config' access","fromName":"Sam Vilain","fromEmail":"sam.vilain@catalyst.net.nz","sentAt":"2009-04-06T22:50:59Z","receivedAt":"2009-04-06T22:50:59Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"On Mon, 2009-04-06 at 11:29 +0200, Frank Lichtenheld wrote:\n> On Mon, Apr 06, 2009 at 11:46:15AM +1200, Sam Vilain wrote:\n> > +\tmy ($fh, $c) = $git->command_output_pipe(\n> > +\t\t'config', ( $which ? (\"--$which\") : () ),\n> > +\t\t'--list',\n> > +\t       );\n> Any reason why you don't use --null here? The output of --list without --null\n> is not reliably parsable, since people can put newlines in values.\n\nNo particularly good reason :-)\n\nSubject: [PATCH] perl: make Git::Config use --null\n\nUse the form of 'git-config' designed for parsing by modules like\nthis for safety with values containing embedded line feeds.\n\nSigned-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>\n---\n perl/Git/Config.pm |    6 ++++--\n t/t9700/config.t   |    4 ++++\n 2 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/perl/Git/Config.pm b/perl/Git/Config.pm\nindex a0a6a41..a35d9f3 100644\n--- a/perl/Git/Config.pm\n+++ b/perl/Git/Config.pm\n@@ -179,12 +179,14 @@ sub read {\n \n \tmy ($fh, $c) = $git->command_output_pipe(\n \t\t'config', ( $which ? (\"--$which\") : () ),\n-\t\t'--list',\n+\t\t '--null', '--list',\n \t       );\n \tmy $read_state = {};\n \n+\tlocal($/)=\"\\0\";\n \twhile (<$fh>) {\n-\t\tmy ($item, $value) = m{(.*?)=(.*)};\n+\t\tmy ($item, $value) = m{(.*?)\\n((?s:.*))\\0}\n+\t\t\tor die \"failed to parse it; \\$_='$_'\";\n \t\tmy $sl = \\( $read_state->{$item} );\n \t\tif (!defined $$sl) {\n \t\t\t$$sl = $value;\ndiff --git a/t/t9700/config.t b/t/t9700/config.t\nindex 395a5c9..f0f7d2d 100644\n--- a/t/t9700/config.t\n+++ b/t/t9700/config.t\n@@ -16,6 +16,7 @@ in_empty_repo sub {\n \t$git->command_oneline(\"config\", \"foo.intval\", \"12g\");\n \t$git->command_oneline(\"config\", \"foo.false.val\", \"false\");\n \t$git->command_oneline(\"config\", \"foo.true.val\", \"yes\");\n+\t$git->command_oneline(\"config\", \"multiline.val\", \"hello\\nmultiline.val=world\");\n \n \tmy $conf = Git::Config->new();\n \tok($conf, \"constructed a new Git::Config\");\n@@ -92,6 +93,9 @@ in_empty_repo sub {\n \tis($unset, undef,\n \t   \"boolean thaw - not present\");\n \n+\tlike($conf->config(\"multiline.val\"), qr{\\n},\n+\t     \"parsing multi-line values\");\n+\n \t$git->command_oneline(\"config\", \"foo.intval\", \"12g\");\n \t$git->command_oneline(\"config\", \"foo.falseval\", \"false\");\n \t$git->command_oneline(\"config\", \"foo.trueval\", \"on\");\n-- \ndebian.1.5.6.1\n"},{"id":"110698","messageId":"m3prfo1xh6.fsf@localhost.localdomain","threadId":"18739","inReplyTo":"1239058276.31863.19.camel@maia.lan","subject":"Re: [PATCH] perl: add new module Git::Config for cached 'git config' access","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-04-07T12:01:37Z","receivedAt":"2009-04-07T12:01:37Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Sam Vilain <sam.vilain@catalyst.net.nz> writes:\n\n> On Mon, 2009-04-06 at 11:29 +0200, Frank Lichtenheld wrote:\n> > On Mon, Apr 06, 2009 at 11:46:15AM +1200, Sam Vilain wrote:\n\n> > > +\tmy ($fh, $c) = $git->command_output_pipe(\n> > > +\t\t'config', ( $which ? (\"--$which\") : () ),\n> > > +\t\t'--list',\n> > > +\t       );\n> >\n> > Any reason why you don't use --null here? The output of --list\n> > without --null is not reliably parsable, since people can put\n> > newlines in values.\n> \n> No particularly good reason :-)\n> \n> Subject: [PATCH] perl: make Git::Config use --null\n> \n> Use the form of 'git-config' designed for parsing by modules like\n> this for safety with values containing embedded line feeds.\n> \n> Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>\n\n> diff --git a/perl/Git/Config.pm b/perl/Git/Config.pm\n> index a0a6a41..a35d9f3 100644\n> --- a/perl/Git/Config.pm\n> +++ b/perl/Git/Config.pm\n> @@ -179,12 +179,14 @@ sub read {\n>  \n>  \tmy ($fh, $c) = $git->command_output_pipe(\n>  \t\t'config', ( $which ? (\"--$which\") : () ),\n> -\t\t'--list',\n> +\t\t '--null', '--list',\n>  \t       );\n>  \tmy $read_state = {};\n>  \n> +\tlocal($/)=\"\\0\";\n>  \twhile (<$fh>) {\n> -\t\tmy ($item, $value) = m{(.*?)=(.*)};\n> +\t\tmy ($item, $value) = m{(.*?)\\n((?s:.*))\\0}\n> +\t\t\tor die \"failed to parse it; \\$_='$_'\";\n>  \t\tmy $sl = \\( $read_state->{$item} );\n>  \t\tif (!defined $$sl) {\n>  \t\t\t$$sl = $value;\n\nErrr... wouldn't it be better to simply use \n\n+\t\tmy ($item, $value) = split(\"\\n\", $_, 2)\n\nhere? Have you tested Git::Config with a \"null\" value, i.e. something\nlike\n\n    [section]\n        noval\n\nin the config file (which evaluates to 'true' with '--bool' option)?\nBecause from what I remember from the discussion on the \n\"git config --null --list\" format the lack of \"\\n\" is used to\ndistinguish between noval (which is equivalent to 'true'), and empty\nvalue (which is equivalent to 'false')\n\n    [boolean\n        noval        # equivalent to 'true'\n        empty1 =     # equivalent to 'false'\n        empty2 = \"\"  # equivalent to 'false'\n\n> diff --git a/t/t9700/config.t b/t/t9700/config.t\n> index 395a5c9..f0f7d2d 100644\n> --- a/t/t9700/config.t\n> +++ b/t/t9700/config.t\n> @@ -16,6 +16,7 @@ in_empty_repo sub {\n>  \t$git->command_oneline(\"config\", \"foo.intval\", \"12g\");\n>  \t$git->command_oneline(\"config\", \"foo.false.val\", \"false\");\n>  \t$git->command_oneline(\"config\", \"foo.true.val\", \"yes\");\n> +\t$git->command_oneline(\"config\", \"multiline.val\", \"hello\\nmultiline.val=world\");\n>  \n>  \tmy $conf = Git::Config->new();\n>  \tok($conf, \"constructed a new Git::Config\");\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"110796","messageId":"49DC3ADD.5000902@catalyst.net.nz","threadId":"18739","inReplyTo":"m3prfo1xh6.fsf@localhost.localdomain","subject":"Re: [PATCH] perl: add new module Git::Config for cached 'git config' access","fromName":"Sam Vilain","fromEmail":"sam.vilain@catalyst.net.nz","sentAt":"2009-04-08T05:49:17Z","receivedAt":"2009-04-08T05:49:17Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"Jakub Narebski wrote:\n>> -\t\tmy ($item, $value) = m{(.*?)=(.*)};\n>> +\t\tmy ($item, $value) = m{(.*?)\\n((?s:.*))\\0}\n>> +\t\t\tor die \"failed to parse it; \\$_='$_'\";\n> \n> Errr... wouldn't it be better to simply use \n> \n> +\t\tmy ($item, $value) = split(\"\\n\", $_, 2)\n> \n> here?\n\nYeah, I guess that's easier to read and possibly faster; both are\nusing the regexp engine and using COW strings though, so it's probably\nnot as bad as one might think.\n\n> Have you tested Git::Config with a \"null\" value, i.e. something\n> like\n> \n>     [section]\n>         noval\n> \n> in the config file (which evaluates to 'true' with '--bool' option)?\n> Because from what I remember from the discussion on the \n> \"git config --null --list\" format the lack of \"\\n\" is used to\n> distinguish between noval (which is equivalent to 'true'), and empty\n> value (which is equivalent to 'false')\n> \n>     [boolean]\n>         noval        # equivalent to 'true'\n>         empty1 =     # equivalent to 'false'\n>         empty2 = \"\"  # equivalent to 'false'\n\nThat I didn't consider.  Below is a patch for this.  Any more gremlins?\n\nSubject: perl: fix no value items in Git::Config\n\nWhen interpreted as boolean, items in the configuration which do not\nhave an '=' are interpreted as true.  Parse for this situation, and\nrepresent it with an object in the state hash which works a bit like\nundef, but isn't.  Various internal tests that items were multiple\nvalues with ref() must become stricter.  Reported by Jakub Narebski.\nSneak a couple of vim footer changes in too.\n\nSigned-off-by: Sam Vilain <sam@vilain.net>\n---\n  pull 'perl-Config' from git://github.com/samv/git for a rebased\n  version without the vim footer changes, and with cleaner whitespace.\n\n perl/Git/Config.pm |   36 +++++++++++++++++++++++++++---------\n t/t9700/config.t   |   22 +++++++++++++++++++++-\n 2 files changed, 48 insertions(+), 10 deletions(-)\n\ndiff --git a/perl/Git/Config.pm b/perl/Git/Config.pm\nindex a35d9f3..6b4d928 100644\n--- a/perl/Git/Config.pm\n+++ b/perl/Git/Config.pm\n@@ -144,7 +144,7 @@ sub _config {\n \t}\n \n \tif (defined wantarray) {\n-\t\tmy @values = ref $state ? @$state :\n+\t\tmy @values = ref($state) eq \"ARRAY\" ? @$state :\n \t\t\tdefined $state ? ($state) : ();\n \n \t\tif ( my $type = $self->type( $item ) ) {\n@@ -171,6 +171,8 @@ Reads the current state of the configuration file.\n \n =cut\n \n+our $NOVALUE = bless [__PACKAGE__.\"/NOVALUE\"], \"Git::Config::novalue\";\n+\n sub read {\n \tmy $self = shift;\n \tmy $which = shift;\n@@ -185,13 +187,19 @@ sub read {\n \n \tlocal($/)=\"\\0\";\n \twhile (<$fh>) {\n-\t\tmy ($item, $value) = m{(.*?)\\n((?s:.*))\\0}\n-\t\t\tor die \"failed to parse it; \\$_='$_'\";\n+\t\tmy ($item, $value) = split \"\\n\", $_, 2;\n+\t\tif (defined $value) {\n+\t\t\tchop($value);\n+\t\t} else {\n+\t\t\tchop($item);\n+\t\t\t$value = $NOVALUE;\n+\t\t}\n+\t\tmy $exists = exists $read_state->{$item};\n \t\tmy $sl = \\( $read_state->{$item} );\n-\t\tif (!defined $$sl) {\n+\t\tif (!$exists) {\n \t\t\t$$sl = $value;\n \t\t}\n-\t\telsif (!ref $$sl) {\n+\t\telsif (!ref $$sl or ref $$sl ne \"ARRAY\") {\n \t\t\t$$sl = [ $$sl, $value ];\n \t\t}\n \t\telse {\n@@ -325,7 +333,7 @@ sub _write {\n \t\tif ($type ne \"string\") {\n \t\t\tpush @cmd, \"--$type\";\n \t\t}\n-\t\tif (ref $value) {\n+\t\tif (ref $value eq \"ARRAY\") {\n \t\t\t$git->command_oneline (\n \t\t\t\t\"config\", @cmd, \"--replace-all\",\n \t\t\t\t $item, $value->[0],\n@@ -378,7 +386,7 @@ sub thaw {\n {\n \tpackage Git::Config::string;\n \tsub freeze { shift }\n-\tsub thaw   { shift }\n+\tsub thaw   { (shift).\"\" }\n }\n {\n \tpackage Git::Config::integer;\n@@ -408,6 +416,15 @@ sub thaw {\n \t}\n }\n {\n+\tpackage Git::Config::novalue;\n+\tsub as_string { \"\" }\n+\tsub as_num    {  0 }\n+\tuse overload\n+\t\t'\"\"' => \\&as_string,\n+\t\t'0+' => \\&as_num,\n+\t\tfallback => 1;\n+}\n+{\n \tpackage Git::Config::boolean;\n \tour @true = qw(true yes 1);\n \tour @false = qw(false no 0);\n@@ -424,7 +441,8 @@ sub thaw {\n \t}\n \tsub thaw {\n \t\tmy $val = shift;\n-\t\tif ($val =~ m{$true_re}) {\n+\t\tif (eval{$val->isa(\"Git::Config::novalue\")}\n+\t\t\t    or $val =~ m{$true_re}) {\n \t\t\t1;\n \t\t}\n \t\telsif ($val =~ m{$false_re}) {\n@@ -464,4 +482,4 @@ Perl Artistic License 2.0 or later, or the GPL v2 or later.\n #   cperl-indent-wrt-brace: nil\n # End:\n #\n-# vim: vim:tw=78:sts=0:noet\n+# vim: tw=78:sts=0:noet\ndiff --git a/t/t9700/config.t b/t/t9700/config.t\nindex f0f7d2d..9d7860f 100644\n--- a/t/t9700/config.t\n+++ b/t/t9700/config.t\n@@ -17,6 +17,14 @@ in_empty_repo sub {\n \t$git->command_oneline(\"config\", \"foo.false.val\", \"false\");\n \t$git->command_oneline(\"config\", \"foo.true.val\", \"yes\");\n \t$git->command_oneline(\"config\", \"multiline.val\", \"hello\\nmultiline.val=world\");\n+\topen(CONFIG, \">>.git/config\") or die $!;\n+\tprint CONFIG <<CONF;\n+[boolean]\n+   noval\n+   empty1 =\n+   empty2 = \"\"\n+CONF\n+\tclose CONFIG;\n \n \tmy $conf = Git::Config->new();\n \tok($conf, \"constructed a new Git::Config\");\n@@ -100,6 +108,18 @@ in_empty_repo sub {\n \t$git->command_oneline(\"config\", \"foo.falseval\", \"false\");\n \t$git->command_oneline(\"config\", \"foo.trueval\", \"on\");\n \n+\tis($conf->config(\"boolean.noval\"), \"\", \"noval: string\");\n+\tis($conf->config(\"boolean.empty1\"), \"\", \"empty1: string\");\n+\tis($conf->config(\"boolean.empty2\"), \"\", \"empty2: string\");\n+\n+\t$conf->type(\"boolean.*\" => \"boolean\");\n+\n+\tok($conf->config(\"boolean.noval\"), \"noval: boolean\");\n+\teval{my $x = $conf->config(\"boolean.empty1\")};\n+\tok($@, \"empty1: boolean\");\n+\teval{my $x = $conf->config(\"boolean.empty2\")};\n+\tok($@, \"empty2: boolean\");\n+\n \tSKIP:{\n \t\tif (eval {\n \t\t\t$git->command(\n@@ -128,4 +148,4 @@ in_empty_repo sub {\n #   cperl-indent-wrt-brace: nil\n # End:\n #\n-# vim: vim:tw=78:sts=0:noet\n+# vim: tw=78:sts=0:noet\n-- \n1.6.0\n"},{"id":"110794","messageId":"7vbpr7r72w.fsf@gitster.siamese.dyndns.org","threadId":"18739","inReplyTo":"m3prfo1xh6.fsf@localhost.localdomain","subject":"Re: [PATCH] perl: add new module Git::Config for cached 'git config' access","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-08T06:29:59Z","receivedAt":"2009-04-08T06:29:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Errr... wouldn't it be better to simply use \n>\n> +\t\tmy ($item, $value) = split(\"\\n\", $_, 2)\n>\n> here? Have you tested Git::Config with a \"null\" value, i.e. something\n> like\n>\n>     [section]\n>         noval\n>\n> in the config file (which evaluates to 'true' with '--bool' option)?\n> Because from what I remember from the discussion on the \n> \"git config --null --list\" format the lack of \"\\n\" is used to\n> distinguish between noval (which is equivalent to 'true'), and empty\n> value (which is equivalent to 'false')\n>\n>     [boolean\n>         noval        # equivalent to 'true'\n>         empty1 =     # equivalent to 'false'\n>         empty2 = \"\"  # equivalent to 'false'\n\nI do not mind if the _write method always wrote out\n\n\t[core]\n        \tautocrlf = true\n\nfor a variable that is true, but it should be able to read existing\n\n\t[core]\n        \tautocrlf\n\ncorrectly.\n\nSam, I think you meant to make me squash the \"Oops, for no good reason,\nhere is a fix-up\" into the previous one, but for this case, I'd appreciate\na re-roll of the series, that includes a test to read from an existing\nconfiguration file that contains such \"presense of the name alone means\nboolean true\" variables.\n"},{"id":"110809","messageId":"20090408081221.GG8940@machine.or.cz","threadId":"18739","inReplyTo":"1238975176-14354-1-git-send-email-sam.vilain@catalyst.net.nz","subject":"Re: [PATCH] perl: add new module Git::Config for cached 'git config' access","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2009-04-08T08:12:21Z","receivedAt":"2009-04-08T08:12:21Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Mon, Apr 06, 2009 at 11:46:15AM +1200, Sam Vilain wrote:\n> Add a new module, Git::Config, for a better Git configuration API.\n> \n> Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>\n\nI'm really sorry that I probably won't have time soon to properly review\nthis patch. :-(\n\n> +\t\t\tthrow Error::Simple (\n> +\t\t\t\t\"'$item' is specified multiple times\",\n> +\t\t\t       );\n\nSo just one comment - in general people seem to be unhappy with this way\nof exception handling, preferring die-eval to throw-catch. We might\nseize the opportunity here and start using die in all new modules,\nkeeping Error::Simple only in legacy Git.pm (and its wrappers for the\nGit::Config stuff).\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nThe average, healthy, well-adjusted adult gets up at seven-thirty\nin the morning feeling just terrible. -- Jean Kerr\n"},{"id":"110813","messageId":"49DC736F.1030007@catalyst.net.nz","threadId":"18739","inReplyTo":"7vbpr7r72w.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] perl: add new module Git::Config for cached 'git config' access","fromName":"Sam Vilain","fromEmail":"samv@catalyst.net.nz","sentAt":"2009-04-08T09:50:39Z","receivedAt":"2009-04-08T09:50:39Z","isPatch":true,"sender":{"key":"samv@catalyst.net.nz","avatar":null},"body":"Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n>\n>   \n>> Errr... wouldn't it be better to simply use \n>>\n>> +\t\tmy ($item, $value) = split(\"\\n\", $_, 2)\n>>\n>> here? Have you tested Git::Config with a \"null\" value, i.e. something\n>> like\n>>\n>>     [section]\n>>         noval\n>>\n>> in the config file (which evaluates to 'true' with '--bool' option)?\n>> Because from what I remember from the discussion on the \n>> \"git config --null --list\" format the lack of \"\\n\" is used to\n>> distinguish between noval (which is equivalent to 'true'), and empty\n>> value (which is equivalent to 'false')\n>>\n>>     [boolean\n>>         noval        # equivalent to 'true'\n>>         empty1 =     # equivalent to 'false'\n>>         empty2 = \"\"  # equivalent to 'false'\n>>     \n>\n> I do not mind if the _write method always wrote out\n>\n> \t[core]\n>         \tautocrlf = true\n>\n> for a variable that is true, but it should be able to read existing\n>\n> \t[core]\n>         \tautocrlf\n>\n> correctly.\n>   \n\nYep - that's what I thought was reasonable behaviour as well and what my \nsubmission does.\n\n> Sam, I think you meant to make me squash the \"Oops, for no good reason,\n> here is a fix-up\" into the previous one, but for this case, I'd appreciate\n> a re-roll of the series, that includes a test to read from an existing\n> configuration file that contains such \"presense of the name alone means\n> boolean true\" variables.\n>   \n\nSure, I rebased the series to have the fix-ups at the right places, but \ndidn't think it was an interesting enough change to rate a full \nre-submission. The series at git://github.com/samv/git branch \nperl-Config has the minor change put into the place it was introduced. I \nput a little note to this effect after the --- line.\n\nI'm not quite sure what you want squashed where, maybe just edit the \nbelow list to be how you'd like it,\n\npick d43238e perl: add new module Git::Config for cached 'git config' access\npick 5ea135d perl: make Git.pm use new Git::Config module\npick b2865bc perl: make Git::Config use --null\npick 28eecdc perl: fix no value items in Git::Config\n\n:-)\n\nSam\n"},{"id":"110824","messageId":"200904081218.39984.jnareb@gmail.com","threadId":"18739","inReplyTo":"49DC3ADD.5000902@catalyst.net.nz","subject":"Re: [PATCH] perl: add new module Git::Config for cached 'git config' access","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-04-08T10:18:38Z","receivedAt":"2009-04-08T10:18:38Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"By the way, did you take a look how cached 'git config' access and\ntypecasting is done in gitweb?  See commit b201927 (gitweb: Read\nrepo config using 'git config -z -l') and following similar commits.\n\nOn Wed, 8 April 2009, Sam Vilain wrote:\n> Jakub Narebski wrote:\n\n>>> -\t\tmy ($item, $value) = m{(.*?)=(.*)};\n>>> +\t\tmy ($item, $value) = m{(.*?)\\n((?s:.*))\\0}\n>>> +\t\t\tor die \"failed to parse it; \\$_='$_'\";\n>> \n>> Errr... wouldn't it be better to simply use \n>> \n>> +\t\tmy ($item, $value) = split(\"\\n\", $_, 2)\n>> \n>> here?\n> \n> Yeah, I guess that's easier to read and possibly faster; both are\n> using the regexp engine and using COW strings though, so it's probably\n> not as bad as one might think.\n\nThe version using 'split' has the advantage that for config variable\nwith no value (e.g. \"[section] noval\") it sets $item (why this variable\nis called $item and not $var, $variable or $key, BTW.?) to fully \nqualified variable name (e.g. \"section.noval\"), and sets $value to \nundef, instead of failing like your original version using regexp.\n\nAnd I also think that this version is easier to understand, and might be \na bit faster as well; but it is more important to be easier to \nunderstand.\n\n>> Have you tested Git::Config with a \"null\" value, i.e. something\n>> like\n>> \n>>     [section]\n>>         noval\n>> \n>> in the config file (which evaluates to 'true' with '--bool' option)?\n>> Because from what I remember from the discussion on the \n>> \"git config --null --list\" format the lack of \"\\n\" is used to\n>> distinguish between noval (which is equivalent to 'true'), and empty\n>> value (which is equivalent to 'false')\n>> \n>>     [boolean]\n>>         noval        # equivalent to 'true'\n>>         empty1 =     # equivalent to 'false'\n>>         empty2 = \"\"  # equivalent to 'false'\n> \n> That I didn't consider.  Below is a patch for this.  Any more\n> gremlins? \n\nI have nor examined your patch in detail; I'll try to do it soon,\nbut with git config file parsing there lies following traps.\n\n1. In fully qualified variable name section name and variable name\n   have to be compared case insensitive (or normalized, i.e.\n   lowercased), while subsection part (if it exists) is case sensitive.\n\n2. When coercing type to bool, you need to remember (and test) that\n   there are values which are truish (no value, 'true', 'yes', non-zero\n   integer usually 1), values which are falsish (empry, 'false', 'no',\n   0); other values IIRC are truish too.\n\n3. When coercing type to int, you need to remember about optional\n   value suffixes: 'k', 'm' or 'g'.\n\n4. I don't know if you remembered about 'colorbool' and 'color'; the\n   latter would probably require some extra CPAN module for ANSI color\n   escapes... or copying color codes from the C version.\n\n> \n> Subject: perl: fix no value items in Git::Config\n> \n> When interpreted as boolean, items in the configuration which do not\n> have an '=' are interpreted as true.  Parse for this situation, and\n> represent it with an object in the state hash which works a bit like\n> undef, but isn't.\n\nWhy not represent it simply as an 'undef'? You can always distinguish \nbetween not defined and not existing by using 'exists'...\n\n> Sneak a couple of vim footer changes in too.\n\nHmmm...\n\n> \n> Signed-off-by: Sam Vilain <sam@vilain.net>\n\n[...]\n-- \nJakub Narebski\nPoland\n"},{"id":"110826","messageId":"49DC8002.6050503@catalyst.net.nz","threadId":"18739","inReplyTo":"200904081218.39984.jnareb@gmail.com","subject":"Re: [PATCH] perl: add new module Git::Config for cached 'git config' access","fromName":"Sam Vilain","fromEmail":"samv@catalyst.net.nz","sentAt":"2009-04-08T10:44:18Z","receivedAt":"2009-04-08T10:44:18Z","isPatch":true,"sender":{"key":"samv@catalyst.net.nz","avatar":null},"body":"Jakub Narebski wrote:\n> By the way, did you take a look how cached 'git config' access and\n> typecasting is done in gitweb?  See commit b201927 (gitweb: Read\n> repo config using 'git config -z -l') and following similar commits.\n>   \n\nRight ... sure, looks fairly straightforward.  I guess gitweb could \npotentially use this tested module instead of including that code \nitself.  Also various parts of git-svn... anything really.\n\nI actually wrote this code because I wanted something a bit nicer for \nwriting the mirror-sync initial implementations.  And I wanted to have a \nbit of control over when values get committed, and save work for \nreading, so I wrote this.\n\n>> Any more gremlins? \n>>     \n> I have nor examined your patch in detail; I'll try to do it soon,\n> but with git config file parsing there lies following traps.\n>\n> 1. In fully qualified variable name section name and variable name\n>    have to be compared case insensitive (or normalized, i.e.\n>    lowercased), while subsection part (if it exists) is case sensitive.\n>   \n\nI noticed that 'git config' hides this by normalising the case of what \nit outputs with 'git config --list'; do you think anything special is \nrequired in light of this?\n\n> 2. When coercing type to bool, you need to remember (and test) that\n>    there are values which are truish (no value, 'true', 'yes', non-zero\n>    integer usually 1), values which are falsish (empry, 'false', 'no',\n>    0); other values IIRC are truish too.\n>   \n\nYep, see the Git::Config::boolean mini-package which has a list of \nthose.  I think I used the documented legal values, which are 'true', \n'yes' and '1' for affirmative and 'false', 'no' and '0' for negative.  I \nguess I could make that include non-zero integers as well.\n\n> 3. When coercing type to int, you need to remember about optional\n>    value suffixes: 'k', 'm' or 'g'.\n>   \n\nYep, covered on input and output :-).  See Git::Config::integer for the \nconversion functions.\n\n> 4. I don't know if you remembered about 'colorbool' and 'color'; the\n>    latter would probably require some extra CPAN module for ANSI color\n>    escapes... or copying color codes from the C version.\n>   \n\nYeah, I thought those could probably be done with a follow-up patch.  \nIt's just a matter of writing functions Git::Config::color::thaw and \n::freeze.\n\n> Why not represent it simply as an 'undef'? You can always distinguish \n> between not defined and not existing by using 'exists'...\n>   \n\nI don't like 'undef' being a data value.  In this case I was already \nusing setting a value to undef to tell the module to remove the key from \nthe config file.  But in any case you should not need to care what form \nthe values exist in the internal ->{read_state} hash, as you should \nalways be retrieving them using the ->config method, which will marshall \nthem into the type you want.  Note it's always the same object, just \nlike Perl's &PL_undef via the C API.\n\n>> Sneak a couple of vim footer changes in too.\n>>     \n>\n> Hmmm...\n>   \n\nGuess someone noticed them.  Oh well, rebase time ...\n\nThanks for your input Jakub, I'll incorporate your suggestions.\n\nSam\n"},{"id":"110859","messageId":"7vtz4zm114.fsf@gitster.siamese.dyndns.org","threadId":"18739","inReplyTo":"49DC736F.1030007@catalyst.net.nz","subject":"Re: [PATCH] perl: add new module Git::Config for cached 'git config' access","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-08T18:51:51Z","receivedAt":"2009-04-08T18:51:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sam Vilain <samv@catalyst.net.nz> writes:\n\n> I'm not quite sure what you want squashed where, maybe just edit the\n> below list to be how you'd like it,\n>\n> pick d43238e perl: add new module Git::Config for cached 'git config' access\n> pick 5ea135d perl: make Git.pm use new Git::Config module\n> pick b2865bc perl: make Git::Config use --null\n> pick 28eecdc perl: fix no value items in Git::Config\n\nFor example, if Git::Config is a new thing that this series brings in, and\nif the final decision made before the series is integrated into my tree is\nthat it should read from --null output, then that makes the third one an\n\"Oops, it was a thinko---I should have used --null from day one\" fix-up\nthat shouldn't be in the resulting series as a separate commit, doesn't\nit?  I think the same applies to the last \"fix no value 'true' variables\"\none.\n"},{"id":"110879","messageId":"200904090113.54272.jnareb@gmail.com","threadId":"18739","inReplyTo":"49DC8002.6050503@catalyst.net.nz","subject":"Re: [PATCH] perl: add new module Git::Config for cached 'git config' access","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-04-08T23:13:53Z","receivedAt":"2009-04-08T23:13:53Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 8 Apr 2009, Sam Vilain wrote:\n> Jakub Narebski wrote:\n>>\n>> By the way, did you take a look how cached 'git config' access and\n>> typecasting is done in gitweb?  See commit b201927 (gitweb: Read\n>> repo config using 'git config -z -l') and following similar commits.\n> \n> Right ... sure, looks fairly straightforward.  I guess gitweb could \n> potentially use this tested module instead of including that code \n> itself.  Also various parts of git-svn... anything really.\n\nWell... first, gitweb use of config files is simplified to what, I guess,\nyou want, because it only considers _reading_ config, and doesn't worry\nabout writing and (what is I guess most difficult) rewriting config file.\n\nSecond, gitweb doesn't even use Git.pm (although \"gitweb caching\" project\nby Lea Wiemann from GSoC 2008 introduced Git::Repo, the alternate OO\ninterface, and used it in caching gitweb).  This has the advantage of\nbeing slightly easier to install... but we require git anyway, so it\nis not much more diffucult requiring perl-Git / Git.pm.\n\n> \n> I actually wrote this code because I wanted something a bit nicer for \n> writing the mirror-sync initial implementations.  And I wanted to have a \n> bit of control over when values get committed, and save work for \n> reading, so I wrote this.\n\nWell, you could have written in C instead ;-)\n\n>>> Any more gremlins? \n>>>     \n>> I have nor examined your patch in detail; I'll try to do it soon,\n>> but with git config file parsing there lies following traps.\n>>\n>> 1. In fully qualified variable name section name and variable name\n>>    have to be compared case insensitive (or normalized, i.e.\n>>    lowercased), while subsection part (if it exists) is case sensitive.\n> \n> I noticed that 'git config' hides this by normalising the case of what \n> it outputs with 'git config --list'; do you think anything special is \n> required in light of this?\n\nI'm not sure. I was thinking that get() method should normalize its\narguments before comparing... but I am not sure if it is necessary\n(or even if it is a good idea).\n\n>> 2. When coercing type to bool, you need to remember (and test) that\n>>    there are values which are truish (no value, 'true', 'yes', non-zero\n>>    integer usually 1), values which are falsish (empry, 'false', 'no',\n>>    0); other values IIRC are truish too.\n> \n> Yep, see the Git::Config::boolean mini-package which has a list of \n> those.  I think I used the documented legal values, which are 'true', \n> 'yes' and '1' for affirmative and 'false', 'no' and '0' for negative.  I \n> guess I could make that include non-zero integers as well.\n\nThey are, from what I understand, empty value, 'false', 'no' and '0' for\nnegative, all else is positive (which includes no value, 'true', 'yes'\nand '1').  But you'd better check the C code yourself.\n\n[...]\n>> Why not represent it simply as an 'undef'? You can always distinguish \n>> between not defined and not existing by using 'exists'...\n>>   \n> \n> I don't like 'undef' being a data value.\n\nWhy not? It is IMHO the most natural way.\n\n>                                           In this case I was already  \n> using setting a value to undef to tell the module to remove the key from \n> the config file.\n\nWhy not use 'delete' to remove hash element, and 'exists' to check\nwhether it exists?\n\n\n-- \nJakub Narebski\nPoland\n"}]}