[darcs-devel] darcs patch: add sendmail-command option (#115)

David Roundy droundy at abridgegame.org
Wed Mar 30 04:31:21 PST 2005


On Tue, Mar 29, 2005 at 12:15:09PM -0500, Benedikt Schmidt wrote:
> Sun Mar 27 05:07:47 CEST 2005  Benedikt Schmidt <beschmi at cloaked.de>
>   * add sendmail-command option (#115)
>   - sendmail-command specifies the commandline used to send mail, the
>     commandline can contain some format strings to specify the recipient
>   - the sendmail-command option is used on windows and is used instead of
>     mapi if specified
> 
> There is no documentation yet and i'm not sure if i like the name
> "sendmail-command" but you can already use it to send a patch bundle
> with eg evolution or msmtp:
> darcs send --sendmail-command='evolution "mailto:%T?subject=%S&cc=%C&body=%B&attachment=%A'
> darcs send --sendmail-command='msmtp %s %<'

Looks mostly good.  Thanks!

Adding docs would definitely be a good idea... I'm ambivalent about the
option name.

> hunk ./CommandLine.lhs 1
> +A parser for commandlines, returns an arg list and expands

Could you add a license and copyright note at the top of the file? I'm not
sure about the legality of distributing code without a license.  Perhaps
the fact that you sent it here to a GPLed project is enough that we can
assume it's licensed under the GPL, but I think it's better if the author
supplies a copyright statement.

> +get_sendmail_cmd :: [DarcsFlag] -> IO String
> +get_sendmail_cmd (SendmailCmd a:_) = return a
> +get_sendmail_cmd (_:flags) = get_sendmail_cmd flags
> +get_sendmail_cmd [] = do scmds <- get_preflist "sendmail-command"
> +                         scmdgl <- get_global "sendmail-command"
> +                         case scmds++scmdgl of [] -> return ""
> +					       (c:_) -> return c

I'd rather not have the get_preflist and get_global here, and force the
user to use the defaults mechanism if they want to set a
repository-specific or global default sendmail command.  The "prefs"
mechanism is really intended for things like "test" or "boringfile" which
should be inherited by children of a given repository.

It would also be nice to have a bit of documentation on your CommandLine
API, since it's not immediately obvious (to me) from the type signatures
what the commands do.
-- 
David Roundy
http://www.darcs.net




More information about the darcs-devel mailing list