{"thread":{"id":"24988","subject":"[PATCH v3 0/3] Add support for SMTP server options","startedAt":"2010-09-05T17:48:57Z","lastAt":"2010-09-06T21:13:51Z","messageCount":10,"participants":["Pascal Obry","Ævar Arnfjörð Bjarmason","Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"149979","messageId":"1283708940-2172-1-git-send-email-pascal@obry.net","threadId":"24988","inReplyTo":null,"subject":"[PATCH v3 0/3] Add support for SMTP server options","fromName":"Pascal Obry","fromEmail":"pascal@obry.net","sentAt":"2010-09-05T17:48:57Z","receivedAt":"2010-09-05T17:48:57Z","isPatch":true,"sender":{"key":"pascal@obry.net","avatar":"https://avatars.githubusercontent.com/u/467069?v=4"},"body":"I'm not familiar at all with Perl so comments on style or usage most\nwelcomed. This patch is to introduce a way to pass specific options to the\nSMTP server used by git-send-email.\n\nI need that to be able to use different SMTP account (wanadoo, gmail...) on\nsome Git repositories to send over proper identity.\n\nThis is v3 of the patch thanks to Junio and AEvar for the review and help.\n\nThe two first patches are really code clean-up found while working on this\nnew feature. The last patch is the actual implementation of this new\nfeature.\n\nPascal Obry (3):\n  Minor indentation fix.\n  Remove @smtp_host_parts variable as not used.\n  New send-email option smtpserveroption.\n\n Documentation/git-send-email.txt |    8 ++++++++\n git-send-email.perl              |   12 +++++++++---\n 2 files changed, 17 insertions(+), 3 deletions(-)\n\n-- \n1.7.2.2.277.gb49c4\n"},{"id":"149981","messageId":"1283708940-2172-2-git-send-email-pascal@obry.net","threadId":"24988","inReplyTo":"1283708940-2172-1-git-send-email-pascal@obry.net","subject":"[PATCH v3 1/3] Minor indentation fix.","fromName":"Pascal Obry","fromEmail":"pascal@obry.net","sentAt":"2010-09-05T17:48:58Z","receivedAt":"2010-09-05T17:48:58Z","isPatch":true,"sender":{"key":"pascal@obry.net","avatar":"https://avatars.githubusercontent.com/u/467069?v=4"},"body":"---\n git-send-email.perl |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 6dab3bf..0063606 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -212,7 +212,7 @@ my %config_settings = (\n     \"smtpserverport\" => \\$smtp_server_port,\n     \"smtpuser\" => \\$smtp_authuser,\n     \"smtppass\" => \\$smtp_authpass,\n-\t\"smtpdomain\" => \\$smtp_domain,\n+    \"smtpdomain\" => \\$smtp_domain,\n     \"to\" => \\@to,\n     \"cc\" => \\@initial_cc,\n     \"cccmd\" => \\$cc_cmd,\n-- \n1.7.2.3.316.ga4c47\n"},{"id":"149980","messageId":"1283708940-2172-3-git-send-email-pascal@obry.net","threadId":"24988","inReplyTo":"1283708940-2172-1-git-send-email-pascal@obry.net","subject":"[PATCH v3 2/3] Remove @smtp_host_parts variable as not used.","fromName":"Pascal Obry","fromEmail":"pascal@obry.net","sentAt":"2010-09-05T17:48:59Z","receivedAt":"2010-09-05T17:48:59Z","isPatch":true,"sender":{"key":"pascal@obry.net","avatar":"https://avatars.githubusercontent.com/u/467069?v=4"},"body":"---\n git-send-email.perl |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 0063606..39cb5af 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -189,7 +189,7 @@ sub do_edit {\n # Variables with corresponding config settings\n my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);\n my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);\n-my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts, $smtp_domain);\n+my ($identity, $aliasfiletype, @alias_files, $smtp_domain);\n my ($validate, $confirm);\n my (@suppress_cc);\n my ($auto_8bit_encoding);\n-- \n1.7.2.3.316.ga4c47\n"},{"id":"149982","messageId":"1283708940-2172-4-git-send-email-pascal@obry.net","threadId":"24988","inReplyTo":"1283708940-2172-1-git-send-email-pascal@obry.net","subject":"[PATCH v3 3/3] New send-email option smtpserveroption.","fromName":"Pascal Obry","fromEmail":"pascal@obry.net","sentAt":"2010-09-05T17:49:00Z","receivedAt":"2010-09-05T17:49:00Z","isPatch":true,"sender":{"key":"pascal@obry.net","avatar":"https://avatars.githubusercontent.com/u/467069?v=4"},"body":"The new command line parameter --smtp-server-option or default\nconfiguration sendemail.smtpserveroption can be used to pass\nspecific options to the SMTP server. Update the documentation\naccordingly.\n---\n Documentation/git-send-email.txt |    8 ++++++++\n git-send-email.perl              |    8 +++++++-\n 2 files changed, 15 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex c283084..5af05bc 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -157,6 +157,14 @@ user is prompted for a password while the input is masked for privacy.\n \t`/usr/lib/sendmail` if such program is available, or\n \t`localhost` otherwise.\n \n+--smtp-server-option=<option>::\n+\tIf set, specifies the outgoing SMTP server option to use.\n+\tDefault value can be specified by the 'sendemail.smtpserveroption'\n+\tconfiguration option.\n++\n+The --smtp-server-option option must be repeated for each option you want\n+to pass to the server.\n+\n --smtp-server-port=<port>::\n \tSpecifies a port different from the default port (SMTP\n \tservers typically listen to smtp port 25, but may also listen to\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 39cb5af..47989fe 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -60,6 +60,7 @@ git send-email [options] <file | directory | rev-list options >\n     --envelope-sender       <str>  * Email envelope sender.\n     --smtp-server       <str:int>  * Outgoing SMTP server to use. The port\n                                      is optional. Default 'localhost'.\n+    --smtp-server-option    <str>  * Outgoing SMTP server option to use.\n     --smtp-server-port      <int>  * Outgoing SMTP server port.\n     --smtp-user             <str>  * Username for SMTP-AUTH.\n     --smtp-pass             <str>  * Password for SMTP-AUTH; not necessary.\n@@ -188,7 +189,8 @@ sub do_edit {\n \n # Variables with corresponding config settings\n my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);\n-my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);\n+my ($smtp_server, $smtp_server_port, @smtp_server_options);\n+my ($smtp_authuser, $smtp_encryption);\n my ($identity, $aliasfiletype, @alias_files, $smtp_domain);\n my ($validate, $confirm);\n my (@suppress_cc);\n@@ -210,6 +212,7 @@ my %config_bool_settings = (\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@@ -279,6 +282,7 @@ my $rc = GetOptions(\"sender|from=s\" => \\$sender,\n \t\t    \"no-bcc\" => \\$no_bcc,\n \t\t    \"chain-reply-to!\" => \\$chain_reply_to,\n \t\t    \"smtp-server=s\" => \\$smtp_server,\n+\t\t    \"smtp-server-option=s\" => \\@smtp_server_options,\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@@ -1015,6 +1019,8 @@ X-Mailer: git-send-email $gitversion\n \t\t}\n \t}\n \n+\tunshift (@sendmail_parameters, @smtp_server_options);\n+\n \tif ($dry_run) {\n \t\t# We don't want to send the email.\n \t} elsif ($smtp_server =~ m#^/#) {\n-- \n1.7.2.3.316.ga4c47\n"},{"id":"149997","messageId":"AANLkTikH1nkVftN7fADWoMbocYVZ+MexSoFYWHjUUK-M@mail.gmail.com","threadId":"24988","inReplyTo":"1283708940-2172-1-git-send-email-pascal@obry.net","subject":"Re: [PATCH v3 0/3] Add support for SMTP server options","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-05T20:34:05Z","receivedAt":"2010-09-05T20:34:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, Sep 5, 2010 at 17:48, Pascal Obry <pascal@obry.net> wrote:\n> I'm not familiar at all with Perl so comments on style or usage most\n> welcomed. This patch is to introduce a way to pass specific options to the\n> SMTP server used by git-send-email.\n>\n> I need that to be able to use different SMTP account (wanadoo, gmail...) on\n> some Git repositories to send over proper identity.\n>\n> This is v3 of the patch thanks to Junio and AEvar for the review and help.\n>\n> The two first patches are really code clean-up found while working on this\n> new feature. The last patch is the actual implementation of this new\n> feature.\n\nI looked these over and they look good, thanks.\n"},{"id":"150005","messageId":"m3lj7fn9oy.fsf@localhost.localdomain","threadId":"24988","inReplyTo":"1283708940-2172-4-git-send-email-pascal@obry.net","subject":"Re: [PATCH v3 3/3] New send-email option smtpserveroption.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-09-05T21:49:15Z","receivedAt":"2010-09-05T21:49:15Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Pascal Obry <pascal@obry.net> writes:\n\n> The new command line parameter --smtp-server-option or default\n> configuration sendemail.smtpserveroption can be used to pass\n> specific options to the SMTP server. Update the documentation\n> accordingly.\n\nSign-off?  (See Documentation/SubmittingPatches).\n\n> ---\n>  Documentation/git-send-email.txt |    8 ++++++++\n>  git-send-email.perl              |    8 +++++++-\n>  2 files changed, 15 insertions(+), 1 deletions(-)\n\nNeeds update to Documentation/config.txt, adding line about\nsendemail.smtpserveroption.\n\n> \n> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n> index c283084..5af05bc 100644\n> --- a/Documentation/git-send-email.txt\n> +++ b/Documentation/git-send-email.txt\n> @@ -157,6 +157,14 @@ user is prompted for a password while the input is masked for privacy.\n>  \t`/usr/lib/sendmail` if such program is available, or\n>  \t`localhost` otherwise.\n>  \n> +--smtp-server-option=<option>::\n> +\tIf set, specifies the outgoing SMTP server option to use.\n> +\tDefault value can be specified by the 'sendemail.smtpserveroption'\n> +\tconfiguration option.\n> ++\n> +The --smtp-server-option option must be repeated for each option you want\n> +to pass to the server.\n\nJust a nitpick.\n\nHow do multiple options are supported with sendemail.smtpserveroption?\nThis also needs to be described, I think.\n\n> +\n>  --smtp-server-port=<port>::\n>  \tSpecifies a port different from the default port (SMTP\n>  \tservers typically listen to smtp port 25, but may also listen to\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 39cb5af..47989fe 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -60,6 +60,7 @@ git send-email [options] <file | directory | rev-list options >\n>      --envelope-sender       <str>  * Email envelope sender.\n>      --smtp-server       <str:int>  * Outgoing SMTP server to use. The port\n>                                       is optional. Default 'localhost'.\n> +    --smtp-server-option    <str>  * Outgoing SMTP server option to use.\n>      --smtp-server-port      <int>  * Outgoing SMTP server port.\n>      --smtp-user             <str>  * Username for SMTP-AUTH.\n>      --smtp-pass             <str>  * Password for SMTP-AUTH; not necessary.\n> @@ -188,7 +189,8 @@ sub do_edit {\n>  \n>  # Variables with corresponding config settings\n>  my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);\n> -my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);\n> +my ($smtp_server, $smtp_server_port, @smtp_server_options);\n> +my ($smtp_authuser, $smtp_encryption);\n>  my ($identity, $aliasfiletype, @alias_files, $smtp_domain);\n>  my ($validate, $confirm);\n>  my (@suppress_cc);\n> @@ -210,6 +212,7 @@ my %config_bool_settings = (\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> @@ -279,6 +282,7 @@ my $rc = GetOptions(\"sender|from=s\" => \\$sender,\n>  \t\t    \"no-bcc\" => \\$no_bcc,\n>  \t\t    \"chain-reply-to!\" => \\$chain_reply_to,\n>  \t\t    \"smtp-server=s\" => \\$smtp_server,\n> +\t\t    \"smtp-server-option=s\" => \\@smtp_server_options,\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> @@ -1015,6 +1019,8 @@ X-Mailer: git-send-email $gitversion\n>  \t\t}\n>  \t}\n>  \n> +\tunshift (@sendmail_parameters, @smtp_server_options);\n> +\n\nI guess that you are following strange style that other 'unshift'\ninvocation uses, but there should be no space between function and\nopening parentheses beginning its arguments, e.g.\n\n  join(\"\\n\", @xh)\n\nnot\n\n  join (\"\\n\", @xh)\n\n>  \tif ($dry_run) {\n>  \t\t# We don't want to send the email.\n>  \t} elsif ($smtp_server =~ m#^/#) {\n> -- \n> 1.7.2.3.316.ga4c47\n> \n> \n> \n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"150049","messageId":"7vpqwrv0l2.fsf@alter.siamese.dyndns.org","threadId":"24988","inReplyTo":"m3lj7fn9oy.fsf@localhost.localdomain","subject":"Re: [PATCH v3 3/3] New send-email option smtpserveroption.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-06T06:38:01Z","receivedAt":"2010-09-06T06:38:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Pascal Obry <pascal@obry.net> writes:\n>\n>> The new command line parameter --smtp-server-option or default\n>> configuration sendemail.smtpserveroption can be used to pass\n>> specific options to the SMTP server. Update the documentation\n>> accordingly.\n>\n> Sign-off?  (See Documentation/SubmittingPatches).\n\nThanks.\n\n>> ---\n>>  Documentation/git-send-email.txt |    8 ++++++++\n>>  git-send-email.perl              |    8 +++++++-\n>>  2 files changed, 15 insertions(+), 1 deletions(-)\n>\n> Needs update to Documentation/config.txt, adding line about\n> sendemail.smtpserveroption.\n\nAnd test if it is easy to arrange (otherwise I'll take a look myself, so\ndo not worry too much about it).\n\n>> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n>> index c283084..5af05bc 100644\n>> --- a/Documentation/git-send-email.txt\n>> +++ b/Documentation/git-send-email.txt\n>> @@ -157,6 +157,14 @@ user is prompted for a password while the input is masked for privacy.\n>>  \t`/usr/lib/sendmail` if such program is available, or\n>>  \t`localhost` otherwise.\n>>  \n>> +--smtp-server-option=<option>::\n>> +\tIf set, specifies the outgoing SMTP server option to use.\n>> +\tDefault value can be specified by the 'sendemail.smtpserveroption'\n>> +\tconfiguration option.\n>> ++\n>> +The --smtp-server-option option must be repeated for each option you want\n>> +to pass to the server.\n>\n> Just a nitpick.\n>\n> How do multiple options are supported with sendemail.smtpserveroption?\n> This also needs to be described, I think.\n\nThat is a good and important point.\n\nWe could\n\n\t[sendemail]\n        \tsmtpserveroption = opt1\n        \tsmtpserveroption = opt2\n                \nor if we choose to split at WS\n\n\t[sendemail]\n        \tsmtpserveroption = \"opt1 opt2\"\n\nbut with the second form there always is this nagging \"how would you\nspecify an option with WS in it\" issue, so the former might be easier.  \n\nIf we take the latter route, we should take a single command line option\nand split it, i.e. --smtp-server-option='opt1 opt2', using the same WS\nquoting mechanism, for consistency between command line and configuration.\n\nI dunno.  Have we solved a similar issue with other parts of the system,\nand how?\n\n>> @@ -1015,6 +1019,8 @@ X-Mailer: git-send-email $gitversion\n>>  \t\t}\n>>  \t}\n>>  \n>> +\tunshift (@sendmail_parameters, @smtp_server_options);\n>> +\n>\n> I guess that you are following strange style that other 'unshift'\n> invocation uses, but there should be no space between function and\n> opening parentheses beginning its arguments, e.g.\n>\n>   join(\"\\n\", @xh)\n>\n> not\n>\n>   join (\"\\n\", @xh)\n\nI tend to prefer shift/unshift/push/pop written without these\nparentheses.  Is it just me?\n"},{"id":"150058","messageId":"AANLkTi=gm6wZ_0Q5-0r+ZSFLMtCzgi0Ah7WFLBWfJer6@mail.gmail.com","threadId":"24988","inReplyTo":"7vpqwrv0l2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 3/3] New send-email option smtpserveroption.","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-06T09:27:17Z","receivedAt":"2010-09-06T09:27:17Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Sep 6, 2010 at 06:38, Junio C Hamano <gitster@pobox.com> wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n>\n>> Pascal Obry <pascal@obry.net> writes:\n>>\n>>> The new command line parameter --smtp-server-option or default\n>>> configuration sendemail.smtpserveroption can be used to pass\n>>> specific options to the SMTP server. Update the documentation\n>>> accordingly.\n>>\n>> Sign-off?  (See Documentation/SubmittingPatches).\n>\n> Thanks.\n>\n>>> ---\n>>>  Documentation/git-send-email.txt |    8 ++++++++\n>>>  git-send-email.perl              |    8 +++++++-\n>>>  2 files changed, 15 insertions(+), 1 deletions(-)\n>>\n>> Needs update to Documentation/config.txt, adding line about\n>> sendemail.smtpserveroption.\n>\n> And test if it is easy to arrange (otherwise I'll take a look myself, so\n> do not worry too much about it).\n>\n>>> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n>>> index c283084..5af05bc 100644\n>>> --- a/Documentation/git-send-email.txt\n>>> +++ b/Documentation/git-send-email.txt\n>>> @@ -157,6 +157,14 @@ user is prompted for a password while the input is masked for privacy.\n>>>      `/usr/lib/sendmail` if such program is available, or\n>>>      `localhost` otherwise.\n>>>\n>>> +--smtp-server-option=<option>::\n>>> +    If set, specifies the outgoing SMTP server option to use.\n>>> +    Default value can be specified by the 'sendemail.smtpserveroption'\n>>> +    configuration option.\n>>> ++\n>>> +The --smtp-server-option option must be repeated for each option you want\n>>> +to pass to the server.\n>>\n>> Just a nitpick.\n>>\n>> How do multiple options are supported with sendemail.smtpserveroption?\n>> This also needs to be described, I think.\n>\n> That is a good and important point.\n>\n> We could\n>\n>        [sendemail]\n>                smtpserveroption = opt1\n>                smtpserveroption = opt2\n>\n> or if we choose to split at WS\n>\n>        [sendemail]\n>                smtpserveroption = \"opt1 opt2\"\n>\n> but with the second form there always is this nagging \"how would you\n> specify an option with WS in it\" issue, so the former might be easier.\n>\n> If we take the latter route, we should take a single command line option\n> and split it, i.e. --smtp-server-option='opt1 opt2', using the same WS\n> quoting mechanism, for consistency between command line and configuration.\n>\n> I dunno.  Have we solved a similar issue with other parts of the system,\n> and how?\n\nI can tell you that I *didn't* solve a similar problem with t/harness:\n\n         ? (test_args    => [ split /\\s+/, $ENV{GIT_TEST_OPTS} ])\n\nThat'll fail if the GIT_TEST_OPTS contains a space,\ne.g. GIT_TEST_OPTS='--root=\"/tmp/a space\"'. But I just ignored it at\nthe time.\n\nIt would be useful for both of these if we had a str_to_options()\nutility function in e.g. Git.pm. If nothing else it's probably more\nuser friendly to specify them in one smtpserveroption string, since\nyou can copy/paste an existing invocation then.\n\nMaybe we can convince a POSIX shell (which we require anyway) to split\nthese up for us? Shelling out from Perl to split these up would be a\ngood solution if it can be done.\n\n>>> @@ -1015,6 +1019,8 @@ X-Mailer: git-send-email $gitversion\n>>>              }\n>>>      }\n>>>\n>>> +    unshift (@sendmail_parameters, @smtp_server_options);\n>>> +\n>>\n>> I guess that you are following strange style that other 'unshift'\n>> invocation uses, but there should be no space between function and\n>> opening parentheses beginning its arguments, e.g.\n>>\n>>   join(\"\\n\", @xh)\n>>\n>> not\n>>\n>>   join (\"\\n\", @xh)\n>\n> I tend to prefer shift/unshift/push/pop written without these\n> parentheses.  Is it just me?\n\nIt's not just you. The customary style in Perl is to avoid parentheses.\n"},{"id":"150061","messageId":"201009061223.59090.jnareb@gmail.com","threadId":"24988","inReplyTo":"7vpqwrv0l2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 3/3] New send-email option smtpserveroption.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-09-06T10:23:57Z","receivedAt":"2010-09-06T10:23:57Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 6 Sep 2010, Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n>> Pascal Obry <pascal@obry.net> writes:\n\n>>> ---\n>>>  Documentation/git-send-email.txt |    8 ++++++++\n>>>  git-send-email.perl              |    8 +++++++-\n>>>  2 files changed, 15 insertions(+), 1 deletions(-)\n>>\n>> Needs update to Documentation/config.txt, adding line about\n>> sendemail.smtpserveroption.\n> \n> And test if it is easy to arrange (otherwise I'll take a look myself, so\n> do not worry too much about it).\n\nActually because *all* options use the same mechanism (%config_settings\nwhich is used in read_config, and GetOptions from Getopt::Long), it would\nbe better to just test the mechanism, I think.\n\nBut that is outside the scope of this patch... and probably require\nmoving (some of) functionality to Git.pm\n \n>>> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n>>> index c283084..5af05bc 100644\n>>> --- a/Documentation/git-send-email.txt\n>>> +++ b/Documentation/git-send-email.txt\n>>> @@ -157,6 +157,14 @@ user is prompted for a password while the input is masked for privacy.\n>>>  \t`/usr/lib/sendmail` if such program is available, or\n>>>  \t`localhost` otherwise.\n>>>  \n>>> +--smtp-server-option=<option>::\n>>> +\tIf set, specifies the outgoing SMTP server option to use.\n>>> +\tDefault value can be specified by the 'sendemail.smtpserveroption'\n>>> +\tconfiguration option.\n>>> ++\n>>> +The --smtp-server-option option must be repeated for each option you want\n>>> +to pass to the server.\n>>\n>> Just a nitpick.\n>>\n>> How do multiple options are supported with sendemail.smtpserveroption?\n>> This also needs to be described, I think.\n> \n> That is a good and important point.\n> \n> We could\n> \n> \t[sendemail]\n>         \tsmtpserveroption = opt1\n>         \tsmtpserveroption = opt2\n> \n\nWe do it this way, the same as for 'sendemail.to'... though I admit\nthat it is much more natural for 'sendemail.to' than for \n'sendemail.smtpServerOption'.\n                \n> or if we choose to split at WS\n> \n> \t[sendemail]\n>         \tsmtpserveroption = \"opt1 opt2\"\n> \n> but with the second form there always is this nagging \"how would you\n> specify an option with WS in it\" issue, so the former might be easier.\n\nYes, there would be a problem with something like this:\n\n  \t[sendemail]\n          \tsmtpserveroption = \"opt1 'opt2_a opt2_b'\"\n\nWe could use extract_delimited or extract_quotelike, or just a regexp\ngenerated using gen_delimited_pat from Text::Balanced.  Or better use\nshellwords from Text::ParseWords\n\nText::ParseWords is in Perl core since Perl 5... which is I think good\nenough for Git.\n\n\nIf we go this route we should probably provide shellwords / shellsplit /\n/ split_sq also for C.\n\n> \n> \n> If we take the latter route, we should take a single command line option\n> and split it, i.e. --smtp-server-option='opt1 opt2', using the same WS\n> quoting mechanism, for consistency between command line and configuration.\n\nRight.\n\n> I dunno.  Have we solved a similar issue with other parts of the system,\n> and how?\n> \n>>> @@ -1015,6 +1019,8 @@ X-Mailer: git-send-email $gitversion\n>>>  \t\t}\n>>>  \t}\n>>>  \n>>> +\tunshift (@sendmail_parameters, @smtp_server_options);\n>>> +\n>>\n>> I guess that you are following strange style that other 'unshift'\n>> invocation uses, but there should be no space between function and\n>> opening parentheses beginning its arguments, e.g.\n>>\n>>   join(\"\\n\", @xh)\n>>\n>> not\n>>\n>>   join (\"\\n\", @xh)\n> \n> I tend to prefer shift/unshift/push/pop written without these\n> parentheses.  Is it just me?\n\nThe Perl-ish way is to avoid parentheses for built-in functions with\nprototypes, such as shift/unshift/join.  But consistency trumps this\nrule, I think (till the cleanup :-))\n\n-- \nJakub Narebski\nPoland\n"},{"id":"150135","messageId":"7vvd6itw1c.fsf@alter.siamese.dyndns.org","threadId":"24988","inReplyTo":"201009061223.59090.jnareb@gmail.com","subject":"Re: [PATCH v3 3/3] New send-email option smtpserveroption.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-06T21:13:51Z","receivedAt":"2010-09-06T21:13:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> On Mon, 6 Sep 2010, Junio C Hamano wrote:\n> ...\n>> And test if it is easy to arrange (otherwise I'll take a look myself, so\n>> do not worry too much about it).\n>\n> Actually because *all* options use the same mechanism (%config_settings\n> which is used in read_config, and GetOptions from Getopt::Long), it would\n> be better to just test the mechanism, I think.\n>\n> But that is outside the scope of this patch...\n\nI didn't mean a test that makes sure --sso=foo is parsed into @sso array;\nthat would be a _mechanism_ test you alluded to, but I think we probably\ndo not need such a test as long as  we trust Getopt::Long().\n\nWhat I meant was to make sure that the result is correctly propagated to\nthe invocation of \"exec($smtp_server, @sendmail_params)\"; presumably the\ncheck will use a custom $smtp_server command that checks individual\ncommand line parameters.  And that _is_ exactly within the scope of this\nchange.\n"}]}