RFR: 8238080: FXMLLoader: if script engines implement javax.script.Compilable compile scripts [v5]

Kevin Rushforth kcr at openjdk.java.net
Fri Jun 26 21:46:24 UTC 2020


On Fri, 26 Jun 2020 18:38:10 GMT, Rony G. Flatscher <github.com+60214806+ronyfla at openjdk.org> wrote:

>> This PR adds a "compile" process instruction to FXML files with the optional PI data "true" (default) and "false". The
>> PI data is turned into a boolean value using "Boolean.parseBoolean(String)".
>> This makes it possible to inject the compile PI everywhere in a FXML file and turn on and off compilation of scripts if
>> the scripting engine implements the javax.script.Compilable interface. The PR adds the ability for a fallback in case
>> compilation of scripts fails, in which case a warning gets issued about this fact and evaluation of the script will be
>> done without compilation. Because of the fallback scripts get compiled with this version by default.
>> ---------
>> ### Progress
>> - [x] Change must not contain extraneous whitespace
>> - [x] Commit message must refer to an issue
>> - [ ] Change must be properly reviewed
>> 
>> ### Issue
>>  * [JDK-8238080](https://bugs.openjdk.java.net/browse/JDK-8238080): FXMLLoader: if script engines implement
>>    javax.script.Compilable compile scripts
>> 
>> 
>> ### Download
>> `$ git fetch https://git.openjdk.java.net/jfx pull/192/head:pull/192`
>> `$ git checkout pull/192`
>
> Rony G. Flatscher has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Incorporating Kevin's review comments.

You got most of them, but there are still a few code style issues. I marked the ones as resolved that were, so you
should be able to look at the above comments for those that are stil ourstanding.

One of the comments is more than just code style. I think the following, which appears in a few test files (I noted it
in `FXMLScriptDeployment2Compile_Fail_Compilation.java`), should use an `int` variable rather than an `Integer` in the
for loop:

         for (Integer invocation = 1; invocation <= invocationList.size(); invocation++) {

modules/javafx.fxml/src/main/java/javafx/fxml/FXMLLoader.java line 1567:

> 1566:                     InputStreamReader scriptReader = null;
> 1567:                     String script=null;
> 1568:                     try {

Here's one more place to add spaces surrounding `=` (I missed it last time)

-------------

PR: https://git.openjdk.java.net/jfx/pull/192


More information about the openjfx-dev mailing list