[Rev 02] RFR: 8234959: FXMLLoader does not populate ENGINE_SCOPE Bindings with FILENAME and ARGV

[Kevin Rushforth]

On Thu, 27 Feb 2020 15:56:25 GMT, Rony G. Flatscher <github.com+60214806+ronyfla at openjdk.org> wrote:

>> …9: FXMLLoader does not populate ENGINE_SCOPE Bindings with FILENAME and ARGV
>
> Rony G. Flatscher has updated the pull request incrementally with one additional commit since the last revision:
> 
>   corrected wrong test string

The fix looks good. I left a few comments on the test. One of them is substantive, the rest are formatting. Once you
make those changes, I'll approve it.

tests/system/src/testscriptapp1/java/mymod/myapp1/FXMLScriptDeployment.java line 104:

> 103:             ioe.printStackTrace();
> 104:             System.exit(-1);
> 105:         }

This should be `System.exit(ERROR_UNEXPECTED_EXCEPTION);`

tests/system/src/testscriptapp1/java/mymod/myapp1/FXMLScriptDeployment.java line 185:

> 184:
> 185:                 // global Bindings
> 186:             Util.assertExists(IDROOT + " in global scope Bindings", globalBindings.containsKey(IDROOT));

Minor: indentation is wrong (indented too far)

tests/system/src/testscriptapp1/java/mymod/myapp1/FXMLScriptDeployment.java line 208:

> 207:
> 208:                 // engine Bindings
> 209:             Util.assertExists(FILENAME + " in engine scope Bindings", engineBindings.containsKey(FILENAME));

Minor: indentation

tests/system/src/testscriptapp1/java/mymod/pseudoScriptEngine/InvocationInfos.java line 55:

> 54:      * @return string formatted to ease debugging
> 55:     */
> 56:     public String toDebugFormat(String indentation) {

Minor: the `/**` should be on a line by itself. Also, the closing `*/` needs one more space of indentation.

tests/system/src/testscriptapp1/java/mymod/myapp1/FXMLScriptDeployment.java line 109:

> 108:         btn.fireEvent(new ActionEvent());
> 109:         btn.fireEvent(new MouseEvent( MouseEvent.MOUSE_CLICKED,
> 110:                                        0,       // double x,

Minor: remove the extra space after the last `(`

tests/system/src/testscriptapp1/java/mymod/myapp1/FXMLScriptDeployment.java line 229:

> 228:                 }
> 229:                 else {
> 230:                     Util.assertType(EVENT, ActionEvent.class, obj);

Minor: the `}` should be on the same line as the `else {`

tests/system/src/testscriptapp1/java/mymod/pseudoScriptEngine/RgfPseudoScriptEngine.java line 47:

> 46: public class RgfPseudoScriptEngine extends AbstractScriptEngine
> 47: {
> 48:     static final boolean bDebug = false; // true;

The `{` should be on the previous line.

tests/system/src/testscriptapp1/java/mymod/pseudoScriptEngine/RgfPseudoScriptEngine.java line 89:

> 88:             // create copies of the Bindings for later inspection as they may
> 89:             // get reused and changed on each eval() invocation
> 90:         TreeMap<Integer,TreeMap> bindings = new TreeMap();

Minor: indentation

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

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