Looks good. I like it. Thanks, Marius
On Fri, Sep 19, 2014 at 1:04 AM, [email protected] <[email protected]> wrote: > > > On 18 Sep 2014 at 19:41:56, [email protected] > ([email protected](mailto:[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’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, [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 _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
