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

[Eric Rostetter]

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

> 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.

Yes, I would expect it to show an error, and AFAIK it does, just not
in that code, but in the main.php code.  The code there reads:

             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');
             }
         }
         break;

so if it returns "false" then it displays $driver->err_str, which should
display the error.  Are you saying it isn't in the current HEAD code for
some reason?

The only reason it won't get displayed is if either it is over-written
with another error message, or if the code has been broken, or if you
are refering to another invocation of it that doesn't display the
error text returned.

Now, I suppose we could stack the messages at in the driver, for 
display later,
if we wanted.  But this could result in a overload of error messages.  It is
something we could consider, but I've not really looked at the code in a while
so I can't say the cons/pros of directly calling $notification->push() in
the drivers rather than returning a string that gets pushed later.

> 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.

Where are you calling it from?  It is the calling code's job (currently) to
display the error.

> Of course, if error messages are considered an annoyance, then I suppose that
> is all right? Then I will just leave it at that.

It depends.  In some cases, it is an annoyance.  In other cases, displaying
them is a requirement.

>> What do you mean by "trigger an error" ?
>
> Show an error message to the user.

Okay, I wasn't sure what you meant.  That's clear enough.

> 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

Well, it used to ignore it, but now it checks the return (Y/N/other).
But while it doesn't ignore the error, it does (at least sometimes) ignore
the error text.  This is because it is doing something behind the scenes
without user action.  If the user requests the action, then it should return
the error text.  But if it is just trying to pre-emptively help the user
out without the user's knowledge, and the success or failure is of no
real importance to the user, then it silently discards the error text.

>> 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.

Yes, that is clear now.  I'm still not sure what the problem is, but I know
what you are talking about at least.  I guess I need to dig up your parser
patch to see what you are trying to do.

> 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?

You should probably do it, and you should probably do it either returning
the error text to something that will show it, or by calling
$notification->push() directly.

--
Eric Rostetter
The Department of Physics
The University of Texas at Austin

Why get even? Get odd!