[Catalyst-dev] Re: jshirley/jrockway
Evan Carroll
lists at evancarroll.com
Sat Jul 28 22:41:45 GMT 2007
> Message: 2
> Date: Fri, 27 Jul 2007 18:27:30 -0500
> From: "Jonathan T. Rockway" <jon at jrock.us>
> Subject: Re: [Catalyst-dev] Catalyst::View::Email patch courtesy of
> the one and only.
> To: Development of the elegant MVC web framework
> <catalyst-dev at lists.rawmode.org>
> Message-ID: <20070727232730.GA32243 at jrock.us>
> Content-Type: text/plain; charset=us-ascii
>
> On Fri, Jul 27, 2007 at 05:06:38PM -0500, Evan Carroll wrote:
> > --- a/Email.pm
> > +++ b/Email.pm
> > @@ -35,9 +35,12 @@ In your app configuration (example in L<YAML>):
> > method: SMTP
> > # mailer_args is passed directly into Email::Send
> > mailer_args:
> > - - Host: smtp.example.com
> > - - username: username
> > - - password: password
> > + - Host
> > + - smtp.example.com
> > + - username
> > + - mySMPTUsername
> > + - password
> > + - mySMTPPassword
>
> Why are you doing this?
Because Email::Send does not take an Array of HashRefs it just takes
an Array. And this is a wrapper around Email::Send and pod and code
says these are ->{mailer_args}
>
> Regards,
> Jonathan Rockway
>
>
>
> ------------------------------
>
> Message: 3
> Date: Fri, 27 Jul 2007 20:38:35 -0700
> From: "J. Shirley" <jshirley at gmail.com>
> Subject: Re: [Catalyst-dev] Catalyst::View::Email patch courtesy of
> the one and only.
> To: "Development of the elegant MVC web framework"
> <catalyst-dev at lists.rawmode.org>
> Message-ID:
> <756703690707272038j263cca32j8c8500adbf4919e2 at mail.gmail.com>
> Content-Type: text/plain; charset=ISO-8859-1
>
> On 7/27/07, Evan Carroll <lists at evancarroll.com> wrote:
> > added verbosity to error to reflect nested args ie AoH throwing off error check
> >
> > fixed typo in docs, mailer_args is supposed to be an array not a hash,
> > now yaml shows it.
> >
> > removed old * for ->sender->{mailer_args}
> > we now support the documented fashion!! but we carp - because it isn't
> > supported by Email::Send ie we now coerce into ArrayRef is HashRef is
> > supplied.
> >
> > We also croak if neither HashRef nor ArrayRef is supplied
> >
> > We now check that Host is supplied if the ->mailer is SMTP
> >
> >
> > diff --git a/Email.pm b/Email.pm
> > index e4be27c..b422e54 100644
> > --- a/Email.pm
> > +++ b/Email.pm
> > @@ -35,9 +35,12 @@ In your app configuration (example in L<YAML>):
> > method: SMTP
> > # mailer_args is passed directly into Email::Send
> > mailer_args:
> > - - Host: smtp.example.com
> > - - username: username
> > - - password: password
> > + - Host
> > + - smtp.example.com
> > + - username
> > + - mySMPTUsername
> > + - password
> > + - mySMTPPassword
> >
> > =cut
> >
> > @@ -110,24 +113,64 @@ Then, Catalyst::View::Email will forward to your
> > View::TT by default.
> >
> > sub new {
> > my $self = shift->next::method(@_);
> > -
> > my ( $c, $arguments ) = @_;
> >
> > my $mailer = Email::Send->new;
> >
> > - if ( my $method = $self->sender->{method} ) {
> > - croak "$method is not supported, see Email::Send"
> > - unless $mailer->mailer_available($method);
> > - $mailer->mailer($method);
> > - } else {
> > - # Default case, run through the most likely options first.
> > - for ( qw/SMTP Sendmail Qmail/ ) {
> > - $mailer->mailer($_) and last if $mailer->mailer_available($_);
> > + my $chosen_method;
> > +
> > + ##
> > + ## If mailer is provided - check to make sure it is avail.
> > + ##
> > + if ( $chosen_method = $self->sender->{method} ) {
> > + croak "$chosen_method is not supported, see Email::Send"
> > + unless $mailer->mailer_available($chosen_method)
> > + ;
> > + $mailer->mailer($chosen_method);
> > + }
> > +
> > + ##
> > + ## Default case, run through the most likely options first.
> > + ##
> > + else {
> > + for my $default_method ( qw/SMTP Sendmail Qmail/ ) {
> > + if ( $mailer->available($default_method) ) {
> > + $mailer->mailer($default_method);
> > + carp "Warning making assumption that $default_method is
> > the desired method";
> > + last;
> > + }
> > }
> > }
> >
> > - if ( $self->sender->{mailer_args} ) {
> > - $mailer->mailer_args($self->sender->{mailer_args});
> > + ##
> > + ## Runtime checks to make sure that an Array is provided if anything
> > + ##
> > + my $mailer_args = $self->sender->{mailer_args};
> > + if ( $mailer_args ) {
> > +
> > + if ( ref $mailer_args eq 'HASH' ) {
> > + carp 'Attempting to coerce HashRef ->{mailer_args} '
> > + . 'into Email::Send required ArrayRef'
> > + ;
> > + $mailer_args = [%{$mailer_args}];
> > + }
> > + elsif ( ref $mailer_args ne 'ARRAY' ) {
> > + croak 'Please send something non-retarded for ->sender->{mailer_args}';
> > + }
> > +
> > + ##
> > + ## Further checks here against thine ->{mailer_args}
> > + ## Total bull shit Email::Send does not croak without this
> > + ##
> > + my %hash_args = ( @$mailer_args );
> > + croak 'No ->{Host} provided with method SMTP, '
> > + . 'check to make sure data structure sent to ->{mailer_args}
> > is not nested'
> > + if $chosen_method eq 'SMTP'
> > + && !defined $hash_args{Host}
> > + ;
> > +
> > + $mailer->mailer_args( $mailer_args );
> > +
> > }
> >
> > $self->mailer($mailer);
> >
> >
> >
> > I am awesome.
> >
>
> Your style changes are not going to be accepted.
>
> Adding in the type check on mailer_args is a good idea, and is implemented.
>
> The croak on Host not being specified is just plain dumb. That
> doesn't belong here, that belongs in Email::Send::SMTP, and they
> default to 'localhost'. This is pretty common (although undocumented)
> behavior. I'd recommend you file a ticket against Email::Send::SMTP
> asking them to document or change behavior to suit your desires.
>
> I'm not going to change their behavior, since I do specifically state
> that Catalyst::View::Email is a gateway to Email::Send. I will,
> however, point it out in the Pod.
Good enough for me. I didn't know it was using localhost, only that it
was neither dieing nor was traffic going out eth1. So it appeared to
just be silently failing. I'd still keep the part there about
$chosen_method and make it accessible...
And update the POD to reflect an Array rather than an AoH
>
> -Jay
>
> --
> J. Shirley :: jshirley at gmail.com :: Killing two stones with one bird...
> http://www.toeat.com
>
--
Evan Carroll
System Lord of the Internets
me at evancarroll.com
832-445-8877
More information about the Catalyst-dev
mailing list