scijava-common icon indicating copy to clipboard operation
scijava-common copied to clipboard

Avoid processing parameters and directives in multi-line strings

Open imagejan opened this issue 8 years ago • 4 comments

Currently it's not possible to use script parameters inside scripts that are defined as multi-line strings inside other scripts. The following Groovy script illustrates this:

#@ ScriptService scripts

script = """
#@OUTPUT String sum
#@OUTPUT Double constant

constant = 2.4
sum = "foo" + "bar"
"""

println script

module = scripts.run("foo.groovy", script, true).get()

sum = "bar" + "foo"
constant = 42

You might expect the return value of the inner script being a HashMap containing constant and sum, and the return value of the outer script being the implicit result with a value of 42.

However, the actual return values are result: foobar from the inner script, and {sum: barfoo; constant: 42} from the outer script.

If you replace the line breaks in the multi-line string by \n, you'll get the expected behavior:

#@ ScriptService scripts

script = """\n#@OUTPUT String sum\n#@OUTPUT Double constant

constant = 2.4
sum = "foo" + "bar"
"""

println script

module = scripts.run("foo.groovy", script, true).get()

sum = "bar" + "foo"
constant = 42

This behavior is the result of the simple line-by-line parsing of the ScriptProcessor, but it isn't intuitive from the user perspective. I suggest to implement some (language-agnostic) parsing that respects and ignores multi-line strings.


@ctrueden Or should we revert to only allowing parameters at the start of scripts altogether? (While keeping the new language-agnostic #@ syntax, of course...)

imagejan avatar Aug 20 '17 15:08 imagejan

I do not think we should revert the behavior. There must be a better way. Maybe we can add some kind of "and now the script parameters are done" symbol like:

#@ ScriptService scripts
#@----

ctrueden avatar Aug 23 '17 00:08 ctrueden

@imagejan What do you think? Do you like the idea of a terminating directive?

ctrueden avatar Aug 23 '17 19:08 ctrueden

Yes, #@---- is a good idea. When I first discovered this issue, I was concerned that the current behavior is against the principle of least astonishment: I'd assume that the script acts the same whether I use \n or true line breaks in a multi-line string. But the global #@ parameter parsing has a lot of advantages, and as long as you can use #@---- for the admittedly rare use case of coding scripts in multi-line strings, I'm fine with that.

I'll open a PR including the proposed directive in the ParameterScriptProcessor (unless you're faster than me).

Should this also be introduced to scijava-grab to be consistent then?

imagejan avatar Aug 23 '17 20:08 imagejan

Should this also be introduced to scijava-grab to be consistent then?

Yeah, we'll have to think carefully about the most extension-friendly way of doing this. It might be nice if #@---- caused the loop itself to stop feeding lines to the processors, so that no one can code a buggy processor. On the other hand, there could be legitimate preprocessing cases prevented by forcing the issue at that level. Alternately, we can of course update every existing ScriptProcessor plugin to respect the terminator.

ctrueden avatar Aug 24 '17 15:08 ctrueden