Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
imagejan opened this issue Aug 20, 2017 · 4 comments
Open

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

imagejan opened this issue Aug 20, 2017 · 4 comments

Comments

@imagejan
Copy link
Member

imagejan commented Aug 20, 2017

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
Copy link
Member

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
Copy link
Member

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

@imagejan
Copy link
Member Author

imagejan commented Aug 23, 2017

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants