search

atom / fuzzaldrin

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

Give exact file name matches a perfect score. Fixes #11. #12

Closed jmacdonald closed 10 years ago

kevinsawicki commented 10 years ago

Thanks for this

kevinsawicki commented 10 years ago

After looking at this more, I think there was a different issue in #11, that issue was forward slashes always being used instead of the OS slashes, the paths in the screenshots were from a Windows machine.

I reverted this change in https://github.com/atom/fuzzaldrin/commit/3532a81d01337c8287adf24363902428157da060 since the spec still passed without it.

Was there some other case you were seeing it fail for?

jmacdonald commented 10 years ago

I was able to reproduce the issue when the exact match was deeper in the filesystem (within at least one directory) than the fuzzy match. The spec added in the PR fails without the corresponding implementation on OS X. I'm also able to see the issue in Atom itself. Is the spec passing without the implementation on your machine?

On Tuesday, September 2, 2014, Kevin Sawicki notifications@github.com wrote:

After looking at this more, I think there was a different issue in #11 https://github.com/atom/fuzzaldrin/issues/11, that issue was forward slashes always being used instead of the OS slashes, the paths in the screenshots were from a Windows machine.

I reverted this change in 3532a81 https://github.com/atom/fuzzaldrin/commit/3532a81d01337c8287adf24363902428157da060 since the spec still passed without it.

Was there some other case you were seeing it fail for?

— Reply to this email directly or view it on GitHub https://github.com/atom/fuzzaldrin/pull/12#issuecomment-54239820.

kevinsawicki commented 10 years ago

Yeah, it passed for me locally, as does master right now, also seems to pass on Travis, https://travis-ci.org/atom/fuzzaldrin/builds/34244283

kevinsawicki commented 10 years ago

Does master right now fail for you when you run npm test?

jmacdonald commented 10 years ago

In my infinite wisdom, I refactored the spec (for the sake of clarity) but forgot to test it without the implementation to ensure that it still broke without it. :disappointed:

The latest pull request (#14) fails without the provided implementation, as expected. :wink:

Sorry about the mix-up.