On Thu, Aug 9, 2012 at 6:32 PM, Claude Brisson <cla...@renegat.net> wrote:
> On Tue, 7 Aug 2012 23:26:53 +0530 > Dishara Wijewardana <ddwijeward...@gmail.com> wrote: > > > > > I think I got you ;-). I have commited the fix which sets and gets > > the file name as a context attribute. And if we are doing so we have > > to recommend users to provide the file name of the template (which is > > on classpath) when providing the script itself. > > > > I also got the point of caching and will commit when the fix is ready > > and tests passed. > > > > You are right, if we're not given the script filename, we cannot do any > caching. Please forget this for now and come back to what you had -what > I had in mind was not to use geTemplate(), but to use a specific > Velocity resource loader. > +1 and done. > > I looked at your VelocityScriptEngine implementation, and I have some > remarks: > > - you can factorize eval(String,ScriptContext) and > eval(Reader,ScriptContext) with a StringReader. > > Done > - when an exception is thrown, its stack trace should be displayed on > the stream returned by scriptContext.getErrorWriter() > > Done > - when scriptContext.getWriter() is null, the default writer shoud be > System.out (right now, the output is lost...). > > Done > - "return new ScriptException(exp)" should be: > throw new ScriptException(exp) > > Done > That's all I see for now. > > Claude > -- Thanks /Dishara
