Add fuzzy matching to name completion#2563
Add fuzzy matching to name completion#2563przepompownia wants to merge 1 commit intophpactor:masterfrom
Conversation
przepompownia
changed the title
ShortNameFuzzilyMatchesTo Criteria
| return false; | ||
| } | ||
|
|
||
| $regex = '#' . implode('.*', array_map(preg_quote(...), str_split($this->name))) . '#i'; |
There was a problem hiding this comment.
Couldn't we move this to the constructor? That way we don't have to build the regex every time.
|
Alternatively you could try something less regex: function fuzzySearch(string $search, string $subject): bool
{
$index = -1;
foreach(str_split($search) as $char){
$index = stripos($subject, $char, $index + 1);
if ($index === false) {
return false;
}
}
return true;
}Would need some benchmarking though. |
|
First I needed to learn how to write my first benchmark for phpbench and used existing Regex fuzzy looks ~50% slower than lead string equivalent. |
|
Maybe more examples are needed - at the moment I do not se any significant difference: Details\Phpactor\Indexer\Tests\Benchmark\Model\Query\Criteria\ShortNameMatchesToBench |
|
As expected, the result of the compound check is between str_starts_with and str_starts_with + fuzzy (depending on example) |
|
so it's probably to remove although looks more elegant. |
|
I'd like to clean this PR from redundant implementations but waiting for comments from @dantleech before. |
|
Have been testing, and notice that with the PR it can no longer find
Without this PR:
|
|
Is there a test that checks if a full match still works with fuzzy matching? |
|
The hint where the response can be truncated by exceeding any time limit (on the server side) could be helpful for me. On the other side I need to prepare (probably with |
|
At the moment I want to defer expreiments but have the example #!/usr/bin/env bash
# see: man systemd.resource-control
# see: man systemd-run
systemd-run \
--quiet \
--user \
--pty \
--pipe \
--same-dir \
--property=PrivateUsers=yes \
--property=CPUQuota=10% \
--property=IOWeight=10 \
--setenv=XDG_CACHE_HOME="${XDG_CACHE_HOME}" \
-- \
"$(command -v phpactor)" "$@"that seems to be too good (at least to displaying help - I did not check if it communicates with my client): |
there is notthing in phpactor that will truncate the number of completion results based on time limit - or I don't understand the question. There is a limit to the number of completion results returned: Possibly this is important because we do some reflection/processing on each entry, so if we return 1000 or 10,000 or even 100,000 entries, then this could have a massive performance impact. |
|
You understood my question - at that moment I didn't suspect there is any amount limit. In practice there isn't unless the user has set |
|
It's so convenient that it's worth overcoming the obstacles. Maybe the fuzzy matching alternative ( IIRC PhpStorm had years ago an intermediate (in sense of results amount) form of matching that attach to uppercase characters. In that form https://www.jetbrains.com/help/rider/Coding_Assistance__Code_Completion__Smart.html#using-camelhumps |
|
Please test and suggest better names than the current 😀 |
|
@mamazu your proposition seemed to be easiest to convert to "camel matching". |
|
|
|
Hello, I'm using this branch for almost 3 weeks now, and here are a few thoughts:
Thanks for the work! PHP LOC |
|
Nice to read. Do you use the recent ("camel case") implementation? I was going to clean this PR from other implementations but still wait for opinions (especially from @dantleech and @mamazu). |
|
I don't think so, I cloned the repo on the 6th of March and didn't pull after that. I was using this commit all along: przepompownia@492bc9e5 |
|
Looks good to me, haven't tried it on my projects out yet. I think something that we should also maybe look into is trying to sort them by namespace. (Like own code before vendor code) that would also help if you have the same classes in both directories. |
Good idea but let do it in a separate PR. |
|
Yet one addition: |
|
I reduced this PR to "begin or camel-fuzzy" proposition to make it ready to review. If you want to compare the current matcher with any previously tried, switch back to 64b46aa. |
|
I need to test this, if there is any performance penalty or if it significantly changes the behavior it will need to be configurable. |
|
Please possibly change the way of this configuration (including especially renaming, spell mistakes, etc.). I started with a choice between the old and the new method but replaced it with a simple boolean option ( As a side effect I added tests for both methods: they revealed missing |
|
Hi, something new about this feature? 🙏 |
przepompownia
force-pushed
the
fuzzy
branch
7 times, most recently
from
ff4c56d to
fd9fd44
Compare
Co-authored-by: @mamazu Resolves phpactor#2562
5 participants
Co-authored-by: @mamazu
Resolves #2562