git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: Junio C Hamano <gitster@pobox.com>, Eric Wong <normalperson@yhbt.net>
Cc: Cord Seele <cowose@googlemail.com>,
	Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>,
	git@vger.kernel.org, Cord Seele <cowose@gmail.com>
Subject: [PATCH/RFC 3/2] Refactor Git::config_*
Date: Fri, 7 Oct 2011 23:17:16 +0200	[thread overview]
Message-ID: <201110072317.17436.jnareb@gmail.com> (raw)
In-Reply-To: <7voby1oesm.fsf@alter.siamese.dyndns.org>

From: Junio C Hamano <gitster@pobox.com>

There is, especially with addition of Git::config_path(), much code
repetition in the Git::config_* family of subroutines.  Refactor
common parts of Git::config(), Git::config_bool(), Git::config_int()
and Git::config_path() into _config_common() subroutine.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is version which has fixed style to be more Perl-ish, and which
actually works (i.e. t9700 passes).

I have also moved _config_common() after commands that use it, just like
it is done with other "private" methods (methods with names starting with
'_'), and excluded this private detail of implementation from docs.

 perl/Git.pm |   85 ++++++++++++++---------------------------------------------
 1 files changed, 20 insertions(+), 65 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index c279bfb..d6df2e8 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -562,7 +562,6 @@ sub wc_chdir {
 	$self->{opts}->{WorkingSubdir} = $subdir;
 }
 
-
 =item config ( VARIABLE )
 
 Retrieve the configuration C<VARIABLE> in the same manner as C<config>
@@ -570,30 +569,11 @@ does. In scalar context requires the variable to be set only one time
 (exception is thrown otherwise), in array context returns allows the
 variable to be set multiple times and returns all the values.
 
-This currently wraps command('config') so it is not so fast.
-
 =cut
 
 sub config {
 	my ($self, $var) = _maybe_self(@_);
-
-	try {
-		my @cmd = ('config');
-		unshift @cmd, $self if $self;
-		if (wantarray) {
-			return command(@cmd, '--get-all', $var);
-		} else {
-			return command_oneline(@cmd, '--get', $var);
-		}
-	} catch Git::Error::Command with {
-		my $E = shift;
-		if ($E->value() == 1) {
-			# Key not found.
-			return;
-		} else {
-			throw $E;
-		}
-	};
+	return _config_common($self, $var);
 }
 
 
@@ -603,60 +583,24 @@ Retrieve the bool configuration C<VARIABLE>. The return value
 is usable as a boolean in perl (and C<undef> if it's not defined,
 of course).
 
-This currently wraps command('config') so it is not so fast.
-
 =cut
 
 sub config_bool {
 	my ($self, $var) = _maybe_self(@_);
-
-	try {
-		my @cmd = ('config', '--bool', '--get', $var);
-		unshift @cmd, $self if $self;
-		my $val = command_oneline(@cmd);
-		return undef unless defined $val;
-		return $val eq 'true';
-	} catch Git::Error::Command with {
-		my $E = shift;
-		if ($E->value() == 1) {
-			# Key not found.
-			return undef;
-		} else {
-			throw $E;
-		}
-	};
+	my $val = scalar _config_common($self, $var, {'kind' => '--bool'});
+	return (defined $val && $val eq 'true');
 }
 
-
 =item config_path ( VARIABLE )
 
 Retrieve the path configuration C<VARIABLE>. The return value
 is an expanded path or C<undef> if it's not defined.
 
-This currently wraps command('config') so it is not so fast.
-
 =cut
 
 sub config_path {
 	my ($self, $var) = _maybe_self(@_);
-
-	try {
-		my @cmd = ('config', '--path');
-		unshift @cmd, $self if $self;
-		if (wantarray) {
-			return command(@cmd, '--get-all', $var);
-		} else {
-			return command_oneline(@cmd, '--get', $var);
-		}
-	} catch Git::Error::Command with {
-		my $E = shift;
-		if ($E->value() == 1) {
-			# Key not found.
-			return undef;
-		} else {
-			throw $E;
-		}
-	};
+	return _config_common($self, $var, {'kind' => '--path'});
 }
 
 =item config_int ( VARIABLE )
@@ -667,28 +611,39 @@ or 'g' in the config file will cause the value to be multiplied
 by 1024, 1048576 (1024^2), or 1073741824 (1024^3) prior to output.
 It would return C<undef> if configuration variable is not defined,
 
-This currently wraps command('config') so it is not so fast.
-
 =cut
 
 sub config_int {
 	my ($self, $var) = _maybe_self(@_);
+	return scalar _config_common($self, $var, {'kind' => '--int'});
+}
+
+# Common subroutine to implement bulk of what the config* family of methods
+# do. This wraps command('config') so it is not so fast.
+sub _config_common {
+	my ($self, $var, $opts) = _maybe_self(@_);
 
 	try {
-		my @cmd = ('config', '--int', '--get', $var);
+		my @cmd = ('config', $opts->{'kind'} ? $opts->{'kind'} : ());
 		unshift @cmd, $self if $self;
-		return command_oneline(@cmd);
+		if (wantarray) {
+			return command(@cmd, '--get-all', $var);
+		} else {
+			return command_oneline(@cmd, '--get', $var);
+		}
 	} catch Git::Error::Command with {
 		my $E = shift;
 		if ($E->value() == 1) {
 			# Key not found.
-			return undef;
+			return;
 		} else {
 			throw $E;
 		}
 	};
+
 }
 
+
 =item get_colorbool ( NAME )
 
 Finds if color should be used for NAMEd operation from the configuration,
-- 
1.7.6

  reply	other threads:[~2011-10-07 21:17 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-28 13:13 [PATCH] git-send-email.perl: expand filename of aliasesfile Cord Seele
2011-09-28 13:42 ` Matthieu Moy
2011-09-28 14:40   ` Cord Seele
2011-09-28 14:47     ` Matthieu Moy
2011-09-30 10:52       ` [PATCH v2] git-send-email: allow filename expansion Cord Seele
2011-09-30 10:52         ` [PATCH 1/2] Add Git::config_path() Cord Seele
2011-10-07 20:32           ` Jakub Narebski
2011-10-07 21:35             ` Junio C Hamano
2011-10-07 21:44               ` Jakub Narebski
2011-10-07 22:26                 ` Junio C Hamano
2011-09-30 10:52         ` [PATCH 2/2] use new Git::config_path() for aliasesfile Cord Seele
2011-09-30 19:55           ` Junio C Hamano
2011-09-30 21:16             ` Cord Seele
2011-09-30 22:00             ` Jakub Narebski
2011-09-30 22:18               ` Junio C Hamano
2011-10-07 21:17                 ` Jakub Narebski [this message]
2011-10-17 20:50                   ` [PATCH/RFC 3/2] Refactor Git::config_* Junio C Hamano
2011-10-17 21:47                     ` Jakub Narebski
2011-10-17 23:25                       ` Junio C Hamano
2011-10-18  9:47                       ` [PATCH/RFC 3/2 (fixed)] " Jakub Narebski
2011-10-18 19:52                         ` Junio C Hamano
2011-10-18 22:09                           ` [PATCH/RFC 3/2 v3] " Jakub Narebski
2011-10-18 23:25                             ` Junio C Hamano
2011-10-19  0:19                               ` Jakub Narebski

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201110072317.17436.jnareb@gmail.com \
    --to=jnareb@gmail.com \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=cowose@gmail.com \
    --cc=cowose@googlemail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=normalperson@yhbt.net \
    /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.
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 NNTP newsgroup(s).