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
