Re: [PATCHv3 2/3] cvsimport: fix the parsing of uppercase config options

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Jeff King <peff@xxxxxxxx> writes:
>
>> If there are short config options that work, though, we should probably
>> keep them. It surely can't be that hard for the perl code to accept both
>> "cvsimport.r" and "cvsimport.remote" for "-r" and
>> "cvsimport.generaterevisions" (or whatever) for "-R"?
>
> I'd agree.  The current parameter set we give to GetOptions is:
>
>     haivmkuo:d:p:r:C:z:s:M:P:A:S:L:R
>
> and I notice that we cannot sanely access A, M, P, R, S and their
> lowercase counterparts.
>
> It shouldn't be too hard to interpret the single letter options for the
> currently supported set but only for lowercase letters plus '-C <arg>' and
> '-L <arg>' (because there are no lowercase ones that crash with them), and
> give long config names to at least the ones inaccessible with the above.
>
> Wouldn't that give us a regression-free solution to the immediate problem
> at hand?
>
> Supporting long options on the command line, and giving long config name
> synonyms to the lowercase ones, would be a plus for consistency's sake,
> but that probably is a separate topic.

So here is an attempt to do that first step, taking the list of long names
from Jonathan's message earlier in the thread.  If we start giving set of
long options, long option names may come from a different table which
would likely to have names in the dashed form, so this code already
expects names with dashes, e.g. "track-revisions", and contracts them to
names for configuration files, e.g. "trackRevisions".

 git-cvsimport.perl   |   19 ++++++++++++++++++-
 t/t9600-cvsimport.sh |    7 +++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 2b07c72..8c916d9 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -89,6 +89,14 @@ sub write_author_info($) {
 }
 
 # convert getopts specs for use by git config
+my %longmap = (
+	'A:' => 'authors-file',
+	'M:' => 'merge-regex',
+	'P:' => undef,
+	'R' => 'track-revisions',
+	'S:' => 'ignore-paths',
+);
+
 sub read_repo_config {
 	# Split the string between characters, unless there is a ':'
 	# So "abc:de" becomes ["a", "b", "c:", "d", "e"]
@@ -98,8 +106,17 @@ sub read_repo_config {
 		$key =~ s/://g;
 		my $arg = 'git config';
 		$arg .= ' --bool' if ($o !~ /:$/);
-
-		chomp(my $tmp = `$arg --get cvsimport.$key`);
+		my $ckey = $key;
+
+		if (exists $longmap{$o}) {
+			# An uppercase option like -R cannot be
+			# expressed in the configuration, as the
+			# variable names are downcased.
+			$ckey = $longmap{$o};
+			next if (! defined $ckey);
+			$ckey =~ s/-//g;
+		}
+		chomp(my $tmp = `$arg --get cvsimport.$ckey`);
 		if ($tmp && !($arg =~ /--bool/ && $tmp eq 'false')) {
 			no strict 'refs';
 			my $opt_name = "opt_" . $key;
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index b572ce3..6ede84c 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -93,7 +93,8 @@ EOF
 test_expect_success 'update git module' '
 
 	cd module-git &&
-	git cvsimport -a -R -z 0 module &&
+	git config cvsimport.trackRevisions true &&
+	git cvsimport -a -z 0 module &&
 	git merge origin &&
 	cd .. &&
 	test_cmp module-cvs/o_fortuna module-git/o_fortuna
@@ -122,7 +123,8 @@ test_expect_success 'cvsimport.module config works' '
 
 	cd module-git &&
 		git config cvsimport.module module &&
-		git cvsimport -a -R -z0 &&
+		git config cvsimport.trackRevisions true &&
+		git cvsimport -a -z0 &&
 		git merge origin &&
 	cd .. &&
 	test_cmp module-cvs/tick module-git/tick
@@ -142,6 +144,7 @@ test_expect_success 'import from a CVS working tree' '
 
 	$CVS co -d import-from-wt module &&
 	cd import-from-wt &&
+		git config cvsimport.trackRevisions false &&
 		git cvsimport -a -z0 &&
 		echo 1 >expect &&
 		git log -1 --pretty=format:%s%n >actual &&
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]