Last modified: 2013-01-06 11:55:16 UTC
If a function written with help from Validator issues a error (non fatal one) there are two problems: 1. Error message is not visually distinct from normal text (no color or other highlighting is applied). 2. Function `#iferror' does not recognize function failure. For example: {{ #iferror: {{ #subpagecount: Page | format = list }} | <!-- Never get here. --> | <!-- Always get here. --> }} Function `#subpagecount' (implemented in extension SubPageList) does not have parameter `format' so the result will include error message "format is not a valid parameter." (wording is not exact). However, `#iferror' fails to detect the error. Both problems can be solved by enclosing error message generated by Validator into element `span' with class `error'.
Created attachment 9274 [details] Proposed fix.
What component is #iferror part of? My concern with this patch is that this pretty much forces extensions to get the errors in a span with class error. IIRC there also is a "warning" class, which might be more appropriate for some extensions. Having an extra function like this one would be more flexible: function getErrorSpan() { return span $this->renderErrors /span }
`#iferror' is a part of widely used ParserFunctions. Attachment is just a proposed patch, I am not insist on any particular implementation. My concerns is that (1) error and warnings are not distinguishable from normal text, so some kind of <span> is required for formatting purposes, for handling with JQuery scripts, etc; (2) well-known and widely used ParserFunctions' `#iferror' does not recognize errors. Introducing a new function is ok if it will be called "behind the scene". I mean, usual, currently working code: class MyExtension extends ParserHook { function getName() { return 'MyExtension'; } function getParameterInfo( $type ) { $params = array(); $params[ 'param1' ] = new Parameter( … ); … return $params; } function render( ) { return 'result'; }; } should return error message properly formatted (with <span class="error">/</span>) with no changes. > My concern with this patch is that this pretty much forces extensions to get the errors in a span with class error. To me this is right behaviour bcause: 1. Span of class `error' will be properly formatted, because it is present in `skins/common/shared.css': .error { color: red; font-size: larger; } 2. `span' is safest HTML element, suitable to appear inside any other elements, either inline or block. About warnings: Many different message levels (fatal error, errors, warnings; btw, syslog supports 8 levels) would be nice, but… To me it is not a high priority task. I would like to have better reporting first. For example, I want an error message includes error location: source page name, line and position number. I want it includes "stack unwinding" info, so I can easily track the error. For example, now I see an error message (wording is not exact): syntax error: escape is not a valid parameter. But where is the error? What function/template generated it? From which template is is called? Each time I have to guess, to look trough my templates searching the exact location of the problem. Ok, get back to the stream. I think Validator can support multiple message levels, warning/errors/fatal errors, etc. However, by default all the level should be splitted into 2 groups: (1) fatal errors and (2) non-fatal errors. 1. Fatal errors are returned instead of normal result, and surrounded with <span class="error">/</span>. 2. Non-fatal errors are discarded, normal result is returned. If FirePHP is installed, it is used for reporting: if ( isset( $wgFirePHP ) ) { $wgFirePHP->log( ... ); # or $wgFirePHP->info( ... ); # or $wgFirePHP->warn( ... ); # or $wgFirePHP->error( ... ); # depending on message level }
-need-review, +reviewed since Jeroen De Dauw has looked at it.
Validator actually has some notion of different error levels. However, this is not fully implemented. The part that is implemented is differentiation between fatal and non-fatal errors, the fatal ones causing the regular output to not be shown at all (ie render not getting called). It also has some support for choosing which errors you want to see (never show errors, show all but the most trivial ones (ie deprecation warnings)), ect. Again, this has not been fully implemented yet. Returning the span only for fatal errors might actually break some current code. Map for example will put the error message in a div with class error when there is a fatal error. This also ought to work with #iferror I guess. Even if it doesn't break it, it's still silly to do it at two points. Having some kind of stack trace would be nice, but might be difficult. No idea what this Fire PHP thing is.
