scijava / scijava-common

A plugin framework and application container with built-in extensibility mechanism :electric_plug:
BSD 2-Clause "Simplified" License
87 stars 52 forks source link

Avoid processing parameters and directives in multi-line strings #287

Open imagejan opened 7 years ago

imagejan commented 7 years ago

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...)

ctrueden commented 7 years ago

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 commented 7 years ago

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

imagejan commented 7 years ago

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?

ctrueden commented 7 years ago

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.