Re: [xwiki-devs] [xwiki-notifications] r14298 - in platform/core/trunk: . xwiki-core xwiki-core/src/main/java/com/xpn/xwiki/render xwiki-core/src/main/java/com/xpn/xwiki/render/groovy xwiki-macros xwiki-macros/xwiki-macro-groovy xwiki-macros/xwiki-ma
On Fri, Nov 21, 2008 at 7:28 AM, Sergiu Dumitriu <sergiu@xwiki.com> wrote:
tmortagne (SVN) wrote:
Author: tmortagne Date: 2008-11-19 15:57:38 +0100 (Wed, 19 Nov 2008) New Revision: 14298
Log: XWIKI-2825: Create a generic script macro
A quick review of the xwiki-script module:
- DefaultScriptContextManager and ScriptExecutionContextInitializer should be in the internal package. - ScriptExecutionContextInitializer is not declared in components.xml, thus it is not used. - getScriptContext calls the initializers on each call. Is that correct? Shouldn't the context be initialized only once?
It in initialized only once (there is an if if (scriptContext.getAttribute("util") == null) in it). I was in a hurry for the 1.7M3 release so I just follow the way it's done for Velocity.
- scriptContextInitializerList (which also contains a typo in its name) could be completely injected by Plexus, thus there's no need for Initializable and Composable on this class. Just declare a dependency on ScriptContextInitializer components.
How do you do that ? The context initializers are retrieved the same way in DefaultExecutionContextInitializerManager so I assumed it was not possible to inject a list of dependencies in component.xml.
- The javadocs on the interfaces could be improved... They are rather short right now. -- Sergiu Dumitriu http://purl.org/net/sergiu/ _______________________________________________ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
Thomas Mortagne wrote:
On Fri, Nov 21, 2008 at 7:28 AM, Sergiu Dumitriu <sergiu@xwiki.com> wrote:
tmortagne (SVN) wrote:
Author: tmortagne Date: 2008-11-19 15:57:38 +0100 (Wed, 19 Nov 2008) New Revision: 14298 Log: XWIKI-2825: Create a generic script macro A quick review of the xwiki-script module:
- DefaultScriptContextManager and ScriptExecutionContextInitializer should be in the internal package. - ScriptExecutionContextInitializer is not declared in components.xml, thus it is not used. - getScriptContext calls the initializers on each call. Is that correct? Shouldn't the context be initialized only once?
It in initialized only once (there is an if if (scriptContext.getAttribute("util") == null) in it). I was in a hurry for the 1.7M3 release so I just follow the way it's done for Velocity.
I can't see it, perhaps you forgot to commit it?
- scriptContextInitializerList (which also contains a typo in its name) could be completely injected by Plexus, thus there's no need for Initializable and Composable on this class. Just declare a dependency on ScriptContextInitializer components.
How do you do that ? The context initializers are retrieved the same way in DefaultExecutionContextInitializerManager so I assumed it was not possible to inject a list of dependencies in component.xml.
Just declare it like a normal dependency. Plexus can see that it is a list and populate with all the implementations. I did this in the localization component.
- The javadocs on the interfaces could be improved... They are rather short right now.
-- Sergiu Dumitriu http://purl.org/net/sergiu/
participants (2)
-
Sergiu Dumitriu -
Thomas Mortagne