On Thu, 9 Jul 2020 20:29:51 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Robert Lichtenberger has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8176270: Adding ChangeListener to TextField.selectedTextProperty causes
>> StringOutOfBoundsException
>>
>> Clamping is no longer needed, removed it.
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/TextFieldTest.java
> line 475:
>
>> 474: assertEquals(4, txtField.getSelection().getStart());
>> 475: assertEquals(4, txtField.getSelection().getStart());
>> 476: }
>
> Did you mean to check `getStart()` twice?
No I didn't. Good catch. Fixed in next commit.
> modules/javafx.controls/src/test/java/test/javafx/scene/control/TextInputControlTest.java
> line 1924:
>
>> 1923: textInput.selectionProperty().addListener((__, ___, selection)
>> -> selectionLog.append("|" +
>> selection.getStart() + "," + selection.getEnd())); 1924:
>> textInput.textProperty().addListener((__, ___,
>> text) -> textLog.append("|" + text)); 1925:
>
> The pattern of using multiple underscores for unused variables isn't one we
> use. I recommend using something more
> common like `obs` and `o` (or similar).
Replaced the variable names in next commit. It looks like "observable" /
"oldValue" / "newValue" are almost exclusively
used in the current code base so I changed them to that.
-------------
PR: https://git.openjdk.java.net/jfx/pull/138
