{"thread":{"id":"50905","subject":"[PATCH 1/1] send-email: fix transferencoding config option","startedAt":"2019-04-09T19:28:27Z","lastAt":"2019-05-29T09:11:39Z","messageCount":39,"participants":["Heinrich Schuchardt","Jonathan Nieder","brian m. carlson","Junio C Hamano","Eric Sunshine","Ævar Arnfjörð Bjarmason","Stephen Boyd","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"373530","messageId":"20190409192733.10173-1-xypron.glpk@gmx.de","threadId":"50905","inReplyTo":null,"subject":"[PATCH 1/1] send-email: fix transferencoding config option","fromName":"Heinrich Schuchardt","fromEmail":"xypron.glpk@gmx.de","sentAt":"2019-04-09T19:27:33Z","receivedAt":"2019-04-09T19:28:27Z","isPatch":true,"sender":{"key":"xypron.glpk@gmx.de","avatar":"https://gravatar.com/avatar/ecfc545fcbe89519e2ffdf4eccf04756d830b2a7a26780b6f5b3d5d569d1d362?d=mp&s=160"},"body":"Since e67a228cd8a (\"send-email: automatically determine transfer-encoding\")\nthe value of sendmail.transferencoding is ignored because when parsing\nthe configuration $target_xfer_encoding is not initial anymore.\n\nInstead of initializing variable $target_xfer_encoding on definition we\nhave to set it to the default value of 'auto' if is initial after parsing\nthe configuration files.\n\nThe documentation erroneously mentions the option as\nsendmail.transferEncoding. Fix that typo.\n\nFixes: e67a228cd8a (\"send-email: automatically determine transfer-encoding\")\nSigned-off-by: Heinrich Schuchardt \n---\n Documentation/git-send-email.txt | 2 +-\n git-send-email.perl | 4 +++-\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 1afe9fc858..884e776add 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -146,7 +146,7 @@ Note that no attempts whatsoever are made to validate the encoding.\n \teven more opaque. auto will use 8bit when possible, and quoted-printable\n \totherwise.\n +\n-Default is the value of the `sendemail.transferEncoding` configuration\n+Default is the value of the `sendemail.transferencoding` configuration\n value; if that is unspecified, default to `auto`.\n\n --xmailer::\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 8200d58cdc..0e23193939 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -239,7 +239,7 @@ sub do_edit {\n my (@suppress_cc);\n my ($auto_8bit_encoding);\n my ($compose_encoding);\n-my $target_xfer_encoding = 'auto';\n+my ($target_xfer_encoding);\n\n my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n\n@@ -446,6 +446,8 @@ sub read_config {\n \t\t\t$smtp_encryption = 'ssl';\n \t\t}\n \t}\n+\n+\t$target_xfer_encoding = 'auto' unless (defined $target_xfer_encoding);\n }\n\n # read configuration from [sendemail \"$identity\"], fall back on [sendemail]\n--\n2.20.1\n\n"},{"id":"373537","messageId":"20190409215856.GD92879@google.com","threadId":"50905","inReplyTo":"20190409192733.10173-1-xypron.glpk@gmx.de","subject":"Re: [PATCH 1/1] send-email: fix transferencoding config option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2019-04-09T21:58:56Z","receivedAt":"2019-04-09T21:59:03Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(thanks for cc-ing bmc!)\nHi,\n\nHeinrich Schuchardt wrote:\n\n> Subject: send-email: fix transferencoding config option\n\nnit: \"fix\" doesn't tell me what was broken and what you improved about\nit. Here, I think you mean \"respect transferencoding config option\".\n\n> Since e67a228cd8a (\"send-email: automatically determine transfer-encoding\")\n> the value of sendmail.transferencoding is ignored because when parsing\n> the configuration $target_xfer_encoding is not initial anymore.\n\nnit: I was confused when first reading this, since I read \"the\nconfiguration $target_xfer_encoding\" as a single phrase. A comma\nafter \"configuration\" might help.\n\n> Instead of initializing variable $target_xfer_encoding on definition we\n> have to set it to the default value of 'auto' if is initial after parsing\n> the configuration files.\n\nrun-on sentence. I'm having trouble parsing this part.\n\nCan you start from the beginning and describe again what this does?\nIn other words, tell me\n\n- What is the user-facing effect of the change? What workflow is it\n part of?\n\n- Any risks or complications?\n\n- Any technical details that might be interesting to the later reader?\n\n- What does this allow me to do that I couldn't do before?\n\nThe code can speak for itself, so this should primarily focus on the\nintention behind the change.\n\n[...]\n> --- a/Documentation/git-send-email.txt\n> +++ b/Documentation/git-send-email.txt\n> @@ -146,7 +146,7 @@ Note that no attempts whatsoever are made to validate the encoding.\n> \teven more opaque. auto will use 8bit when possible, and quoted-printable\n> \totherwise.\n> +\n> -Default is the value of the `sendemail.transferEncoding` configuration\n> +Default is the value of the `sendemail.transferencoding` configuration\n\nUnrelated change?\n\n[...]\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -239,7 +239,7 @@ sub do_edit {\n> my (@suppress_cc);\n> my ($auto_8bit_encoding);\n> my ($compose_encoding);\n> -my $target_xfer_encoding = 'auto';\n> +my ($target_xfer_encoding);\n> \n> my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n> \n> @@ -446,6 +446,8 @@ sub read_config {\n> \t\t\t$smtp_encryption = 'ssl';\n> \t\t}\n> \t}\n> +\n> +\t$target_xfer_encoding = 'auto' unless (defined $target_xfer_encoding);\n\nMakes sense.\n\nIs there a way to cover this in tests (t/t9001-send-email.sh) so we\ncan avoid regressing again?\n\nThe rest looks good.\n\nThanks for noticing, and hope that helps,\nJonathan\n"},{"id":"373538","messageId":"20190409225531.GI12419@genre.crustytoothpaste.net","threadId":"50905","inReplyTo":"20190409192733.10173-1-xypron.glpk@gmx.de","subject":"Re: [PATCH 1/1] send-email: fix transferencoding config option","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-04-09T22:55:31Z","receivedAt":"2019-04-09T22:55:42Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Apr 09, 2019 at 09:27:33PM +0200, Heinrich Schuchardt wrote:\n> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n> index 1afe9fc858..884e776add 100644\n> --- a/Documentation/git-send-email.txt\n> +++ b/Documentation/git-send-email.txt\n> @@ -146,7 +146,7 @@ Note that no attempts whatsoever are made to validate the encoding.\n> \teven more opaque. auto will use 8bit when possible, and quoted-printable\n> \totherwise.\n> +\n> -Default is the value of the `sendemail.transferEncoding` configuration\n> +Default is the value of the `sendemail.transferencoding` configuration\n> value; if that is unspecified, default to `auto`.\n\nIn Git, two-part config settings are case-insensitive. We traditionally\nwrite them in lower camel case because it's easier for people to read.\nGit will canonicalize the values when \"git config\" runs.\n\nSo I don't think we should change this.\n\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 8200d58cdc..0e23193939 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -239,7 +239,7 @@ sub do_edit {\n> my (@suppress_cc);\n> my ($auto_8bit_encoding);\n> my ($compose_encoding);\n> -my $target_xfer_encoding = 'auto';\n> +my ($target_xfer_encoding);\n> \n> my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n> \n> @@ -446,6 +446,8 @@ sub read_config {\n> \t\t\t$smtp_encryption = 'ssl';\n> \t\t}\n> \t}\n> +\n> +\t$target_xfer_encoding = 'auto' unless (defined $target_xfer_encoding);\n> }\n> \n> # read configuration from [sendemail \"$identity\"], fall back on [sendemail]\n\nThanks for fixing this. I didn't realize that we only set values if the\nvariable holding them is undef. Would you mind adding a test for this\ncase so we won't regress it in the future?\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"373539","messageId":"bd431817-3c47-c05c-5a92-b26eb2b25d4d@gmx.de","threadId":"50905","inReplyTo":"20190409225531.GI12419@genre.crustytoothpaste.net","subject":"Re: [PATCH 1/1] send-email: fix transferencoding config option","fromName":"Heinrich Schuchardt","fromEmail":"xypron.glpk@gmx.de","sentAt":"2019-04-09T23:06:22Z","receivedAt":"2019-04-09T23:06:34Z","isPatch":true,"sender":{"key":"xypron.glpk@gmx.de","avatar":"https://gravatar.com/avatar/ecfc545fcbe89519e2ffdf4eccf04756d830b2a7a26780b6f5b3d5d569d1d362?d=mp&s=160"},"body":"On 4/10/19 12:55 AM, brian m. carlson wrote:\n> On Tue, Apr 09, 2019 at 09:27:33PM +0200, Heinrich Schuchardt wrote:\n>> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n>> index 1afe9fc858..884e776add 100644\n>> --- a/Documentation/git-send-email.txt\n>> +++ b/Documentation/git-send-email.txt\n>> @@ -146,7 +146,7 @@ Note that no attempts whatsoever are made to validate the encoding.\n>> \teven more opaque. auto will use 8bit when possible, and quoted-printable\n>> \totherwise.\n>> +\n>> -Default is the value of the `sendemail.transferEncoding` configuration\n>> +Default is the value of the `sendemail.transferencoding` configuration\n>> value; if that is unspecified, default to `auto`.\n>\n> In Git, two-part config settings are case-insensitive. We traditionally\n> write them in lower camel case because it's easier for people to read.\n> Git will canonicalize the values when \"git config\" runs.\n>\n> So I don't think we should change this.\n\nThanks for the hint.\n\n>\n>> diff --git a/git-send-email.perl b/git-send-email.perl\n>> index 8200d58cdc..0e23193939 100755\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -239,7 +239,7 @@ sub do_edit {\n>> my (@suppress_cc);\n>> my ($auto_8bit_encoding);\n>> my ($compose_encoding);\n>> -my $target_xfer_encoding = 'auto';\n>> +my ($target_xfer_encoding);\n>>\n>> my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n>>\n>> @@ -446,6 +446,8 @@ sub read_config {\n>> \t\t\t$smtp_encryption = 'ssl';\n>> \t\t}\n>> \t}\n>> +\n>> +\t$target_xfer_encoding = 'auto' unless (defined $target_xfer_encoding);\n>> }\n>>\n>> # read configuration from [sendemail \"$identity\"], fall back on [sendemail]\n>\n> Thanks for fixing this. I didn't realize that we only set values if the\n> variable holding them is undef. Would you mind adding a test for this\n> case so we won't regress it in the future?\n>\n\nNice challenge for my first patch for git :)\nI will give it a try.\n\nBest regards\n\nHeinrich\n"},{"id":"373544","messageId":"44afdc46-260d-2ad3-0a00-5789615146a8@gmx.de","threadId":"50905","inReplyTo":"20190409215856.GD92879@google.com","subject":"Re: [PATCH 1/1] send-email: fix transferencoding config option","fromName":"Heinrich Schuchardt","fromEmail":"xypron.glpk@gmx.de","sentAt":"2019-04-09T23:39:03Z","receivedAt":"2019-04-09T23:39:19Z","isPatch":true,"sender":{"key":"xypron.glpk@gmx.de","avatar":"https://gravatar.com/avatar/ecfc545fcbe89519e2ffdf4eccf04756d830b2a7a26780b6f5b3d5d569d1d362?d=mp&s=160"},"body":"On 4/9/19 11:58 PM, Jonathan Nieder wrote:\n> (thanks for cc-ing bmc!)\n> Hi,\n>\n> Heinrich Schuchardt wrote:\n>\n>> Subject: send-email: fix transferencoding config option\n>\n> nit: \"fix\" doesn't tell me what was broken and what you improved about\n> it. Here, I think you mean \"respect transferencoding config option\".\n>\n>> Since e67a228cd8a (\"send-email: automatically determine transfer-encoding\")\n>> the value of sendmail.transferencoding is ignored because when parsing\n>> the configuration $target_xfer_encoding is not initial anymore.\n>\n> nit: I was confused when first reading this, since I read \"the\n> configuration $target_xfer_encoding\" as a single phrase. A comma\n> after \"configuration\" might help.\n>\n>> Instead of initializing variable $target_xfer_encoding on definition we\n>> have to set it to the default value of 'auto' if is initial after parsing\n>> the configuration files.\n>\n> run-on sentence. I'm having trouble parsing this part.\n>\n> Can you start from the beginning and describe again what this does?\n> In other words, tell me\n>\n> - What is the user-facing effect of the change? What workflow is it\n> part of?\n\nI am working with a repository which uses CRLF line endings. So when\nsending patches I should use an appropriate encoding. There should be\ntwo ways to do it:\n\n- call git-send-email with --transfer-encoding base64\n- git config --global sendmail.transferencoding base64\n\nUnfortunately the latter method did not show the expected result. The\nsetting was simply ignored.\n\n>\n> - Any risks or complications?\n\nNone that I am aware of.\n\n>\n> - Any technical details that might be interesting to the later reader?\n\nAs I tried to explain above the setting is ignored because a variable is\ninitialized too early.\n\n>\n> - What does this allow me to do that I couldn't do before?\n\nYou can use a global setting for the transfer encoding.\n\n>\n> The code can speak for itself, so this should primarily focus on the\n> intention behind the change.\n>\n> [...]\n>> --- a/Documentation/git-send-email.txt\n>> +++ b/Documentation/git-send-email.txt\n>> @@ -146,7 +146,7 @@ Note that no attempts whatsoever are made to validate the encoding.\n>> \teven more opaque. auto will use 8bit when possible, and quoted-printable\n>> \totherwise.\n>> +\n>> -Default is the value of the `sendemail.transferEncoding` configuration\n>> +Default is the value of the `sendemail.transferencoding` configuration\n>\n> Unrelated change?\n>\n> [...]\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -239,7 +239,7 @@ sub do_edit {\n>> my (@suppress_cc);\n>> my ($auto_8bit_encoding);\n>> my ($compose_encoding);\n>> -my $target_xfer_encoding = 'auto';\n>> +my ($target_xfer_encoding);\n>>\n>> my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n>>\n>> @@ -446,6 +446,8 @@ sub read_config {\n>> \t\t\t$smtp_encryption = 'ssl';\n>> \t\t}\n>> \t}\n>> +\n>> +\t$target_xfer_encoding = 'auto' unless (defined $target_xfer_encoding);\n>\n> Makes sense.\n>\n> Is there a way to cover this in tests (t/t9001-send-email.sh) so we\n> can avoid regressing again?\n\nI will give it a try.\n\n>\n> The rest looks good.\n>\n> Thanks for noticing, and hope that helps,\n> Jonathan\n>\nThanks for reviewing.\n\nBest regards\n\nHeinrich\n"},{"id":"373564","messageId":"xmqq8swi34h5.fsf@gitster-ct.c.googlers.com","threadId":"50905","inReplyTo":"20190409215856.GD92879@google.com","subject":"Re: [PATCH 1/1] send-email: fix transferencoding config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-10T03:48:38Z","receivedAt":"2019-04-10T03:48:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> nit: I was confused when first reading this, since I read \"the\n> configuration $target_xfer_encoding\" as a single phrase. A comma\n> after \"configuration\" might help.\n> ...\n> run-on sentence. I'm having trouble parsing this part.\n\nI had the same issue with the wording. Without addressing other\nparts of the suggestions in the thread (like describing the\nmotivating use case, and protecting this with the test), here is\nwhat I have tentatively queued.\n\nAs all the $scalar variables that are referenced by %config_settings\netc. all potentially share this issue, I wonder if it makes sense to\nhave a validation at the very beginning of the read_config sub,\nsomething along the lines of....\n\n\tsub read_config {\n\t\tmy ($prefix) = @_;\n\n\t\twhile (my ($k, $v) = each %config_bool_settings) {\n \tif (defined $$v) {\n\t\t\t\tdie \"BUG: \\%config_bool_settings{$k} is not undef\\n\";\n\t\t\t}\n\t\t}\n\t\t... similarly for %config_path_settings and %config_settings ...\n\n\t\t... then the original code ...\n\t\tforeach my $setting (keys %config_bool_settings) {\n\t\t\t...\n\t}\n\nBy the way, if we look more closely to the two callsites of\nread_config(), however, we realize that Heinrich's patch is a wrong\nsolution to the problem.\n\nWhat happens when \"sendemail..xferencoding\" is not set, but\n\"sendemail.xferencoding\" is, with the updated code? The \"ah, the\nconfiguration file did not define the xfer-encoding, so let's set it\nto auto\" at the end of read_config is done still too early. After\nchecking \"sendemail..*\", the code added by the patch under\nreview assigns 'auto' to $target_xfer_encoding and this assignment\ncauses \"sendemail.xferencoding\" to be ignored, just like BMC's bug.\n\nIn other words, the patch is reproducing the same bug it is\nattempting to fix; a quick-and-dirty and obvious band-aid is to move\nthe assignment of 'auto' further down, outside the read_config()\nsub, after two calls to the sub is made by the caller, but singling\nthis single variable out is very unsatisfactory.\n\nI wonder if we can follow the pattern used by the code to handle the\nfallback for %config_bool_settings we can see immediately after\nthese two calls to read_config()? That is, each of the element in\nthe %config_* hash is not merely a pointer to where the value is\nstored, but also knows what the default fallback value should be,\nand a loop _in the caller of_ read_config(), after it finishes\nmaking calls to the read_config function, fills in the missing\ndefault?\n\n-- >8 --\nFrom: Heinrich Schuchardt \nDate: Tue, 9 Apr 2019 21:27:33 +0200\nSubject: [PATCH] send-email: honor transferencoding config option again\n\nSince e67a228cd8a (\"send-email: automatically determine\ntransfer-encoding\"), the value of sendmail.transferencoding in the\nconfiguration file is ignored, because $target_xfer_encoding is\nalready defined read_config sub parses the configuration file.\n\nInstead of initializing variable $target_xfer_encoding to 'auto' on\ndefinition, we have to set it to the default value of 'auto' if is\nundefined after parsing the configuration files.\n\nSigned-off-by: Heinrich Schuchardt \nSigned-off-by: Junio C Hamano \n---\n git-send-email.perl | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex f4c07908d2..db32cddbde 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -231,7 +231,7 @@ sub do_edit {\n my (@suppress_cc);\n my ($auto_8bit_encoding);\n my ($compose_encoding);\n-my $target_xfer_encoding = 'auto';\n+my ($target_xfer_encoding);\n \n my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n \n@@ -434,6 +434,8 @@ sub read_config {\n \t\t\t$smtp_encryption = 'ssl';\n \t\t}\n \t}\n+\n+\t$target_xfer_encoding = 'auto' unless (defined $target_xfer_encoding);\n }\n \n # read configuration from [sendemail \"$identity\"], fall back on [sendemail]\n-- \n2.21.0-313-ge35b8cb8e2\n\n\n\n"},{"id":"373611","messageId":"668fc68f-4d94-39e8-256d-55229472761f@gmx.de","threadId":"50905","inReplyTo":"xmqq8swi34h5.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] send-email: fix transferencoding config option","fromName":"Heinrich Schuchardt","fromEmail":"xypron.glpk@gmx.de","sentAt":"2019-04-10T20:40:51Z","receivedAt":"2019-04-10T20:41:08Z","isPatch":true,"sender":{"key":"xypron.glpk@gmx.de","avatar":"https://gravatar.com/avatar/ecfc545fcbe89519e2ffdf4eccf04756d830b2a7a26780b6f5b3d5d569d1d362?d=mp&s=160"},"body":"On 4/10/19 5:48 AM, Junio C Hamano wrote:\n> Jonathan Nieder writes:\n>\n>> nit: I was confused when first reading this, since I read \"the\n>> configuration $target_xfer_encoding\" as a single phrase. A comma\n>> after \"configuration\" might help.\n>> ...\n>> run-on sentence. I'm having trouble parsing this part.\n>\n> I had the same issue with the wording. Without addressing other\n> parts of the suggestions in the thread (like describing the\n> motivating use case, and protecting this with the test), here is\n> what I have tentatively queued.\n>\n> As all the $scalar variables that are referenced by %config_settings\n> etc. all potentially share this issue, I wonder if it makes sense to\n> have a validation at the very beginning of the read_config sub,\n> something along the lines of....\n>\n> \tsub read_config {\n> \t\tmy ($prefix) = @_;\n>\n> \t\twhile (my ($k, $v) = each %config_bool_settings) {\n> \tif (defined $$v) {\n> \t\t\t\tdie \"BUG: \\%config_bool_settings{$k} is not undef\\n\";\n> \t\t\t}\n> \t\t}\n> \t\t... similarly for %config_path_settings and %config_settings ...\n>\n> \t\t... then the original code ...\n> \t\tforeach my $setting (keys %config_bool_settings) {\n> \t\t\t...\n> \t}\n>\n> By the way, if we look more closely to the two callsites of\n> read_config(), however, we realize that Heinrich's patch is a wrong\n> solution to the problem.\n>\n> What happens when \"sendemail..xferencoding\" is not set, but\n> \"sendemail.xferencoding\" is, with the updated code? The \"ah, the\n> configuration file did not define the xfer-encoding, so let's set it\n> to auto\" at the end of read_config is done still too early. After\n> checking \"sendemail..*\", the code added by the patch under\n> review assigns 'auto' to $target_xfer_encoding and this assignment\n> causes \"sendemail.xferencoding\" to be ignored, just like BMC's bug.\n>\n> In other words, the patch is reproducing the same bug it is\n> attempting to fix; a quick-and-dirty and obvious band-aid is to move\n> the assignment of 'auto' further down, outside the read_config()\n> sub, after two calls to the sub is made by the caller, but singling\n> this single variable out is very unsatisfactory.\n>\n> I wonder if we can follow the pattern used by the code to handle the\n> fallback for %config_bool_settings we can see immediately after\n> these two calls to read_config()? That is, each of the element in\n> the %config_* hash is not merely a pointer to where the value is\n> stored, but also knows what the default fallback value should be,\n> and a loop _in the caller of_ read_config(), after it finishes\n> making calls to the read_config function, fills in the missing\n> default?\n\nSounds reasonable. But including the tests requested nothing I could\neasily shoulder.\n\nJust a quite different thought:\n\n'auto' should discover a safe transfer encoding. Why does 'auto' not\ndiscover that a patch contains carriage returns and should be base64\nencoded (see subroutine apply_transfer_encoding())?\n\nBest regards\n\nHeinrich\n\n>\n> -- >8 --\n> From: Heinrich Schuchardt \n> Date: Tue, 9 Apr 2019 21:27:33 +0200\n> Subject: [PATCH] send-email: honor transferencoding config option again\n>\n> Since e67a228cd8a (\"send-email: automatically determine\n> transfer-encoding\"), the value of sendmail.transferencoding in the\n> configuration file is ignored, because $target_xfer_encoding is\n> already defined read_config sub parses the configuration file.\n>\n> Instead of initializing variable $target_xfer_encoding to 'auto' on\n> definition, we have to set it to the default value of 'auto' if is\n> undefined after parsing the configuration files.\n>\n> Signed-off-by: Heinrich Schuchardt \n> Signed-off-by: Junio C Hamano \n> ---\n> git-send-email.perl | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index f4c07908d2..db32cddbde 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -231,7 +231,7 @@ sub do_edit {\n> my (@suppress_cc);\n> my ($auto_8bit_encoding);\n> my ($compose_encoding);\n> -my $target_xfer_encoding = 'auto';\n> +my ($target_xfer_encoding);\n>\n> my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n>\n> @@ -434,6 +434,8 @@ sub read_config {\n> \t\t\t$smtp_encryption = 'ssl';\n> \t\t}\n> \t}\n> +\n> +\t$target_xfer_encoding = 'auto' unless (defined $target_xfer_encoding);\n> }\n>\n> # read configuration from [sendemail \"$identity\"], fall back on [sendemail]\n>\n\n"},{"id":"373619","messageId":"20190410224250.GJ12419@genre.crustytoothpaste.net","threadId":"50905","inReplyTo":"668fc68f-4d94-39e8-256d-55229472761f@gmx.de","subject":"Re: [PATCH 1/1] send-email: fix transferencoding config option","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-04-10T22:42:50Z","receivedAt":"2019-04-10T22:43:00Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Apr 10, 2019 at 10:40:51PM +0200, Heinrich Schuchardt wrote:\n> Sounds reasonable. But including the tests requested nothing I could\n> easily shoulder.\n> \n> Just a quite different thought:\n> \n> 'auto' should discover a safe transfer encoding. Why does 'auto' not\n> discover that a patch contains carriage returns and should be base64\n> encoded (see subroutine apply_transfer_encoding())?\n\nBecause nobody thought of it. The original rationale for \"auto\" was to\nreplace \"8bit\" with something sane that worked due to long lines as an\nalternative to forcing people to choose it themselves.\n\nAnd as the commit message says:\n\n Choose quoted-printable instead of base64, since base64-encoded plain\n text is treated as suspicious by some spam filters.\n\nI'll try to see if I can write something up to handle this better. If\nquoted-printable works, I'll pick that; otherwise, I'll choose base64.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"375098","messageId":"xmqqef59gy10.fsf_-_@gitster-ct.c.googlers.com","threadId":"50905","inReplyTo":"xmqq8swi34h5.fsf@gitster-ct.c.googlers.com","subject":"Re* [PATCH 1/1] send-email: fix transferencoding config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-08T08:18:03Z","receivedAt":"2019-05-08T08:18:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I wonder if we can follow the pattern used by the code to handle the\n> fallback for %config_bool_settings we can see immediately after\n> these two calls to read_config()? That is, each of the element in\n> the %config_* hash is not merely a pointer to where the value is\n> stored, but also knows what the default fallback value should be,\n> and a loop _in the caller of_ read_config(), after it finishes\n> making calls to the read_config function, fills in the missing\n> default?\n\nSo, here is a two-patch series that tries to do so, primarily done\nto gauge if there still is the level of interest needed to make it\nworth for us to pursue this topic. Here is the first one; I'll send\nthe second one that takes advantage of this change separately (but\nit should be trivial to imagine what that step would involve).\n\n-- >8 --\nSubject: [PATCH 1/2] send-email: update the mechanism to set default configuration values\n\nThe program has a good mechanism to specify the fallback default\nvalues for boolean configuration variables after two invocations of\nread_config() for \"sendmail.$ident.$var\" and \"sendemail.$var\" have\nnot found any configuration. Imitate it so that we can set the\ndefault values for non-boolean variables as well.\n\nSigned-off-by: Junio C Hamano \n---\n git-send-email.perl | 51 ++++++++++++++++++++++++++-------------------\n 1 file changed, 29 insertions(+), 22 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex f4c07908d2..ca7faff094 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -250,28 +250,28 @@ sub do_edit {\n );\n \n my %config_settings = (\n- \"smtpserver\" => \\$smtp_server,\n- \"smtpserverport\" => \\$smtp_server_port,\n- \"smtpserveroption\" => \\@smtp_server_options,\n- \"smtpuser\" => \\$smtp_authuser,\n- \"smtppass\" => \\$smtp_authpass,\n- \"smtpdomain\" => \\$smtp_domain,\n- \"smtpauth\" => \\$smtp_auth,\n- \"smtpbatchsize\" => \\$batch_size,\n- \"smtprelogindelay\" => \\$relogin_delay,\n- \"to\" => \\@initial_to,\n- \"tocmd\" => \\$to_cmd,\n- \"cc\" => \\@initial_cc,\n- \"cccmd\" => \\$cc_cmd,\n- \"aliasfiletype\" => \\$aliasfiletype,\n- \"bcc\" => \\@bcclist,\n- \"suppresscc\" => \\@suppress_cc,\n- \"envelopesender\" => \\$envelope_sender,\n- \"confirm\" => \\$confirm,\n- \"from\" => \\$sender,\n- \"assume8bitencoding\" => \\$auto_8bit_encoding,\n- \"composeencoding\" => \\$compose_encoding,\n- \"transferencoding\" => \\$target_xfer_encoding,\n+ \"smtpserver\" => [\\$smtp_server],\n+ \"smtpserverport\" => [\\$smtp_server_port],\n+ \"smtpserveroption\" => [\\@smtp_server_options],\n+ \"smtpuser\" => [\\$smtp_authuser],\n+ \"smtppass\" => [\\$smtp_authpass],\n+ \"smtpdomain\" => [\\$smtp_domain],\n+ \"smtpauth\" => [\\$smtp_auth],\n+ \"smtpbatchsize\" => [\\$batch_size],\n+ \"smtprelogindelay\" => [\\$relogin_delay],\n+ \"to\" => [\\@initial_to],\n+ \"tocmd\" => [\\$to_cmd],\n+ \"cc\" => [\\@initial_cc],\n+ \"cccmd\" => [\\$cc_cmd],\n+ \"aliasfiletype\" => [\\$aliasfiletype],\n+ \"bcc\" => [\\@bcclist],\n+ \"suppresscc\" => [\\@suppress_cc],\n+ \"envelopesender\" => [\\$envelope_sender],\n+ \"confirm\" => [\\$confirm],\n+ \"from\" => [\\$sender],\n+ \"assume8bitencoding\" => [\\$auto_8bit_encoding],\n+ \"composeencoding\" => [\\$compose_encoding],\n+ \"transferencoding\" => [\\$target_xfer_encoding],\n );\n \n my %config_path_settings = (\n@@ -446,6 +446,13 @@ sub read_config {\n \t${$setting->[0]} = $setting->[1] unless (defined (${$setting->[0]}));\n }\n \n+# fall back to builtin defaults\n+for my $setting (values %config_settings) {\n+\tif (@$setting == 2 && !defined (${$setting->[0]})) {\n+\t\t${$setting->[0]} = $setting->[1];\n+\t}\n+}\n+\n # 'default' encryption is none -- this only prevents a warning\n $smtp_encryption = '' unless (defined $smtp_encryption);\n \n-- \n2.21.0-777-g83232e3864\n\n"},{"id":"375099","messageId":"xmqqa7fxgxwj.fsf_-_@gitster-ct.c.googlers.com","threadId":"50905","inReplyTo":"xmqqef59gy10.fsf_-_@gitster-ct.c.googlers.com","subject":"[PATCH 2/2] send-email: honor transferencoding config option again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-08T08:20:44Z","receivedAt":"2019-05-08T08:20:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Since e67a228cd8a (\"send-email: automatically determine\ntransfer-encoding\"), the value of sendmail.transferencoding in the\nconfiguration file is ignored, because $target_xfer_encoding is\nalready defined read_config sub parses the configuration file.\n\nInstead of initializing variable $target_xfer_encoding to 'auto' on\ndefinition, we have to set it to the default value of 'auto' if is\nundefined after calling read_config() twice to parsing the\nconfiguration files for \"sendemail.transferencoding\" and\n\"sendemail.$ident.transferencoding\".\n\nThis is made trivial by the previous change.\n\nSigned-off-by: Junio C Hamano \n---\n\n * And this is the second one. Totally untested, though.\n\n git-send-email.perl | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex ca7faff094..7458a0d1ef 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -231,7 +231,7 @@ sub do_edit {\n my (@suppress_cc);\n my ($auto_8bit_encoding);\n my ($compose_encoding);\n-my $target_xfer_encoding = 'auto';\n+my ($target_xfer_encoding);\n \n my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n \n@@ -271,7 +271,7 @@ sub do_edit {\n \"from\" => [\\$sender],\n \"assume8bitencoding\" => [\\$auto_8bit_encoding],\n \"composeencoding\" => [\\$compose_encoding],\n- \"transferencoding\" => [\\$target_xfer_encoding],\n+ \"transferencoding\" => [\\$target_xfer_encoding, 'auto'],\n );\n \n my %config_path_settings = (\n-- \n2.21.0-777-g83232e3864\n\n"},{"id":"375102","messageId":"xmqqzhnxfe3p.fsf@gitster-ct.c.googlers.com","threadId":"50905","inReplyTo":"xmqqef59gy10.fsf_-_@gitster-ct.c.googlers.com","subject":"Re: Re* [PATCH 1/1] send-email: fix transferencoding config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-08T10:13:46Z","receivedAt":"2019-05-08T10:13:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> So, here is a two-patch series that tries to do so, primarily done\n> to gauge if there still is the level of interest needed to make it\n> worth for us to pursue this topic. Here is the first one; I'll send\n> the second one that takes advantage of this change separately (but\n> it should be trivial to imagine what that step would involve).\n>\n> -- >8 --\n> Subject: [PATCH 1/2] send-email: update the mechanism to set default configuration values\n>\n> The program has a good mechanism to specify the fallback default\n> values for boolean configuration variables after two invocations of\n> read_config() for \"sendmail.$ident.$var\" and \"sendemail.$var\" have\n> not found any configuration. Imitate it so that we can set the\n> default values for non-boolean variables as well.\n>\n> Signed-off-by: Junio C Hamano \n> ---\n> git-send-email.perl | 51 ++++++++++++++++++++++++++-------------------\n> 1 file changed, 29 insertions(+), 22 deletions(-)\n\nThis one was embarrassingly buggy, and needs the following squashed\nin.\n\nSorry about that.\n\n git-send-email.perl | 13 ++++++++-----\n 1 file changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex ca7faff094..831947c7ed 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -411,7 +411,7 @@ sub read_config {\n \t}\n \n \tforeach my $setting (keys %config_settings) {\n-\t\tmy $target = $config_settings{$setting};\n+\t\tmy $target = $config_settings{$setting}->[0];\n \t\tnext if $setting eq \"to\" and defined $no_to;\n \t\tnext if $setting eq \"cc\" and defined $no_cc;\n \t\tnext if $setting eq \"bcc\" and defined $no_bcc;\n@@ -447,10 +447,13 @@ sub read_config {\n }\n \n # fall back to builtin defaults\n-for my $setting (values %config_settings) {\n-\tif (@$setting == 2 && !defined (${$setting->[0]})) {\n-\t\t${$setting->[0]} = $setting->[1];\n-\t}\n+while (my ($name, $setting) = each %config_settings) {\n+\tnext unless @$setting == 2;\n+\n+\tmy ($target, $default) = @$setting;\n+\tif (ref($target) eq \"SCALAR\") {\n+\t\t$$target = $default unless defined $target;\n+\t} # elsif ... for other types later.\n }\n \n # 'default' encryption is none -- this only prevents a warning\n\n"},{"id":"375106","messageId":"20190508105607.178244-1-gitster@pobox.com","threadId":"50905","inReplyTo":"xmqqzhnxfe3p.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 0/2] send-email: set xfer encoding correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-08T10:56:05Z","receivedAt":"2019-05-08T10:56:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Earlier I sent a series of \"oops that was wrong\" which was rather\nembarrassing. This is what I have to replace Heinrich's patch that\nwas not quite right, while making it harder to make the same mistake\nwhile defining fallback default values to configuration variables.\n\nJunio C Hamano (2):\n send-email: update the mechanism to set default configuration values\n send-email: honor transferencoding config option again\n\n git-send-email.perl | 58 ++++++++++++++++++++++++++-------------------\n 1 file changed, 34 insertions(+), 24 deletions(-)\n\n-- \n2.21.0-777-g83232e3864\n\n"},{"id":"375107","messageId":"20190508105607.178244-3-gitster@pobox.com","threadId":"50905","inReplyTo":"xmqqzhnxfe3p.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 2/2] send-email: honor transferencoding config option again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-08T10:56:07Z","receivedAt":"2019-05-08T10:56:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Since e67a228cd8a (\"send-email: automatically determine\ntransfer-encoding\"), the value of sendmail.transferencoding in the\nconfiguration file is ignored, because $target_xfer_encoding is\nalready defined read_config sub parses the configuration file.\n\nInstead of initializing variable $target_xfer_encoding to 'auto' on\ndefinition, we have to set it to the default value of 'auto' if is\nundefined after calling read_config() twice to parsing the\nconfiguration files for \"sendemail.transferencoding\" and\n\"sendemail.$ident.transferencoding\".\n\nIt was made trivial to do so by the previous change.\n\nSigned-off-by: Junio C Hamano \n---\n git-send-email.perl | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 98bc295c6e..b9dd775b63 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -231,7 +231,7 @@ sub do_edit {\n my (@suppress_cc);\n my ($auto_8bit_encoding);\n my ($compose_encoding);\n-my $target_xfer_encoding = 'auto';\n+my ($target_xfer_encoding);\n \n my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n \n@@ -271,7 +271,7 @@ sub do_edit {\n \"from\" => [\\$sender],\n \"assume8bitencoding\" => [\\$auto_8bit_encoding],\n \"composeencoding\" => [\\$compose_encoding],\n- \"transferencoding\" => [\\$target_xfer_encoding],\n+ \"transferencoding\" => [\\$target_xfer_encoding, 'auto'],\n );\n \n my %config_path_settings = (\n-- \n2.21.0-777-g83232e3864\n\n"},{"id":"375108","messageId":"20190508105607.178244-2-gitster@pobox.com","threadId":"50905","inReplyTo":"xmqqzhnxfe3p.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 1/2] send-email: update the mechanism to set default configuration values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-08T10:56:06Z","receivedAt":"2019-05-08T10:56:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The program has a good mechanism to specify the fallback default\nvalues for boolean configuration variables after two invocations of\nread_config() for \"sendmail.$ident.$var\" and \"sendemail.$var\" have\nnot found any configuration. Imitate it so that we can set the\ndefault values for non-boolean variables as well.\n\nSigned-off-by: Junio C Hamano \n---\n git-send-email.perl | 56 ++++++++++++++++++++++++++-------------------\n 1 file changed, 33 insertions(+), 23 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex f4c07908d2..98bc295c6e 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -250,28 +250,28 @@ sub do_edit {\n );\n \n my %config_settings = (\n- \"smtpserver\" => \\$smtp_server,\n- \"smtpserverport\" => \\$smtp_server_port,\n- \"smtpserveroption\" => \\@smtp_server_options,\n- \"smtpuser\" => \\$smtp_authuser,\n- \"smtppass\" => \\$smtp_authpass,\n- \"smtpdomain\" => \\$smtp_domain,\n- \"smtpauth\" => \\$smtp_auth,\n- \"smtpbatchsize\" => \\$batch_size,\n- \"smtprelogindelay\" => \\$relogin_delay,\n- \"to\" => \\@initial_to,\n- \"tocmd\" => \\$to_cmd,\n- \"cc\" => \\@initial_cc,\n- \"cccmd\" => \\$cc_cmd,\n- \"aliasfiletype\" => \\$aliasfiletype,\n- \"bcc\" => \\@bcclist,\n- \"suppresscc\" => \\@suppress_cc,\n- \"envelopesender\" => \\$envelope_sender,\n- \"confirm\" => \\$confirm,\n- \"from\" => \\$sender,\n- \"assume8bitencoding\" => \\$auto_8bit_encoding,\n- \"composeencoding\" => \\$compose_encoding,\n- \"transferencoding\" => \\$target_xfer_encoding,\n+ \"smtpserver\" => [\\$smtp_server],\n+ \"smtpserverport\" => [\\$smtp_server_port],\n+ \"smtpserveroption\" => [\\@smtp_server_options],\n+ \"smtpuser\" => [\\$smtp_authuser],\n+ \"smtppass\" => [\\$smtp_authpass],\n+ \"smtpdomain\" => [\\$smtp_domain],\n+ \"smtpauth\" => [\\$smtp_auth],\n+ \"smtpbatchsize\" => [\\$batch_size],\n+ \"smtprelogindelay\" => [\\$relogin_delay],\n+ \"to\" => [\\@initial_to],\n+ \"tocmd\" => [\\$to_cmd],\n+ \"cc\" => [\\@initial_cc],\n+ \"cccmd\" => [\\$cc_cmd],\n+ \"aliasfiletype\" => [\\$aliasfiletype],\n+ \"bcc\" => [\\@bcclist],\n+ \"suppresscc\" => [\\@suppress_cc],\n+ \"envelopesender\" => [\\$envelope_sender],\n+ \"confirm\" => [\\$confirm],\n+ \"from\" => [\\$sender],\n+ \"assume8bitencoding\" => [\\$auto_8bit_encoding],\n+ \"composeencoding\" => [\\$compose_encoding],\n+ \"transferencoding\" => [\\$target_xfer_encoding],\n );\n \n my %config_path_settings = (\n@@ -411,7 +411,7 @@ sub read_config {\n \t}\n \n \tforeach my $setting (keys %config_settings) {\n-\t\tmy $target = $config_settings{$setting};\n+\t\tmy $target = $config_settings{$setting}->[0];\n \t\tnext if $setting eq \"to\" and defined $no_to;\n \t\tnext if $setting eq \"cc\" and defined $no_cc;\n \t\tnext if $setting eq \"bcc\" and defined $no_bcc;\n@@ -446,6 +446,16 @@ sub read_config {\n \t${$setting->[0]} = $setting->[1] unless (defined (${$setting->[0]}));\n }\n \n+# fall back to builtin defaults\n+while (my ($name, $setting) = each %config_settings) {\n+\tnext unless @$setting == 2;\n+\n+\tmy ($target, $default) = @$setting;\n+\tif (ref($target) eq \"SCALAR\") {\n+\t\t$$target = $default unless defined $$target;\n+\t} # elsif ... for other types later.\n+}\n+\n # 'default' encryption is none -- this only prevents a warning\n $smtp_encryption = '' unless (defined $smtp_encryption);\n \n-- \n2.21.0-777-g83232e3864\n\n"},{"id":"375186","messageId":"CAPig+cRrPsm+8Ja16A0t1ihM0OKZUn=pexhWaKGjUst-EvSOLw@mail.gmail.com","threadId":"50905","inReplyTo":"20190508105607.178244-3-gitster@pobox.com","subject":"Re: [PATCH v2 2/2] send-email: honor transferencoding config option again","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-08T21:24:39Z","receivedAt":"2019-05-08T21:24:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, May 8, 2019 at 6:56 AM Junio C Hamano wrote:\n> Since e67a228cd8a (\"send-email: automatically determine\n> transfer-encoding\"), the value of sendmail.transferencoding in the\n> configuration file is ignored, because $target_xfer_encoding is\n\ns/,//\n\n> already defined read_config sub parses the configuration file.\n\ns/defined/& when/ --or -- s/defined/& by the time/\n\n> Instead of initializing variable $target_xfer_encoding to 'auto' on\n> definition, we have to set it to the default value of 'auto' if is\n\ns/if is/if it is/\n\n> undefined after calling read_config() twice to parsing the\n\ns/parsing/parse/\n\n> configuration files for \"sendemail.transferencoding\" and\n\ns/files/file/ (I think)\n\n> \"sendemail.$ident.transferencoding\".\n>\n> It was made trivial to do so by the previous change.\n>\n> Signed-off-by: Junio C Hamano \n"},{"id":"375195","messageId":"20190508231228.GC7458@genre.crustytoothpaste.net","threadId":"50905","inReplyTo":"20190508105607.178244-3-gitster@pobox.com","subject":"Re: [PATCH v2 2/2] send-email: honor transferencoding config option again","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-05-08T23:12:28Z","receivedAt":"2019-05-08T23:12:37Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, May 08, 2019 at 07:56:07PM +0900, Junio C Hamano wrote:\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 98bc295c6e..b9dd775b63 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -231,7 +231,7 @@ sub do_edit {\n> my (@suppress_cc);\n> my ($auto_8bit_encoding);\n> my ($compose_encoding);\n> -my $target_xfer_encoding = 'auto';\n> +my ($target_xfer_encoding);\n> \n> my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n> \n> @@ -271,7 +271,7 @@ sub do_edit {\n> \"from\" => [\\$sender],\n> \"assume8bitencoding\" => [\\$auto_8bit_encoding],\n> \"composeencoding\" => [\\$compose_encoding],\n> - \"transferencoding\" => [\\$target_xfer_encoding],\n> + \"transferencoding\" => [\\$target_xfer_encoding, 'auto'],\n> );\n\nThese changes seem sane and the series makes the second one\nstraightforward. It's also a nice way to add defaults in the future, so\nthanks for that.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"375202","messageId":"xmqqzhnwdszt.fsf@gitster-ct.c.googlers.com","threadId":"50905","inReplyTo":"CAPig+cRrPsm+8Ja16A0t1ihM0OKZUn=pexhWaKGjUst-EvSOLw@mail.gmail.com","subject":"Re: [PATCH v2 2/2] send-email: honor transferencoding config option again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-09T06:47:18Z","receivedAt":"2019-05-09T06:47:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Wed, May 8, 2019 at 6:56 AM Junio C Hamano wrote:\n>> Since e67a228cd8a (\"send-email: automatically determine\n>> transfer-encoding\"), the value of sendmail.transferencoding in the\n>> configuration file is ignored, because $target_xfer_encoding is\n>\n> s/,//\n\n(also all other typofixes are obviously correct)\n\nThanks.\n"},{"id":"375215","messageId":"20190509114830.29647-1-avarab@gmail.com","threadId":"50905","inReplyTo":"20190508105607.178244-1-gitster@pobox.com","subject":"[PATCH v3 0/3] send-email: fix cli->config parsing crazyness","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-09T11:48:27Z","receivedAt":"2019-05-09T11:48:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is a proposed replacement for Junio's version of the\nsendemail.transferencoding bugfix. As explained in 3/3 I think the\nroot cause is that we're needlessly doing the config->cli parsing in\nthe wrong order, so let's just fix that.\n\nIt fixes the bug at hand, and makes the coge less fragile for future\nmaintenance.\n\nÆvar Arnfjörð Bjarmason (3):\n send-email: move the read_config() function above getopts\n send-email: rename the @bcclist variable for consistency\n send-email: do defaults -> config -> getopt in that order\n\n git-send-email.perl | 179 ++++++++++++++++++++++--------------------\n t/t9001-send-email.sh | 13 ++-\n 2 files changed, 105 insertions(+), 87 deletions(-)\n\n-- \n2.21.0.1020.gf2820cf01a\n\n"},{"id":"375216","messageId":"20190509114830.29647-2-avarab@gmail.com","threadId":"50905","inReplyTo":"20190508105607.178244-1-gitster@pobox.com","subject":"[PATCH v3 1/3] send-email: move the read_config() function above getopts","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-09T11:48:28Z","receivedAt":"2019-05-09T11:48:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is in preparation for a later change where we'll read the config\nfirst before parsing command-line options. As the move detection will\nshow no lines (except one line of comment) is changed here, just moved\naround.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n git-send-email.perl | 97 ++++++++++++++++++++++-----------------------\n 1 file changed, 48 insertions(+), 49 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 24859a7bc3..0d87ed2b5d 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -315,6 +315,54 @@ sub signal_handler {\n $SIG{TERM} = \\&signal_handler;\n $SIG{INT} = \\&signal_handler;\n \n+# Read our sendemail.* config\n+sub read_config {\n+\tmy ($prefix) = @_;\n+\n+\tforeach my $setting (keys %config_bool_settings) {\n+\t\tmy $target = $config_bool_settings{$setting}->[0];\n+\t\t$$target = Git::config_bool(@repo, \"$prefix.$setting\") unless (defined $$target);\n+\t}\n+\n+\tforeach my $setting (keys %config_path_settings) {\n+\t\tmy $target = $config_path_settings{$setting};\n+\t\tif (ref($target) eq \"ARRAY\") {\n+\t\t\tunless (@$target) {\n+\t\t\t\tmy @values = Git::config_path(@repo, \"$prefix.$setting\");\n+\t\t\t\t@$target = @values if (@values && defined $values[0]);\n+\t\t\t}\n+\t\t}\n+\t\telse {\n+\t\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\") unless (defined $$target);\n+\t\t}\n+\t}\n+\n+\tforeach my $setting (keys %config_settings) {\n+\t\tmy $target = $config_settings{$setting};\n+\t\tnext if $setting eq \"to\" and defined $no_to;\n+\t\tnext if $setting eq \"cc\" and defined $no_cc;\n+\t\tnext if $setting eq \"bcc\" and defined $no_bcc;\n+\t\tif (ref($target) eq \"ARRAY\") {\n+\t\t\tunless (@$target) {\n+\t\t\t\tmy @values = Git::config(@repo, \"$prefix.$setting\");\n+\t\t\t\t@$target = @values if (@values && defined $values[0]);\n+\t\t\t}\n+\t\t}\n+\t\telse {\n+\t\t\t$$target = Git::config(@repo, \"$prefix.$setting\") unless (defined $$target);\n+\t\t}\n+\t}\n+\n+\tif (!defined $smtp_encryption) {\n+\t\tmy $enc = Git::config(@repo, \"$prefix.smtpencryption\");\n+\t\tif (defined $enc) {\n+\t\t\t$smtp_encryption = $enc;\n+\t\t} elsif (Git::config_bool(@repo, \"$prefix.smtpssl\")) {\n+\t\t\t$smtp_encryption = 'ssl';\n+\t\t}\n+\t}\n+}\n+\n # Begin by accumulating all the variables (defined above), that we will end up\n # needing, first, from the command line:\n \n@@ -399,55 +447,6 @@ sub signal_handler {\n \t\"(via command-line or configuration option)\\n\")\n \tif defined $relogin_delay and not defined $batch_size;\n \n-# Now, let's fill any that aren't set in with defaults:\n-\n-sub read_config {\n-\tmy ($prefix) = @_;\n-\n-\tforeach my $setting (keys %config_bool_settings) {\n-\t\tmy $target = $config_bool_settings{$setting}->[0];\n-\t\t$$target = Git::config_bool(@repo, \"$prefix.$setting\") unless (defined $$target);\n-\t}\n-\n-\tforeach my $setting (keys %config_path_settings) {\n-\t\tmy $target = $config_path_settings{$setting};\n-\t\tif (ref($target) eq \"ARRAY\") {\n-\t\t\tunless (@$target) {\n-\t\t\t\tmy @values = Git::config_path(@repo, \"$prefix.$setting\");\n-\t\t\t\t@$target = @values if (@values && defined $values[0]);\n-\t\t\t}\n-\t\t}\n-\t\telse {\n-\t\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\") unless (defined $$target);\n-\t\t}\n-\t}\n-\n-\tforeach my $setting (keys %config_settings) {\n-\t\tmy $target = $config_settings{$setting};\n-\t\tnext if $setting eq \"to\" and defined $no_to;\n-\t\tnext if $setting eq \"cc\" and defined $no_cc;\n-\t\tnext if $setting eq \"bcc\" and defined $no_bcc;\n-\t\tif (ref($target) eq \"ARRAY\") {\n-\t\t\tunless (@$target) {\n-\t\t\t\tmy @values = Git::config(@repo, \"$prefix.$setting\");\n-\t\t\t\t@$target = @values if (@values && defined $values[0]);\n-\t\t\t}\n-\t\t}\n-\t\telse {\n-\t\t\t$$target = Git::config(@repo, \"$prefix.$setting\") unless (defined $$target);\n-\t\t}\n-\t}\n-\n-\tif (!defined $smtp_encryption) {\n-\t\tmy $enc = Git::config(@repo, \"$prefix.smtpencryption\");\n-\t\tif (defined $enc) {\n-\t\t\t$smtp_encryption = $enc;\n-\t\t} elsif (Git::config_bool(@repo, \"$prefix.smtpssl\")) {\n-\t\t\t$smtp_encryption = 'ssl';\n-\t\t}\n-\t}\n-}\n-\n # read configuration from [sendemail \"$identity\"], fall back on [sendemail]\n $identity = Git::config(@repo, \"sendemail.identity\") unless (defined $identity);\n read_config(\"sendemail.$identity\") if (defined $identity);\n-- \n2.21.0.1020.gf2820cf01a\n\n"},{"id":"375217","messageId":"20190509114830.29647-3-avarab@gmail.com","threadId":"50905","inReplyTo":"20190508105607.178244-1-gitster@pobox.com","subject":"[PATCH v3 2/3] send-email: rename the @bcclist variable for consistency","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-09T11:48:29Z","receivedAt":"2019-05-09T11:48:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"to\" and \"cc\" variables are named @initial_{to,cc}, let's rename\nthis one to match them.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n git-send-email.perl | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 0d87ed2b5d..48ed18a85c 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -177,7 +177,7 @@ sub format_2822_time {\n my $re_encoded_word = qr/=\\?($re_token)\\?($re_token)\\?($re_encoded_text)\\?=/;\n \n # Variables we fill in automatically, or via prompting:\n-my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,\n+my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@initial_bcc,$no_bcc,@xh,\n \t$initial_in_reply_to,$reply_to,$initial_subject,@files,\n \t$author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time);\n \n@@ -272,7 +272,7 @@ sub do_edit {\n \"cc\" => \\@initial_cc,\n \"cccmd\" => \\$cc_cmd,\n \"aliasfiletype\" => \\$aliasfiletype,\n- \"bcc\" => \\@bcclist,\n+ \"bcc\" => \\@initial_bcc,\n \"suppresscc\" => \\@suppress_cc,\n \"envelopesender\" => \\$envelope_sender,\n \"confirm\" => \\$confirm,\n@@ -383,7 +383,7 @@ sub read_config {\n \t\t \"no-to\" => \\$no_to,\n \t\t \"cc=s\" => \\@initial_cc,\n \t\t \"no-cc\" => \\$no_cc,\n-\t\t \"bcc=s\" => \\@bcclist,\n+\t\t \"bcc=s\" => \\@initial_bcc,\n \t\t \"no-bcc\" => \\$no_bcc,\n \t\t \"chain-reply-to!\" => \\$chain_reply_to,\n \t\t \"no-chain-reply-to\" => sub {$chain_reply_to = 0},\n@@ -940,7 +940,7 @@ sub expand_one_alias {\n \n @initial_to = process_address_list(@initial_to);\n @initial_cc = process_address_list(@initial_cc);\n-@bcclist = process_address_list(@bcclist);\n+@initial_bcc = process_address_list(@initial_bcc);\n \n if ($thread && !defined $initial_in_reply_to && $prompting) {\n \t$initial_in_reply_to = ask(\n@@ -1363,7 +1363,7 @@ sub send_message {\n \t\t }\n \t @cc);\n \tmy $to = join (\",\\n\\t\", @recipients);\n-\t@recipients = unique_email_list(@recipients,@cc,@bcclist);\n+\t@recipients = unique_email_list(@recipients,@cc,@initial_bcc);\n \t@recipients = (map { extract_valid_address_or_die($_) } @recipients);\n \tmy $date = format_2822_time($time++);\n \tmy $gitversion = '@@GIT_VERSION@@';\n-- \n2.21.0.1020.gf2820cf01a\n\n"},{"id":"375218","messageId":"20190509114830.29647-4-avarab@gmail.com","threadId":"50905","inReplyTo":"20190508105607.178244-1-gitster@pobox.com","subject":"[PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-09T11:48:30Z","receivedAt":"2019-05-09T11:48:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the git-send-email command-line argument parsing and config\nreading code to parse those two in the right order. I.e. first we set\nour hardcoded defaults, then we read our config, and finally we read\nthe command-line, with later sets overriding earlier sets.\n\nThis fixes a bug introduced in e67a228cd8 (\"send-email: automatically\ndetermine transfer-encoding\", 2018-07-08). That change broke the broke\nthe reading of sendmail.transferencoding because it wasn't careful to\nupdate our fragile code dealing with doing this in the previous\n\"defaults -> getopt -> config\" order..\n\nBut as we can see from the history for this file doing it this way was\nnever what we actually wanted, it just something we grew organically\nas of 5483c71d7a (\"git-send-email: make options easier to configure.\",\n2007-06-27) and have been dealing with the fallout since, e.g. in\n463b0ea22b (\"send-email: Fix %config_path_settings handling\",\n2011-10-14).\n\nAs can be seen in this change the only place where we actually want to\ndo something clever is with the to/cc/bcc variables, where setting\nthem on the command-line (or using --no-{to,cc,bcc}) should clear out\nvalues we grab from the config.\n\nAll the rest are things where the command-line should simply override\nthe config values, and by reading the config first the config code\ndoesn't need all this \"let's not set it was on the command-line\"\nspecial-casing, as [1] shows we'd otherwise need to care about the\ndifference between whether something was a default or present in\nconfig to fix the bug in e67a228cd8.\n\n1. https://public-inbox.org/git/20190508105607.178244-2-gitster@pobox.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n git-send-email.perl | 92 +++++++++++++++++++++++--------------------\n t/t9001-send-email.sh | 13 +++++-\n 2 files changed, 62 insertions(+), 43 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 48ed18a85c..fab255249f 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -177,11 +177,15 @@ sub format_2822_time {\n my $re_encoded_word = qr/=\\?($re_token)\\?($re_token)\\?($re_encoded_text)\\?=/;\n \n # Variables we fill in automatically, or via prompting:\n-my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@initial_bcc,$no_bcc,@xh,\n+my (@to,@cc,,@xh,$envelope_sender,\n \t$initial_in_reply_to,$reply_to,$initial_subject,@files,\n-\t$author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time);\n-\n-my $envelope_sender;\n+\t$author,$sender,$smtp_authpass,$annotate,$compose,$time);\n+# Things we either get from config, *or* are overridden on the\n+# command-line.\n+my ($no_cc, $no_to, $no_bcc);\n+my (@config_to, @getopt_to);\n+my (@config_cc, @getopt_cc);\n+my (@config_bcc, @getopt_bcc);\n \n # Example reply to:\n #$initial_in_reply_to = ''; #<20050203173208.GA23964@foobar.com>';\n@@ -228,33 +232,37 @@ sub do_edit {\n }\n \n # Variables with corresponding config settings\n-my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc);\n+my ($suppress_from, $signed_off_by_cc);\n my ($cover_cc, $cover_to);\n my ($to_cmd, $cc_cmd);\n my ($smtp_server, $smtp_server_port, @smtp_server_options);\n my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);\n my ($batch_size, $relogin_delay);\n my ($identity, $aliasfiletype, @alias_files, $smtp_domain, $smtp_auth);\n-my ($validate, $confirm);\n+my ($confirm);\n my (@suppress_cc);\n my ($auto_8bit_encoding);\n my ($compose_encoding);\n-my $target_xfer_encoding = 'auto';\n-\n+# Variables with corresponding config settings & hardcoded defaults\n my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n+my $thread = 1;\n+my $chain_reply_to = 0;\n+my $use_xmailer = 1;\n+my $validate = 1;\n+my $target_xfer_encoding = 'auto';\n \n my %config_bool_settings = (\n- \"thread\" => [\\$thread, 1],\n- \"chainreplyto\" => [\\$chain_reply_to, 0],\n- \"suppressfrom\" => [\\$suppress_from, undef],\n- \"signedoffbycc\" => [\\$signed_off_by_cc, undef],\n- \"cccover\" => [\\$cover_cc, undef],\n- \"tocover\" => [\\$cover_to, undef],\n- \"signedoffcc\" => [\\$signed_off_by_cc, undef], # Deprecated\n- \"validate\" => [\\$validate, 1],\n- \"multiedit\" => [\\$multiedit, undef],\n- \"annotate\" => [\\$annotate, undef],\n- \"xmailer\" => [\\$use_xmailer, 1]\n+ \"thread\" => \\$thread,\n+ \"chainreplyto\" => \\$chain_reply_to,\n+ \"suppressfrom\" => \\$suppress_from,\n+ \"signedoffbycc\" => \\$signed_off_by_cc,\n+ \"cccover\" => \\$cover_cc,\n+ \"tocover\" => \\$cover_to,\n+ \"signedoffcc\" => \\$signed_off_by_cc,\n+ \"validate\" => \\$validate,\n+ \"multiedit\" => \\$multiedit,\n+ \"annotate\" => \\$annotate,\n+ \"xmailer\" => \\$use_xmailer,\n );\n \n my %config_settings = (\n@@ -267,12 +275,12 @@ sub do_edit {\n \"smtpauth\" => \\$smtp_auth,\n \"smtpbatchsize\" => \\$batch_size,\n \"smtprelogindelay\" => \\$relogin_delay,\n- \"to\" => \\@initial_to,\n+ \"to\" => \\@config_to,\n \"tocmd\" => \\$to_cmd,\n- \"cc\" => \\@initial_cc,\n+ \"cc\" => \\@config_cc,\n \"cccmd\" => \\$cc_cmd,\n \"aliasfiletype\" => \\$aliasfiletype,\n- \"bcc\" => \\@initial_bcc,\n+ \"bcc\" => \\@config_bcc,\n \"suppresscc\" => \\@suppress_cc,\n \"envelopesender\" => \\$envelope_sender,\n \"confirm\" => \\$confirm,\n@@ -320,8 +328,9 @@ sub read_config {\n \tmy ($prefix) = @_;\n \n \tforeach my $setting (keys %config_bool_settings) {\n-\t\tmy $target = $config_bool_settings{$setting}->[0];\n-\t\t$$target = Git::config_bool(@repo, \"$prefix.$setting\") unless (defined $$target);\n+\t\tmy $target = $config_bool_settings{$setting};\n+\t\tmy $v = Git::config_bool(@repo, \"$prefix.$setting\");\n+\t\t$$target = $v if defined $v;\n \t}\n \n \tforeach my $setting (keys %config_path_settings) {\n@@ -333,15 +342,13 @@ sub read_config {\n \t\t\t}\n \t\t}\n \t\telse {\n-\t\t\t$$target = Git::config_path(@repo, \"$prefix.$setting\") unless (defined $$target);\n+\t\t\tmy $v = Git::config_path(@repo, \"$prefix.$setting\");\n+\t\t\t$$target = $v if defined $v;\n \t\t}\n \t}\n \n \tforeach my $setting (keys %config_settings) {\n \t\tmy $target = $config_settings{$setting};\n-\t\tnext if $setting eq \"to\" and defined $no_to;\n-\t\tnext if $setting eq \"cc\" and defined $no_cc;\n-\t\tnext if $setting eq \"bcc\" and defined $no_bcc;\n \t\tif (ref($target) eq \"ARRAY\") {\n \t\t\tunless (@$target) {\n \t\t\t\tmy @values = Git::config(@repo, \"$prefix.$setting\");\n@@ -349,7 +356,8 @@ sub read_config {\n \t\t\t}\n \t\t}\n \t\telse {\n-\t\t\t$$target = Git::config(@repo, \"$prefix.$setting\") unless (defined $$target);\n+\t\t\tmy $v = Git::config(@repo, \"$prefix.$setting\");\n+\t\t\t$$target = $v if defined $v;\n \t\t}\n \t}\n \n@@ -363,6 +371,11 @@ sub read_config {\n \t}\n }\n \n+$identity = Git::config(@repo, \"sendemail.identity\");\n+read_config(\"sendemail.$identity\") if defined $identity;\n+read_config(\"sendemail\");\n+read_config(\"sendemail\");\n+\n # Begin by accumulating all the variables (defined above), that we will end up\n # needing, first, from the command line:\n \n@@ -378,12 +391,12 @@ sub read_config {\n \"in-reply-to=s\" => \\$initial_in_reply_to,\n \t\t \"reply-to=s\" => \\$reply_to,\n \t\t \"subject=s\" => \\$initial_subject,\n-\t\t \"to=s\" => \\@initial_to,\n+\t\t \"to=s\" => \\@getopt_to,\n \t\t \"to-cmd=s\" => \\$to_cmd,\n \t\t \"no-to\" => \\$no_to,\n-\t\t \"cc=s\" => \\@initial_cc,\n+\t\t \"cc=s\" => \\@getopt_cc,\n \t\t \"no-cc\" => \\$no_cc,\n-\t\t \"bcc=s\" => \\@initial_bcc,\n+\t\t \"bcc=s\" => \\@getopt_bcc,\n \t\t \"no-bcc\" => \\$no_bcc,\n \t\t \"chain-reply-to!\" => \\$chain_reply_to,\n \t\t \"no-chain-reply-to\" => sub {$chain_reply_to = 0},\n@@ -434,6 +447,11 @@ sub read_config {\n \t\t \"git-completion-helper\" => \\$git_completion_helper,\n \t );\n \n+# Munge any \"either config or getopt, not both\" variables\n+my @initial_to = @getopt_to ? @getopt_to : ($no_to ? () : @config_to);\n+my @initial_cc = @getopt_cc ? @getopt_cc : ($no_cc ? () : @config_cc);\n+my @initial_bcc = @getopt_bcc ? @getopt_bcc : ($no_bcc ? () : @config_bcc);\n+\n usage() if $help;\n completion_helper() if $git_completion_helper;\n unless ($rc) {\n@@ -447,16 +465,6 @@ sub read_config {\n \t\"(via command-line or configuration option)\\n\")\n \tif defined $relogin_delay and not defined $batch_size;\n \n-# read configuration from [sendemail \"$identity\"], fall back on [sendemail]\n-$identity = Git::config(@repo, \"sendemail.identity\") unless (defined $identity);\n-read_config(\"sendemail.$identity\") if (defined $identity);\n-read_config(\"sendemail\");\n-\n-# fall back on builtin bool defaults\n-foreach my $setting (values %config_bool_settings) {\n-\t${$setting->[0]} = $setting->[1] unless (defined (${$setting->[0]}));\n-}\n-\n # 'default' encryption is none -- this only prevents a warning\n $smtp_encryption = '' unless (defined $smtp_encryption);\n \ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 1e3ac3c384..1b18201ce2 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1461,7 +1461,18 @@ test_expect_success $PREREQ '--transfer-encoding overrides sendemail.transferEnc\n \ttest -z \"$(ls msgtxt*)\"\n '\n \n-test_expect_success $PREREQ 'sendemail.transferencoding=8bit' '\n+test_expect_success $PREREQ 'sendemail.transferencoding=8bit via config' '\n+\tclean_fake_sendmail &&\n+\tgit -c sendemail.transferencoding=8bit send-email \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\temail-using-8bit \\\n+\t\t2>errors >out &&\n+\tsed '1,/^$/d' msgtxt1 >actual &&\n+\tsed '1,/^$/d' email-using-8bit >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success $PREREQ 'sendemail.transferencoding=8bit via cli' '\n \tclean_fake_sendmail &&\n \tgit send-email \\\n \t\t--transfer-encoding=8bit \\\n-- \n2.21.0.1020.gf2820cf01a\n\n"},{"id":"375248","messageId":"CAPig+cTJFTkJ7N+ksrN-nHeLzNmCbDpVgLRBJZtMjTsKSE_ijA@mail.gmail.com","threadId":"50905","inReplyTo":"20190509114830.29647-4-avarab@gmail.com","subject":"Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-09T18:04:23Z","receivedAt":"2019-05-09T18:04:36Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, May 9, 2019 at 7:48 AM Ævar Arnfjörð Bjarmason wrote:\n> Change the git-send-email command-line argument parsing and config\n> reading code to parse those two in the right order. I.e. first we set\n> our hardcoded defaults, then we read our config, and finally we read\n> the command-line, with later sets overriding earlier sets.\n>\n> This fixes a bug introduced in e67a228cd8 (\"send-email: automatically\n> determine transfer-encoding\", 2018-07-08). That change broke the broke\n\ns/broke the broke/broke/\n\n> the reading of sendmail.transferencoding because it wasn't careful to\n> update our fragile code dealing with doing this in the previous\n> \"defaults -> getopt -> config\" order..\n\ns/\\.\\.$/./\n\n> But as we can see from the history for this file doing it this way was\n> never what we actually wanted, it just something we grew organically\n\ns/it/it's/\n\n> as of 5483c71d7a (\"git-send-email: make options easier to configure.\",\n> 2007-06-27) and have been dealing with the fallout since, e.g. in\n> 463b0ea22b (\"send-email: Fix %config_path_settings handling\",\n> 2011-10-14).\n>\n> As can be seen in this change the only place where we actually want to\n> do something clever is with the to/cc/bcc variables, where setting\n> them on the command-line (or using --no-{to,cc,bcc}) should clear out\n> values we grab from the config.\n>\n> All the rest are things where the command-line should simply override\n> the config values, and by reading the config first the config code\n> doesn't need all this \"let's not set it was on the command-line\"\n\nECANTPARSE \"let's not set it was on the command-line\"\n\n> special-casing, as [1] shows we'd otherwise need to care about the\n> difference between whether something was a default or present in\n> config to fix the bug in e67a228cd8.\n>\n> 1. https://public-inbox.org/git/20190508105607.178244-2-gitster@pobox.com/\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n"},{"id":"375275","messageId":"20190509235112.GD7458@genre.crustytoothpaste.net","threadId":"50905","inReplyTo":"20190509114830.29647-4-avarab@gmail.com","subject":"Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-05-09T23:51:12Z","receivedAt":"2019-05-09T23:51:21Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, May 09, 2019 at 01:48:30PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> @@ -363,6 +371,11 @@ sub read_config {\n> \t}\n> }\n> \n> +$identity = Git::config(@repo, \"sendemail.identity\");\n> +read_config(\"sendemail.$identity\") if defined $identity;\n> +read_config(\"sendemail\");\n> +read_config(\"sendemail\");\n\nDid we really want to call this function twice with the same argument?\nIf so, perhaps a comment is appropriate explaining why.\n\nIn general, I'm not opposed to this approach, either. It does make it\neasier to handle parsing in general. However, having said that, I'm fine\nwith either your series or Junio's.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"375295","messageId":"xmqqr296e7ur.fsf@gitster-ct.c.googlers.com","threadId":"50905","inReplyTo":"20190509114830.29647-1-avarab@gmail.com","subject":"Re: [PATCH v3 0/3] send-email: fix cli->config parsing crazyness","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-10T13:50:52Z","receivedAt":"2019-05-10T13:51:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> This is a proposed replacement for Junio's version of the\n> ... the\n> root cause is that we're needlessly doing the config->cli parsing in\n> the wrong order, so let's just fix that.\n\nYup, that's absolutely the right approach. I just wanted to avoid\nrestructuring that heavily, but I have no problem as long as it is\nsomebody else, and somebody who is more competent in writing Perl\nthan I am, doing the heavy lifting ;-)\n\n"},{"id":"375369","messageId":"xmqqr292bv24.fsf@gitster-ct.c.googlers.com","threadId":"50905","inReplyTo":"CAPig+cTJFTkJ7N+ksrN-nHeLzNmCbDpVgLRBJZtMjTsKSE_ijA@mail.gmail.com","subject":"Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-13T08:46:59Z","receivedAt":"2019-05-13T08:47:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n>> All the rest are things where the command-line should simply override\n>> the config values, and by reading the config first the config code\n>> doesn't need all this \"let's not set it was on the command-line\"\n>\n> ECANTPARSE \"let's not set it was on the command-line\"\n\nI took it as \"let's not set it, if it was on the command-line\".\n"},{"id":"375370","messageId":"xmqqmujqbuw4.fsf@gitster-ct.c.googlers.com","threadId":"50905","inReplyTo":"20190509114830.29647-4-avarab@gmail.com","subject":"Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-13T08:50:35Z","receivedAt":"2019-05-13T08:50:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> # Variables we fill in automatically, or via prompting:\n> -my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@initial_bcc,$no_bcc,@xh,\n> +my (@to,@cc,,@xh,$envelope_sender,\n\n,,???\n"},{"id":"375419","messageId":"87r292vyzu.fsf@evledraar.gmail.com","threadId":"50905","inReplyTo":"xmqqmujqbuw4.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-13T21:13:57Z","receivedAt":"2019-05-13T21:14:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, May 13 2019, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> # Variables we fill in automatically, or via prompting:\n>> -my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@initial_bcc,$no_bcc,@xh,\n>> +my (@to,@cc,,@xh,$envelope_sender,\n>\n> ,,???\n\nJust a typo. I see you fixed this and the other issues noted in your\npush-out to \"next\", thanks! Sorry about not getting to this before that.\n"},{"id":"375757","messageId":"5cddeb61.1c69fb81.47ed4.e648@mx.google.com","threadId":"50905","inReplyTo":"20190509114830.29647-4-avarab@gmail.com","subject":"Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order","fromName":"Stephen Boyd","fromEmail":"swboyd@chromium.org","sentAt":"2019-05-16T22:59:44Z","receivedAt":"2019-05-16T22:59:47Z","isPatch":true,"sender":{"key":"swboyd@chromium.org","avatar":null},"body":"(Sorry for weird reply, I'm not subscribed and my MUA is not prepared\nfor this)\n\n> Change the git-send-email command-line argument parsing and config\n> reading code to parse those two in the right order. I.e. first we set\n> our hardcoded defaults, then we read our config, and finally we read\n> the command-line, with later sets overriding earlier sets.\n> \n> This fixes a bug introduced in e67a228cd8 (\"send-email: automatically\n> determine transfer-encoding\", 2018-07-08). That change broke the broke\n> the reading of sendmail.transferencoding because it wasn't careful to\n> update our fragile code dealing with doing this in the previous\n> \"defaults -> getopt -> config\" order..\n> \n> But as we can see from the history for this file doing it this way was\n> never what we actually wanted, it just something we grew organically\n> as of 5483c71d7a (\"git-send-email: make options easier to configure.\",\n> 2007-06-27) and have been dealing with the fallout since, e.g. in\n> 463b0ea22b (\"send-email: Fix %config_path_settings handling\",\n> 2011-10-14).\n> \n> As can be seen in this change the only place where we actually want to\n> do something clever is with the to/cc/bcc variables, where setting\n> them on the command-line (or using --no-{to,cc,bcc}) should clear out\n> values we grab from the config.\n> \n> All the rest are things where the command-line should simply override\n> the config values, and by reading the config first the config code\n> doesn't need all this \"let's not set it was on the command-line\"\n> special-casing, as [1] shows we'd otherwise need to care about the\n> difference between whether something was a default or present in\n> config to fix the bug in e67a228cd8.\n\nThis broke my workflow.\n\nI specify --identity= on the commandline and I want that to\npick out my send-email config from my global .gitconfig file that\ncorresponds to that identity. With this change, the config is parsed\nbefore the getopt part so --identity on the commandline is a nop and\nnever looks into the config file to figure this out. So at least\n--identity is special in addition to --to,cc,bcc.\n\n"},{"id":"375759","messageId":"xmqqh89u3sdw.fsf@gitster-ct.c.googlers.com","threadId":"50905","inReplyTo":"5cddeb61.1c69fb81.47ed4.e648@mx.google.com","subject":"Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-16T23:13:15Z","receivedAt":"2019-05-16T23:13:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stephen Boyd writes:\n\n>> As can be seen in this change the only place where we actually want to\n>> do something clever is with the to/cc/bcc variables, where setting\n>> them on the command-line (or using --no-{to,cc,bcc}) should clear out\n>> values we grab from the config.\n>> \n>> All the rest are things where the command-line should simply override\n>> the config values, and by reading the config first the config code\n>> doesn't need all this \"let's not set it was on the command-line\"\n>> special-casing, as [1] shows we'd otherwise need to care about the\n>> difference between whether something was a default or present in\n>> config to fix the bug in e67a228cd8.\n>\n> This broke my workflow.\n>\n> I specify --identity= on the commandline and I want that to\n> pick out my send-email config from my global .gitconfig file that\n> corresponds to that identity. With this change, the config is parsed\n> before the getopt part so --identity on the commandline is a nop and\n> never looks into the config file to figure this out. So at least\n> --identity is special in addition to --to,cc,bcc.\n\nAh, sorry that nobody noticed that case, but you are right. Because\nthe ident is used as a part of the key to find identity-specific\nconfiguration values, if the command line gives one, we must have an\naccess to it before we start reading the configuration. In that sense,\nit is more fundamental to special-case the option.\n\nWe are past -rc0, so I am inclined to revert the change (and perhaps\nreplace it with the other \"fix\" that did not break the parsing order\nlike these patches did), with an expectation that a clever fix will\nbe found later, *unless* a simple and correct fix is found quickly.\n\n"},{"id":"375783","messageId":"xmqqsgtd3fw3.fsf@gitster-ct.c.googlers.com","threadId":"50905","inReplyTo":"xmqqh89u3sdw.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-17T03:43:08Z","receivedAt":"2019-05-17T03:43:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ah, sorry that nobody noticed that case, but you are right. Because\n> the ident is used as a part of the key to find identity-specific\n> configuration values, if the command line gives one, we must have an\n> access to it before we start reading the configuration. In that sense,\n> it is more fundamental to special-case the option.\n>\n> We are past -rc0, so I am inclined to revert the change (and perhaps\n> replace it with the other \"fix\" that did not break the parsing order\n> like these patches did), with an expectation that a clever fix will\n> be found later, *unless* a simple and correct fix is found quickly.\n\nOops, spoke too soon. The topic hasn't escaped to 'master' yet, so\nI'll make sure it stays that way. Thanks for catching a regression\nbefore it gets too late.\n\n\n"},{"id":"375821","messageId":"20190517195545.29729-1-avarab@gmail.com","threadId":"50905","inReplyTo":"xmqqsgtd3fw3.fsf@gitster-ct.c.googlers.com","subject":"[PATCH 0/5] ab/send-email-transferencoding-fix-for-the-fix","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-17T19:55:40Z","receivedAt":"2019-05-17T19:56:16Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This ab/send-email-transferencoding-fix-for-the-fix series goes on top\nof the ab/send-email-transferencoding-fix merged into \"next\". It fixes\nthe sendemail.identity issue Stephen Boyd reported, and then some.\n\nMaybe it would be more understandable to rewind the existing topic out\nof next and produce a new rebased series, but I think this is actually\neasier to review, and documents some of the tricky edge cases for\nfuture contributors.\n\nI did make sure that the new tests I added for existing behavior were\nactually testing existing behavior, and not whatever bugs I got into\n\"next\" by running the test on top of \"master\", only the expected (now\nfixed) tests fail there, not any tests for existing sendemail.identity\nbehavior.\n\nÆvar Arnfjörð Bjarmason (5):\n send-email: remove cargo-culted multi-patch pattern in tests\n send-email: fix broken transferEncoding tests\n send-email: document --no-[to|cc|bcc]\n send-email: fix regression in sendemail.identity parsing\n send-email: remove support for deprecated sendemail.smtpssl\n\n Documentation/config/sendemail.txt | 3 -\n Documentation/git-send-email.txt | 11 ++-\n git-send-email.perl | 78 ++++++++++--------\n t/t9001-send-email.sh | 122 +++++++++++++++++++++--------\n 4 files changed, 144 insertions(+), 70 deletions(-)\n\n-- \n2.21.0.1020.gf2820cf01a\n\n"},{"id":"375822","messageId":"20190517195545.29729-2-avarab@gmail.com","threadId":"50905","inReplyTo":"xmqqsgtd3fw3.fsf@gitster-ct.c.googlers.com","subject":"[PATCH 1/5] send-email: remove cargo-culted multi-patch pattern in tests","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-17T19:55:41Z","receivedAt":"2019-05-17T19:56:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change test code added in f434c083a0 (\"send-email: add --no-cc,\n--no-to, and --no-bcc\", 2010-03-07) which blindly copied a pattern\nfrom an earlier test added in 32ae83194b (\"add a test for\ngit-send-email for non-threaded mails\", 2009-06-12) where the\n\"$patches\" variable was supplied more than once.\n\nAs it turns out we didn't need more than one \"$patches\" for the test\nadded in 32ae83194b either. The only tests that actually needed this\nsort of invocation were the tests added in 54aae5e1a0 (\"t9001:\nsend-email interation with --in-reply-to and --chain-reply-to\",\n2010-10-19).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t9001-send-email.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 1b18201ce2..13de44686b 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1204,7 +1204,7 @@ test_expect_success $PREREQ 'no in-reply-to and no threading' '\n \t\t--from=\"Example \" \\\n \t\t--to=nobody@example.com \\\n \t\t--no-thread \\\n-\t\t$patches $patches >stdout &&\n+\t\t$patches >stdout &&\n \t! grep \"In-Reply-To: \" stdout\n '\n \n@@ -1224,7 +1224,7 @@ test_expect_success $PREREQ 'sendemail.to works' '\n \tgit send-email \\\n \t\t--dry-run \\\n \t\t--from=\"Example \" \\\n-\t\t$patches $patches >stdout &&\n+\t\t$patches >stdout &&\n \tgrep \"To: Somebody \" stdout\n '\n \n@@ -1234,7 +1234,7 @@ test_expect_success $PREREQ '--no-to overrides sendemail.to' '\n \t\t--from=\"Example \" \\\n \t\t--no-to \\\n \t\t--to=nobody@example.com \\\n-\t\t$patches $patches >stdout &&\n+\t\t$patches >stdout &&\n \tgrep \"To: nobody@example.com\" stdout &&\n \t! grep \"To: Somebody \" stdout\n '\n@@ -1245,7 +1245,7 @@ test_expect_success $PREREQ 'sendemail.cc works' '\n \t\t--dry-run \\\n \t\t--from=\"Example \" \\\n \t\t--to=nobody@example.com \\\n-\t\t$patches $patches >stdout &&\n+\t\t$patches >stdout &&\n \tgrep \"Cc: Somebody \" stdout\n '\n \n@@ -1256,7 +1256,7 @@ test_expect_success $PREREQ '--no-cc overrides sendemail.cc' '\n \t\t--no-cc \\\n \t\t--cc=bodies@example.com \\\n \t\t--to=nobody@example.com \\\n-\t\t$patches $patches >stdout &&\n+\t\t$patches >stdout &&\n \tgrep \"Cc: bodies@example.com\" stdout &&\n \t! grep \"Cc: Somebody \" stdout\n '\n@@ -1268,7 +1268,7 @@ test_expect_success $PREREQ 'sendemail.bcc works' '\n \t\t--from=\"Example \" \\\n \t\t--to=nobody@example.com \\\n \t\t--smtp-server relay.example.com \\\n-\t\t$patches $patches >stdout &&\n+\t\t$patches >stdout &&\n \tgrep \"RCPT TO:\" stdout\n '\n \n@@ -1280,7 +1280,7 @@ test_expect_success $PREREQ '--no-bcc overrides sendemail.bcc' '\n \t\t--bcc=bodies@example.com \\\n \t\t--to=nobody@example.com \\\n \t\t--smtp-server relay.example.com \\\n-\t\t$patches $patches >stdout &&\n+\t\t$patches >stdout &&\n \tgrep \"RCPT TO:\" stdout &&\n \t! grep \"RCPT TO:\" stdout\n '\n-- \n2.21.0.1020.gf2820cf01a\n\n"},{"id":"375823","messageId":"20190517195545.29729-3-avarab@gmail.com","threadId":"50905","inReplyTo":"xmqqsgtd3fw3.fsf@gitster-ct.c.googlers.com","subject":"[PATCH 2/5] send-email: fix broken transferEncoding tests","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-17T19:55:42Z","receivedAt":"2019-05-17T19:56:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"I fixed a bug that had broken the reading of sendmail.transferEncoding\nin 3494dfd3ee (\"send-email: do defaults -> config -> getopt in that\norder\", 2019-05-09), but the test I added in that commit did nothing\nto assert the bug had been fixed.\n\nThat issue originates in 8d81408435 (\"git-send-email: add\n--transfer-encoding option\", 2014-11-25) which first added the\n\"sendemail.transferencoding=8bit\".\n\nThat test has never done anything meaningful. It tested that the\n\"--transfer-encoding=8bit\" option would turn on the 8bit\nTransfer-Encoding, but that was the default at the time (and now). As\nchecking out 8d81408435 and editing the test to remove that option\nwill reveal, supplying it never did anything.\n\nSo when I copied it thinking it would work in 3494dfd3ee I copied a\npreviously broken test, although I was making sure it did the right\nthing via da-hoc debugger inspection, so the bug was fixed.\n\nSo fix the test I added in 3494dfd3ee, as well as the long-standing\ntest added in 8d81408435. To test if we're actually setting the\nTransfer-Encoding let's set it to 7bit, not 8bit, as 7bit will error\nout on \"email-using-8bit\".\n\nThis means that we can remove the \"sendemail.transferencoding=7bit\nfails on 8bit data\" test, since it was redundant, we now have other\ntests that assert that that'll fail.\n\nWhile I'm at it convert \"git config \" in the test setup\nto just \"-c =\" on the command-line. Then we don't need to\ncleanup after these tests, and there's no sense in asserting where\nconfig values come from in these tests, we can take that as a given.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t9001-send-email.sh | 35 +++++++++++------------------------\n 1 file changed, 11 insertions(+), 24 deletions(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 13de44686b..61d484d1a6 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1437,10 +1437,10 @@ test_expect_success $PREREQ 'setup expect' '\n \tEOF\n '\n \n-test_expect_success $PREREQ 'sendemail.transferencoding=7bit fails on 8bit data' '\n+test_expect_success $PREREQ '--transfer-encoding overrides sendemail.transferEncoding' '\n \tclean_fake_sendmail &&\n-\tgit config sendemail.transferEncoding 7bit &&\n-\ttest_must_fail git send-email \\\n+\ttest_must_fail git -c sendemail.transferEncoding=8bit \\\n+\t\tsend-email \\\n \t\t--transfer-encoding=7bit \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n \t\temail-using-8bit \\\n@@ -1449,11 +1449,10 @@ test_expect_success $PREREQ 'sendemail.transferencoding=7bit fails on 8bit data'\n \ttest -z \"$(ls msgtxt*)\"\n '\n \n-test_expect_success $PREREQ '--transfer-encoding overrides sendemail.transferEncoding' '\n+test_expect_success $PREREQ 'sendemail.transferEncoding via config' '\n \tclean_fake_sendmail &&\n-\tgit config sendemail.transferEncoding 8bit &&\n-\ttest_must_fail git send-email \\\n-\t\t--transfer-encoding=7bit \\\n+\ttest_must_fail git -c sendemail.transferEncoding=7bit \\\n+\t\tsend-email \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n \t\temail-using-8bit \\\n \t\t2>errors >out &&\n@@ -1461,27 +1460,15 @@ test_expect_success $PREREQ '--transfer-encoding overrides sendemail.transferEnc\n \ttest -z \"$(ls msgtxt*)\"\n '\n \n-test_expect_success $PREREQ 'sendemail.transferencoding=8bit via config' '\n+test_expect_success $PREREQ 'sendemail.transferEncoding via cli' '\n \tclean_fake_sendmail &&\n-\tgit -c sendemail.transferencoding=8bit send-email \\\n-\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n-\t\temail-using-8bit \\\n-\t\t2>errors >out &&\n-\tsed '1,/^$/d' msgtxt1 >actual &&\n-\tsed '1,/^$/d' email-using-8bit >expected &&\n-\ttest_cmp expected actual\n-'\n-\n-test_expect_success $PREREQ 'sendemail.transferencoding=8bit via cli' '\n-\tclean_fake_sendmail &&\n-\tgit send-email \\\n-\t\t--transfer-encoding=8bit \\\n+\ttest_must_fail git send-email \\\n+\t\t--transfer-encoding=7bit \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n \t\temail-using-8bit \\\n \t\t2>errors >out &&\n-\tsed '1,/^$/d' msgtxt1 >actual &&\n-\tsed '1,/^$/d' email-using-8bit >expected &&\n-\ttest_cmp expected actual\n+\tgrep \"cannot send message as 7bit\" errors &&\n+\ttest -z \"$(ls msgtxt*)\"\n '\n \n test_expect_success $PREREQ 'setup expect' '\n-- \n2.21.0.1020.gf2820cf01a\n\n"},{"id":"375824","messageId":"20190517195545.29729-4-avarab@gmail.com","threadId":"50905","inReplyTo":"xmqqsgtd3fw3.fsf@gitster-ct.c.googlers.com","subject":"[PATCH 3/5] send-email: document --no-[to|cc|bcc]","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-17T19:55:43Z","receivedAt":"2019-05-17T19:56:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"These options added in f434c083a0 (\"send-email: add --no-cc, --no-to,\nand --no-bcc\", 2010-03-07) were never documented.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-send-email.txt | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 1afe9fc858..8100ff4b0f 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -278,6 +278,10 @@ must be used for each option.\n Automating\n ~~~~~~~~~~\n \n+--no-[to|cc|bcc]::\n+\tClears any list of \"To:\", \"Cc:\", \"Bcc:\" addresses previously\n+\tset via config.\n+\n --to-cmd=::\n \tSpecify a command to execute once per patch file which\n \tshould generate patch file specific \"To:\" entries.\n-- \n2.21.0.1020.gf2820cf01a\n\n"},{"id":"375825","messageId":"20190517195545.29729-5-avarab@gmail.com","threadId":"50905","inReplyTo":"xmqqsgtd3fw3.fsf@gitster-ct.c.googlers.com","subject":"[PATCH 4/5] send-email: fix regression in sendemail.identity parsing","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-17T19:55:44Z","receivedAt":"2019-05-17T19:56:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a regression in my recent 3494dfd3ee (\"send-email: do defaults ->\nconfig -> getopt in that order\", 2019-05-09). I missed that the\n$identity variable needs to be extracted from the command-line before\nwe do the config reading, as it determines which config variable we\nshould read first. See [1] for the report.\n\nThe sendemail.identity feature was added back in\n34cc60ce2b (\"send-email: Add support for SSL and SMTP-AUTH\",\n2007-09-03), there were no tests to assert that it worked properly.\n\nSo let's fix both the regression, and add some tests to assert that\nthis is being parsed properly. While I'm at it I'm adding a\n--no-identity option to go with --[to|cc|bcc] variable, since the\nsemantics are similar. It's like to/cc/bcc except that unlike those we\ndon't support multiple identities, but we could now easily add it\nsupport for it if anyone cares.\n\nIn just fixing the --identity command-line parsing bug I discovered\nthat a narrow fix to that wouldn't do. In read_config() we had a state\nmachine that would only set config values if they weren't set already,\nand thus by proxy we wouldn't e.g. set \"to\" based on sendemail.to if\nwe'd seen sendemail.gmail.to before, with --identity=gmail.\n\nI'd modified some of the relevant code in 3494dfd3ee, but just\nreverting to that wouldn't do, since it would bring back the\nregression fixed in that commit.\n\nRefactor read_config() do what we actually mean here. We don't want to\nset a given sendemail.VAR if a sendemail.$identity.VAR previously set\nit. The old code was conflating this desire with the hardcoded\ndefaults for these variables, and as discussed in 3494dfd3ee that was\nnever going to work. Instead pass along the state of whether an\nidentity config set something before, as distinguished from the state\nof the default just being false, or the default being a non-bool or\ntrue (e.g. --transferencoding).\n\nI'm still not happy with the test coverage here, e.g. there's nothing\ntesting sendemail.smtpEncryption, but I only have so much time to fix\nthis code.\n\n1. https://public-inbox.org/git/5cddeb61.1c69fb81.47ed4.e648@mx.google.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-send-email.txt | 4 ++\n git-send-email.perl | 60 ++++++++++++++++++++----------\n t/t9001-send-email.sh | 64 ++++++++++++++++++++++++++++++++\n 3 files changed, 108 insertions(+), 20 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 8100ff4b0f..a861934c69 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -282,6 +282,10 @@ Automating\n \tClears any list of \"To:\", \"Cc:\", \"Bcc:\" addresses previously\n \tset via config.\n \n+--no-identity::\n+\tClears the previously read value of `sendemail.identity` set\n+\tvia config, if any.\n+\n --to-cmd=::\n \tSpecify a command to execute once per patch file which\n \tshould generate patch file specific \"To:\" entries.\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex b2c4a77671..80cbbfd2b8 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -182,7 +182,7 @@ sub format_2822_time {\n \t$author,$sender,$smtp_authpass,$annotate,$compose,$time);\n # Things we either get from config, *or* are overridden on the\n # command-line.\n-my ($no_cc, $no_to, $no_bcc);\n+my ($no_cc, $no_to, $no_bcc, $no_identity);\n my (@config_to, @getopt_to);\n my (@config_cc, @getopt_cc);\n my (@config_bcc, @getopt_bcc);\n@@ -325,44 +325,53 @@ sub signal_handler {\n \n # Read our sendemail.* config\n sub read_config {\n-\tmy ($prefix) = @_;\n+\tmy ($configured, $prefix) = @_;\n \n \tforeach my $setting (keys %config_bool_settings) {\n \t\tmy $target = $config_bool_settings{$setting};\n \t\tmy $v = Git::config_bool(@repo, \"$prefix.$setting\");\n-\t\t$$target = $v if defined $v;\n+\t\tnext unless defined $v;\n+\t\tnext if $configured->{$setting}++;\n+\t\t$$target = $v;\n \t}\n \n \tforeach my $setting (keys %config_path_settings) {\n \t\tmy $target = $config_path_settings{$setting};\n \t\tif (ref($target) eq \"ARRAY\") {\n-\t\t\tunless (@$target) {\n-\t\t\t\tmy @values = Git::config_path(@repo, \"$prefix.$setting\");\n-\t\t\t\t@$target = @values if (@values && defined $values[0]);\n-\t\t\t}\n+\t\t\tmy @values = Git::config_path(@repo, \"$prefix.$setting\");\n+\t\t\tnext unless @values;\n+\t\t\tnext if $configured->{$setting}++;\n+\t\t\t@$target = @values;\n \t\t}\n \t\telse {\n \t\t\tmy $v = Git::config_path(@repo, \"$prefix.$setting\");\n-\t\t\t$$target = $v if defined $v;\n+\t\t\tnext unless defined $v;\n+\t\t\tnext if $configured->{$setting}++;\n+\t\t\t$$target = $v;\n \t\t}\n \t}\n \n \tforeach my $setting (keys %config_settings) {\n \t\tmy $target = $config_settings{$setting};\n \t\tif (ref($target) eq \"ARRAY\") {\n-\t\t\tunless (@$target) {\n-\t\t\t\tmy @values = Git::config(@repo, \"$prefix.$setting\");\n-\t\t\t\t@$target = @values if (@values && defined $values[0]);\n-\t\t\t}\n+\t\t\tmy @values = Git::config(@repo, \"$prefix.$setting\");\n+\t\t\tnext unless @values;\n+\t\t\tnext if $configured->{$setting}++;\n+\t\t\t@$target = @values;\n \t\t}\n \t\telse {\n \t\t\tmy $v = Git::config(@repo, \"$prefix.$setting\");\n-\t\t\t$$target = $v if defined $v;\n+\t\t\tnext unless defined $v;\n+\t\t\tnext if $configured->{$setting}++;\n+\t\t\t$$target = $v;\n \t\t}\n \t}\n \n \tif (!defined $smtp_encryption) {\n-\t\tmy $enc = Git::config(@repo, \"$prefix.smtpencryption\");\n+\t\tmy $setting = \"$prefix.smtpencryption\";\n+\t\tmy $enc = Git::config(@repo, $setting);\n+\t\treturn unless defined $enc;\n+\t\treturn if $configured->{$setting}++;\n \t\tif (defined $enc) {\n \t\t\t$smtp_encryption = $enc;\n \t\t} elsif (Git::config_bool(@repo, \"$prefix.smtpssl\")) {\n@@ -371,17 +380,30 @@ sub read_config {\n \t}\n }\n \n+# sendemail.identity yields to --identity. We must parse this\n+# special-case first before the rest of the config is read.\n $identity = Git::config(@repo, \"sendemail.identity\");\n-read_config(\"sendemail.$identity\") if defined $identity;\n-read_config(\"sendemail\");\n+my $rc = GetOptions(\n+\t\"identity=s\" => \\$identity,\n+\t\"no-identity\" => \\$no_identity,\n+);\n+usage() unless $rc;\n+undef $identity if $no_identity;\n+\n+# Now we know enough to read the config\n+{\n+ my %configured;\n+ read_config(\\%configured, \"sendemail.$identity\") if defined $identity;\n+ read_config(\\%configured, \"sendemail\");\n+}\n \n # Begin by accumulating all the variables (defined above), that we will end up\n # needing, first, from the command line:\n \n my $help;\n my $git_completion_helper;\n-my $rc = GetOptions(\"h\" => \\$help,\n- \"dump-aliases\" => \\$dump_aliases);\n+$rc = GetOptions(\"h\" => \\$help,\n+ \"dump-aliases\" => \\$dump_aliases);\n usage() unless $rc;\n die __(\"--dump-aliases incompatible with other options\\n\")\n if !$help and $dump_aliases and @ARGV;\n@@ -410,8 +432,6 @@ sub read_config {\n \t\t \"smtp-debug:i\" => \\$debug_net_smtp,\n \t\t \"smtp-domain:s\" => \\$smtp_domain,\n \t\t \"smtp-auth=s\" => \\$smtp_auth,\n-\t\t \"no-smtp-auth\" => sub {$smtp_auth = 'none'},\n-\t\t \"identity=s\" => \\$identity,\n \t\t \"annotate!\" => \\$annotate,\n \t\t \"no-annotate\" => sub {$annotate = 0},\n \t\t \"compose\" => \\$compose,\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 61d484d1a6..890e2874c3 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1228,6 +1228,61 @@ test_expect_success $PREREQ 'sendemail.to works' '\n \tgrep \"To: Somebody \" stdout\n '\n \n+test_expect_success $PREREQ 'setup sendemail.identity' '\n+\tgit config --replace-all sendemail.to \"default@example.com\" &&\n+\tgit config --replace-all sendemail.isp.to \"isp@example.com\" &&\n+\tgit config --replace-all sendemail.cloud.to \"cloud@example.com\"\n+'\n+\n+test_expect_success $PREREQ 'sendemail.identity: reads the correct identity config' '\n+\tgit -c sendemail.identity=cloud send-email \\\n+\t\t--dry-run \\\n+\t\t--from=\"nobody@example.com\" \\\n+\t\t$patches >stdout &&\n+\tgrep \"To: cloud@example.com\" stdout\n+'\n+\n+test_expect_success $PREREQ 'sendemail.identity: identity overrides sendemail.identity' '\n+\tgit -c sendemail.identity=cloud send-email \\\n+\t\t--identity=isp \\\n+\t\t--dry-run \\\n+\t\t--from=\"nobody@example.com\" \\\n+\t\t$patches >stdout &&\n+\tgrep \"To: isp@example.com\" stdout\n+'\n+\n+test_expect_success $PREREQ 'sendemail.identity: --no-identity clears previous identity' '\n+\tgit -c sendemail.identity=cloud send-email \\\n+\t\t--no-identity \\\n+\t\t--dry-run \\\n+\t\t--from=\"nobody@example.com\" \\\n+\t\t$patches >stdout &&\n+\tgrep \"To: default@example.com\" stdout\n+'\n+\n+test_expect_success $PREREQ 'sendemail.identity: bool identity variable existance overrides' '\n+\tgit -c sendemail.identity=cloud \\\n+\t\t-c sendemail.xmailer=true \\\n+\t\t-c sendemail.cloud.xmailer=false \\\n+\t\tsend-email \\\n+\t\t--dry-run \\\n+\t\t--from=\"nobody@example.com\" \\\n+\t\t$patches >stdout &&\n+\tgrep \"To: cloud@example.com\" stdout &&\n+\t! grep \"X-Mailer\" stdout\n+'\n+\n+test_expect_success $PREREQ 'sendemail.identity: bool variable fallback' '\n+\tgit -c sendemail.identity=cloud \\\n+\t\t-c sendemail.xmailer=false \\\n+\t\tsend-email \\\n+\t\t--dry-run \\\n+\t\t--from=\"nobody@example.com\" \\\n+\t\t$patches >stdout &&\n+\tgrep \"To: cloud@example.com\" stdout &&\n+\t! grep \"X-Mailer\" stdout\n+'\n+\n test_expect_success $PREREQ '--no-to overrides sendemail.to' '\n \tgit send-email \\\n \t\t--dry-run \\\n@@ -1785,6 +1840,15 @@ test_expect_success '--dump-aliases must be used alone' '\n \ttest_must_fail git send-email --dump-aliases --to=janice@example.com -1 refs/heads/accounting\n '\n \n+test_expect_success 'aliases and sendemail.identity' '\n+\ttest_must_fail git \\\n+\t\t-c sendemail.identity=cloud \\\n+\t\t-c sendemail.aliasesfile=default-aliases \\\n+\t\t-c sendemail.cloud.aliasesfile=cloud-aliases \\\n+\t\tsend-email -1 2>stderr &&\n+\ttest_i18ngrep \"cloud-aliases\" stderr\n+'\n+\n test_sendmail_aliases () {\n \tmsg=\"$1\" && shift &&\n \texpect=\"$@\" &&\n-- \n2.21.0.1020.gf2820cf01a\n\n"},{"id":"375826","messageId":"20190517195545.29729-6-avarab@gmail.com","threadId":"50905","inReplyTo":"xmqqsgtd3fw3.fsf@gitster-ct.c.googlers.com","subject":"[PATCH 5/5] send-email: remove support for deprecated sendemail.smtpssl","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-17T19:55:45Z","receivedAt":"2019-05-17T19:56:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the sendemail.smtpssl configuration variable and its associated\n--smtp-ssl command-line option. This has been documented as deprecated\nsince f6bebd121a (\"git-send-email: add support for TLS via\nNet::SMTP::SSL\", 2008-06-25) and 65180c6618 (\"List send-email config\noptions in config.txt.\", 2009-07-22), respectively. Waiting for 10\nyears should be enough.\n\nThis allows us to fix a special case in read_config(). We couldn't\njust parse sendemail.smtpEncryption like everything else because we'd\nneed to fall back on sendemail.smtpssl. Now that we don't need to do\nthat we don't need this special case anymore. Let's still find out if\nsomeone's using this and die() with a helpful message if that's the\ncase.\n\nBecause of my recent improvements to the command-line and config\nparsing we can also revert fa835cd572 (\"git-send-email: prevent\nundefined variable warnings if no encryption is set\", 2008-06-26),\nsince we now sensibly support setting defaults for these mixed config\n& command-line options.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/config/sendemail.txt | 3 ---\n Documentation/git-send-email.txt | 3 ---\n git-send-email.perl | 26 +++++++++-----------------\n t/t9001-send-email.sh | 9 +++++++++\n 4 files changed, 18 insertions(+), 23 deletions(-)\n\ndiff --git a/Documentation/config/sendemail.txt b/Documentation/config/sendemail.txt\nindex 0006faf800..d7855bff1f 100644\n--- a/Documentation/config/sendemail.txt\n+++ b/Documentation/config/sendemail.txt\n@@ -8,9 +8,6 @@ sendemail.smtpEncryption::\n \tSee linkgit:git-send-email[1] for description. Note that this\n \tsetting is not subject to the 'identity' mechanism.\n \n-sendemail.smtpssl (deprecated)::\n-\tDeprecated alias for 'sendemail.smtpEncryption = ssl'.\n-\n sendemail.smtpsslcertpath::\n \tPath to ca-certificates (either a directory or a single file).\n \tSet it to an empty string to disable certificate verification.\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex a861934c69..6cf5c32ce8 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -236,9 +236,6 @@ The --smtp-server-option option must be repeated for each option you want\n to pass to the server. Likewise, different lines in the configuration files\n must be used for each option.\n \n---smtp-ssl::\n-\tLegacy alias for '--smtp-encryption ssl'.\n-\n --smtp-ssl-cert-path::\n \tPath to a store of trusted CA certificates for SMTP SSL/TLS\n \tcertificate validation (either a directory that has been processed\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 80cbbfd2b8..b1ed45b907 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -77,7 +77,6 @@ sub usage {\n --smtp-user * Username for SMTP-AUTH.\n --smtp-pass * Password for SMTP-AUTH; not necessary.\n --smtp-encryption * tls or ssl; anything else disables.\n- --smtp-ssl * Deprecated. Use '--smtp-encryption ssl'.\n --smtp-ssl-cert-path * Path to ca-certificates (either directory or file).\n Pass an empty string to disable certificate\n verification.\n@@ -236,7 +235,7 @@ sub do_edit {\n my ($cover_cc, $cover_to);\n my ($to_cmd, $cc_cmd);\n my ($smtp_server, $smtp_server_port, @smtp_server_options);\n-my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);\n+my ($smtp_authuser, $smtp_ssl_cert_path);\n my ($batch_size, $relogin_delay);\n my ($identity, $aliasfiletype, @alias_files, $smtp_domain, $smtp_auth);\n my ($confirm);\n@@ -250,6 +249,9 @@ sub do_edit {\n my $use_xmailer = 1;\n my $validate = 1;\n my $target_xfer_encoding = 'auto';\n+my $smtp_encryption = '';\n+# Deprecated variables\n+my $deprecated_smtp_ssl;\n \n my %config_bool_settings = (\n \"thread\" => \\$thread,\n@@ -266,10 +268,12 @@ sub do_edit {\n );\n \n my %config_settings = (\n+ \"smtpencryption\" => \\$smtp_encryption,\n \"smtpserver\" => \\$smtp_server,\n \"smtpserverport\" => \\$smtp_server_port,\n \"smtpserveroption\" => \\@smtp_server_options,\n \"smtpuser\" => \\$smtp_authuser,\n+ \"smtpssl\" => \\$deprecated_smtp_ssl,\n \"smtppass\" => \\$smtp_authpass,\n \"smtpdomain\" => \\$smtp_domain,\n \"smtpauth\" => \\$smtp_auth,\n@@ -366,18 +370,6 @@ sub read_config {\n \t\t\t$$target = $v;\n \t\t}\n \t}\n-\n-\tif (!defined $smtp_encryption) {\n-\t\tmy $setting = \"$prefix.smtpencryption\";\n-\t\tmy $enc = Git::config(@repo, $setting);\n-\t\treturn unless defined $enc;\n-\t\treturn if $configured->{$setting}++;\n-\t\tif (defined $enc) {\n-\t\t\t$smtp_encryption = $enc;\n-\t\t} elsif (Git::config_bool(@repo, \"$prefix.smtpssl\")) {\n-\t\t\t$smtp_encryption = 'ssl';\n-\t\t}\n-\t}\n }\n \n # sendemail.identity yields to --identity. We must parse this\n@@ -426,7 +418,7 @@ sub read_config {\n \t\t \"smtp-server-port=s\" => \\$smtp_server_port,\n \t\t \"smtp-user=s\" => \\$smtp_authuser,\n \t\t \"smtp-pass:s\" => \\$smtp_authpass,\n-\t\t \"smtp-ssl\" => sub { $smtp_encryption = 'ssl' },\n+\t\t \"smtp-ssl\" => sub { $deprecated_smtp_ssl = 1 },\n \t\t \"smtp-encryption=s\" => \\$smtp_encryption,\n \t\t \"smtp-ssl-cert-path=s\" => \\$smtp_ssl_cert_path,\n \t\t \"smtp-debug:i\" => \\$debug_net_smtp,\n@@ -484,8 +476,8 @@ sub read_config {\n \t\"(via command-line or configuration option)\\n\")\n \tif defined $relogin_delay and not defined $batch_size;\n \n-# 'default' encryption is none -- this only prevents a warning\n-$smtp_encryption = '' unless (defined $smtp_encryption);\n+die __(\"Use of deprecated option --smtp-ssl (or smtp.smtpssl config), use --smtp-encryption=ssl (or sendemail.smtpEncryption=ssl) instead\\n\")\n+\tif $deprecated_smtp_ssl;\n \n # Set CC suppressions\n my(%suppress_cc);\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 890e2874c3..b5ccfa8737 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1849,6 +1849,15 @@ test_expect_success 'aliases and sendemail.identity' '\n \ttest_i18ngrep \"cloud-aliases\" stderr\n '\n \n+test_expect_success 'deprecated --smtp-ssl (or sendemail.smtpssl=true)' '\n+\ttest_must_fail git -c sendemail.smtpssl=true send-email -1 2>stderr &&\n+\ttest_i18ngrep \"deprecated option.*or.*config\" stderr &&\n+\ttest_must_fail git send-email --smtp-ssl -1 &&\n+\ttest_i18ngrep \"deprecated option.*or.*config\" stderr &&\n+\ttest_must_fail git -c sendemail.identity=test -c sendemail.test.smtpssl=true send-email -1 &&\n+\ttest_i18ngrep \"deprecated option.*or.*config\" stderr\n+'\n+\n test_sendmail_aliases () {\n \tmsg=\"$1\" && shift &&\n \texpect=\"$@\" &&\n-- \n2.21.0.1020.gf2820cf01a\n\n"},{"id":"375866","messageId":"xmqq1s0v2pvv.fsf@gitster-ct.c.googlers.com","threadId":"50905","inReplyTo":"20190517195545.29729-5-avarab@gmail.com","subject":"Re: [PATCH 4/5] send-email: fix regression in sendemail.identity parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-05-19T01:29:24Z","receivedAt":"2019-05-19T01:29:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Fix a regression in my recent 3494dfd3ee (\"send-email: do defaults ->\n> config -> getopt in that order\", 2019-05-09). I missed that the\n> $identity variable needs to be extracted from the command-line before\n> we do the config reading, as it determines which config variable we\n> should read first. See [1] for the report.\n> ...\n> Refactor read_config() do what we actually mean here. We don't want to\n> set a given sendemail.VAR if a sendemail.$identity.VAR previously set\n> it. The old code was conflating this desire with the hardcoded\n> defaults for these variables, and as discussed in 3494dfd3ee that was\n> never going to work.\n\nI am not sure if the \"never going to work\" claim is a correct one.\nThe \"no hardcoded default in the variable, read command line, fill\nmissing ones from the two config files and finally apply the default\nfor the ones that are still missing\" was cumbersome, error-prone\nwithout a table, but did work.\n\nIt seems that no matter how we cut it, the cumbersomeness has to\nexist, as long as the command line --identity needs to be taken care\nof. Without that complication, I really liked the base series---the\n\"set var to hardcoded default, overwrite with config and then\noverwrite with command line, without having to check if we already\ngot value in an earlier step\" was so much simpler and easy to\nexplain X-<.\n\n> @@ -371,17 +380,30 @@ sub read_config {\n> ...\n> my $help;\n> my $git_completion_helper;\n> -my $rc = GetOptions(\"h\" => \\$help,\n> - \"dump-aliases\" => \\$dump_aliases);\n> ...\n> @@ -410,8 +432,6 @@ sub read_config {\n> ...\n> \t\t \"smtp-auth=s\" => \\$smtp_auth,\n> -\t\t \"no-smtp-auth\" => sub {$smtp_auth = 'none'},\n> -\t\t \"identity=s\" => \\$identity,\n\nYou seem to be building on top of ab/send-email-transferencoding-fix\nand something else, and these two hunks did not apply. I think I\nmanaged to wiggle the patch in correctly, though.\n\nThanks.\n"},{"id":"376080","messageId":"nycvar.QRO.7.76.6.1905222223380.46@tvgsbejvaqbjf.bet","threadId":"50905","inReplyTo":"20190517195545.29729-5-avarab@gmail.com","subject":"Re: [PATCH 4/5] send-email: fix regression in sendemail.identity parsing","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-05-22T20:25:42Z","receivedAt":"2019-05-22T20:27:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Fri, 17 May 2019, Ævar Arnfjörð Bjarmason wrote:\n\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index 61d484d1a6..890e2874c3 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -1228,6 +1228,61 @@ test_expect_success $PREREQ 'sendemail.to works' '\n> \tgrep \"To: Somebody \" stdout\n> '\n>\n> +test_expect_success $PREREQ 'setup sendemail.identity' '\n> +\tgit config --replace-all sendemail.to \"default@example.com\" &&\n> +\tgit config --replace-all sendemail.isp.to \"isp@example.com\" &&\n> +\tgit config --replace-all sendemail.cloud.to \"cloud@example.com\"\n> +'\n> +\n> +test_expect_success $PREREQ 'sendemail.identity: reads the correct identity config' '\n> +\tgit -c sendemail.identity=cloud send-email \\\n> +\t\t--dry-run \\\n> +\t\t--from=\"nobody@example.com\" \\\n> +\t\t$patches >stdout &&\n> +\tgrep \"To: cloud@example.com\" stdout\n> +'\n> +\n> +test_expect_success $PREREQ 'sendemail.identity: identity overrides sendemail.identity' '\n> +\tgit -c sendemail.identity=cloud send-email \\\n> +\t\t--identity=isp \\\n> +\t\t--dry-run \\\n> +\t\t--from=\"nobody@example.com\" \\\n> +\t\t$patches >stdout &&\n> +\tgrep \"To: isp@example.com\" stdout\n> +'\n> +\n> +test_expect_success $PREREQ 'sendemail.identity: --no-identity clears previous identity' '\n> +\tgit -c sendemail.identity=cloud send-email \\\n> +\t\t--no-identity \\\n> +\t\t--dry-run \\\n> +\t\t--from=\"nobody@example.com\" \\\n> +\t\t$patches >stdout &&\n> +\tgrep \"To: default@example.com\" stdout\n> +'\n> +\n> +test_expect_success $PREREQ 'sendemail.identity: bool identity variable existance overrides' '\n> +\tgit -c sendemail.identity=cloud \\\n> +\t\t-c sendemail.xmailer=true \\\n> +\t\t-c sendemail.cloud.xmailer=false \\\n> +\t\tsend-email \\\n> +\t\t--dry-run \\\n> +\t\t--from=\"nobody@example.com\" \\\n> +\t\t$patches >stdout &&\n> +\tgrep \"To: cloud@example.com\" stdout &&\n> +\t! grep \"X-Mailer\" stdout\n> +'\n> +\n> +test_expect_success $PREREQ 'sendemail.identity: bool variable fallback' '\n> +\tgit -c sendemail.identity=cloud \\\n> +\t\t-c sendemail.xmailer=false \\\n> +\t\tsend-email \\\n> +\t\t--dry-run \\\n> +\t\t--from=\"nobody@example.com\" \\\n> +\t\t$patches >stdout &&\n> +\tgrep \"To: cloud@example.com\" stdout &&\n> +\t! grep \"X-Mailer\" stdout\n> +'\n> +\n\nThese test cases all diligently use the `$PREREQ` prerequisite, but...\n\n> test_expect_success $PREREQ '--no-to overrides sendemail.to' '\n> \tgit send-email \\\n> \t\t--dry-run \\\n> @@ -1785,6 +1840,15 @@ test_expect_success '--dump-aliases must be used alone' '\n> \ttest_must_fail git send-email --dump-aliases --to=janice@example.com -1 refs/heads/accounting\n> '\n>\n> +test_expect_success 'aliases and sendemail.identity' '\n> +\ttest_must_fail git \\\n> +\t\t-c sendemail.identity=cloud \\\n> +\t\t-c sendemail.aliasesfile=default-aliases \\\n> +\t\t-c sendemail.cloud.aliasesfile=cloud-aliases \\\n> +\t\tsend-email -1 2>stderr &&\n> +\ttest_i18ngrep \"cloud-aliases\" stderr\n> +'\n> +\n\nThis one is missing it. That breaks the Windows job in our Azure Pipeline\nwhere we leave out all of the Perl bits (to accelerate the tests somewhat).\n\nCiao,\nDscho\n\n> test_sendmail_aliases () {\n> \tmsg=\"$1\" && shift &&\n> \texpect=\"$@\" &&\n> --\n> 2.21.0.1020.gf2820cf01a\n>\n>\n>\n"},{"id":"376387","messageId":"nycvar.QRO.7.76.6.1905291106540.44@tvgsbejvaqbjf.bet","threadId":"50905","inReplyTo":"nycvar.QRO.7.76.6.1905222223380.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 4/5] send-email: fix regression in sendemail.identity parsing","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-05-29T09:10:53Z","receivedAt":"2019-05-29T09:11:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio & Ævar,\n\nOn Wed, 22 May 2019, Johannes Schindelin wrote:\n\n> On Fri, 17 May 2019, Ævar Arnfjörð Bjarmason wrote:\n>\n> > [...]\n> > +test_expect_success $PREREQ 'sendemail.identity: bool variable fallback' '\n> > +\tgit -c sendemail.identity=cloud \\\n> > +\t\t-c sendemail.xmailer=false \\\n> > +\t\tsend-email \\\n> > +\t\t--dry-run \\\n> > +\t\t--from=\"nobody@example.com\" \\\n> > +\t\t$patches >stdout &&\n> > +\tgrep \"To: cloud@example.com\" stdout &&\n> > +\t! grep \"X-Mailer\" stdout\n> > +'\n> > +\n>\n> These test cases all diligently use the `$PREREQ` prerequisite, but...\n>\n> > test_expect_success $PREREQ '--no-to overrides sendemail.to' '\n> > \tgit send-email \\\n> > \t\t--dry-run \\\n> > @@ -1785,6 +1840,15 @@ test_expect_success '--dump-aliases must be used alone' '\n> > \ttest_must_fail git send-email --dump-aliases --to=janice@example.com -1 refs/heads/accounting\n> > '\n> >\n> > +test_expect_success 'aliases and sendemail.identity' '\n> > +\ttest_must_fail git \\\n> > +\t\t-c sendemail.identity=cloud \\\n> > +\t\t-c sendemail.aliasesfile=default-aliases \\\n> > +\t\t-c sendemail.cloud.aliasesfile=cloud-aliases \\\n> > +\t\tsend-email -1 2>stderr &&\n> > +\ttest_i18ngrep \"cloud-aliases\" stderr\n> > +'\n> > +\n>\n> This one is missing it. That breaks the Windows job in our Azure Pipeline\n> where we leave out all of the Perl bits (to accelerate the tests somewhat).\n\nFor the record, this is still breaking our Azure Pipeline build.\n\nJunio, would you terribly mind applying this on top (or fetching it from\nthe `shears/pu` branch at https://github.com/git-for-windows/git)?\n\n-- snipsnap --\nFrom 4c946f956d894676f68f5b130ab414d8ea75e97d Mon Sep 17 00:00:00 2001\nFrom: Johannes Schindelin \nDate: Wed, 29 May 2019 11:09:23 +0200\nSubject: [PATCH] SQUASH???\n\nSigned-off-by: Johannes Schindelin \n---\n t/t9001-send-email.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 7caab8160fcf..7c5ef114ac90 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1840,7 +1840,7 @@ test_expect_success '--dump-aliases must be used alone' '\n \ttest_must_fail git send-email --dump-aliases --to=janice@example.com -1 refs/heads/accounting\n '\n\n-test_expect_success 'aliases and sendemail.identity' '\n+test_expect_success $PREREQ 'aliases and sendemail.identity' '\n \ttest_must_fail git \\\n \t\t-c sendemail.identity=cloud \\\n \t\t-c sendemail.aliasesfile=default-aliases \\\n--\n2.22.0.rc1.windows.1.19.g571a93d65ff3\n"}]}