Remove duplicate entry #348
Conversation
|
Union won't work like this. This Union will create a IEnumerable that has been unioned, but not put the results back to the list. It would be better to use HashSet if you want to do the same job. |
|
Or actually using an distinct will work, but will create another hashset to do the same job, so it would be better to simply do that with a hashset. |
|
I actually think we should maybe do this so all plugin results will benefit, so plugins dont have to worry about dup results. What do you think |
Agree with you, it is easy to do that. Just adding a distinct to the NewResults method should work |
Add distinct to NewResults to try avoiding duplicate result
|
@jjw24 done, please take a look |
taooceros
changed the title
|
|
||
| if (quickaccessLinks.Count > 0) | ||
| results.AddRange(quickaccessLinks); | ||
| results.Union(quickaccessLinks); |
There was a problem hiding this comment.
Distinct hasnt solved it yet, this change has been reverted. We probably need to write our own comparer
There was a problem hiding this comment.
Yeah I think you are right. By seeing the code of the equality comparator of Result class, it requires more than just title match. Maybe we still need to do that separately for plugins instead of distinct overall because we seldom will add two results that will be identical (they need to be equal at title, score, and highlight data) to the result list.
There was a problem hiding this comment.
yeah i agree. Perhaps we can create and add it to IPublicAPI so plugins can utilise the return distinct results themselves
There was a problem hiding this comment.
Well I don't think it should be included in IPublicAPI, because how they should filter the result should be decided by them. If we allow Equaltycomparater for that, I don't see the different between putting that in IPublicAPI and doing that by distinct or hashset.
There was a problem hiding this comment.
Let me add a customized add a customized equality comparator to do this job by comparing the subtitle (it is supposed to be the path) to remove duplicate for this one.
|
Done, please take a look @jjw24 |
|
should we revert commit 4a3ad08? |
Yeah we can do that. Seems that a simple distinct doesn't work well. |
|
@taooceros good to roll? |
When result exists from both Quick Access and file search, use unionwith and custom comparer instead of append, so this will return distinct results of duplicates