On Thu, Sep 18, 2014 at 7:41 PM, [email protected] <[email protected]> wrote:
> Hi Sergiu,
>
> On 18 Sep 2014 at 19:29:52, Sergiu Dumitriu
> ([email protected](mailto:[email protected])) 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 said I did not know, not that it was not possible for sure.
>
>> 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 =
> "<pre>$escapetool.html($tdoc.getRenderedContent($outputSyntax))</pre>")
> #end
> #else
> #set ($renderedContent = $tdoc.getRenderedContent())
> #end
> #catch()
> …
> <div id="xwikicontent">
> #if ("$!exception" != '')
> #displayException($exception)
> #else
> $renderedContent
> #end
> </div>
> …
>
> Thanks
> -Vincent
>
>> On 09/18/2014 09:28 AM, [email protected] 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
> [email protected]
> http://lists.xwiki.org/mailman/listinfo/devs
--
Thomas Mortagne
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
