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

Fuzzy matching instead of startsWith for filtering #734

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

WebFreak001
Copy link
Member

@WebFreak001 WebFreak001 commented Mar 20, 2023

Experimental, subject to change

This way, when you have a function parseJsonString and you only type parseString or search for jsonString, you will find the symbols.

Additional benefit of this: we are losing a bunch of GC allocations for every request. I haven't measured what kind of improvements we would get from this yet though.

Note that without the IDE/editor sorting imports in a decent manner, this will quickly overwhelm users auto-complete.

With VSCode's default sorting and filtering this works very well though.

Feel free to give this PR a try and give your feedback. (cc @ryuukk @vushu)

Ideally for code cleanliness, prettyFuzzyMatch should not be part of dsymbol, but only of dcd.

Side-note for UFCS in dsymbol causing the fuzzymatch dependency in there:

I think we should move out the UFCS auto-complete / cursor token matching code out of dsymbol into DCD, and only let dsymbol do all the UFCS processing, e.g returning UFCS symbols joined into the regular symbols using the usual (or to-be-created) listing APIs. (indicating the UFCS symbols just using flags)

I noticed that the UFCS matcher also just started to reimplement bunch of stuff from DCD and still misses things, such as recognizing int as partially typed identifier. Splitting the matcher from UFCS symbol generation and moving just the matcher into DCD's existing matching code would deduplicate a lot there.

@vushu
Copy link
Contributor

vushu commented Mar 20, 2023

Very cool, makes the completion feel more modern, I discovered that it doesn't fuzzy match on UFCS function name lolexex

int lolexex(int x)
{
    return 5;
}

void main()
{
    int number = 42;
    number.
}

i try to write xex but doesn't match with lolexex function name shouldn't it?
I agree that we could move some of the logic out to DCD. Reimplementing some of the stuff was a good learning experience. Generally I think the DCD's business logic is hard to read and sometimes I'm not sure WTH is happening.

@rikkimax
Copy link
Contributor

Note that without the IDE/editor sorting imports in a decent manner, this will quickly overwhelm users auto-complete.

If this can produce too many results, it should probably be opt-in and let dcd-client pick the ones that it thinks are the most likely.

@ryuukk
Copy link
Contributor

ryuukk commented Mar 20, 2023

I don't know what fuzzymatch does, i took a look quickly and i suggest not to use a dub dependency wich does useless downloads only just for 1 function..

https://github.com/WebFreak001/FuzzyMatch/blob/master/source/fuzzymatch.d

Copy paste the function, better

@vushu
Copy link
Contributor

vushu commented Mar 20, 2023

The idea is to proably use FuzzyMatch in other spaces, hence packaging it within a library is most pratical for distribution and for updating future changes.

@WebFreak001
Copy link
Member Author

yeah I copy pasted the function out of serve-d, where it has worked pretty well already for some auto-complete extensions. Making it a package is the cleanest thing and I think it makes sense to share this kind of logic between IDE tooling. (+ optimization only needs to be done once for these kinds of potentially error-prone unicode handling functions)

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.

4 participants