Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

QuickOpen Filter #204

Merged
merged 17 commits into from
Apr 9, 2019
Merged

QuickOpen Filter #204

merged 17 commits into from
Apr 9, 2019

Conversation

CrossR
Copy link
Member

@CrossR CrossR commented Mar 31, 2019

Started to work on the QuickOpen filter stuff.

This adds a basic filter.
There is a few other things we can do to make it feel better:

  1. Split up the paths into name + description (ie src/Editor/Core and Filter.re) such that we can match on both and combine the score, and give a higher score for name matches. The logic there might be a bit awkward, but should make it feel nicer.
  2. Hook up index highlights.
  3. Look into "smart" search settings, since there is definitely times I felt the search should have come back with my file, but it was getting a lower than expected score due to the case.

@romgrk
Copy link
Contributor

romgrk commented Apr 1, 2019

One good matching algorithm to take inspiration from could be fzy (explanation here).

@bryphe
Copy link
Member

bryphe commented Apr 1, 2019

It already feels really good @CrossR ! Awesome to see it hooked up with the work @Akin909 did. I believe we'll be able to offer a better experience than Oni1 performance-wise with QuickOpen.

Split up the paths into name + description (ie src/Editor/Core and Filter.re) such that we can match on both and combine the score, and give a higher score for name matches. The logic there might be a bit awkward, but should make it feel nicer.

Cool - this sounds like it'll help a lot.

Hook up index highlights.

Yeah! I miss this too! We don't have good font support for bold/italic at the moment - but we could at least color the matches differently.

Look into "smart" search settings, since there is definitely times I felt the search should have come back with my file, but it was getting a lower than expected score due to the case.

Makes sense. I really liked the 'refine' functionality you added in QuickOpen for Oni1 too - that's something we could consider adding in the future.

It looks like you'll need to update the bench lockfile too (via esy '@bench' install - seems like thats why the CI is failing).

@CrossR CrossR marked this pull request as ready for review April 5, 2019 21:15
@CrossR
Copy link
Member Author

CrossR commented Apr 5, 2019

With smart search enabled, the search feels a lot better, to me at least.

The two other issues outlined in the first post can be added in a follow up (if that is okay with everyone). I'd like to get his merged so we have some form of QuickOpen since I'm missing having one in general!

reasonFuzz does have a fuzzy match alg that will return the indexes for the highlighting, its just waiting for me to write a compareScores equivalent for it (indexCompareScores etc), which should let that work once the style of the menu is updated accordingly.

The splitting is still potentially useful, but the smart search seems to have fixed the issues I was having in the Oni2 repo at least.

@CrossR CrossR requested a review from bryphe April 9, 2019 18:10
Copy link
Member

@bryphe bryphe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @CrossR - thank you!

@bryphe
Copy link
Member

bryphe commented Apr 9, 2019

The two other issues outlined in the first post can be added in a follow up (if that is okay with everyone). I'd like to get his merged so we have some form of QuickOpen since I'm missing having one in general!

Sounds reasonable! I'll merge this in so we can get going with this implementation 👍 Thanks @CrossR !

@bryphe bryphe merged commit d9231dd into onivim:master Apr 9, 2019
@CrossR CrossR mentioned this pull request Apr 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants