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

[Rony G.Flatscher]

On Fri, 19 Jun 2020 00:04:37 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Rony G. Flatscher has updated the pull request with a new target base due to a merge or a rebase. The pull request now
>> contains 27 commits:
>>  - Updates to meet Kevin's comment in PR 192 as of May 27th.
>>  - Merge remote-tracking branch 'upstream/master' into scriptCompilablePIcompileFallback
>>  - Reword the compile processing instructions (replace 'Hint' with 'Note:', adjust text to context), remove spurious empty
>>    line in script example code.
>>  - Document the compile processing instruction for scripts.
>>  - Add missing language processing instruction.
>>  - Correct typo, replace tabs, remove trailing blanks.
>>  - Make sure we test the default behaviour to compile script by leaving out the compile PI.
>>  - Revert temporary rename of test method.
>>  - Correct ModuleLauncherTest (remove non-existing test), correct formatting.
>>  - Always supply the script's filename in the error message first to further ease spotting the location of script
>>    exceptions.
>>  - ... and 17 more: https://git.openjdk.java.net/jfx/compare/6bd0e22d...7ef1bd68
>
> I reviewed the implementation, and I think this is close to being ready, but I have a couple questions. I also still
> need to review the test and run it, but that will be later.
> I also noticed a few places with minor formatting issues -- mostly missing spaces and extra blank lines added to some
> files, but also `else {` when it should be `} else {`. I'll wait until the substantive part of the review is done to
> note them all (so if you can fix them ahead of time, that would be good).

@CSR-update: looks good!

> modules/javafx.fxml/src/main/java/javafx/fxml/FXMLLoader.java line 1769:
> 
>> 1768:             try {
>> 1769:                 if (isCompiled) {
>> 1770:                    compiledScript.eval(localBindings);
> 
> I think there may be other places you need to set isCompiled (it isn't set in the first couple of methods where you
> compile scripts). Can you check this?

isCompiled gets set explicitly to false at object creation time. It only will be changed to true, if the script was
successfully compiled.

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

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