Glenn Rempe <glenn@xxxxxxxx> writes:
> git-send-email.perl | 48 +++++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 4031e86..7c9c302 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -79,6 +79,10 @@ Options:
> --smtp-server If set, specifies the outgoing SMTP server to use.
> Defaults to localhost.
>
> + --smtp-server-port If set, specifies the port on the outgoing SMTP
> + server to use. Defaults to port 25 unless --smtp-ssl is set in
> + which case it will default to port 465.
> +
This paragraph look inconsistent with different indentation
for second and subsequent lines.
> @@ -375,6 +381,14 @@ if (!defined $smtp_server) {
> $smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*
> }
>
> +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
> @@ -604,20 +618,32 @@ X-Mailer: git-send-email $gitversion
> } else {
> if ($smtp_ssl) {
> require Net::SMTP::SSL;
> - $smtp ||= Net::SMTP::SSL->new( $smtp_server, Port => 465 );
> + $smtp ||= Net::SMTP::SSL->new( $smtp_server, Port => $smtp_server_port );
> }
> else {
> require Net::SMTP;
> - $smtp ||= Net::SMTP->new( $smtp_server );
> + $smtp ||= Net::SMTP->new( $smtp_server . ":" . $smtp_server_port );
> }
This change suggests that, although undocumented, existing users
could have already been using
--smtp-server=smtp.myisp.com:26
to specify a nonstandard port, and this patch, while bringing in
the support for a nonstandard port as an official feature, would
break such a setup. I wonder how real the issue is, and if we
can work it around easily. For example,
(1) drop the "default to 25 for smtp if undefined" part we saw
earlier;
(2) redo this part as
if ($smtp_ssl) {
... as you have it ...
} else {
require Net::SMTP;
$smtp ||= Net::SMTP->new((defined $smtp_server_port) ?
"$smtp_server:$smtp_server_port" :
$smtp_server);
}
> - $smtp->auth( $smtp_authuser, $smtp_authpass )
> - or die $smtp->message if (defined $smtp_authuser);
> ...
> +
> + # we'll get an ugly error if $smtp was undefined above.
> + # If so we'll catch it and present something friendlier.
> + if ($smtp) {
> +
> + if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
> + $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
> + }
> + ...
> + } else {
> + die "Unable to initialize SMTP properly. Is there something wrong with your config?";
> + }
I'd prefer the check done the opposite order, like
if (!$smtp) {
die ...
}
without an else clause.
-
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
