Skip to content

Commit d3cbee4

Browse files
committed
Merge pull request #237 from scijava/multiple-choice
Use SJEP to parse script attributes
2 parents ea8c647 + 66fc545 commit d3cbee4

File tree

3 files changed

+107
-93
lines changed

3 files changed

+107
-93
lines changed

pom.xml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,16 @@
116116

117117
<properties>
118118
<scijava.jvm.test.version>1.8</scijava.jvm.test.version>
119+
<scijava-expression-parser.version>3.0.0</scijava-expression-parser.version>
119120
</properties>
120121

121122
<dependencies>
123+
<!-- SciJava dependencies -->
124+
<dependency>
125+
<groupId>org.scijava</groupId>
126+
<artifactId>scijava-expression-parser</artifactId>
127+
</dependency>
128+
122129
<!-- Third-party dependencies -->
123130
<dependency>
124131
<groupId>com.googlecode.gentyref</groupId>

src/main/java/org/scijava/script/ScriptInfo.java

Lines changed: 82 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,11 @@
3838
import java.io.Reader;
3939
import java.io.StringReader;
4040
import java.text.SimpleDateFormat;
41+
import java.util.ArrayList;
42+
import java.util.Collections;
4143
import java.util.Date;
4244
import java.util.HashMap;
45+
import java.util.List;
4346
import java.util.Map;
4447

4548
import javax.script.ScriptException;
@@ -56,6 +59,8 @@
5659
import org.scijava.module.DefaultMutableModuleItem;
5760
import org.scijava.module.ModuleException;
5861
import org.scijava.plugin.Parameter;
62+
import org.scijava.sjep.Variable;
63+
import org.scijava.sjep.eval.DefaultEvaluator;
5964
import org.scijava.util.DigestUtils;
6065
import org.scijava.util.FileUtils;
6166

@@ -337,17 +342,17 @@ private void parseParam(final String param) throws ScriptException {
337342
if (rParen < lParen) {
338343
throw new ScriptException("Invalid parameter: " + param);
339344
}
340-
if (lParen < 0) parseParam(param, parseAttrs(""));
345+
if (lParen < 0) parseParam(param, parseAttrs("()"));
341346
else {
342347
final String cutParam =
343348
param.substring(0, lParen) + param.substring(rParen + 1);
344-
final String attrs = param.substring(lParen + 1, rParen);
349+
final String attrs = param.substring(lParen, rParen + 1);
345350
parseParam(cutParam, parseAttrs(attrs));
346351
}
347352
}
348353

349354
private void parseParam(final String param,
350-
final HashMap<String, String> attrs) throws ScriptException
355+
final HashMap<String, Object> attrs) throws ScriptException
351356
{
352357
final String[] tokens = param.trim().split("[ \t\n]+");
353358
checkValid(tokens.length >= 1, param);
@@ -371,23 +376,42 @@ private void parseParam(final String param,
371376
}
372377

373378
/** Parses a comma-delimited list of {@code key=value} pairs into a map. */
374-
private HashMap<String, String> parseAttrs(final String attrs)
379+
private HashMap<String, Object> parseAttrs(final String attrs)
375380
throws ScriptException
376381
{
377-
// TODO: We probably want to use a real CSV parser.
378-
final HashMap<String, String> attrsMap = new HashMap<String, String>();
379-
for (final String token : attrs.split(",")) {
380-
if (token.isEmpty()) continue;
381-
final int equals = token.indexOf("=");
382-
if (equals < 0) throw new ScriptException("Invalid attribute: " + token);
383-
final String key = token.substring(0, equals).trim();
384-
String value = token.substring(equals + 1).trim();
385-
if (value.startsWith("\"") && value.endsWith("\"")) {
386-
value = value.substring(1, value.length() - 1);
382+
// NB: Parse the attributes using the SciJava Expression Parser.
383+
final DefaultEvaluator e = new DefaultEvaluator();
384+
try {
385+
final Object result = e.evaluate(attrs);
386+
if (result == null) throw new ScriptException("Unparseable attributes");
387+
final List<?> list;
388+
if (result instanceof List) list = (List<?>) result;
389+
else if (result instanceof Variable) {
390+
list = Collections.singletonList(result);
391+
}
392+
else {
393+
throw new ScriptException("Unexpected attributes type: " +
394+
result.getClass().getName());
395+
}
396+
397+
final HashMap<String, Object> attrsMap = new HashMap<String, Object>();
398+
for (final Object o : list) {
399+
if (o instanceof Variable) {
400+
final Variable v = (Variable) o;
401+
attrsMap.put(v.getToken(), e.value(v));
402+
}
403+
else {
404+
throw new ScriptException("Invalid attribute: " + o);
405+
}
387406
}
388-
attrsMap.put(key, value);
407+
return attrsMap;
408+
}
409+
catch (final IllegalArgumentException exc) {
410+
final ScriptException se = new ScriptException(
411+
"Error parsing attributes");
412+
se.initCause(exc);
413+
throw se;
389414
}
390-
return attrsMap;
391415
}
392416

393417
private boolean isIOType(final String token) {
@@ -402,97 +426,68 @@ private void checkValid(final boolean valid, final String param)
402426

403427
/** Adds an output for the value returned by the script itself. */
404428
private void addReturnValue() throws ScriptException {
405-
final HashMap<String, String> attrs = new HashMap<String, String>();
429+
final HashMap<String, Object> attrs = new HashMap<String, Object>();
406430
attrs.put("type", "OUTPUT");
407431
addItem(ScriptModule.RETURN_VALUE, Object.class, attrs);
408432
}
409433

410434
private <T> void addItem(final String name, final Class<T> type,
411-
final Map<String, String> attrs) throws ScriptException
435+
final Map<String, Object> attrs) throws ScriptException
412436
{
413437
final DefaultMutableModuleItem<T> item =
414438
new DefaultMutableModuleItem<T>(this, name, type);
415439
for (final String key : attrs.keySet()) {
416-
final String value = attrs.get(key);
440+
final Object value = attrs.get(key);
417441
assignAttribute(item, key, value);
418442
}
419443
if (item.isInput()) registerInput(item);
420444
if (item.isOutput()) registerOutput(item);
421445
}
422446

423447
private <T> void assignAttribute(final DefaultMutableModuleItem<T> item,
424-
final String key, final String value) throws ScriptException
448+
final String k, final Object v) throws ScriptException
425449
{
426450
// CTR: There must be an easier way to do this.
427451
// Just compile the thing using javac? Or parse via javascript, maybe?
428-
if ("callback".equalsIgnoreCase(key)) {
429-
item.setCallback(value);
430-
}
431-
else if ("choices".equalsIgnoreCase(key)) {
432-
// FIXME: Regex above won't handle {a,b,c} syntax.
433-
// item.setChoices(choices);
434-
}
435-
else if ("columns".equalsIgnoreCase(key)) {
436-
item.setColumnCount(convertService.convert(value, int.class));
437-
}
438-
else if ("description".equalsIgnoreCase(key)) {
439-
item.setDescription(value);
440-
}
441-
else if ("initializer".equalsIgnoreCase(key)) {
442-
item.setInitializer(value);
443-
}
444-
else if ("type".equalsIgnoreCase(key)) {
445-
item.setIOType(convertService.convert(value, ItemIO.class));
446-
}
447-
else if ("label".equalsIgnoreCase(key)) {
448-
item.setLabel(value);
449-
}
450-
else if ("max".equalsIgnoreCase(key)) {
451-
item.setMaximumValue(convertService.convert(value, item.getType()));
452-
}
453-
else if ("min".equalsIgnoreCase(key)) {
454-
item.setMinimumValue(convertService.convert(value, item.getType()));
455-
}
456-
else if ("name".equalsIgnoreCase(key)) {
457-
item.setName(value);
458-
}
459-
else if ("persist".equalsIgnoreCase(key)) {
460-
item.setPersisted(convertService.convert(value, boolean.class));
461-
}
462-
else if ("persistKey".equalsIgnoreCase(key)) {
463-
item.setPersistKey(value);
464-
}
465-
else if ("required".equalsIgnoreCase(key)) {
466-
item.setRequired(convertService.convert(value, boolean.class));
467-
}
468-
else if ("softMax".equalsIgnoreCase(key)) {
469-
item.setSoftMaximum(convertService.convert(value, item.getType()));
470-
}
471-
else if ("softMin".equalsIgnoreCase(key)) {
472-
item.setSoftMinimum(convertService.convert(value, item.getType()));
473-
}
474-
else if ("stepSize".equalsIgnoreCase(key)) {
475-
try {
476-
final double stepSize = Double.parseDouble(value);
477-
item.setStepSize(stepSize);
478-
}
479-
catch (final NumberFormatException exc) {
480-
log.warn("Script parameter " + item.getName() +
481-
" has an invalid stepSize: " + value);
482-
}
483-
}
484-
else if ("style".equalsIgnoreCase(key)) {
485-
item.setWidgetStyle(value);
486-
}
487-
else if ("visibility".equalsIgnoreCase(key)) {
488-
item.setVisibility(convertService.convert(value, ItemVisibility.class));
489-
}
490-
else if ("value".equalsIgnoreCase(key)) {
491-
item.setDefaultValue(convertService.convert(value, item.getType()));
492-
}
493-
else {
494-
throw new ScriptException("Invalid attribute name: " + key);
452+
if (is(k, "callback")) item.setCallback(as(v, String.class));
453+
else if (is(k, "choices")) item.setChoices(asList(v, item.getType()));
454+
else if (is(k, "columns")) item.setColumnCount(as(v, int.class));
455+
else if (is(k, "description")) item.setDescription(as(v, String.class));
456+
else if (is(k, "initializer")) item.setInitializer(as(v, String.class));
457+
else if (is(k, "type")) item.setIOType(as(v, ItemIO.class));
458+
else if (is(k, "label")) item.setLabel(as(v, String.class));
459+
else if (is(k, "max")) item.setMaximumValue(as(v, item.getType()));
460+
else if (is(k, "min")) item.setMinimumValue(as(v, item.getType()));
461+
else if (is(k, "name")) item.setName(as(v, String.class));
462+
else if (is(k, "persist")) item.setPersisted(as(v, boolean.class));
463+
else if (is(k, "persistKey")) item.setPersistKey(as(v, String.class));
464+
else if (is(k, "required")) item.setRequired(as(v, boolean.class));
465+
else if (is(k, "softMax")) item.setSoftMaximum(as(v, item.getType()));
466+
else if (is(k, "softMin")) item.setSoftMinimum(as(v, item.getType()));
467+
else if (is(k, "stepSize")) item.setStepSize(as(v, double.class));
468+
else if (is(k, "style")) item.setWidgetStyle(as(v, String.class));
469+
else if (is(k, "visibility")) item.setVisibility(as(v, ItemVisibility.class));
470+
else if (is(k, "value")) item.setDefaultValue(as(v, item.getType()));
471+
else throw new ScriptException("Invalid attribute name: " + k);
472+
}
473+
474+
/** Super terse comparison helper method. */
475+
private boolean is(final String key, final String desired) {
476+
return desired.equalsIgnoreCase(key);
477+
}
478+
479+
/** Super terse conversion helper method. */
480+
private <T> T as(final Object v, final Class<T> type) {
481+
return convertService.convert(v, type);
482+
}
483+
484+
private <T> List<T> asList(final Object v, final Class<T> type) {
485+
final ArrayList<T> result = new ArrayList<T>();
486+
final List<?> list = as(v, List.class);
487+
for (final Object item : list) {
488+
result.add(as(item, type));
495489
}
490+
return result;
496491
}
497492

498493
/**

src/test/java/org/scijava/script/ScriptInfoTest.java

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import java.io.Reader;
4343
import java.io.StringReader;
4444
import java.util.Arrays;
45+
import java.util.Collections;
4546
import java.util.HashMap;
4647
import java.util.List;
4748

@@ -141,29 +142,39 @@ public void testParameters() {
141142
"% @LogService(required = false) log\n" + //
142143
"% @int(label=\"Slider Value\", softMin=5, softMax=15, " + //
143144
"stepSize=3, value=11, style=\"slider\") sliderValue\n" + //
145+
"% @String(persist = false, " + //
146+
"choices={'quick brown fox', 'lazy dog'}) animal\n" + //
144147
"% @BOTH java.lang.StringBuilder buffer";
145148

146149
final ScriptInfo info =
147150
new ScriptInfo(context, "params.bsizes", new StringReader(script));
148151

152+
final List<?> noChoices = Collections.emptyList();
153+
149154
final ModuleItem<?> log = info.getInput("log");
150155
assertItem("log", LogService.class, null, ItemIO.INPUT, false, true, null,
151-
null, null, null, null, null, null, null, log);
156+
null, null, null, null, null, null, null, noChoices, log);
152157

153158
final ModuleItem<?> sliderValue = info.getInput("sliderValue");
154159
assertItem("sliderValue", int.class, "Slider Value", ItemIO.INPUT, true,
155-
true, null, "slider", 11, null, null, 5, 15, 3.0, sliderValue);
160+
true, null, "slider", 11, null, null, 5, 15, 3.0, noChoices, sliderValue);
161+
162+
final ModuleItem<?> animal = info.getInput("animal");
163+
final List<String> animalChoices = //
164+
Arrays.asList("quick brown fox", "lazy dog");
165+
assertItem("animal", String.class, null, ItemIO.INPUT, true, false,
166+
null, null, null, null, null, null, null, null, animalChoices, animal);
156167

157168
final ModuleItem<?> buffer = info.getOutput("buffer");
158169
assertItem("buffer", StringBuilder.class, null, ItemIO.BOTH, true, true,
159-
null, null, null, null, null, null, null, null, buffer);
170+
null, null, null, null, null, null, null, null, noChoices, buffer);
160171

161172
final ModuleItem<?> result = info.getOutput("result");
162173
assertItem("result", Object.class, null, ItemIO.OUTPUT, true, true, null,
163-
null, null, null, null, null, null, null, result);
174+
null, null, null, null, null, null, null, noChoices, result);
164175

165176
int inputCount = 0;
166-
final ModuleItem<?>[] inputs = { log, sliderValue, buffer };
177+
final ModuleItem<?>[] inputs = { log, sliderValue, animal, buffer };
167178
for (final ModuleItem<?> inItem : info.inputs()) {
168179
assertSame(inputs[inputCount++], inItem);
169180
}
@@ -180,7 +191,7 @@ private void assertItem(final String name, final Class<?> type,
180191
final boolean persist, final String persistKey, final String style,
181192
final Object value, final Object min, final Object max,
182193
final Object softMin, final Object softMax, final Number stepSize,
183-
final ModuleItem<?> item)
194+
final List<?> choices, final ModuleItem<?> item)
184195
{
185196
assertEquals(name, item.getName());
186197
assertSame(type, item.getType());
@@ -196,6 +207,7 @@ private void assertItem(final String name, final Class<?> type,
196207
assertEquals(softMin, item.getSoftMinimum());
197208
assertEquals(softMax, item.getSoftMaximum());
198209
assertEquals(stepSize, item.getStepSize());
210+
assertEquals(choices, item.getChoices());
199211
}
200212

201213
/**

0 commit comments

Comments
 (0)