On Mon, 6 Sep 2010, Junio C Hamano wrote:
> Jakub Narebski <jnareb@xxxxxxxxx> writes:
>> Pascal Obry <pascal@xxxxxxxx> writes:
>>> ---
>>> 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.
>
> And test if it is easy to arrange (otherwise I'll take a look myself, so
> do not worry too much about it).
Actually because *all* options use the same mechanism (%config_settings
which is used in read_config, and GetOptions from Getopt::Long), it would
be better to just test the mechanism, I think.
But that is outside the scope of this patch... and probably require
moving (some of) functionality to Git.pm
>>> 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.
>
> That is a good and important point.
>
> We could
>
> [sendemail]
> smtpserveroption = opt1
> smtpserveroption = opt2
>
We do it this way, the same as for 'sendemail.to'... though I admit
that it is much more natural for 'sendemail.to' than for
'sendemail.smtpServerOption'.
> or if we choose to split at WS
>
> [sendemail]
> smtpserveroption = "opt1 opt2"
>
> but with the second form there always is this nagging "how would you
> specify an option with WS in it" issue, so the former might be easier.
Yes, there would be a problem with something like this:
[sendemail]
smtpserveroption = "opt1 'opt2_a opt2_b'"
We could use extract_delimited or extract_quotelike, or just a regexp
generated using gen_delimited_pat from Text::Balanced. Or better use
shellwords from Text::ParseWords
Text::ParseWords is in Perl core since Perl 5... which is I think good
enough for Git.
If we go this route we should probably provide shellwords / shellsplit /
/ split_sq also for C.
>
>
> If we take the latter route, we should take a single command line option
> and split it, i.e. --smtp-server-option='opt1 opt2', using the same WS
> quoting mechanism, for consistency between command line and configuration.
Right.
> I dunno. Have we solved a similar issue with other parts of the system,
> and how?
>
>>> @@ -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)
>
> I tend to prefer shift/unshift/push/pop written without these
> parentheses. Is it just me?
The Perl-ish way is to avoid parentheses for built-in functions with
prototypes, such as shift/unshift/join. But consistency trumps this
rule, I think (till the cleanup :-))
--
Jakub Narebski
Poland
--
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