I'm inclined to do this on top of yours.
* As you mentioned, ssmtp plus undocumented "server:port"
syntax never worked. There is no point supporting the
combination. Also I do not think it is worth additional
lines of code to even say "server:port" plus --server-port
option are not meant to be used together.
* People who used the undocumented "server:port" syntax did not
use the new --smtp-server-port option anyway.
* If somebody goes over plain smtp specifies server without
port, we can let the Net::SMTP to handle the default port.
No need for _us_ to say that the default is 25.
* I do not see much point insisting that port to be numeric; I
do not know what Net::SMTP accepts, but if it accepts
my.isp.com:smtp instead of my.isp.com:25, that is fine. This
has the side effect of keeping people's existing set-up
working.
* The indentation was horrible. Maybe your tabstop is set
incorrectly?
* As I am inclined not to insist on numeric port numbers,
the additional tests become pointless.
The result is much simpler, and I think it is more readable.
---
git-send-email.perl | 64 +++++++++++++-----------------------------------
t/t9001-send-email.sh | 12 ---------
2 files changed, 18 insertions(+), 58 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 886f78f..62e1429 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -380,29 +380,6 @@ if (!defined $smtp_server) {
$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
}
-# don't allow BOTH forms of port definition to work since we can't guess which one is right.
-if (($smtp_server =~ /:\d+/) && (defined $smtp_server_port)) {
- die "You must specify the port using either hostname:port OR --smtp-server-port but not both!"
-}
-
-# setup smtp_server var if it was passed in as host:port format
-if ( $smtp_server =~ /:\d+/) {
- # if they do pass a host:port form then split it and use the parts
- @smtp_host_parts = split(/:/, $smtp_server);
- $smtp_server = $smtp_host_parts[0];
- $smtp_server_port = $smtp_host_parts[1];
-}
-
-# setup reasonable defaults if neither host:port or --smtp-server-port were passed
-if ( !defined $smtp_server_port) {
- if ($smtp_ssl) {
- $smtp_server_port = 465 # SSL port
- } else {
- $smtp_server_port = 25 # Non-SSL port
- }
-}
-
-
if ($compose) {
# Note that this does not need to be secure, but we will make a small
# effort to have it be unique
@@ -632,39 +609,34 @@ X-Mailer: git-send-email $gitversion
} else {
if (!defined $smtp_server) {
- die "The required SMTP server is not properly defined."
- }
-
- if (!defined $smtp_server_port || !$smtp_server_port =~ /^\d+$/ ) {
- die "The required SMTP server port is not properly defined."
+ die "The required SMTP server is not properly defined."
}
if ($smtp_ssl) {
+ $smtp_server_port ||= 465; # ssmtp
require Net::SMTP::SSL;
- $smtp ||= Net::SMTP::SSL->new( $smtp_server, Port => $smtp_server_port );
+ $smtp ||= Net::SMTP::SSL->new($smtp_server, Port => $smtp_server_port);
}
else {
require Net::SMTP;
- $smtp ||= Net::SMTP->new($smtp_server . ":" . $smtp_server_port);
+ $smtp ||= Net::SMTP->new((defined $smtp_server_port)
+ ? "$smtp_server:$smtp_server_port"
+ : $smtp_server);
}
- # we'll get an ugly error if $smtp was undefined above.
- # If so we'll catch it and present something friendlier.
- if (!$smtp) {
- die "Unable to initialize SMTP properly. Is there something wrong with your config?";
- }
-
- if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
- $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
- }
-
- $smtp->mail( $raw_from ) or die $smtp->message;
- $smtp->to( @recipients ) or die $smtp->message;
- $smtp->data or die $smtp->message;
- $smtp->datasend("$header\n$message") or die $smtp->message;
- $smtp->dataend() or die $smtp->message;
- $smtp->ok or die "Failed to send $subject\n".$smtp->message;
+ if (!$smtp) {
+ die "Unable to initialize SMTP properly. Is there something wrong with your config?";
+ }
+ if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
+ $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
+ }
+ $smtp->mail( $raw_from ) or die $smtp->message;
+ $smtp->to( @recipients ) or die $smtp->message;
+ $smtp->data or die $smtp->message;
+ $smtp->datasend("$header\n$message") or die $smtp->message;
+ $smtp->dataend() or die $smtp->message;
+ $smtp->ok or die "Failed to send $subject\n".$smtp->message;
}
if ($quiet) {
printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index d32907d..83f9470 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -41,16 +41,4 @@ test_expect_success \
'Verify commandline' \
'diff commandline expected'
-test_expect_failure 'Passing in both host:port form AND --smtp-server-port' '
- git send-email --from="Example <nobody@xxxxxxxxxxx>" --to=nobody@xxxxxxxxxxx --smtp-server smtp.foo.com:66 --smtp-server-port 77" $patches 2>errors
-'
-
-test_expect_failure 'Passing in non-numeric server port with host:port form' '
- git send-email --from="Example <nobody@xxxxxxxxxxx>" --to=nobody@xxxxxxxxxxx --smtp-server smtp.foo.com:bar" $patches 2>errors
-'
-
-test_expect_failure 'Passing in non-numeric server port with --smtp-server-port form' '
- git send-email --from="Example <nobody@xxxxxxxxxxx>" --to=nobody@xxxxxxxxxxx --smtp-server smtp.foo.com --smtp-server-port bar" $patches 2>errors
-'
-
test_done
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
