{"thread":{"id":"10019","subject":"[PATCH] Add ability to specify SMTP server port when using git-send-email.","startedAt":"2007-09-25T22:38:47Z","lastAt":"2007-09-26T10:31:36Z","messageCount":10,"participants":["Glenn Rempe","Johannes Schindelin","Junio C Hamano","Andreas Ericsson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"54052","messageId":"1190759927-19493-1-git-send-email-glenn@rempe.us","threadId":"10019","inReplyTo":null,"subject":"[PATCH] Add ability to specify SMTP server port when using git-send-email.","fromName":"Glenn Rempe","fromEmail":"glenn@rempe.us","sentAt":"2007-09-25T22:38:47Z","receivedAt":"2007-09-25T22:38:47Z","isPatch":true,"sender":{"key":"glenn@rempe.us","avatar":"https://gravatar.com/avatar/5febe06f1feb5a86bed5aee7b660b18f010bb0b00c48e91af84c2e179e344c50?d=mp&s=160"},"body":"Add ability to specify custom SMTP server port using\nsmtpserverport config value or --smtp-server-port command\nline option.\n\nAbility to specify custom SMTP server port using\n--smtp-server host:port syntax.\n\nWill default to port 25 if smtpssl config is set\nto false or --smtp-ssl command line is unset and port\nis not explicitly defined.\n\nWill default to port 465 if smtpssl config is set\nto true or --smtp-ssl is set and port is not explicity\ndefined.\n\nUsers should be aware that sending auth info over non-ssl\nconnections may be unsafe or just may not work at all\ndepending on SMTP server config.\n\nAdded some negative test cases.\n\nSigned-off-by: Glenn Rempe <glenn@rempe.us>\n---\n git-send-email.perl   |   73 ++++++++++++++++++++++++++++++++++++++++---------\n t/t9001-send-email.sh |   12 ++++++++\n 2 files changed, 72 insertions(+), 13 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 4031e86..969cb39 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -77,7 +77,10 @@ Options:\n                   the default section.\n \n    --smtp-server  If set, specifies the outgoing SMTP server to use.\n-                  Defaults to localhost.\n+                  Defaults to localhost.  Port number can be specified here with\n+                  hostname:port format or by using --smtp-server-port option.\n+\n+   --smtp-server-port Specify a port on the outgoing SMTP server to connect to.\n \n    --smtp-user    The username for SMTP-AUTH.\n \n@@ -172,8 +175,8 @@ my ($quiet, $dry_run) = (0, 0);\n \n # Variables with corresponding config settings\n my ($thread, $chain_reply_to, $suppress_from, $signed_off_cc, $cc_cmd);\n-my ($smtp_server, $smtp_authuser, $smtp_authpass, $smtp_ssl);\n-my ($identity, $aliasfiletype, @alias_files);\n+my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_authpass, $smtp_ssl);\n+my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);\n \n my %config_bool_settings = (\n     \"thread\" => [\\$thread, 1],\n@@ -185,6 +188,7 @@ my %config_bool_settings = (\n \n my %config_settings = (\n     \"smtpserver\" => \\$smtp_server,\n+    \"smtpserverport\" => \\$smtp_server_port,\n     \"smtpuser\" => \\$smtp_authuser,\n     \"smtppass\" => \\$smtp_authpass,\n     \"cccmd\" => \\$cc_cmd,\n@@ -204,6 +208,7 @@ my $rc = GetOptions(\"sender|from=s\" => \\$sender,\n \t\t    \"bcc=s\" => \\@bcclist,\n \t\t    \"chain-reply-to!\" => \\$chain_reply_to,\n \t\t    \"smtp-server=s\" => \\$smtp_server,\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!\" => \\$smtp_ssl,\n@@ -375,6 +380,29 @@ if (!defined $smtp_server) {\n \t$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*\n }\n \n+# don't allow BOTH forms of port definition to work since we can't guess which one is right.\n+if (($smtp_server =~ /:\\d+/) && (defined $smtp_server_port)) {\n+  die \"You must specify the port using either hostname:port OR --smtp-server-port but not both!\"\n+}\n+\n+# setup smtp_server var if it was passed in as host:port format\n+if ( $smtp_server =~ /:\\d+/) {\n+  # if they do pass a host:port form then split it and use the parts\n+  @smtp_host_parts = split(/:/, $smtp_server);\n+  $smtp_server = $smtp_host_parts[0];\n+  $smtp_server_port = $smtp_host_parts[1];\n+}\n+\n+# setup reasonable defaults if neither host:port or --smtp-server-port were passed\n+if ( !defined $smtp_server_port) {\n+  if ($smtp_ssl) {\n+    $smtp_server_port = 465  # SSL port\n+  } else {\n+    $smtp_server_port = 25  # Non-SSL port\n+  }\n+}\n+\n+\n if ($compose) {\n \t# Note that this does not need to be secure, but we will make a small\n \t# effort to have it be unique\n@@ -602,22 +630,41 @@ X-Mailer: git-send-email $gitversion\n \t\tprint $sm \"$header\\n$message\";\n \t\tclose $sm or die $?;\n \t} else {\n+\n+\t\tif (!defined $smtp_server) {\n+\t\t  die \"The required SMTP server is not properly defined.\"\n+\t\t}\n+\n+\t\tif (!defined $smtp_server_port || !$smtp_server_port =~ /^\\d+$/ ) {\n+\t\t  die \"The required SMTP server port is not properly defined.\"\n+\t\t}\n+\n \t\tif ($smtp_ssl) {\n \t\t\trequire Net::SMTP::SSL;\n-\t\t\t$smtp ||= Net::SMTP::SSL->new( $smtp_server, Port => 465 );\n+\t\t\t$smtp ||= Net::SMTP::SSL->new( $smtp_server, Port => $smtp_server_port );\n \t\t}\n \t\telse {\n \t\t\trequire Net::SMTP;\n-\t\t\t$smtp ||= Net::SMTP->new( $smtp_server );\n+\t\t\t$smtp ||= Net::SMTP->new($smtp_server . \":\" . $smtp_server_port);\n \t\t}\n-\t\t$smtp->auth( $smtp_authuser, $smtp_authpass )\n-\t\t\tor die $smtp->message if (defined $smtp_authuser);\n-\t\t$smtp->mail( $raw_from ) or die $smtp->message;\n-\t\t$smtp->to( @recipients ) or die $smtp->message;\n-\t\t$smtp->data or die $smtp->message;\n-\t\t$smtp->datasend(\"$header\\n$message\") or die $smtp->message;\n-\t\t$smtp->dataend() or die $smtp->message;\n-\t\t$smtp->ok or die \"Failed to send $subject\\n\".$smtp->message;\n+    \n+    # we'll get an ugly error if $smtp was undefined above.\n+    # If so we'll catch it and present something friendlier.\n+    if (!$smtp) {\n+      die \"Unable to initialize SMTP properly.  Is there something wrong with your config?\";\n+    }\n+\n+    if ((defined $smtp_authuser) && (defined $smtp_authpass)) {\n+      $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;\n+    }\n+\n+    $smtp->mail( $raw_from ) or die $smtp->message;\n+    $smtp->to( @recipients ) or die $smtp->message;\n+    $smtp->data or die $smtp->message;\n+    $smtp->datasend(\"$header\\n$message\") or die $smtp->message;\n+    $smtp->dataend() or die $smtp->message;\n+    $smtp->ok or die \"Failed to send $subject\\n\".$smtp->message;\n+\n \t}\n \tif ($quiet) {\n \t\tprintf (($dry_run ? \"Dry-\" : \"\").\"Sent %s\\n\", $subject);\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 83f9470..d32907d 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -41,4 +41,16 @@ test_expect_success \\\n     'Verify commandline' \\\n     'diff commandline expected'\n \n+test_expect_failure 'Passing in both host:port form AND --smtp-server-port' '\n+  git send-email --from=\"Example <nobody@example.com>\" --to=nobody@example.com --smtp-server smtp.foo.com:66 --smtp-server-port 77\" $patches 2>errors\n+'\n+\n+test_expect_failure 'Passing in non-numeric server port with host:port form' '\n+  git send-email --from=\"Example <nobody@example.com>\" --to=nobody@example.com --smtp-server smtp.foo.com:bar\" $patches 2>errors\n+'\n+\n+test_expect_failure 'Passing in non-numeric server port with --smtp-server-port form' '\n+  git send-email --from=\"Example <nobody@example.com>\" --to=nobody@example.com --smtp-server smtp.foo.com --smtp-server-port bar\" $patches 2>errors\n+'\n+\n test_done\n-- \n1.5.3.2.105.g23fe\n"},{"id":"54060","messageId":"Pine.LNX.4.64.0709260004090.28395@racer.site","threadId":"10019","inReplyTo":"1190759927-19493-1-git-send-email-glenn@rempe.us","subject":"Re: [PATCH] Add ability to specify SMTP server port when using git-send-email.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-25T23:05:02Z","receivedAt":"2007-09-25T23:05:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 25 Sep 2007, Glenn Rempe wrote:\n\n> +if (($smtp_server =~ /:\\d+/) && (defined $smtp_server_port)) {\n\nNot that I want to be a PITA, but this breaks down with IPv6, right?\n\nCiao,\nDscho\n"},{"id":"54061","messageId":"7vzlza2vcl.fsf@gitster.siamese.dyndns.org","threadId":"10019","inReplyTo":"Pine.LNX.4.64.0709260004090.28395@racer.site","subject":"Re: [PATCH] Add ability to specify SMTP server port when using git-send-email.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-25T23:12:10Z","receivedAt":"2007-09-25T23:12:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> On Tue, 25 Sep 2007, Glenn Rempe wrote:\n>\n>> +if (($smtp_server =~ /:\\d+/) && (defined $smtp_server_port)) {\n>\n> Not that I want to be a PITA, but this breaks down with IPv6, right?\n\nRight.  Do we care about symbolic \"server.addre.ss:smtp\"\nnotation as well, I wonder?\n"},{"id":"54064","messageId":"7vmyva2uqd.fsf@gitster.siamese.dyndns.org","threadId":"10019","inReplyTo":"7vzlza2vcl.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Add ability to specify SMTP server port when using git-send-email.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-25T23:25:30Z","receivedAt":"2007-09-25T23:25:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n>> On Tue, 25 Sep 2007, Glenn Rempe wrote:\n>>\n>>> +if (($smtp_server =~ /:\\d+/) && (defined $smtp_server_port)) {\n>>\n>> Not that I want to be a PITA, but this breaks down with IPv6, right?\n>\n> Right.  Do we care about symbolic \"server.addre.ss:smtp\"\n> notation as well, I wonder?\n\nWell, does it break?\n\nBTW, I do not think we care about \":smtp\"; it was a\ntongue-in-cheek comment.\n"},{"id":"54068","messageId":"F45A8184-2867-47A4-87D1-64EB48340E3A@mac.com","threadId":"10019","inReplyTo":"7vmyva2uqd.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Add ability to specify SMTP server port when using git-send-email.","fromName":"Glenn Rempe","fromEmail":"glenn.rempe@mac.com","sentAt":"2007-09-26T00:23:03Z","receivedAt":"2007-09-26T00:23:03Z","isPatch":true,"sender":{"key":"glenn.rempe@mac.com","avatar":null},"body":"\nOn Sep 25, 2007, at 4:25 PM, Junio C Hamano wrote:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>>\n>>> On Tue, 25 Sep 2007, Glenn Rempe wrote:\n>>>\n>>>> +if (($smtp_server =~ /:\\d+/) && (defined $smtp_server_port)) {\n>>>\n>>> Not that I want to be a PITA, but this breaks down with IPv6, right?\n>>\n>> Right.  Do we care about symbolic \"server.addre.ss:smtp\"\n>> notation as well, I wonder?\n>\n> Well, does it break?\n>\n> BTW, I do not think we care about \":smtp\"; it was a\n> tongue-in-cheek comment.\n\nUnfortunately, I know little about IPv6 and whether this breaks IPv6  \naddressing or not.  So I'll leave that question for others. Does the  \nunpatched code work with IPv6?  Does anyone currently use it that way?\n\nJunio, are you suggesting that I should remove the host:port form  \nsupport entirely and leave only the --smtp-server port option as  \nvalid?  I kind of liked that this new method allows both forms of  \nspecifying port as equal citizens.  :-)  If you are suggesting that  \nit be removed, I think we would have to reject the host:port form  \nsmtp-server addresses so we don't break when both --smtp- \nserver=host:port and -smtp-ssl are provided. (which brings us back to  \nthe ipv6 question).  No?\n"},{"id":"54069","messageId":"7vabra2rv3.fsf@gitster.siamese.dyndns.org","threadId":"10019","inReplyTo":"1190759927-19493-1-git-send-email-glenn@rempe.us","subject":"Re: [PATCH] Add ability to specify SMTP server port when using git-send-email.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-26T00:27:28Z","receivedAt":"2007-09-26T00:27:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I'm inclined to do this on top of yours.\n\n * As you mentioned, ssmtp plus undocumented \"server:port\"\n   syntax never worked.  There is no point supporting the\n   combination.  Also I do not think it is worth additional\n   lines of code to even say \"server:port\" plus --server-port\n   option are not meant to be used together.\n\n * People who used the undocumented \"server:port\" syntax did not\n   use the new --smtp-server-port option anyway.\n\n * If somebody goes over plain smtp specifies server without\n   port, we can let the Net::SMTP to handle the default port.\n   No need for _us_ to say that the default is 25.\n\n * I do not see much point insisting that port to be numeric; I\n   do not know what Net::SMTP accepts, but if it accepts\n   my.isp.com:smtp instead of my.isp.com:25, that is fine. This\n   has the side effect of keeping people's existing set-up\n   working.\n\n * The indentation was horrible.  Maybe your tabstop is set\n   incorrectly?\n\n * As I am inclined not to insist on numeric port numbers,\n   the additional tests become pointless.\n\nThe result is much simpler, and I think it is more readable.\n\n---\n\n git-send-email.perl   |   64 +++++++++++++-----------------------------------\n t/t9001-send-email.sh |   12 ---------\n 2 files changed, 18 insertions(+), 58 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 886f78f..62e1429 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -380,29 +380,6 @@ if (!defined $smtp_server) {\n \t$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*\n }\n \n-# don't allow BOTH forms of port definition to work since we can't guess which one is right.\n-if (($smtp_server =~ /:\\d+/) && (defined $smtp_server_port)) {\n-  die \"You must specify the port using either hostname:port OR --smtp-server-port but not both!\"\n-}\n-\n-# setup smtp_server var if it was passed in as host:port format\n-if ( $smtp_server =~ /:\\d+/) {\n-  # if they do pass a host:port form then split it and use the parts\n-  @smtp_host_parts = split(/:/, $smtp_server);\n-  $smtp_server = $smtp_host_parts[0];\n-  $smtp_server_port = $smtp_host_parts[1];\n-}\n-\n-# setup reasonable defaults if neither host:port or --smtp-server-port were passed\n-if ( !defined $smtp_server_port) {\n-  if ($smtp_ssl) {\n-    $smtp_server_port = 465  # SSL port\n-  } else {\n-    $smtp_server_port = 25  # Non-SSL port\n-  }\n-}\n-\n-\n if ($compose) {\n \t# Note that this does not need to be secure, but we will make a small\n \t# effort to have it be unique\n@@ -632,39 +609,34 @@ X-Mailer: git-send-email $gitversion\n \t} else {\n \n \t\tif (!defined $smtp_server) {\n-\t\t  die \"The required SMTP server is not properly defined.\"\n-\t\t}\n-\n-\t\tif (!defined $smtp_server_port || !$smtp_server_port =~ /^\\d+$/ ) {\n-\t\t  die \"The required SMTP server port is not properly defined.\"\n+\t\t\tdie \"The required SMTP server is not properly defined.\"\n \t\t}\n \n \t\tif ($smtp_ssl) {\n+\t\t\t$smtp_server_port ||= 465; # ssmtp\n \t\t\trequire Net::SMTP::SSL;\n-\t\t\t$smtp ||= Net::SMTP::SSL->new( $smtp_server, Port => $smtp_server_port );\n+\t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server, Port => $smtp_server_port);\n \t\t}\n \t\telse {\n \t\t\trequire Net::SMTP;\n-\t\t\t$smtp ||= Net::SMTP->new($smtp_server . \":\" . $smtp_server_port);\n+\t\t\t$smtp ||= Net::SMTP->new((defined $smtp_server_port)\n+\t\t\t\t\t\t ? \"$smtp_server:$smtp_server_port\"\n+\t\t\t\t\t\t : $smtp_server);\n \t\t}\n \n-    # we'll get an ugly error if $smtp was undefined above.\n-    # If so we'll catch it and present something friendlier.\n-    if (!$smtp) {\n-      die \"Unable to initialize SMTP properly.  Is there something wrong with your config?\";\n-    }\n-\n-    if ((defined $smtp_authuser) && (defined $smtp_authpass)) {\n-      $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;\n-    }\n-\n-    $smtp->mail( $raw_from ) or die $smtp->message;\n-    $smtp->to( @recipients ) or die $smtp->message;\n-    $smtp->data or die $smtp->message;\n-    $smtp->datasend(\"$header\\n$message\") or die $smtp->message;\n-    $smtp->dataend() or die $smtp->message;\n-    $smtp->ok or die \"Failed to send $subject\\n\".$smtp->message;\n+\t\tif (!$smtp) {\n+\t\t\tdie \"Unable to initialize SMTP properly.  Is there something wrong with your config?\";\n+\t\t}\n \n+\t\tif ((defined $smtp_authuser) && (defined $smtp_authpass)) {\n+\t\t\t$smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;\n+\t\t}\n+\t\t$smtp->mail( $raw_from ) or die $smtp->message;\n+\t\t$smtp->to( @recipients ) or die $smtp->message;\n+\t\t$smtp->data or die $smtp->message;\n+\t\t$smtp->datasend(\"$header\\n$message\") or die $smtp->message;\n+\t\t$smtp->dataend() or die $smtp->message;\n+\t\t$smtp->ok or die \"Failed to send $subject\\n\".$smtp->message;\n \t}\n \tif ($quiet) {\n \t\tprintf (($dry_run ? \"Dry-\" : \"\").\"Sent %s\\n\", $subject);\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex d32907d..83f9470 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -41,16 +41,4 @@ test_expect_success \\\n     'Verify commandline' \\\n     'diff commandline expected'\n \n-test_expect_failure 'Passing in both host:port form AND --smtp-server-port' '\n-  git send-email --from=\"Example <nobody@example.com>\" --to=nobody@example.com --smtp-server smtp.foo.com:66 --smtp-server-port 77\" $patches 2>errors\n-'\n-\n-test_expect_failure 'Passing in non-numeric server port with host:port form' '\n-  git send-email --from=\"Example <nobody@example.com>\" --to=nobody@example.com --smtp-server smtp.foo.com:bar\" $patches 2>errors\n-'\n-\n-test_expect_failure 'Passing in non-numeric server port with --smtp-server-port form' '\n-  git send-email --from=\"Example <nobody@example.com>\" --to=nobody@example.com --smtp-server smtp.foo.com --smtp-server-port bar\" $patches 2>errors\n-'\n-\n test_done\n"},{"id":"54073","messageId":"1A0CAB9D-5C99-4FD7-B3AC-9B3161FD8663@rempe.us","threadId":"10019","inReplyTo":"7vabra2rv3.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Add ability to specify SMTP server port when using git-send-email.","fromName":"Glenn Rempe","fromEmail":"glenn@rempe.us","sentAt":"2007-09-26T01:15:09Z","receivedAt":"2007-09-26T01:15:09Z","isPatch":true,"sender":{"key":"glenn@rempe.us","avatar":"https://gravatar.com/avatar/5febe06f1feb5a86bed5aee7b660b18f010bb0b00c48e91af84c2e179e344c50?d=mp&s=160"},"body":"ok, you're the boss! :-)\n\nComments:\n\n>  * As you mentioned, ssmtp plus undocumented \"server:port\"\n>    syntax never worked.  There is no point supporting the\n>    combination.  Also I do not think it is worth additional\n>    lines of code to even say \"server:port\" plus --server-port\n>    option are not meant to be used together.\n\nok.  I'm new to Git and the project's coding standards.  I just  \nthought that the additional check made the experience friendlier for  \nthe end user in the case of passing ambiguous arguments, even at the  \nexpense of a few lines of code.\n\n>  * People who used the undocumented \"server:port\" syntax did not\n>    use the new --smtp-server-port option anyway.\n\nLikely true.\n\n>\n>  * If somebody goes over plain smtp specifies server without\n>    port, we can let the Net::SMTP to handle the default port.\n>    No need for _us_ to say that the default is 25.\n\nok.\n\n>  * I do not see much point insisting that port to be numeric; I\n>    do not know what Net::SMTP accepts, but if it accepts\n>    my.isp.com:smtp instead of my.isp.com:25, that is fine. This\n>    has the side effect of keeping people's existing set-up\n>    working.\n\nok.  Again just trying to protect against invalid arguments.\n\n>  * The indentation was horrible.  Maybe your tabstop is set\n>    incorrectly?\n\nCan you be more detailed on the definition of 'horrible'? :-) I am  \nusing Textmate on OS X with soft tab stops (2 spaces).  What should  \nit be to look less horrible on your end?  Or is the issue that I  \nindent fewer tabstops than you expect? If so, sorry since perl is not  \nmy usual language and Ruby 2 space (soft tab) indentation looks right  \nto my eye.\n\n>  * As I am inclined not to insist on numeric port numbers,\n>    the additional tests become pointless.\n\nok.  Assuming you want to remove the check on port numbers.\n\n> The result is much simpler, and I think it is more readable.\n>\n\nBack to my first comment.  I agree its more readable with less code.   \nJust weighing the trade off with user experience.  The tool is a bit  \n'sharper' in the hands of the end user now IMHO.\n\nI hope this was helpful. :-)  Its been useful for me in getting to  \nknow Git and the community a bit better.  I'll assume you don't need  \nme to do anything else on this issue?  If thats not correct please  \nlet me know.\n\nGlenn\n\n\nOn Sep 25, 2007, at 5:27 PM, Junio C Hamano wrote:\n\n> I'm inclined to do this on top of yours.\n>\n>  * As you mentioned, ssmtp plus undocumented \"server:port\"\n>    syntax never worked.  There is no point supporting the\n>    combination.  Also I do not think it is worth additional\n>    lines of code to even say \"server:port\" plus --server-port\n>    option are not meant to be used together.\n>\n>  * People who used the undocumented \"server:port\" syntax did not\n>    use the new --smtp-server-port option anyway.\n>\n>  * If somebody goes over plain smtp specifies server without\n>    port, we can let the Net::SMTP to handle the default port.\n>    No need for _us_ to say that the default is 25.\n>\n>  * I do not see much point insisting that port to be numeric; I\n>    do not know what Net::SMTP accepts, but if it accepts\n>    my.isp.com:smtp instead of my.isp.com:25, that is fine. This\n>    has the side effect of keeping people's existing set-up\n>    working.\n>\n>  * The indentation was horrible.  Maybe your tabstop is set\n>    incorrectly?\n>\n>  * As I am inclined not to insist on numeric port numbers,\n>    the additional tests become pointless.\n>\n> The result is much simpler, and I think it is more readable.\n>\n> ---\n>\n>  git-send-email.perl   |   64 ++++++++++++ \n> +-----------------------------------\n>  t/t9001-send-email.sh |   12 ---------\n>  2 files changed, 18 insertions(+), 58 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 886f78f..62e1429 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -380,29 +380,6 @@ if (!defined $smtp_server) {\n>  \t$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*\n>  }\n>\n> -# don't allow BOTH forms of port definition to work since we can't  \n> guess which one is right.\n> -if (($smtp_server =~ /:\\d+/) && (defined $smtp_server_port)) {\n> -  die \"You must specify the port using either hostname:port OR -- \n> smtp-server-port but not both!\"\n> -}\n> -\n> -# setup smtp_server var if it was passed in as host:port format\n> -if ( $smtp_server =~ /:\\d+/) {\n> -  # if they do pass a host:port form then split it and use the parts\n> -  @smtp_host_parts = split(/:/, $smtp_server);\n> -  $smtp_server = $smtp_host_parts[0];\n> -  $smtp_server_port = $smtp_host_parts[1];\n> -}\n> -\n> -# setup reasonable defaults if neither host:port or --smtp-server- \n> port were passed\n> -if ( !defined $smtp_server_port) {\n> -  if ($smtp_ssl) {\n> -    $smtp_server_port = 465  # SSL port\n> -  } else {\n> -    $smtp_server_port = 25  # Non-SSL port\n> -  }\n> -}\n> -\n> -\n>  if ($compose) {\n>  \t# Note that this does not need to be secure, but we will make a  \n> small\n>  \t# effort to have it be unique\n> @@ -632,39 +609,34 @@ X-Mailer: git-send-email $gitversion\n>  \t} else {\n>\n>  \t\tif (!defined $smtp_server) {\n> -\t\t  die \"The required SMTP server is not properly defined.\"\n> -\t\t}\n> -\n> -\t\tif (!defined $smtp_server_port || !$smtp_server_port =~ /^\\d+$/ ) {\n> -\t\t  die \"The required SMTP server port is not properly defined.\"\n> +\t\t\tdie \"The required SMTP server is not properly defined.\"\n>  \t\t}\n>\n>  \t\tif ($smtp_ssl) {\n> +\t\t\t$smtp_server_port ||= 465; # ssmtp\n>  \t\t\trequire Net::SMTP::SSL;\n> -\t\t\t$smtp ||= Net::SMTP::SSL->new( $smtp_server, Port =>  \n> $smtp_server_port );\n> +\t\t\t$smtp ||= Net::SMTP::SSL->new($smtp_server, Port =>  \n> $smtp_server_port);\n>  \t\t}\n>  \t\telse {\n>  \t\t\trequire Net::SMTP;\n> -\t\t\t$smtp ||= Net::SMTP->new($smtp_server . \":\" . $smtp_server_port);\n> +\t\t\t$smtp ||= Net::SMTP->new((defined $smtp_server_port)\n> +\t\t\t\t\t\t ? \"$smtp_server:$smtp_server_port\"\n> +\t\t\t\t\t\t : $smtp_server);\n>  \t\t}\n>\n> -    # we'll get an ugly error if $smtp was undefined above.\n> -    # If so we'll catch it and present something friendlier.\n> -    if (!$smtp) {\n> -      die \"Unable to initialize SMTP properly.  Is there something  \n> wrong with your config?\";\n> -    }\n> -\n> -    if ((defined $smtp_authuser) && (defined $smtp_authpass)) {\n> -      $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp- \n> >message;\n> -    }\n> -\n> -    $smtp->mail( $raw_from ) or die $smtp->message;\n> -    $smtp->to( @recipients ) or die $smtp->message;\n> -    $smtp->data or die $smtp->message;\n> -    $smtp->datasend(\"$header\\n$message\") or die $smtp->message;\n> -    $smtp->dataend() or die $smtp->message;\n> -    $smtp->ok or die \"Failed to send $subject\\n\".$smtp->message;\n> +\t\tif (!$smtp) {\n> +\t\t\tdie \"Unable to initialize SMTP properly.  Is there something  \n> wrong with your config?\";\n> +\t\t}\n>\n> +\t\tif ((defined $smtp_authuser) && (defined $smtp_authpass)) {\n> +\t\t\t$smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp- \n> >message;\n> +\t\t}\n> +\t\t$smtp->mail( $raw_from ) or die $smtp->message;\n> +\t\t$smtp->to( @recipients ) or die $smtp->message;\n> +\t\t$smtp->data or die $smtp->message;\n> +\t\t$smtp->datasend(\"$header\\n$message\") or die $smtp->message;\n> +\t\t$smtp->dataend() or die $smtp->message;\n> +\t\t$smtp->ok or die \"Failed to send $subject\\n\".$smtp->message;\n>  \t}\n>  \tif ($quiet) {\n>  \t\tprintf (($dry_run ? \"Dry-\" : \"\").\"Sent %s\\n\", $subject);\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index d32907d..83f9470 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -41,16 +41,4 @@ test_expect_success \\\n>      'Verify commandline' \\\n>      'diff commandline expected'\n>\n> -test_expect_failure 'Passing in both host:port form AND --smtp- \n> server-port' '\n> -  git send-email --from=\"Example <nobody@example.com>\" -- \n> to=nobody@example.com --smtp-server smtp.foo.com:66 --smtp-server- \n> port 77\" $patches 2>errors\n> -'\n> -\n> -test_expect_failure 'Passing in non-numeric server port with  \n> host:port form' '\n> -  git send-email --from=\"Example <nobody@example.com>\" -- \n> to=nobody@example.com --smtp-server smtp.foo.com:bar\" $patches  \n> 2>errors\n> -'\n> -\n> -test_expect_failure 'Passing in non-numeric server port with -- \n> smtp-server-port form' '\n> -  git send-email --from=\"Example <nobody@example.com>\" -- \n> to=nobody@example.com --smtp-server smtp.foo.com --smtp-server-port  \n> bar\" $patches 2>errors\n> -'\n> -\n>  test_done\n\n--\nGlenn Rempe\nglenn.@rempe.us\n\n\n\n"},{"id":"54076","messageId":"Pine.LNX.4.64.0709260239210.28395@racer.site","threadId":"10019","inReplyTo":"1A0CAB9D-5C99-4FD7-B3AC-9B3161FD8663@rempe.us","subject":"Re: [PATCH] Add ability to specify SMTP server port when using git-send-email.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-26T01:42:12Z","receivedAt":"2007-09-26T01:42:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 25 Sep 2007, Glenn Rempe wrote:\n\n> > * The indentation was horrible.  Maybe your tabstop is set\n> >   incorrectly?\n> \n> Can you be more detailed on the definition of 'horrible'? :-) I am using \n> Textmate on OS X with soft tab stops (2 spaces).  What should it be to \n> look less horrible on your end?  Or is the issue that I indent fewer \n> tabstops than you expect? If so, sorry since perl is not my usual \n> language and Ruby 2 space (soft tab) indentation looks right to my eye.\n\nWe use soft tabs, with the standard up-to 8 spaces, so that short sighted \npeople like me can still see that the line is actually indented.\n\nIn related news, I just saw that we never mention the 80 characters per \nline convention in code, or the 76 characters per line for commit messages \n(because of the 4 space indent git-log does -- ouch ;-)\n\nCiao,\nDscho\n"},{"id":"54086","messageId":"46FA07B9.4000402@op5.se","threadId":"10019","inReplyTo":"Pine.LNX.4.64.0709260239210.28395@racer.site","subject":"Re: [PATCH] Add ability to specify SMTP server port when using git-send-email.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-09-26T07:18:17Z","receivedAt":"2007-09-26T07:18:17Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n> Hi,\n> \n> On Tue, 25 Sep 2007, Glenn Rempe wrote:\n> \n>>> * The indentation was horrible.  Maybe your tabstop is set\n>>>   incorrectly?\n>> Can you be more detailed on the definition of 'horrible'? :-) I am using \n>> Textmate on OS X with soft tab stops (2 spaces).  What should it be to \n>> look less horrible on your end?  Or is the issue that I indent fewer \n>> tabstops than you expect? If so, sorry since perl is not my usual \n>> language and Ruby 2 space (soft tab) indentation looks right to my eye.\n> \n> We use soft tabs, with the standard up-to 8 spaces,\n\nActually, the rest of git's perl-scripts use hard tabs, just as the C-code\nand the shell-code, with tabs for indentation and spaces for alignment.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"},{"id":"54091","messageId":"Pine.LNX.4.64.0709261131100.28395@racer.site","threadId":"10019","inReplyTo":"46FA07B9.4000402@op5.se","subject":"Re: [PATCH] Add ability to specify SMTP server port when using git-send-email.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-09-26T10:31:36Z","receivedAt":"2007-09-26T10:31:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 26 Sep 2007, Andreas Ericsson wrote:\n\n> Actually, the rest of git's perl-scripts use hard tabs, just as the \n> C-code and the shell-code, with tabs for indentation and spaces for \n> alignment.\n\nD'oh.  I meant to say hard tabs.\n\nThanks,\nDscho\n"}]}