Jamorabon / phpliteadmin

Automatically exported from code.google.com/p/phpliteadmin
0 stars 0 forks source link

Centralize message handling #250

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Currently, lots of codes stores messages in global variables such as $completed 
that later produces some message.

I think we should have a central class to which we can push messages at any 
time (especially before HTML output has started) and that will manage the 
messages.

I started a new branch called "messageQueue" for this:
https://code.google.com/p/phpliteadmin/source/browse/?name=messageQueue

It is far from perfect and complete. But I think the right direction.

Feedback welcome.

Original issue reported on code.google.com by crazy4ch...@gmail.com on 24 Apr 2014 at 8:13

GoogleCodeExporter commented 9 years ago
Thanks for starting on this! A few doubts and suggestions after a first read:

- do we need queues? Are we likely to show more than one message?

- adding another exit path might be problematic in the future; I'd rather keep 
flow control to the main script, maybe *one* check "are there fatal errors to 
print? print them and exit"

- css: rather than different message_* classes, I would suggest one .message 
style for the bulk styling, followed by smaller .message.error, 
.message.notice, ... with minimal changes

.message { border: 0.2em solid gray; padding: 0.5em; margin: 1em 0; }
.message.error  { border-color: red; color: orange; }
.message.notice { border-color: lightblue; color: blue; }
[...]

- the message $type could be a class constant rather than a string

- we should rename 'err' in 'error' in the translation files

- if we are going to fix Issue #104, this should probable include some "show 
later" facility, storing messages in $_SESSION maybe

Maybe we should take this slowly: first split the output code into something 
like your Message class, while we survey the different messages and needs we 
might have, and later change the flow code.

E.g.:

class Message {

  const INFO  = 'msg_notice';
  const OK    = 'msg_success';
  const WARN  = 'msg_warning';
  const ERROR = 'msg_error';

  // I'd say public, so we can tweak content if necessary or check the type
  public $type, $message, $title;

  public function __construct($message, $type = self::INFO)
  {
    global $lang;
    $this->type = $type;
    $this->message = $message;
    $this->title = isset($lang[$this->type])
      ? $lang[$this->type]
      : "<code>[{$this->type}]</code>";
  }

  public function __toString()
  {
    return <<<MSG
<div class="message {$this-type}">
  <h3>{$this->title}</h3>
  <p>{$this->message}</p>
</div>
MSG;
  }

}

and for the moment just go for 'echo new Message(...);'  Once we have replaced 
all the messages and updated the theme [and lang files?] then we move to the 
message queue and flashing :)

Original comment by dreadnaut on 26 Apr 2014 at 11:50

GoogleCodeExporter commented 9 years ago
Thanks for your feedback.

- do we need queues? Are we likely to show more than one message?
I found at least one occurrence where we do this: When no db is there, there is 
a message like "you have no db yet, create one" and there *can* be an error 
message saying like "your file extension is not allowed". I always prefer being 
flexible and restricting to one message might be problematic.

The main reason why I introduced the message queue is to decouple message 
generation and output. Because we currently produce lots of messages before we 
output them and this is quite of a mess currently.
I think we really need a mechanism to produce a message before html output has 
started and can rely on the fact that it gets echoed at the right time. But ok, 
we can do it step by step.

- adding another exit path might be problematic 
agreed

- css
agreed

- type constants
agreed

- err -> error
agreed

- issue #104: the message queue could always store messages in the session and 
in case they have not been echoed (because there was only a redirect), print 
them on the next page. So if we introduce a message queue, fixing #104 will be 
a lot easier as we only need to change the messageQueue class and not all the 
places where messages are produced.

Okay, let's do it slowly.

Original comment by crazy4ch...@gmail.com on 26 Apr 2014 at 1:11