[sork] Patch for vacation forwards-driver (.forward-parser)

[Espen Jürgensen]

First of all I have to mention that your emails don't look very good in IMP 
3.... look beneath! Apparently it doesn't like IMP 4?

> if ($driver->setVacation($user, $realm, $password=,                        
> $vacationtxt, $alias)) {
>     $notification= 
> ->push(_("Vacation notice successfully enabled."),
> 'horde.success');
> } = 
[and so on]

Notice that this code only gets called when setting or unsetting, not when just 
trying to show current settings (_getUserDetails), which is the situation I am 
in.

> Ah, so we're not talking about the setVacation() 
> call, but isEnabled()?

Yes, correct, it's about showing current settings.

> The problem as I understand it is that if isEnabled() returns false, you
> don't know if that happens because there was an error or simply because
> the driver doesn't support this.
> The "correct" solution would be to rework the whole driver code to return
> PEAR_Errors on errors instead of false.

Yes, that would be nice. Or just some other way of distinguishing an error 
from "method not supported", so errors can be shown.

Regards,
Espen


Citat Jan Schneider <jan at horde.org>:

> Zitat von Espen J=FCrgensen <espen at hhkol.dk>:=0A=0A> Citat Eric Rostetter
> <=
> eric.rostetter at physics.utexas.edu>:=0A>=0A>> > So my question is, if there
> =
> is any general plan/idea on how to fix this?=0A>>=0A>> I still not sure
> wha=
> t "this" is so I have no general plan or idea on how=0A>> to fix
> it.=0A>=0A=
> > OK, here is another attempt at explaining my problem. From the code
> in=0A=
> > forwards.php:=0A>=0A>   // Try to login with the username/password, no
> re=
> alm.=0A>   $status =3D $_vfs->checkCredentials();=0A>   if (is_a($status,
> '=
> PEAR_Error')) {=0A>       $this->err_str =3D $status->getMessage();=0A>   
> =
>    $this->err_str .=3D '  ' .  _("Check your username and password.");=0A>
> =
>       return false;=0A>   }=0A>=0A> One would think this would show an
> erro=
> r message, right? Well, it =0A> doesn't show=0A> anything. Setting an
> error=
>  string is actually pretty useless.=0A>=0A> Anyway, the parser, which I
> sub=
> mitted in a patch recently, wants to do pretty=0A> much the same: It gets
> c=
> alled, and on error it returns false with an error=0A> message. However,
> li=
> ke the ftp call, no error gets shown.=0A>=0A> Of course, if error messages
> =
> are considered an annoyance, then I suppose that=0A> is all right? Then I
> w=
> ill just leave it at that.=0A=0AThis is of course not right, but when I
> loo=
> k at main.php I see:=0A=0Aif ($driver->setVacation($user, $realm,
> $password=
> ,=0A                          $vacationtxt, $alias)) {=0A    
> $notification=
> ->push(_("Vacation notice successfully enabled."),=0A'horde.success');=0A}
> =
> else {=0A     $notification->push(sprintf(_("Failure in modifying vacation
> =
> notice:=0A%s"), $driver->err_str), 'horde.error');=0A}=0A=0AThat should
> sho=
> w the error. If doesn't get shown for, try to find out why=0Athis piece of
> =
> code doesn't work.=0A=0A>> What do you mean by "trigger an error"
> ?=0A>=0A>=
>  Show an error message to the user.=0A>=0A>> The code hasn't changed much
> f=
> rom back when it was only called once, so I=0A>> doubt it has anything to
> d=
> o it being called twice.=0A>=0A> Yes, you are right, perhaps it has more
> to=
>  do with main.php ignoring errors=0A> from isEnabled. Anyway, I am sure
> you=
>  are more inside the inner =0A> workings of the=0A> module, and can better
> =
> pinpoint the cause.=0A=0AAh, so we're not talking about the setVacation()
> c=
> all, but isEnabled()?=0A=0A>> Yes, and in doing so, you can "return an
> erro=
> r value".  Not sure how that=0A>> differs from "trigger an error" though.=
> =0A>=0A> Sorry for being unclear, I hope the above is more clear.=0A>=0A>
> A=
> ll I want to do is show a nice error message to the user. Should I =0A> do
> =
> it? And=0A> if so, how do I do it?=0A=0AThe problem as I understand it is
> t=
> hat if isEnabled() returns false, you=0Adon't know if that happens because
> =
> there was an error or simply because the=0Adriver doesn't support
> this.=0AT=
> he "correct" solution would be to rework the whole driver code to
> return=0A=
> PEAR_Errors on errors instead of
> false.=0A=0AJan.=0A=0A--=0Ahttp://www.hord=
> e.org - The Horde Project=0Ahttp://www.ammma.de - Neue Wege des
> Lernens=0Ah=
> ttp://www.tip4all.de - Deine private Tippgemeinschaft=0
> -- 
> Sork mailing list - Join the hunt: http://horde.org/bounties/#sork
> Frequently Asked Questions: http://horde.org/faq/
> To unsubscribe, mail: sork-unsubscribe at lists.horde.org
>