demobox commented on this pull request.
> @@ -344,7 +349,17 @@ private void discoverCommands(CommandProcessorImpl
> commandProcessor, ClassLoader
@Override
public Action createNewAction() {
try {
- return ((Class<? extends Action>)
actionClass).newInstance();
+ Action action = ((Class< ? extends Action>)
actionClass).newInstance();
+ if (action instanceof ComputeCommandBase) {
+ ShellTableFactory shellTableFactory =
((ComputeCommandBase) action).getShellTableFactory();
+ if (shellTableFactory instanceof
BasicShellTableFactory) {
+ BasicShellTableFactory factory =
(BasicShellTableFactory) shellTableFactory;
+ if (factory.getScriptEngineManager() ==
null) {
+ factory.setScriptEngineManager(new
ScriptEngineManager());
> That will always be null, but, it is an extra check that could save us some
> debugging if we change things in the future (unlikely, but it's a cheap
> check).
@nacx Unless I'm missing something, the check would ensure that we don't
overwrite any ScriptEngineManagers that miraculously happen to be set. But I
think the intention here is precisely to set an SEM explicitly, even if it were
somehow already set? That way, I think we can at least be sure that all
commands will be initialized the same way.
A comment along the lines of `setting ScriptEngineManager explicitly as it's
expected to be null` or so might help make that clearer.
And yes, I think that can easily wait until after 2.0.0 ;-)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-cli/pull/35