Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Pick the dynamic filter from #353 #364

Merged
merged 55 commits into from
Apr 4, 2020
Merged

Pick the dynamic filter from #353 #364

merged 55 commits into from
Apr 4, 2020

Conversation

liuchengxu
Copy link
Owner

@liuchengxu liuchengxu commented Mar 30, 2020

I have cherry-picked the dynamic filter code from #353, and refacotor it a bit. There are still some details to deal with, but mostly works great in Vim.

[fixed] But I notice something weired in the top results returned during the dynamic update, see 00:30~00:43 in this gif https://asciinema.org/a/FT3magg2dk6VHIVg04PYgfIpw , the top first item looks right and is seemingly stable, while the other top N items looks far from the top first one. In the end, the results looks right suddently. I'm a little confused. @ImmemorConsultrixContrarie

@ImmemorConsultrixContrarie
Copy link
Contributor

But I notice something weired in the top results returned during the dynamic update, see 00:30~00:43 in this gif https://asciinema.org/a/FT3magg2dk6VHIVg04PYgfIpw , the top first item looks right and is seemingly stable, while the other top N items looks far from the top first one. In the end, the results looks right suddently. I'm a little confused.

Looks like a bug in sorting algo for temporary results. I could have done this sort wrong. Also some logic bugs could have crawled into this function or this macro.

@ImmemorConsultrixContrarie
Copy link
Contributor

ImmemorConsultrixContrarie commented Mar 30, 2020

Yeah, it's there.
The chain should be like .iter().enumerate().rev(), but .rev() goes before the enumerate.

@liuchengxu

@liuchengxu
Copy link
Owner Author

Yeah, it's there.
The chain should be like .iter().enumerate().rev(), but .rev() goes before the enumerate.

Yes, that's the culprit 👍, now it looks all right.

@liuchengxu liuchengxu merged commit c1cdfa7 into master Apr 4, 2020
@delete-merged-branch delete-merged-branch bot deleted the pick-dyn-filter branch April 4, 2020 04:23
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants