On Thu, 13 May 2021 12:29:46 GMT, Jeanette Winzenburg <faste...@openjdk.org>
wrote:
>> Florian Kirmaier has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8264127:
>> we now use a try finally statement, to make sure updateEditingIndex is
>> reset!
>
> modules/javafx.controls/src/main/java/javafx/scene/control/ListCell.java line
> 342:
>
>> 340: }
>> 341: updateEditing();
>> 342: }
>
> forgot yesterday: to keep consistent with TreeTableCell (and TreeCell), the
> updateEditing should be moved into the else-block (as last call) - couldn't
> find any difference (in fact couldn't reproduce the error the if/else is
> supposed to solve) as long as updateEditing behaves, though.
I've moved it to the if-else. Probably doesn't matter.
> modules/javafx.controls/src/main/java/javafx/scene/control/ListCell.java line
> 546:
>
>> 544:
>> 545: if (editing && (index == -1 || list == null || index !=
>> editIndex)) {
>> 546: // If my index is not the one being edited then I need to
>> cancel
>
> probably just me not being able to see at a glance if the overall logic in
> the method is the exact same as before the fix (modulo the fix itself :),
> but: I would prefer an extra method used in both early return for the
> -1/null-list and the old else-if.
>
> If the second reviewer sees the equivalence at a glance, I'll believe his
> eyes :)
I've simplified the if-else statements. Should now be obvious that it's the
same as in TreeTableCell.
-------------
PR: https://git.openjdk.java.net/jfx/pull/441
