Skip to content

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Aug 19, 2024

This gives AutocompleteView subclasses the flexibility to have an implementation that's more than a single loop through candidates.

That will be useful for #234 / #889, /cc @sm-sayedi.

To keep the tricky aspects of the main search loop encapsulated, we move them into a helper function filterCandidates which those computeResults implementations can call.

Neither existing implementation of this method uses this parameter;
it seems like the use of it should be the job of testItem anyway,
not of getSortedItemsToTest.
This method, and its caller _startSearch, already assume that the
query they're given is the current `_query`.  Remove the parameter,
to simplify away redundancy and avoid possible confusion on how the
two query values relate.
I think I probably didn't know Dart had this feature when we
originally wrote this loop.
These two methods were in between compareByRecency and compareByDms,
which are closely-related siblings to each other.  So move them
outside that group.
This gives AutocompleteView subclasses the flexibility to have an
implementation that's more than a single loop through candidates.
@gnprice gnprice added the maintainer review PR ready for review by Zulip maintainers label Aug 19, 2024
@chrisbobbe
Copy link
Collaborator

Thanks, LGTM! Merging.

@chrisbobbe chrisbobbe merged commit cfce389 into zulip:main Aug 20, 2024
@gnprice gnprice deleted the pr-autocomplete-loop branch August 21, 2024 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer review PR ready for review by Zulip maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants