On Fri, 26 Jun 2020 18:38:10 GMT, Rony G. Flatscher 
<github.com+60214806+rony...@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

Reply via email to