On Mon, Sep 6, 2010 at 06:38, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Jakub Narebski <jnareb@xxxxxxxxx> writes:
>
>> Pascal Obry <pascal@xxxxxxxx> 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).
>
> Thanks.
>
>>> ---
>>> 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).
>
>>> 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
>
> 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.
>
> 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.
>
> I dunno. Have we solved a similar issue with other parts of the system,
> and how?
I can tell you that I *didn't* solve a similar problem with t/harness:
? (test_args => [ split /\s+/, $ENV{GIT_TEST_OPTS} ])
That'll fail if the GIT_TEST_OPTS contains a space,
e.g. GIT_TEST_OPTS='--root="/tmp/a space"'. But I just ignored it at
the time.
It would be useful for both of these if we had a str_to_options()
utility function in e.g. Git.pm. If nothing else it's probably more
user friendly to specify them in one smtpserveroption string, since
you can copy/paste an existing invocation then.
Maybe we can convince a POSIX shell (which we require anyway) to split
these up for us? Shelling out from Perl to split these up would be a
good solution if it can be done.
>>> @@ -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?
It's not just you. The customary style in Perl is to avoid parentheses.
--
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