Looks good. I like it.
Thanks,
Marius
On Fri, Sep 19, 2014 at 1:04 AM, vincent(a)massol.net <vincent(a)massol.net> wrote:
On 18 Sep 2014 at 19:41:56, vincent(a)massol.net
(vincent@massol.net(mailto:vincent@massol.net)) wrote:
Hi Sergiu,
On 18 Sep 2014 at 19:29:52, Sergiu Dumitriu (sergiu@xwiki.org(mailto:sergiu@xwiki.org))
wrote:
1. I would rather add proper directives instead
of macros and
uberspector hacks:
http://stackoverflow.com/q/159292/620249
Argh… I wanted that and I asked Thomas and he told me it wasn’t possible and I didn’t
double check. I should have since I’ve now finished my implementation and redoing it as a
directive means dumping what I’ve done a third time in the end. Anyway I agree that a
directive is better so I’ll redo it.
I’ve now implemented a directive and it’s so cool :)
It’s so simple compared to what I had, I love it.
Here’s the usage code now:
#try()
#set($outputSyntax = $xwiki.getAvailableRendererSyntax($request.outputSyntax,
$request.outputSyntaxVersion))
#if ($outputSyntax)
## If the passed syntax is not an HTML-compatible syntax we need to HTML-escape it so
that it can be
## displayed fine in HTML (since at the point this vm file is called we're
already inside an HTML page with
## panels on the side, header, etc).
#set($syntaxType = $outputSyntax.type.toIdString())
#if (($syntaxType == "xhtml") || ($syntaxType == "html"))
#set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
#else
## Make sure to print correctly the result when it's not HTML
#set ($renderedContent =
"<pre>$escapetool.html($tdoc.getRenderedContent($outputSyntax))</pre>")
#end
#else
#set ($renderedContent = $tdoc.getRenderedContent())
#end
#end
…
<div id="xwikicontent">
#if ("$!exception" != '')
#displayException($exception)
#else
$renderedContent
#end
</div>
…
I like it so much that I’m going to commit it and if someone doesn’t agree I’ll revert
the commit :)
Thanks Sergiu
-Vincent
2. So far
it has been the job of our scriptable wrappers to catch
exceptions, does this proposal mean that:
a. We should stop doing that and write try-catch logic in our
templates/scripts? IOW, catching exceptions in wrappers is a bad design
pattern?
I’ve not answered this question because I don’t know yet. I think we’ll discover that as
we start using the new try/catch. ATM I’m only planning to use it in contentview.vm.
b. We didn't do that enough, and as a
workaround we're adding exception
handling in Velocity?
Yes, that’s true. We now have both cases in our code (code throwing exceptions, like a
lot of methods in Document, XWiki) and others catching exceptions.
This proposals offers a finer-grained control in vm while not breaking backward
compatibility. Any change in our script APIs would break compatibility (this is what I
started doing before realizing it would break too many things)…
FTR here’s what I have so far in term of usage, in contentview.vm:
#try()
#set($outputSyntax = $xwiki.getAvailableRendererSyntax($request.outputSyntax,
$request.outputSyntaxVersion))
#if ($outputSyntax)
## If the passed syntax is not an HTML-compatible syntax we need to HTML-escape it so
that it can be
## displayed fine in HTML (since at the point this vm file is called we're already
inside an HTML page with
## panels on the side, header, etc).
#set($syntaxType = $outputSyntax.type.toIdString())
#if (($syntaxType == "xhtml") || ($syntaxType == "html"))
#set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
#else
## Make sure to print correctly the result when it's not HTML
#set ($renderedContent = "
$escapetool.html($tdoc.getRenderedContent($outputSyntax))
")
#end
#else
#set ($renderedContent = $tdoc.getRenderedContent())
#end
#catch()
…
#if ("$!exception" != '')
#displayException($exception)
#else
$renderedContent
#end
…
Thanks
-Vincent
> On 09/18/2014 09:28 AM, vincent(a)massol.net wrote:
> > Hi Devs,
> >
> > Example of a use case:
> >
> > * In contentview.vm we need to be able to catch when getRenderedContent()
throws an exception in order to be able to still display the page title, content menu and
last modification line. We want only the content part to display an error.
> >
> > I’ve tried implementing the following:
> > * Modified Document.getRenderedContent() to not throw exceptions any more
> > * I’ve added Document.setError() and getError()
> > * Then added a #displayRenderedContent($renderedContent) velocimacro to display
an error, calling Document.getError()
> >
> > However that’s no good because we have lots of places in our code (and in
extensions we don’t control) that call getRenderedContent() and they expect the whole page
rendering to fail in case of an error…
> >
> > Thus after discussing quickly on IRC and with Thomas we’d like to propose
instead:
> >
> > * Add a new #try() velocimacro that would push a new empty List in the
Execution Context to a Stack variable
> > * Add a new uberspector that would, based on this flag do a try/catch around
the method called (can be done easily by returning a custom VelMethod)
> > * The uberspector catch would add the exception to the List in the Execution
Context
> > * Add a new #catch() velocimacro that would pop from the Stack in the Execution
Context and that would set 2 variables in the Velocity Context:
> > ** $exception: the first exception
> > ** $exceptions: the list of all exceptions that happened between the #try() and
#catch()
> >
> > The good part with this is:
> > * No change in behavior to all existing code
> > * Code that want to do something in case of error can do it using
#try()/#catch() for java methods throwing exceptions (like getRenderedContent())
> >
> > Of course for the future we would need to decide if script services should stop
doing try/catch or not but that’s another debate I’d prefer to separate from this thread.
> >
> > WDYT?
> >
> > Thanks
> > -Vincent
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs