On Fri, 19 Jun 2020 00:04:37 GMT, Kevin Rushforth <k...@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
