From: Jakub Narebski <jnareb@gmail.com>
Date: Sun, 05 Sep 2010 21:49:15 GMT
Subject: Re: [PATCH v3 3/3] New send-email option smtpserveroption.
Message-ID: <m3lj7fn9oy.fsf@localhost.localdomain>
In-Reply-To: <1283708940-2172-4-git-send-email-pascal@obry.net>

Pascal Obry <pascal@obry.net> writes:

> The new command line parameter --smtp-server-option or default
> configuration sendemail.smtpserveroption can be used to pass
> specific options to the SMTP server. Update the documentation
> accordingly.

Sign-off?  (See Documentation/SubmittingPatches).

> ---
>  Documentation/git-send-email.txt |    8 ++++++++
>  git-send-email.perl              |    8 +++++++-
>  2 files changed, 15 insertions(+), 1 deletions(-)

Needs update to Documentation/config.txt, adding line about
sendemail.smtpserveroption.

> 
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index c283084..5af05bc 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -157,6 +157,14 @@ user is prompted for a password while the input is masked for privacy.
>  	`/usr/lib/sendmail` if such program is available, or
>  	`localhost` otherwise.
>  
> +--smtp-server-option=<option>::
> +	If set, specifies the outgoing SMTP server option to use.
> +	Default value can be specified by the 'sendemail.smtpserveroption'
> +	configuration option.
> ++
> +The --smtp-server-option option must be repeated for each option you want
> +to pass to the server.

Just a nitpick.

How do multiple options are supported with sendemail.smtpserveroption?
This also needs to be described, I think.

> +
>  --smtp-server-port=<port>::
>  	Specifies a port different from the default port (SMTP
>  	servers typically listen to smtp port 25, but may also listen to
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 39cb5af..47989fe 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -60,6 +60,7 @@ git send-email [options] <file | directory | rev-list options >
>      --envelope-sender       <str>  * Email envelope sender.
>      --smtp-server       <str:int>  * Outgoing SMTP server to use. The port
>                                       is optional. Default 'localhost'.
> +    --smtp-server-option    <str>  * Outgoing SMTP server option to use.
>      --smtp-server-port      <int>  * Outgoing SMTP server port.
>      --smtp-user             <str>  * Username for SMTP-AUTH.
>      --smtp-pass             <str>  * Password for SMTP-AUTH; not necessary.
> @@ -188,7 +189,8 @@ sub do_edit {
>  
>  # Variables with corresponding config settings
>  my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd);
> -my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption);
> +my ($smtp_server, $smtp_server_port, @smtp_server_options);
> +my ($smtp_authuser, $smtp_encryption);
>  my ($identity, $aliasfiletype, @alias_files, $smtp_domain);
>  my ($validate, $confirm);
>  my (@suppress_cc);
> @@ -210,6 +212,7 @@ my %config_bool_settings = (
>  my %config_settings = (
>      "smtpserver" => \$smtp_server,
>      "smtpserverport" => \$smtp_server_port,
> +    "smtpserveroption" => \@smtp_server_options,
>      "smtpuser" => \$smtp_authuser,
>      "smtppass" => \$smtp_authpass,
>      "smtpdomain" => \$smtp_domain,
> @@ -279,6 +282,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
>  		    "no-bcc" => \$no_bcc,
>  		    "chain-reply-to!" => \$chain_reply_to,
>  		    "smtp-server=s" => \$smtp_server,
> +		    "smtp-server-option=s" => \@smtp_server_options,
>  		    "smtp-server-port=s" => \$smtp_server_port,
>  		    "smtp-user=s" => \$smtp_authuser,
>  		    "smtp-pass:s" => \$smtp_authpass,
> @@ -1015,6 +1019,8 @@ X-Mailer: git-send-email $gitversion
>  		}
>  	}
>  
> +	unshift (@sendmail_parameters, @smtp_server_options);
> +

I guess that you are following strange style that other 'unshift'
invocation uses, but there should be no space between function and
opening parentheses beginning its arguments, e.g.

  join("\n", @xh)

not

  join ("\n", @xh)

>  	if ($dry_run) {
>  		# We don't want to send the email.
>  	} elsif ($smtp_server =~ m#^/#) {
> -- 
> 1.7.2.3.316.ga4c47
> 
> 
> 

-- 
Jakub Narebski
Poland
ShadeHawk on #git