{"thread":{"id":"25840","subject":"[PATCH 3/3] cvsimport.txt: document the mapping between config and options","startedAt":"2010-11-25T15:10:54Z","lastAt":"2010-12-03T02:18:08Z","messageCount":36,"participants":["Michael J Gruber","Junio C Hamano","Martin Langhoff","Jonathan Nieder","Jakub Narebski","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"156556","messageId":"939dff25f5a58b6f37cf21bf490b2782f6e96f39.1290697830.git.git@drmicha.warpmail.net","threadId":"25840","inReplyTo":null,"subject":"[PATCH 1/3] cvsimport: partial whitespace cleanup","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-11-25T15:10:54Z","receivedAt":"2010-11-25T15:10:54Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"in preparation of the config parse patch\n\nSigned-off-by: Michael J Gruber \n---\n git-cvsimport.perl | 16 ++++++++--------\n 1 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex d27abfe..f955295 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -91,8 +91,8 @@ sub write_author_info($) {\n \n # convert getopts specs for use by git config\n sub read_repo_config {\n- # Split the string between characters, unless there is a ':'\n- # So \"abc:de\" becomes [\"a\", \"b\", \"c:\", \"d\", \"e\"]\n+\t# Split the string between characters, unless there is a ':'\n+\t# So \"abc:de\" becomes [\"a\", \"b\", \"c:\", \"d\", \"e\"]\n \tmy @opts = split(/ *(?!:)/, shift);\n \tforeach my $o (@opts) {\n \t\tmy $key = $o;\n@@ -100,13 +100,13 @@ sub read_repo_config {\n \t\tmy $arg = 'git config';\n \t\t$arg .= ' --bool' if ($o !~ /:$/);\n \n- chomp(my $tmp = `$arg --get cvsimport.$key`);\n+\t\tchomp(my $tmp = `$arg --get cvsimport.$ckey`);\n \t\tif ($tmp && !($arg =~ /--bool/ && $tmp eq 'false')) {\n- no strict 'refs';\n- my $opt_name = \"opt_\" . $key;\n- if (!$$opt_name) {\n- $$opt_name = $tmp;\n- }\n+\t\t\tno strict 'refs';\n+\t\t\tmy $opt_name = \"opt_\" . $key;\n+\t\t\tif (!$$opt_name) {\n+\t\t\t\t$$opt_name = $tmp;\n+\t\t\t}\n \t\t}\n \t}\n }\n-- \n1.7.3.2.614.g03864.dirty\n"},{"id":"156555","messageId":"ad36bc3f6a00c5f7bc643be3f97aa2bcfda990ff.1290697830.git.git@drmicha.warpmail.net","threadId":"25840","inReplyTo":"939dff25f5a58b6f37cf21bf490b2782f6e96f39.1290697830.git.git@drmicha.warpmail.net","subject":"[PATCH 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-11-25T15:10:55Z","receivedAt":"2010-11-25T15:10:55Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"The current code leads to\n\nfatal: bad config value for 'cvsimport.r' in .git/config\n\nfor a standard use case with cvsimport.r set:\n\ncvsimport sets internal variables by checking the config for each\npossible command line option. The problem is that config items are case\ninsensitive, so config.r and config.R are the same. The ugly error is\ndue to that fact that cvsimport expects a bool for -R (and thus\nconfig.R) but a remote name for -r (and thus config.r).\n\nFix this by making cvsimport expect the config item \"cvsimport.RR\"\nfor the command line option \"-R\" etc.\n\nSigned-off-by: Michael J Gruber \n---\n git-cvsimport.perl | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex f955295..736a7bf 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -97,6 +97,8 @@ sub read_repo_config {\n \tforeach my $o (@opts) {\n \t\tmy $key = $o;\n \t\t$key =~ s/://g;\n+\t\tmy $ckey = $key;\n+\t\t$ckey .= $ckey if ($key eq uc($key));\n \t\tmy $arg = 'git config';\n \t\t$arg .= ' --bool' if ($o !~ /:$/);\n \n-- \n1.7.3.2.614.g03864.dirty\n"},{"id":"156554","messageId":"d7bf21ade4924949266ac850930a1b208658d22b.1290697830.git.git@drmicha.warpmail.net","threadId":"25840","inReplyTo":"939dff25f5a58b6f37cf21bf490b2782f6e96f39.1290697830.git.git@drmicha.warpmail.net","subject":"[PATCH 3/3] cvsimport.txt: document the mapping between config and options","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-11-25T15:10:56Z","receivedAt":"2010-11-25T15:10:56Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Signed-off-by: Michael J Gruber \n---\n Documentation/git-cvsimport.txt | 7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt\nindex 608cd63..b5d5b27 100644\n--- a/Documentation/git-cvsimport.txt\n+++ b/Documentation/git-cvsimport.txt\n@@ -176,6 +176,13 @@ messages, bug-tracking systems, email archives, and the like.\n -h::\n \tPrint a short usage message and exit.\n \n+CONFIG\n+------\n+For any option '-x' you can set the config variable 'cvsimport.x' to the value\n+you would specify for '-x', or to 'true' for a boolean option. For an\n+uppercase option '-X' use the config variable 'cvsimport.xx' (or\n+'cvsimport.XX').\n+\n OUTPUT\n ------\n If '-v' is specified, the script reports what it is doing.\n-- \n1.7.3.2.614.g03864.dirty\n"},{"id":"156716","messageId":"7v8w0fp978.fsf@alter.siamese.dyndns.org","threadId":"25840","inReplyTo":"939dff25f5a58b6f37cf21bf490b2782f6e96f39.1290697830.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 1/3] cvsimport: partial whitespace cleanup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-27T06:33:15Z","receivedAt":"2010-11-27T06:33:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> @@ -100,13 +100,13 @@ sub read_repo_config {\n> \t\tmy $arg = 'git config';\n> \t\t$arg .= ' --bool' if ($o !~ /:$/);\n> \n> - chomp(my $tmp = `$arg --get cvsimport.$key`);\n> +\t\tchomp(my $tmp = `$arg --get cvsimport.$ckey`);\n\nAt this point in the series you haven't introduced $ckey yet, hence this\nwill break, no?\n"},{"id":"156717","messageId":"7vwrnznueh.fsf@alter.siamese.dyndns.org","threadId":"25840","inReplyTo":"ad36bc3f6a00c5f7bc643be3f97aa2bcfda990ff.1290697830.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-27T06:38:14Z","receivedAt":"2010-11-27T06:38:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> The current code leads to\n>\n> fatal: bad config value for 'cvsimport.r' in .git/config\n>\n> for a standard use case with cvsimport.r set:\n>\n> cvsimport sets internal variables by checking the config for each\n> possible command line option. The problem is that config items are case\n> insensitive, so config.r and config.R are the same. The ugly error is\n> due to that fact that cvsimport expects a bool for -R (and thus\n> config.R) but a remote name for -r (and thus config.r).\n>\n> Fix this by making cvsimport expect the config item \"cvsimport.RR\"\n> for the command line option \"-R\" etc.\n\nI do not think this is \"fixing\" per-se. Isn't it more like \"We didn't\nhave a way to use the configuration file to specify uppercase option; now\nwe do thanks to this patch, and here is how\"?\n\nAnd the \"here is how\" workaround, while it may be a reasonable way out, is\nso obscure that it needs to be documented, no? Ahh, that is what [3/3] is.\n\nThe $ckey change from 1/3 needs to be done here, I think.\n"},{"id":"156764","messageId":"4CF2ADC9.8020504@drmicha.warpmail.net","threadId":"25840","inReplyTo":"7vwrnznueh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-11-28T19:30:17Z","receivedAt":"2010-11-28T19:30:17Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 27.11.2010 07:38:\n> Michael J Gruber writes:\n> \n>> The current code leads to\n>>\n>> fatal: bad config value for 'cvsimport.r' in .git/config\n>>\n>> for a standard use case with cvsimport.r set:\n>>\n>> cvsimport sets internal variables by checking the config for each\n>> possible command line option. The problem is that config items are case\n>> insensitive, so config.r and config.R are the same. The ugly error is\n>> due to that fact that cvsimport expects a bool for -R (and thus\n>> config.R) but a remote name for -r (and thus config.r).\n>>\n>> Fix this by making cvsimport expect the config item \"cvsimport.RR\"\n>> for the command line option \"-R\" etc.\n> \n> I do not think this is \"fixing\" per-se. Isn't it more like \"We didn't\n> have a way to use the configuration file to specify uppercase option; now\n> we do thanks to this patch, and here is how\"?\n\nIt is a fix because the the existing code base checks for \"cvsimport.r\"\nin order to set \"opt_r\" and for \"cvsimport.R\" in order to set \"opt_R\"\n(etc). That is, it sets \"opt_R\" from a config variable which should have\nnothing to do with it (cvsimport.r=cvsimport.R).\n\nIn the case when R expects a boolean you'll notice because \"config get\n--bool\" will die, but if both \"x\" and \"X\" expect the same type then you\nset defaults for \"opt_x\" and \"opt_X\" from the single \"cvsimport.x\".\n\n> And the \"here is how\" workaround, while it may be a reasonable way out, is\n> so obscure that it needs to be documented, no? Ahh, that is what [3/3] is.\n\nYes, 3/3. Should I squash this with 2?\n\n> The $ckey change from 1/3 needs to be done here, I think.\n\nYes, sorry, this was an \"add -p\" related oversight on my side. I'll resend.\n\nMichael\n"},{"id":"156765","messageId":"468f815beeede312f98daa66986ce8bf887724b3.1290973138.git.git@drmicha.warpmail.net","threadId":"25840","inReplyTo":"4CF2ADC9.8020504@drmicha.warpmail.net","subject":"[PATCHv2 1/3] cvsimport: partial whitespace cleanup","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-11-28T19:39:45Z","receivedAt":"2010-11-28T19:39:45Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"in preparation of the config parse patch\n\nSigned-off-by: Michael J Gruber \n---\n git-cvsimport.perl | 16 ++++++++--------\n 1 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex d27abfe..7888b77 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -91,8 +91,8 @@ sub write_author_info($) {\n \n # convert getopts specs for use by git config\n sub read_repo_config {\n- # Split the string between characters, unless there is a ':'\n- # So \"abc:de\" becomes [\"a\", \"b\", \"c:\", \"d\", \"e\"]\n+\t# Split the string between characters, unless there is a ':'\n+\t# So \"abc:de\" becomes [\"a\", \"b\", \"c:\", \"d\", \"e\"]\n \tmy @opts = split(/ *(?!:)/, shift);\n \tforeach my $o (@opts) {\n \t\tmy $key = $o;\n@@ -100,13 +100,13 @@ sub read_repo_config {\n \t\tmy $arg = 'git config';\n \t\t$arg .= ' --bool' if ($o !~ /:$/);\n \n- chomp(my $tmp = `$arg --get cvsimport.$key`);\n+\t\tchomp(my $tmp = `$arg --get cvsimport.$key`);\n \t\tif ($tmp && !($arg =~ /--bool/ && $tmp eq 'false')) {\n- no strict 'refs';\n- my $opt_name = \"opt_\" . $key;\n- if (!$$opt_name) {\n- $$opt_name = $tmp;\n- }\n+\t\t\tno strict 'refs';\n+\t\t\tmy $opt_name = \"opt_\" . $key;\n+\t\t\tif (!$$opt_name) {\n+\t\t\t\t$$opt_name = $tmp;\n+\t\t\t}\n \t\t}\n \t}\n }\n-- \n1.7.3.2.614.g03864.dirty\n"},{"id":"156766","messageId":"d501b7423a370f9b7f95c8f23c4b6adfc2963643.1290973138.git.git@drmicha.warpmail.net","threadId":"25840","inReplyTo":"4CF2ADC9.8020504@drmicha.warpmail.net","subject":"[PATCHv2 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-11-28T19:39:46Z","receivedAt":"2010-11-28T19:39:46Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"The current code leads to\n\nfatal: bad config value for 'cvsimport.r' in .git/config\n\nfor a standard use case with cvsimport.r set:\n\ncvsimport sets internal variables by checking the config for each\npossible command line option. The problem is that config items are case\ninsensitive, so config.r and config.R are the same. The ugly error is\ndue to that fact that cvsimport expects a bool for -R (and thus\nconfig.R) but a remote name for -r (and thus config.r).\n\nFix this by making cvsimport expect the config item \"cvsimport.RR\"\nfor the command line option \"-R\" etc.\n\nSigned-off-by: Michael J Gruber \n---\n git-cvsimport.perl | 4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 7888b77..736a7bf 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -97,10 +97,12 @@ sub read_repo_config {\n \tforeach my $o (@opts) {\n \t\tmy $key = $o;\n \t\t$key =~ s/://g;\n+\t\tmy $ckey = $key;\n+\t\t$ckey .= $ckey if ($key eq uc($key));\n \t\tmy $arg = 'git config';\n \t\t$arg .= ' --bool' if ($o !~ /:$/);\n \n-\t\tchomp(my $tmp = `$arg --get cvsimport.$key`);\n+\t\tchomp(my $tmp = `$arg --get cvsimport.$ckey`);\n \t\tif ($tmp && !($arg =~ /--bool/ && $tmp eq 'false')) {\n \t\t\tno strict 'refs';\n \t\t\tmy $opt_name = \"opt_\" . $key;\n-- \n1.7.3.2.614.g03864.dirty\n"},{"id":"156767","messageId":"317523448870044a68311f811b42bd9461b90d43.1290973138.git.git@drmicha.warpmail.net","threadId":"25840","inReplyTo":"4CF2ADC9.8020504@drmicha.warpmail.net","subject":"[PATCHv2 3/3] cvsimport.txt: document the mapping between config and options","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-11-28T19:39:47Z","receivedAt":"2010-11-28T19:39:47Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Signed-off-by: Michael J Gruber \n---\n Documentation/git-cvsimport.txt | 7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt\nindex 608cd63..b5d5b27 100644\n--- a/Documentation/git-cvsimport.txt\n+++ b/Documentation/git-cvsimport.txt\n@@ -176,6 +176,13 @@ messages, bug-tracking systems, email archives, and the like.\n -h::\n \tPrint a short usage message and exit.\n \n+CONFIG\n+------\n+For any option '-x' you can set the config variable 'cvsimport.x' to the value\n+you would specify for '-x', or to 'true' for a boolean option. For an\n+uppercase option '-X' use the config variable 'cvsimport.xx' (or\n+'cvsimport.XX').\n+\n OUTPUT\n ------\n If '-v' is specified, the script reports what it is doing.\n-- \n1.7.3.2.614.g03864.dirty\n"},{"id":"156835","messageId":"7v7hfv3mli.fsf@alter.siamese.dyndns.org","threadId":"25840","inReplyTo":"317523448870044a68311f811b42bd9461b90d43.1290973138.git.git@drmicha.warpmail.net","subject":"Re: [PATCHv2 3/3] cvsimport.txt: document the mapping between config and options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-29T20:23:53Z","receivedAt":"2010-11-29T20:23:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Signed-off-by: Michael J Gruber \n> ---\n> Documentation/git-cvsimport.txt | 7 +++++++\n> 1 files changed, 7 insertions(+), 0 deletions(-)\n>\n> diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt\n> index 608cd63..b5d5b27 100644\n> --- a/Documentation/git-cvsimport.txt\n> +++ b/Documentation/git-cvsimport.txt\n> @@ -176,6 +176,13 @@ messages, bug-tracking systems, email archives, and the like.\n> -h::\n> \tPrint a short usage message and exit.\n> \n> +CONFIG\n> +------\n> +For any option '-x' you can set the config variable 'cvsimport.x' to the value\n> +you would specify for '-x', or to 'true' for a boolean option. For an\n> +uppercase option '-X' use the config variable 'cvsimport.xx' (or\n> +'cvsimport.XX').\n> +\n\nI still think this is not about fixing \"parsing\" as 2/3 states but about\n\"working around the initial design flaw of how configuration variables are\nused in cvsimport\" in that the initial design didn't take it into account\nthat the last component of a configuration variable is case insensitive.\n\nWhile mapping -X to .xx may be a usable workaround, it looks really ugly.\nWorse, if we are going to give long command line options to the command\nsomeday, we will really regret it doing it the way your patch does.\n\nWould it be a better alternative to give conflicting but rarely used\nuppercase options longer option name synonyms, and have them specified in\nthe gitconfig file in their full names? Then we can disambiguate with\nsomething like\n\n [cvsimport]\n \tgenerate-cvs-revisions = yes\n remote = origin\n\nwhich would be more readable, no?\n"},{"id":"156884","messageId":"4CF4AE49.7090406@drmicha.warpmail.net","threadId":"25840","inReplyTo":"7v7hfv3mli.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2 3/3] cvsimport.txt: document the mapping between config and options","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-11-30T07:56:57Z","receivedAt":"2010-11-30T07:56:57Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 29.11.2010 21:23:\n> Michael J Gruber writes:\n> \n>> Signed-off-by: Michael J Gruber \n>> ---\n>> Documentation/git-cvsimport.txt | 7 +++++++\n>> 1 files changed, 7 insertions(+), 0 deletions(-)\n>>\n>> diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt\n>> index 608cd63..b5d5b27 100644\n>> --- a/Documentation/git-cvsimport.txt\n>> +++ b/Documentation/git-cvsimport.txt\n>> @@ -176,6 +176,13 @@ messages, bug-tracking systems, email archives, and the like.\n>> -h::\n>> \tPrint a short usage message and exit.\n>> \n>> +CONFIG\n>> +------\n>> +For any option '-x' you can set the config variable 'cvsimport.x' to the value\n>> +you would specify for '-x', or to 'true' for a boolean option. For an\n>> +uppercase option '-X' use the config variable 'cvsimport.xx' (or\n>> +'cvsimport.XX').\n>> +\n> \n> I still think this is not about fixing \"parsing\" as 2/3 states but about\n> \"working around the initial design flaw of how configuration variables are\n> used in cvsimport\" in that the initial design didn't take it into account\n> that the last component of a configuration variable is case insensitive.\n\nI don't care too much about the naming. But if I specify a correct\nstring value for \"cvsimport.r\" (correct as in correct for \"-r\", lower\ncase!) and \"git cvsimport\" gives me\n\nfatal: bad config value for 'cvsimport.r' in .git/config\n\nthen I call this a bug, notwithstanding the fact that cvsimport does use\nthe value from cvsimport.r for \"-r\" and continues its operation.\n\nThis occurs really without even any attempt at specfiying values for\nupper case options.\n\n> While mapping -X to .xx may be a usable workaround, it looks really ugly.\n> Worse, if we are going to give long command line options to the command\n> someday, we will really regret it doing it the way your patch does.\n> \n> Would it be a better alternative to give conflicting but rarely used\n> uppercase options longer option name synonyms, and have them specified in\n> the gitconfig file in their full names? Then we can disambiguate with\n> something like\n> \n> [cvsimport]\n> \tgenerate-cvs-revisions = yes\n> remote = origin\n> \n> which would be more readable, no?\n> \n\nWell, cvsimport does not have any long options now, and given the fact\nthat most cvsimport related activity lately has been on documenting its\nshortcomings and promoting cvs2git, I consider that scenario highly\nunlikely. (I'm not hooked on cvsimport - it's simply the only\n*incremental* cvs-to-git importer that I know of, and the only one not\nrequiring local access.)\n\nHow about using a naming scheme like:\n\n[cvsimport]\n\tr = origin\n\tcapital-r = yes\n\nThis would be safe against any possible future long-options, quickly\nimplementable, and we would not have to invent long names now for the\nexisting one-letter options (and thus hindering any future attempts\nalso). Whether this is more or less ugly lies in the eye of the\ns/beholder/maintainer/ :)\n\nMichael\n"},{"id":"156905","messageId":"7vd3pmw9n5.fsf@alter.siamese.dyndns.org","threadId":"25840","inReplyTo":"4CF4AE49.7090406@drmicha.warpmail.net","subject":"Re: [PATCHv2 3/3] cvsimport.txt: document the mapping between config and options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-01T01:43:10Z","receivedAt":"2010-12-01T01:43:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> How about using a naming scheme like:\n>\n> [cvsimport]\n> \tr = origin\n> \tcapital-r = yes\n\nI think we can live with that. If it is easier to implement, that's very\ngood.\n"},{"id":"156915","messageId":"cover.1291207768.git.git@drmicha.warpmail.net","threadId":"25840","inReplyTo":"7vd3pmw9n5.fsf@alter.siamese.dyndns.org","subject":"[PATCHv3 0/3] uppercase config options for cvsimport","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-12-01T12:53:19Z","receivedAt":"2010-12-01T12:53:19Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"v3 changes the naming to \"cvsimport.capital-x\" for the config variable\ncorresponding to \"-X\", and amends the commit message in 2/3 to make it\nclear that this \"fix\" addresses an undocumented feature of cvsimport\n(as far as included documentation goes).\n\nAlso, v3 adds a cover letter :)\n\nMichael J Gruber (3):\n cvsimport: partial whitespace cleanup\n cvsimport: fix the parsing of uppercase config options\n cvsimport.txt: document the mapping between config and options\n\n Documentation/git-cvsimport.txt | 7 +++++++\n git-cvsimport.perl | 18 ++++++++++--------\n 2 files changed, 17 insertions(+), 8 deletions(-)\n\n-- \n1.7.3.2.617.g84f63\n"},{"id":"156917","messageId":"468f815beeede312f98daa66986ce8bf887724b3.1291207768.git.git@drmicha.warpmail.net","threadId":"25840","inReplyTo":"7vd3pmw9n5.fsf@alter.siamese.dyndns.org","subject":"[PATCHv3 1/3] cvsimport: partial whitespace cleanup","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-12-01T12:53:20Z","receivedAt":"2010-12-01T12:53:20Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"in preparation of the config parse patch\n\nSigned-off-by: Michael J Gruber \n---\n git-cvsimport.perl | 16 ++++++++--------\n 1 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex d27abfe..7888b77 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -91,8 +91,8 @@ sub write_author_info($) {\n \n # convert getopts specs for use by git config\n sub read_repo_config {\n- # Split the string between characters, unless there is a ':'\n- # So \"abc:de\" becomes [\"a\", \"b\", \"c:\", \"d\", \"e\"]\n+\t# Split the string between characters, unless there is a ':'\n+\t# So \"abc:de\" becomes [\"a\", \"b\", \"c:\", \"d\", \"e\"]\n \tmy @opts = split(/ *(?!:)/, shift);\n \tforeach my $o (@opts) {\n \t\tmy $key = $o;\n@@ -100,13 +100,13 @@ sub read_repo_config {\n \t\tmy $arg = 'git config';\n \t\t$arg .= ' --bool' if ($o !~ /:$/);\n \n- chomp(my $tmp = `$arg --get cvsimport.$key`);\n+\t\tchomp(my $tmp = `$arg --get cvsimport.$key`);\n \t\tif ($tmp && !($arg =~ /--bool/ && $tmp eq 'false')) {\n- no strict 'refs';\n- my $opt_name = \"opt_\" . $key;\n- if (!$$opt_name) {\n- $$opt_name = $tmp;\n- }\n+\t\t\tno strict 'refs';\n+\t\t\tmy $opt_name = \"opt_\" . $key;\n+\t\t\tif (!$$opt_name) {\n+\t\t\t\t$$opt_name = $tmp;\n+\t\t\t}\n \t\t}\n \t}\n }\n-- \n1.7.3.2.617.g84f63\n"},{"id":"156918","messageId":"b61288036e756c78709438491416d573459a732e.1291207768.git.git@drmicha.warpmail.net","threadId":"25840","inReplyTo":"7vd3pmw9n5.fsf@alter.siamese.dyndns.org","subject":"[PATCHv3 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-12-01T12:53:21Z","receivedAt":"2010-12-01T12:53:21Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"The current code leads to\n\nfatal: bad config value for 'cvsimport.r' in .git/config\n\nfor a standard use case with cvsimport.r set:\n\ncvsimport sets internal variables by checking the config for each\npossible command line option. The problem is that config items are case\ninsensitive, so config.r and config.R are the same. The ugly error is\ndue to that fact that cvsimport expects a bool for -R (and thus\nconfig.R) but a remote name for -r (and thus config.r).\n\nFix this by making cvsimport expect the config item \"cvsimport.capital-r\"\nfor the command line option \"-R\" etc.\n\n(config options for cvsimport have been undocumented so far, though\npresent in the code and advertised in several tutorials. So one may read\n\"enhance\" for \"fix\".)\n\nSigned-off-by: Michael J Gruber \n---\n git-cvsimport.perl | 4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 7888b77..0bb5e32 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -97,10 +97,12 @@ sub read_repo_config {\n \tforeach my $o (@opts) {\n \t\tmy $key = $o;\n \t\t$key =~ s/://g;\n+\t\tmy $ckey = $key;\n+\t\t$ckey = 'capital-' . $ckey if ($key eq uc($key));\n \t\tmy $arg = 'git config';\n \t\t$arg .= ' --bool' if ($o !~ /:$/);\n \n-\t\tchomp(my $tmp = `$arg --get cvsimport.$key`);\n+\t\tchomp(my $tmp = `$arg --get cvsimport.$ckey`);\n \t\tif ($tmp && !($arg =~ /--bool/ && $tmp eq 'false')) {\n \t\t\tno strict 'refs';\n \t\t\tmy $opt_name = \"opt_\" . $key;\n-- \n1.7.3.2.617.g84f63\n"},{"id":"156919","messageId":"84f63e5082da0bf483175584399688385fb6002d.1291207768.git.git@drmicha.warpmail.net","threadId":"25840","inReplyTo":"7vd3pmw9n5.fsf@alter.siamese.dyndns.org","subject":"[PATCHv3 3/3] cvsimport.txt: document the mapping between config and options","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-12-01T12:53:22Z","receivedAt":"2010-12-01T12:53:22Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Signed-off-by: Michael J Gruber \n---\n Documentation/git-cvsimport.txt | 7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt\nindex 608cd63..fc77a3b 100644\n--- a/Documentation/git-cvsimport.txt\n+++ b/Documentation/git-cvsimport.txt\n@@ -176,6 +176,13 @@ messages, bug-tracking systems, email archives, and the like.\n -h::\n \tPrint a short usage message and exit.\n \n+CONFIG\n+------\n+For any option '-x' you can set the config variable 'cvsimport.x' to the value\n+you would specify for '-x', or to 'true' for a boolean option. For an\n+uppercase option '-X' use the config variable 'cvsimport.capital-x' (or\n+'cvsimport.capital-X').\n+\n OUTPUT\n ------\n If '-v' is specified, the script reports what it is doing.\n-- \n1.7.3.2.617.g84f63\n"},{"id":"156936","messageId":"AANLkTi=cb0i28oGi9vrYfEyU_fPHhQGhNNtramtCpCHy@mail.gmail.com","threadId":"25840","inReplyTo":"b61288036e756c78709438491416d573459a732e.1291207768.git.git@drmicha.warpmail.net","subject":"Re: [PATCHv3 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2010-12-01T14:59:42Z","receivedAt":"2010-12-01T14:59:42Z","isPatch":false,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On Wed, Dec 1, 2010 at 7:53 AM, Michael J Gruber\n wrote:\n> Fix this by making cvsimport expect the config item \"cvsimport.capital-r\"\n> for the command line option \"-R\" etc.\n\nGood point to fix this, thanks. But 'capital-r' will muddy the waters\nfurther. When accepting configuration from git-config, make all these\nvalues the full expanded name.\n\nSo cvsimport.remote (for -r) and cvsimport.revisions (or\ntrackrevisions perhaps) seem more appropriate.\n\ncheers,\n\n\nm\n-- \n martin.langhoff@gmail.com\n martin@laptop.org -- School Server Architect\n - ask interesting questions\n - don't get distracted with shiny stuff  - working code first\n - http://wiki.laptop.org/go/User:Martinlanghoff\n"},{"id":"156937","messageId":"AANLkTimEU8RhhaJkv6WdKw3V-6stptRA0Z5SYbLxXNAR@mail.gmail.com","threadId":"25840","inReplyTo":"7vd3pmw9n5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2 3/3] cvsimport.txt: document the mapping between config and options","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2010-12-01T15:02:40Z","receivedAt":"2010-12-01T15:02:40Z","isPatch":false,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On Tue, Nov 30, 2010 at 8:43 PM, Junio C Hamano wrote:\n>> [cvsimport]\n>>       r = origin\n>>       capital-r = yes\n>\n> I think we can live with that.  If it is easier to implement, that's very\n> good.\n\nSorry, had not seen this discussion. Though I will obviously defer to\nyou guys, I don't like it -- not short term, not long term.\n\nShort-name opts should not auto-read from git-config -- it's a misfire.\n\n\n\nm\n-- \n martin.langhoff@gmail.com\n martin@laptop.org -- School Server Architect\n - ask interesting questions\n - don't get distracted with shiny stuff  - working code first\n - http://wiki.laptop.org/go/User:Martinlanghoff\n"},{"id":"156943","messageId":"4CF66B14.7000600@drmicha.warpmail.net","threadId":"25840","inReplyTo":"AANLkTimEU8RhhaJkv6WdKw3V-6stptRA0Z5SYbLxXNAR@mail.gmail.com","subject":"Re: [PATCHv2 3/3] cvsimport.txt: document the mapping between config and options","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-12-01T15:34:44Z","receivedAt":"2010-12-01T15:34:44Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Martin Langhoff venit, vidit, dixit 01.12.2010 16:02:\n> On Tue, Nov 30, 2010 at 8:43 PM, Junio C Hamano wrote:\n>>> [cvsimport]\n>>> r = origin\n>>> capital-r = yes\n>>\n>> I think we can live with that. If it is easier to implement, that's very\n>> good.\n> \n> Sorry, had not seen this discussion. Though I will obviously defer to\n> you guys, I don't like it -- not short term, not long term.\n> \n> Short-name opts should not auto-read from git-config -- it's a misfire.\n\nWell, you're free to change that. There *are no* long options for\ncvsimport right now. And it does accept short config variables (though\nundocumented) right now. That pretty much describes the two obstacles\nyou'd be running into.\n\nMichael\n"},{"id":"156951","messageId":"20101201160503.GB26120@burratino","threadId":"25840","inReplyTo":"AANLkTi=cb0i28oGi9vrYfEyU_fPHhQGhNNtramtCpCHy@mail.gmail.com","subject":"Re: [PATCHv3 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-01T16:05:03Z","receivedAt":"2010-12-01T16:05:03Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Martin Langhoff wrote:\n\n> So cvsimport.remote (for -r) and cvsimport.revisions (or\n> trackrevisions perhaps) seem more appropriate.\n\nI somewhat like this idea. So let's build a full table, shall\nwe? The embedded dashes are meant for the command-line options\nrather than the config file.\n\n\t-v\tverbosity\n\t-d\tcvsroot\n\t-C\t[doesn't make sense in a config file; you've already\n\t\t found where to read the configuration from, right?]\n\t-r\tremote\n\t-o\tmainline\n\t-i\timport-only\n\t-k\tkill-keywords\n\t-u\treplace-underscores\n\t-s\treplace-slashes\n\t-p\tcvsps-options\n\t-z\tfuzz\n\t-P\t[doesn't make much sense in a config file; for one-shot use]\n\t-m\tdetect-merges\n\t-M\tmerge-regex\n\t-S\tignore-paths\n\t-a\timport-all\n\t-L\tmax-commits\n\t-A\tauthors-file\n\t-R\ttrack-revisions\n\t-h\t[doesn't make sense in a config file]\n\nHmm?\nJonathan\n"},{"id":"156953","messageId":"AANLkTinJD_WRBxFBMs-p5ox0wZWZvddhSfan0yhaNyGk@mail.gmail.com","threadId":"25840","inReplyTo":"20101201160503.GB26120@burratino","subject":"Re: [PATCHv3 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2010-12-01T16:18:14Z","receivedAt":"2010-12-01T16:18:14Z","isPatch":false,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On Wed, Dec 1, 2010 at 11:05 AM, Jonathan Nieder wrote:\n> I somewhat like this idea.  So let's build a full table, shall\n> we?  The embedded dashes are meant for the command-line options\n> rather than the config file.\n\nThe suggested expansions sound good to me, with one comment:\n\n>        -o      mainline\n\nI am not sure about this one. The 'o' originally stood for 'origin'\nbranch (back when git didn't use 'origin/master', or maybe I was stuck\nwith Cogito's mental model).\n\nIt is the branchname for CVS HEAD; it defaults to 'origin' -- given\ntoday's conventions it should be 'master'.\n\nI would call it cvshead.\n\n\n\n\nm\n-- \n martin.langhoff@gmail.com\n martin@laptop.org -- School Server Architect\n - ask interesting questions\n - don't get distracted with shiny stuff  - working code first\n - http://wiki.laptop.org/go/User:Martinlanghoff\n"},{"id":"156954","messageId":"m3aakp4g5o.fsf@localhost.localdomain","threadId":"25840","inReplyTo":"20101201160503.GB26120@burratino","subject":"Re: [PATCHv3 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-12-01T16:23:13Z","receivedAt":"2010-12-01T16:23:13Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jonathan Nieder writes:\n\n> Martin Langhoff wrote:\n> \n> > So cvsimport.remote (for -r) and cvsimport.revisions (or\n> > trackrevisions perhaps) seem more appropriate.\n> \n> I somewhat like this idea. So let's build a full table, shall\n> we? The embedded dashes are meant for the command-line options\n> rather than the config file.\n> \n> \t-v\tverbosity\n> \t-d\tcvsroot\n> \t-C\t[doesn't make sense in a config file; you've already\n> \t\t found where to read the configuration from, right?]\n> \t-r\tremote\n> \t-o\tmainline\n> \t-i\timport-only\n> \t-k\tkill-keywords\n> \t-u\treplace-underscores\n> \t-s\treplace-slashes\n> \t-p\tcvsps-options\n> \t-z\tfuzz\n> \t-P\t[doesn't make much sense in a config file; for one-shot use]\n> \t-m\tdetect-merges\n> \t-M\tmerge-regex\n> \t-S\tignore-paths\n> \t-a\timport-all\n> \t-L\tmax-commits\n> \t-A\tauthors-file\n> \t-R\ttrack-revisions\n> \t-h\t[doesn't make sense in a config file]\n> \n> Hmm?\n\nGood idea, though I'd rather we avoid new convention for multi word\nnames separated with '-' (multi-word), but rather use camel case\n(multiWord).\n\nCurrently we have only unfortunate exception of `add.ignore-errors',\nall others use either same case (`core.ignorecase') or camelCase\n(`core.ignoreStat`).\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"156955","messageId":"20101201163406.GD26120@burratino","threadId":"25840","inReplyTo":"m3aakp4g5o.fsf@localhost.localdomain","subject":"Re: [PATCHv3 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-01T16:34:06Z","receivedAt":"2010-12-01T16:34:06Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(+cc: Jeff, config parsing wizard)\n\nJakub Narebski wrote:\n> Jonathan Nieder writes:\n\n>> I somewhat like this idea. So let's build a full table, shall\n>> we? The embedded dashes are meant for the command-line options\n>> rather than the config file.\n[...]\n> Good idea, though I'd rather we avoid new convention for multi word\n> names separated with '-' (multi-word), but rather use camel case\n> (multiWord).\n\nNo disagreement there. :)\n\n> Currently we have only unfortunate exception of `add.ignore-errors',\n\nMaybe we can teach the config file parser to ignore dashes in addition\nto case (except in the names of [genus \"species\"] headings)? That\nwould be an incompatible change for third-party tools, though, so\nmaybe \"git config\" would have to take an explicit --strip-dashes\nflag to do it.\n"},{"id":"156959","messageId":"4CF67D36.9020908@drmicha.warpmail.net","threadId":"25840","inReplyTo":"20101201163406.GD26120@burratino","subject":"Re: [PATCHv3 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-12-01T16:52:06Z","receivedAt":"2010-12-01T16:52:06Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"So you guys are going to break current behaviour (for \"cvsimport.r\" etc.)?\n\nI hate it when simple things get held up like this. Time to go home for\ntoday... Food deprivation makes grumpy.\n\nMichael\n"},{"id":"156960","messageId":"20101201170140.GG26120@burratino","threadId":"25840","inReplyTo":"4CF67D36.9020908@drmicha.warpmail.net","subject":"Re: [PATCHv3 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-01T17:01:40Z","receivedAt":"2010-12-01T17:01:40Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Michael J Gruber wrote:\n\n> So you guys are going to break current behaviour (for \"cvsimport.r\" etc.)?\n\nActual git cvsimport users get the real vote.\n\nBut yes, I would like to break current behavior for cvsimport.r, since\nthe current behavior is insane. On the other hand, I think it is fine\nto preserve the current behavior for cvsimport.d.\n\nMeanwhile we would get better documentation and self-describing\ncommand lines:\n\n\tgit cvsimport --cvshead=master --authors-file=$(pwd)/.git/cvs-authors \\\n\t\t... etc ...\n\n> I hate it when simple things get held up like this.\n\nPerhaps it is a case of everyone knowing what an ugly shed is and thus\nspending the time to make it a little better.\n\nAnyway, feel free to ignore me in this case if you want.\nJonathan\n"},{"id":"156967","messageId":"20101201175517.GA7774@sigill.intra.peff.net","threadId":"25840","inReplyTo":"20101201163406.GD26120@burratino","subject":"Re: [PATCHv3 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-12-01T17:55:17Z","receivedAt":"2010-12-01T17:55:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 01, 2010 at 10:34:06AM -0600, Jonathan Nieder wrote:\n\n> (+cc: Jeff, config parsing wizard)\n\nUgh, that is not a title that I aspire to. :)\n\n> > Good idea, though I'd rather we avoid new convention for multi word\n> > names separated with '-' (multi-word), but rather use camel case\n> > (multiWord).\n> \n> No disagreement there. :)\n> \n> > Currently we have only unfortunate exception of `add.ignore-errors',\n\nAt some point there was talk of making this add.ignoreErrors in the\ndocs, and just keeping the add.ignore-errors alias forever for backwards\ncompatibility.\n\n> Maybe we can teach the config file parser to ignore dashes in addition\n> to case (except in the names of [genus \"species\"] headings)? That\n> would be an incompatible change for third-party tools, though, so\n> maybe \"git config\" would have to take an explicit --strip-dashes\n> flag to do it.\n\nBut if you require --strip-dashes, then you get potentially differing\nbehavior for the same set of options (i.e., one tool may accept \"foobar\"\nbut the other requires \"foo-bar\", because the latter has not been\nupdated to --strip-dashes).\n\nNaively, such a patch would look like:\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 22c6329..944c54f 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -331,7 +331,7 @@ static struct option builtin_add_options[] = {\n \n static int add_config(const char *var, const char *value, void *cb)\n {\n-\tif (!strcasecmp(var, \"add.ignore-errors\")) {\n+\tif (!strcasecmp(var, \"add.ignoreerrors\")) {\n \t\tignore_add_errors = git_config_bool(var, value);\n \t\treturn 0;\n \t}\ndiff --git a/config.c b/config.c\nindex d8ce653..e5cb5f9 100644\n--- a/config.c\n+++ b/config.c\n@@ -211,6 +211,8 @@ static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)\n \t\t\tbreak;\n \t\tif (!iskeychar(c))\n \t\t\tbreak;\n+\t\tif (c == '-')\n+\t\t\tcontinue;\n \t\tname[len++] = tolower(c);\n \t\tif (len >= MAXNAME)\n \t\t\treturn -1;\n\nbut there are a lot of corner cases. What happens with --get-regexp?\nWhat happens with headings like '[foo-bar \"baz\"]'. When we use\n\"git-config\" to edit, do we properly preserve dashes?\n\nI started to think about all of these things, and then realized it\nprobably just isn't worth it. If we care about no-dashes, then let's\nenforce no-dashes in new options. If we care about the existing\nadd.ignore-errors, then let's fix it but keep the old version there for\nbackwards compatibility. Those are easy to do, and then the problem just\ngoes away.\n\n-Peff\n"},{"id":"156968","messageId":"20101201175815.GB7774@sigill.intra.peff.net","threadId":"25840","inReplyTo":"4CF67D36.9020908@drmicha.warpmail.net","subject":"Re: [PATCHv3 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-12-01T17:58:15Z","receivedAt":"2010-12-01T17:58:15Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 01, 2010 at 05:52:06PM +0100, Michael J Gruber wrote:\n\n> So you guys are going to break current behaviour (for \"cvsimport.r\" etc.)?\n\nI have not been following this thread, but as I understand it,\n\"cvsimport.r\" is somewhat broken already, isn't it? I thought the\noriginal problem was that it got parsed as both \"-r\" and \"-R\", and one\nof them tried to enforce some type semantics on the value.\n\nIf there are short config options that work, though, we should probably\nkeep them. It surely can't be that hard for the perl code to accept both\n\"cvsimport.r\" and \"cvsimport.remote\" for \"-r\" and\n\"cvsimport.generaterevisions\" (or whatever) for \"-R\"?\n\n-Peff\n"},{"id":"156978","messageId":"20101201183615.GA27024@burratino","threadId":"25840","inReplyTo":"20101201175517.GA7774@sigill.intra.peff.net","subject":"[PATCH] add: introduce add.ignoreerrors synonym for add.ignore-errors","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-01T18:36:15Z","receivedAt":"2010-12-01T18:36:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The \"[add] ignore-errors\" tweakable introduced by v1.5.6-rc0~30^2 (Add\na config option to ignore errors for git-add, 2008-05-12) does not\nfollow the usual convention for naming values in the git configuration\nfile.\n\nWhat convention? Glad you asked.\n\n\tThe section name indicates the affected subsystem.\n\n\tThe subsection name, if any, indicates which of\n\tan unbound set of things to set the value for.\n\n\tThe variable name describes the effect of tweaking\n\tthis knob.\n\n\tThe section and variable names can be broken into\n\twords using bumpyCaps in documentation as a hint to\n\tthe reader. These word breaks are not significant\n\tat the level of code, since the section and variable\n\tnames are not case sensitive.\n\nThe name \"add.ignore-errors\" includes a dash, meaning a naive\nconfiguration file like\n\n\t[add]\n\t\tignoreerrors\n\ndoes not have any effect. Avoid such confusion by renaming to the\nmore consistent add.ignoreErrors, but keep the old version for\nbackwards compatibility.\n\nSuggested-by: Jeff King \nSigned-off-by: Jonathan Nieder \n---\nJeff King wrote:\n> On Wed, Dec 01, 2010 at 10:34:06AM -0600, Jonathan Nieder wrote:\n\n>> (+cc: Jeff, config parsing wizard)\n>\n> Ugh, that is not a title that I aspire to. :)\n\nYes, one day the code will be clean enough that wizardry is not\nneeded. :)\n\n[...]\n> But if you require --strip-dashes, then you get potentially differing\n> behavior for the same set of options (i.e., one tool may accept \"foobar\"\n> but the other requires \"foo-bar\", because the latter has not been\n> updated to --strip-dashes).\n\nIt was a bad idea, so let's do the simple thing. Like this?\n\n Documentation/config.txt | 1 +\n builtin/add.c | 3 ++-\n 2 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 6a6c0b5..c609de4 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -553,6 +553,7 @@ core.sparseCheckout::\n \tEnable \"sparse checkout\" feature. See section \"Sparse checkout\" in\n \tlinkgit:git-read-tree[1] for more information.\n \n+add.ignoreErrors::\n add.ignore-errors::\n \tTells 'git add' to continue adding files when some files cannot be\n \tadded due to indexing errors. Equivalent to the '--ignore-errors'\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 71f9b04..21dc1f7 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -331,7 +331,8 @@ static struct option builtin_add_options[] = {\n \n static int add_config(const char *var, const char *value, void *cb)\n {\n-\tif (!strcasecmp(var, \"add.ignore-errors\")) {\n+\tif (!strcasecmp(var, \"add.ignoreerrors\") ||\n+\t !strcasecmp(var, \"add.ignore-errors\")) {\n \t\tignore_add_errors = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n-- \n1.7.2.3\n"},{"id":"156980","messageId":"20101201184659.GA9147@sigill.intra.peff.net","threadId":"25840","inReplyTo":"20101201183615.GA27024@burratino","subject":"Re: [PATCH] add: introduce add.ignoreerrors synonym for add.ignore-errors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-12-01T18:46:59Z","receivedAt":"2010-12-01T18:46:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 01, 2010 at 12:36:15PM -0600, Jonathan Nieder wrote:\n\n> It was a bad idea, so let's do the simple thing. Like this?\n> \n> Documentation/config.txt | 1 +\n> builtin/add.c | 3 ++-\n\nYes, looks good to me. You could potentially drop the old one from the\nconfig:\n\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -553,6 +553,7 @@ core.sparseCheckout::\n> \tEnable \"sparse checkout\" feature. See section \"Sparse checkout\" in\n> \tlinkgit:git-read-tree[1] for more information.\n> \n> +add.ignoreErrors::\n> add.ignore-errors::\n> \tTells 'git add' to continue adding files when some files cannot be\n> \tadded due to indexing errors. Equivalent to the '--ignore-errors'\n\nwhich may be less confusing to new users (who might ask \"is there a\ndifference between the two?\").\n\nBut I don't have a strong feeling on it, so either way:\n\nAcked-by: Jeff King \n\n-Peff\n"},{"id":"156983","messageId":"20101201185711.GD27024@burratino","threadId":"25840","inReplyTo":"20101201184659.GA9147@sigill.intra.peff.net","subject":"Re: [PATCH] add: introduce add.ignoreerrors synonym for add.ignore-errors","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-01T18:57:11Z","receivedAt":"2010-12-01T18:57:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> Yes, looks good to me. You could potentially drop the old one from the\n> config:\n>\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -553,6 +553,7 @@ core.sparseCheckout::\n>> \tEnable \"sparse checkout\" feature. See section \"Sparse checkout\" in\n>> \tlinkgit:git-read-tree[1] for more information.\n>> \n>> +add.ignoreErrors::\n>> add.ignore-errors::\n>> \tTells 'git add' to continue adding files when some files cannot be\n>> \tadded due to indexing errors. Equivalent to the '--ignore-errors'\n>\n> which may be less confusing to new users (who might ask \"is there a\n> difference between the two?\").\n\nRight, I prefer to keep it documented for the old and forgetful users\n(who might ask \"why does this configuration work?\").\n\n> But I don't have a strong feeling on it, so either way:\n>\n> Acked-by: Jeff King \n\nThanks again for the help.\n"},{"id":"156997","messageId":"7vvd3duvfy.fsf@alter.siamese.dyndns.org","threadId":"25840","inReplyTo":"20101201175815.GB7774@sigill.intra.peff.net","subject":"Re: [PATCHv3 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-01T19:47:29Z","receivedAt":"2010-12-01T19:47:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> If there are short config options that work, though, we should probably\n> keep them. It surely can't be that hard for the perl code to accept both\n> \"cvsimport.r\" and \"cvsimport.remote\" for \"-r\" and\n> \"cvsimport.generaterevisions\" (or whatever) for \"-R\"?\n\nI'd agree. The current parameter set we give to GetOptions is:\n\n haivmkuo:d:p:r:C:z:s:M:P:A:S:L:R\n\nand I notice that we cannot sanely access A, M, P, R, S and their\nlowercase counterparts.\n\nIt shouldn't be too hard to interpret the single letter options for the\ncurrently supported set but only for lowercase letters plus '-C ' and\n'-L ' (because there are no lowercase ones that crash with them), and\ngive long config names to at least the ones inaccessible with the above.\n\nWouldn't that give us a regression-free solution to the immediate problem\nat hand?\n\nSupporting long options on the command line, and giving long config name\nsynonyms to the lowercase ones, would be a plus for consistency's sake,\nbut that probably is a separate topic.\n"},{"id":"157001","messageId":"7vmxopuv1c.fsf@alter.siamese.dyndns.org","threadId":"25840","inReplyTo":"20101201185711.GD27024@burratino","subject":"Re: [PATCH] add: introduce add.ignoreerrors synonym for add.ignore-errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-01T19:56:15Z","receivedAt":"2010-12-01T19:56:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Jeff King wrote:\n>\n>> Yes, looks good to me. You could potentially drop the old one from the\n>> config:\n>>\n>>> --- a/Documentation/config.txt\n>>> +++ b/Documentation/config.txt\n>>> @@ -553,6 +553,7 @@ core.sparseCheckout::\n>>> \tEnable \"sparse checkout\" feature. See section \"Sparse checkout\" in\n>>> \tlinkgit:git-read-tree[1] for more information.\n>>> \n>>> +add.ignoreErrors::\n>>> add.ignore-errors::\n>>> \tTells 'git add' to continue adding files when some files cannot be\n>>> \tadded due to indexing errors. Equivalent to the '--ignore-errors'\n>>\n>> which may be less confusing to new users (who might ask \"is there a\n>> difference between the two?\").\n>\n> Right, I prefer to keep it documented for the old and forgetful users\n> (who might ask \"why does this configuration work?\").\n>\n>> But I don't have a strong feeling on it, so either way:\n>>\n>> Acked-by: Jeff King \n>\n> Thanks again for the help.\n\nThanks, will queue.\n"},{"id":"157007","messageId":"7vipzduufr.fsf@alter.siamese.dyndns.org","threadId":"25840","inReplyTo":"20101201185711.GD27024@burratino","subject":"Re: [PATCH] add: introduce add.ignoreerrors synonym for add.ignore-errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-01T20:09:12Z","receivedAt":"2010-12-01T20:09:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Jeff King wrote:\n>\n>> Yes, looks good to me. You could potentially drop the old one from the\n>> config:\n>> ...\n>> which may be less confusing to new users (who might ask \"is there a\n>> difference between the two?\").\n>\n> Right, I prefer to keep it documented for the old and forgetful users\n> (who might ask \"why does this configuration work?\").\n\nWe actually need to stress the fact that _only_ newer git will understand\n\"add.ignoreErrors\", so people with older git (especially the ones who need\nto use both older and newer versions of git) may need to keep using the\nolder form in their configuration, no?\n\nPerhaps phrasing it like this, and then issue v1.7.{0,1,2,3}.X maintenance\nrelease to reduce the chance of inconveniencing the users?\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1df0f65..5b6f667 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -539,9 +539,12 @@ core.sparseCheckout::\n \tlinkgit:git-read-tree[1] for more information.\n \n add.ignore-errors::\n+add.ignoreErrors::\n \tTells 'git add' to continue adding files when some files cannot be\n \tadded due to indexing errors. Equivalent to the '--ignore-errors'\n-\toption of linkgit:git-add[1].\n+\toption of linkgit:git-add[1]. Older versions of git accept only\n+\t`add.ignore-errors`, whose name goes against the convention, so\n+\tnewer versions of git honor `add.ignoreErrors` as well.\n \n alias.*::\n \tCommand aliases for the linkgit:git[1] command wrapper - e.g.\n"},{"id":"157024","messageId":"20101201210742.GA13740@sigill.intra.peff.net","threadId":"25840","inReplyTo":"7vipzduufr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add: introduce add.ignoreerrors synonym for add.ignore-errors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-12-01T21:07:42Z","receivedAt":"2010-12-01T21:07:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 01, 2010 at 12:09:12PM -0800, Junio C Hamano wrote:\n\n> We actually need to stress the fact that _only_ newer git will understand\n> \"add.ignoreErrors\", so people with older git (especially the ones who need\n> to use both older and newer versions of git) may need to keep using the\n> older form in their configuration, no?\n> \n> Perhaps phrasing it like this, and then issue v1.7.{0,1,2,3}.X maintenance\n> release to reduce the chance of inconveniencing the users?\n\nYeah, I think it is better to be explicit.\n\n> add.ignore-errors::\n> +add.ignoreErrors::\n> \tTells 'git add' to continue adding files when some files cannot be\n> \tadded due to indexing errors. Equivalent to the '--ignore-errors'\n> -\toption of linkgit:git-add[1].\n> +\toption of linkgit:git-add[1]. Older versions of git accept only\n> +\t`add.ignore-errors`, whose name goes against the convention, so\n> +\tnewer versions of git honor `add.ignoreErrors` as well.\n\nNit: Somehow \"goes against the convention\" doesn't sound quite right to\nme. Astute users can probably figure out which convention, but perhaps\nwe should be more explicit like:\n\n Older versions of git accept only `add.ignore-errors`, which does not\n follow the usual naming convention for configuration variable. Newer\n versions of git honor `add.ignoreErrors` as well.\n\n-Peff\n"},{"id":"157157","messageId":"7vtyivq23z.fsf@alter.siamese.dyndns.org","threadId":"25840","inReplyTo":"7vvd3duvfy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv3 2/3] cvsimport: fix the parsing of uppercase config options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-02T21:46:56Z","receivedAt":"2010-12-02T21:46:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> If there are short config options that work, though, we should probably\n>> keep them. It surely can't be that hard for the perl code to accept both\n>> \"cvsimport.r\" and \"cvsimport.remote\" for \"-r\" and\n>> \"cvsimport.generaterevisions\" (or whatever) for \"-R\"?\n>\n> I'd agree. The current parameter set we give to GetOptions is:\n>\n> haivmkuo:d:p:r:C:z:s:M:P:A:S:L:R\n>\n> and I notice that we cannot sanely access A, M, P, R, S and their\n> lowercase counterparts.\n>\n> It shouldn't be too hard to interpret the single letter options for the\n> currently supported set but only for lowercase letters plus '-C ' and\n> '-L ' (because there are no lowercase ones that crash with them), and\n> give long config names to at least the ones inaccessible with the above.\n>\n> Wouldn't that give us a regression-free solution to the immediate problem\n> at hand?\n>\n> Supporting long options on the command line, and giving long config name\n> synonyms to the lowercase ones, would be a plus for consistency's sake,\n> but that probably is a separate topic.\n\nSo here is an attempt to do that first step, taking the list of long names\nfrom Jonathan's message earlier in the thread. If we start giving set of\nlong options, long option names may come from a different table which\nwould likely to have names in the dashed form, so this code already\nexpects names with dashes, e.g. \"track-revisions\", and contracts them to\nnames for configuration files, e.g. \"trackRevisions\".\n\n git-cvsimport.perl | 19 ++++++++++++++++++-\n t/t9600-cvsimport.sh | 7 +++++--\n 2 files changed, 23 insertions(+), 3 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 2b07c72..8c916d9 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -89,6 +89,14 @@ sub write_author_info($) {\n }\n \n # convert getopts specs for use by git config\n+my %longmap = (\n+\t'A:' => 'authors-file',\n+\t'M:' => 'merge-regex',\n+\t'P:' => undef,\n+\t'R' => 'track-revisions',\n+\t'S:' => 'ignore-paths',\n+);\n+\n sub read_repo_config {\n \t# Split the string between characters, unless there is a ':'\n \t# So \"abc:de\" becomes [\"a\", \"b\", \"c:\", \"d\", \"e\"]\n@@ -98,8 +106,17 @@ sub read_repo_config {\n \t\t$key =~ s/://g;\n \t\tmy $arg = 'git config';\n \t\t$arg .= ' --bool' if ($o !~ /:$/);\n-\n-\t\tchomp(my $tmp = `$arg --get cvsimport.$key`);\n+\t\tmy $ckey = $key;\n+\n+\t\tif (exists $longmap{$o}) {\n+\t\t\t# An uppercase option like -R cannot be\n+\t\t\t# expressed in the configuration, as the\n+\t\t\t# variable names are downcased.\n+\t\t\t$ckey = $longmap{$o};\n+\t\t\tnext if (! defined $ckey);\n+\t\t\t$ckey =~ s/-//g;\n+\t\t}\n+\t\tchomp(my $tmp = `$arg --get cvsimport.$ckey`);\n \t\tif ($tmp && !($arg =~ /--bool/ && $tmp eq 'false')) {\n \t\t\tno strict 'refs';\n \t\t\tmy $opt_name = \"opt_\" . $key;\ndiff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh\nindex b572ce3..6ede84c 100755\n--- a/t/t9600-cvsimport.sh\n+++ b/t/t9600-cvsimport.sh\n@@ -93,7 +93,8 @@ EOF\n test_expect_success 'update git module' '\n \n \tcd module-git &&\n-\tgit cvsimport -a -R -z 0 module &&\n+\tgit config cvsimport.trackRevisions true &&\n+\tgit cvsimport -a -z 0 module &&\n \tgit merge origin &&\n \tcd .. &&\n \ttest_cmp module-cvs/o_fortuna module-git/o_fortuna\n@@ -122,7 +123,8 @@ test_expect_success 'cvsimport.module config works' '\n \n \tcd module-git &&\n \t\tgit config cvsimport.module module &&\n-\t\tgit cvsimport -a -R -z0 &&\n+\t\tgit config cvsimport.trackRevisions true &&\n+\t\tgit cvsimport -a -z0 &&\n \t\tgit merge origin &&\n \tcd .. &&\n \ttest_cmp module-cvs/tick module-git/tick\n@@ -142,6 +144,7 @@ test_expect_success 'import from a CVS working tree' '\n \n \t$CVS co -d import-from-wt module &&\n \tcd import-from-wt &&\n+\t\tgit config cvsimport.trackRevisions false &&\n \t\tgit cvsimport -a -z0 &&\n \t\techo 1 >expect &&\n \t\tgit log -1 --pretty=format:%s%n >actual &&\n"},{"id":"157168","messageId":"7voc93ppjz.fsf@alter.siamese.dyndns.org","threadId":"25840","inReplyTo":"7vipzduufr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add: introduce add.ignoreerrors synonym for add.ignore-errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-03T02:18:08Z","receivedAt":"2010-12-03T02:18:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Perhaps phrasing it like this, and then issue v1.7.{0,1,2,3}.X maintenance\n> release to reduce the chance of inconveniencing the users?\n\nI ended up cutting the maintenance releases for 1.7.[012].X series, which\ncosted a few more hours than I planned to spend on this X-<.\n"}]}