On Monday, June 02, 2014 11:27:26 AM Lior Vernia wrote: > After discussing this with Alex on another thread, I just pushed this: > http://gerrit.ovirt.org/#/c/28268/ > > If it's approved, then it'll take care of the secondary criterion for > you, so you only need to pass the criterion that actually interests you. > Should simplify our work in the next couple of weeks. >
Okay, I think we need to get everyone on the same page with this, right now we have several solutions to the problem, and I think we should pick one and go with that. Right now we have the following solutions available: 1. Attempt to keep the current order in case of 'duplication' in the comparator. So the fallback is to keep the current order. This is implemented in Liors patch [1]. 2. Use the hash code as a fallback in case of 'duplication' in the comparator. Due to hashcode rules this should result in the 'natural' order in case of duplicates. This is implemented in my patch [2]. 3. Do ad-hoc fallback mechanism which can specialize to do the proper thing based on domain knowledge. It looks like this is what Anmol is doing in his gluster sorting patch [3] 4. Replace the SortedSet with a List but lose the ability to automatically sort when calling getItems().add(X). To me it seems that 3 and 4 are off the table as we want to keep the ability to automatically sort. And doing ad hoc solutions for every single sorting column is going to take a lot of time and is going to lead to maintainability issues down the road. That leaves 1 and 2 on which I would like to have a discussion, so we can pick the appropriate method and go forward with that. [1] http://gerrit.ovirt.org/#/c/28268/ [2] http://gerrit.ovirt.org/#/c/28225/ [3] http://gerrit.ovirt.org/#/c/28233/ > On 31/05/14 15:35, Anmol Babu wrote: > > Thanks a lot Alexander.Your idea of having a second criteria based sorting > > just in case of comparing same values(compare returning 0) looks good to > > me and now, I have also done the same in my patch set > > http://gerrit.ovirt.org/#/c/28233/.I have added you as a reviewer as > > well. Thanks, > > Anmol > > > > ----- Original Message ----- > > From: "Alexander Wels" <aw...@redhat.com> > > To: devel@ovirt.org > > Cc: "Anmol Babu" <anb...@redhat.com> > > Sent: Friday, May 30, 2014 5:55:20 PM > > Subject: Re: [ovirt-devel] Sortable Ui Columns > > > > Anmol, > > > > This is due to the fact that the sorting is done by a SortedSet instead of > > a list in the SortedListModel. To fix this we have to do one of two > > things: > > > > 1. Change the sorting to use a list of some sort, but apparently that is > > not much of an option as people want automatic sorting when they add new > > items to the collection. > > 2. Make sure that the comparator never returns 0 for two entities that are > > really not the same. The reason you are seeing the issue is because you > > are > > using a different comparator that does return 0 on whatever field you are > > comparing against. I had the exact same issue and I solved it by using a > > compound comparator. Basically what I did was create a comparator that > > contains the field I am trying to compare on and if that returns 0 then I > > use a different comparator that is guaranteed to not return 0 for the > > entity. > > > > I have a patch that implements sorting for all the data center main tab > > and > > sub tabs here [1] that demonstrates how I solved the issue. This patch > > hasn't been reviewed yet, and my solution might get rejected, but it does > > work and doesn't make your entries disappear when you sort. > > > > > > [1] http://gerrit.ovirt.org/#/c/28225/ > > > > On Friday, May 30, 2014 02:32:17 AM Anmol Babu wrote: > >> Hi, > >> > >> While applying client-side sort using the sorting infra, to the > >> "Server" > >> > >> column of the "Volumes" sub tab "Bricks", I had 2 Bricks with same server > >> name.So,when I sorted it, it removed one of the bricks that had the same > >> server name. I found that this issue occurs when the sort values compared > >> are same(i.e, comparator's compare returns 0). Regards, > >> Anmol.B > >> _______________________________________________ > >> Devel mailing list > >> Devel@ovirt.org > >> http://lists.ovirt.org/mailman/listinfo/devel > > > > _______________________________________________ > > Devel mailing list > > Devel@ovirt.org > > http://lists.ovirt.org/mailman/listinfo/devel _______________________________________________ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel