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

[Jan Schneider]

Zitat von Espen Jürgensen <espen at hhkol.dk>:

> Citat Eric Rostetter <eric.rostetter at physics.utexas.edu>:
>
>> > So my question is, if there is any general plan/idea on how to fix this?
>>
>> I still not sure what "this" is so I have no general plan or idea on how
>> to fix it.
>
> OK, here is another attempt at explaining my problem. From the code in
> forwards.php:
>
>   // Try to login with the username/password, no realm.
>   $status = $_vfs->checkCredentials();
>   if (is_a($status, 'PEAR_Error')) {
>       $this->err_str = $status->getMessage();
>       $this->err_str .= '  ' .  _("Check your username and password.");
>       return false;
>   }
>
> One would think this would show an error message, right? Well, it 
> doesn't show
> anything. Setting an error string is actually pretty useless.
>
> Anyway, the parser, which I submitted in a patch recently, wants to do pretty
> much the same: It gets called, and on error it returns false with an error
> message. However, like the ftp call, no error gets shown.
>
> Of course, if error messages are considered an annoyance, then I suppose that
> is all right? Then I will just leave it at that.

This is of course not right, but when I look at main.php I see:

if ($driver->setVacation($user, $realm, $password,
                          $vacationtxt, $alias)) {
     $notification->push(_("Vacation notice successfully enabled."),
'horde.success');
} else {
     $notification->push(sprintf(_("Failure in modifying vacation notice:
%s"), $driver->err_str), 'horde.error');
}

That should show the error. If doesn't get shown for, try to find out why
this piece of code doesn't work.

>> What do you mean by "trigger an error" ?
>
> Show an error message to the user.
>
>> The code hasn't changed much from back when it was only called once, so I
>> doubt it has anything to do it being called twice.
>
> Yes, you are right, perhaps it has more to do with main.php ignoring errors
> from isEnabled. Anyway, I am sure you are more inside the inner 
> workings of the
> module, and can better pinpoint the cause.

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

>> Yes, and in doing so, you can "return an error value".  Not sure how that
>> differs from "trigger an error" though.
>
> Sorry for being unclear, I hope the above is more clear.
>
> All I want to do is show a nice error message to the user. Should I 
> do it? And
> if so, how do I do it?

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.

Jan.

--
http://www.horde.org - The Horde Project
http://www.ammma.de - Neue Wege des Lernens
http://www.tip4all.de - Deine private Tippgemeinschaft=0