On Tue, 18 Oct 2022 20:01:22 GMT, Alexey Ivanov <[email protected]> wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> insertIndexInterval fix. Add more subtests
>
> src/java.desktop/share/classes/javax/swing/DefaultListSelectionModel.java
> line 721:
>
>> 719: setState(i, value.get(gapLength));
>> 720: break;
>> 721: }
>
> This does not work. It must not break. With this code, 5 of my tests fail.
>
> For example,
>
>
> selectionModel.setSelectionInterval(0, Integer.MAX_VALUE);
> selectionModel.removeIndexInterval(0, Integer.MAX_VALUE);
>
> must result in empty selection. All the possible indexes were selected, you
> removed all the possible indexes (so that “the list” contains no elements at
> all). This test case fails either way, unfortunately:
> `selectionModel.isSelectedIndex(0)` still returns `true`.
>
> This edge case where `rmMinIndex = 0` and `rmMaxIndex = Integer.MAX_VALUE`
> should probably be short-circuited: `setState(i, false)` for all the
> elements. Other values are handled correctly by my suggested code, as far as
> I can see.
>
> This test case:
>
> selectionModel.setSelectionInterval(0, Integer.MAX_VALUE);
> selectionModel.removeIndexInterval(1, Integer.MAX_VALUE);
>
> passed before and now it fails again. Here,
> `selectionModel.isSelectedIndex(0)` must be `true`.
If we handle the edge case separately, it looks better and the first test case
as well as other tests for `remove` pass successfully:
public void removeIndexInterval(int index0, int index1)
{
if (index0 < 0 || index1 < 0) {
throw new IndexOutOfBoundsException("index is negative");
}
int rmMinIndex = Math.min(index0, index1);
int rmMaxIndex = Math.max(index0, index1);
if (rmMinIndex == 0 && rmMaxIndex == Integer.MAX_VALUE) {
for (int i = Integer.MAX_VALUE; i >= 0; i--) {
setState(i, false);
}
// min and max are updated automatically by the for-loop
// TODO Update anchor and lead
return;
}
int gapLength = (rmMaxIndex - rmMinIndex) + 1;
/* Shift the entire bitset to the left to close the index0, index1
* gap.
*/
for(int i = rmMinIndex; i >= 0 && i <= maxIndex; i++) {
setState(i, (i <= Integer.MAX_VALUE - gapLength)
&& (i + gapLength >= minIndex)
&& value.get(i + gapLength));
}
// The rest of the method
}
-------------
PR: https://git.openjdk.org/jdk/pull/10409
