atom / fuzzaldrin

Fuzzy filtering and string scoring
MIT License
319 stars 28 forks source link

Directory depth penalty #27

Closed chamini2 closed 9 years ago

chamini2 commented 9 years ago

I understand the scoring algorithm uses some sort of penalty to a file if it was too deep inside the directory structure, but sometimes this doesn't make sense, I'll post an example and explain:

image

As you can see, it now matches the probably expected file, but when we want to filter by saying "it's in the controllers/ directory"

screen shot 2015-11-02 at 5 08 01 pm

It penalizes the application_controller result because directory depth, or at least that's my guess. So why not start the deepness count of at the first directory match, in this case in the controllers/ directory.

winstliu commented 9 years ago

Have you tried the new alternate scoring method present in Atom 1.1.0? The new scoring method uses fuzzaldrin-plus.

jeancroy commented 9 years ago

So why not start the deepness count of at the first directory match, in this case in the controllers/ directory ?

From what I understand, directory dept penalty can be useful because user may not be aware of what file exist at very deep level. Additionally they may exist as vendor dependency (for example nested node_modules)

Deep path can trigger match length penalty, but not necessarily. Let's take for example : etc/lib/src/config/application_config.cfg. Your proposal would then make that file as good as any file one folder down the root, thus making the above directory depth likelyhood useless.

BTW your proposal would basically be equivalent to minimizing match spread which is an useful but different metric.

I hope that cover the why.

chamini2 commented 9 years ago

This happens using the alternate scoring option, thought I stated it in the original post.

But I do understand the reason @jeancroy, makes sense, thanks for the response.

jeancroy commented 9 years ago

Ok added a test, your use-case works without any change on fuzzaldrin-plus@0.2.0 which adjust some weights vs what is currently in atom.

Issue should probably be closed, if you have another issue please open it on fuzzaldrin-plus

chamini2 commented 9 years ago

The test you added is missing the failing case:

 expect(bestMatch(candidates, 'con/appcon')).toBe candidates[1]
jeancroy commented 9 years ago

Ok sorry for that, I'm working on it. Could you open an issue in the proper repo ? (fuzzaldrin-plus, not classic fuzzaldrin)