Re: RFR: 8196065: ListChangeListener getRemoved() returns items that were not removed. [v9]
On Fri, 25 Jun 2021 12:53:13 GMT, Kevin Rushforth wrote:
>> Michael Strauß has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> changes as per review comments
>
> modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/SelectedItemsReadOnlyObservableListTest.java
> line 105:
>
>> 103: */
>> 104: @Test
>> 105: @Ignore("see JDK-8267951")
>
> The comment by @arapte was not addressed. Please remove `see ` from the
> string passed to `@Ignore`.
Done.
-
PR: https://git.openjdk.java.net/jfx/pull/478
Re: RFR: 8196065: ListChangeListener getRemoved() returns items that were not removed. [v9]
On Fri, 11 Jun 2021 20:26:24 GMT, Michael Strauß wrote:
>> The documentation for `ObservableListBase.nextRemove` states that a single
>> change always refers to the current state of the list, which likely means
>> that multiple disjoint removed ranges need to be applied in order, otherwise
>> the next change's `getFrom` doesn't refer to the correct index.
>>
>> `SelectedItemsReadOnlyObservableList` doesn't apply removals to
>> `itemsRefList`, which means that subsequent removals will refer to the wrong
>> index when retrieving the removed elements. This PR fixes the calculation of
>> the current index.
>
> Michael Strauß has updated the pull request incrementally with one additional
> commit since the last revision:
>
> changes as per review comments
Looks good. I have one minor comment on the test.
modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/SelectedItemsReadOnlyObservableListTest.java
line 105:
> 103: */
> 104: @Test
> 105: @Ignore("see JDK-8267951")
The comment by @arapte was not addressed. Please remove `see ` from the string
passed to `@Ignore`.
-
PR: https://git.openjdk.java.net/jfx/pull/478
Re: RFR: 8196065: ListChangeListener getRemoved() returns items that were not removed. [v9]
On Fri, 11 Jun 2021 20:26:24 GMT, Michael Strauß wrote: >> The documentation for `ObservableListBase.nextRemove` states that a single >> change always refers to the current state of the list, which likely means >> that multiple disjoint removed ranges need to be applied in order, otherwise >> the next change's `getFrom` doesn't refer to the correct index. >> >> `SelectedItemsReadOnlyObservableList` doesn't apply removals to >> `itemsRefList`, which means that subsequent removals will refer to the wrong >> index when retrieving the removed elements. This PR fixes the calculation of >> the current index. > > Michael Strauß has updated the pull request incrementally with one additional > commit since the last revision: > > changes as per review comments Looks good to me. - Marked as reviewed by arapte (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/478
Re: RFR: 8196065: ListChangeListener getRemoved() returns items that were not removed. [v9]
On Fri, 11 Jun 2021 20:26:24 GMT, Michael Strauß wrote: >> The documentation for `ObservableListBase.nextRemove` states that a single >> change always refers to the current state of the list, which likely means >> that multiple disjoint removed ranges need to be applied in order, otherwise >> the next change's `getFrom` doesn't refer to the correct index. >> >> `SelectedItemsReadOnlyObservableList` doesn't apply removals to >> `itemsRefList`, which means that subsequent removals will refer to the wrong >> index when retrieving the removed elements. This PR fixes the calculation of >> the current index. > > Michael Strauß has updated the pull request incrementally with one additional > commit since the last revision: > > changes as per review comments Done. - PR: https://git.openjdk.java.net/jfx/pull/478
Re: RFR: 8196065: ListChangeListener getRemoved() returns items that were not removed. [v9]
> The documentation for `ObservableListBase.nextRemove` states that a single > change always refers to the current state of the list, which likely means > that multiple disjoint removed ranges need to be applied in order, otherwise > the next change's `getFrom` doesn't refer to the correct index. > > `SelectedItemsReadOnlyObservableList` doesn't apply removals to > `itemsRefList`, which means that subsequent removals will refer to the wrong > index when retrieving the removed elements. This PR fixes the calculation of > the current index. Michael Strauß has updated the pull request incrementally with one additional commit since the last revision: changes as per review comments - Changes: - all: https://git.openjdk.java.net/jfx/pull/478/files - new: https://git.openjdk.java.net/jfx/pull/478/files/20fdb393..1672f4da Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=478=08 - incr: https://webrevs.openjdk.java.net/?repo=jfx=478=07-08 Stats: 53 lines in 3 files changed: 0 ins; 34 del; 19 mod Patch: https://git.openjdk.java.net/jfx/pull/478.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/478/head:pull/478 PR: https://git.openjdk.java.net/jfx/pull/478
