Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Regression in highlight with alternate scoring. #296

Closed
jeancroy opened this issue May 20, 2017 · 3 comments
Closed

Regression in highlight with alternate scoring. #296

jeancroy opened this issue May 20, 2017 · 3 comments

Comments

@jeancroy
Copy link
Contributor

Since about atom 1.1, the alternate scoring option is provided that decide both how to rank and highlight the result.

On current version, alternate scoring option only decide ranking, and it seems highlight is always rendered using classic fuzzaldrin. I think it's important to have both together as I consider highlight an explanation of current ranking.

It's very probable the behavior broke when switching the underlying component to remove jquery.
It's hard to detect because there's no test for it and one must be in knowledge of the algorithms to spot the differences.


Here's a recent example, typical of classic fuzzaldrin.

One can see that the word member is split because classic fuzzaldrin select first occurence of m then first occurence of e following that m then first m then ber

img

Here's a reference highlight with fuzz-aldrin-plus outside of atom. One can see that the algorithm try to group matches together instead of processing greedy left to right.

img

Now that algorithm isn't perfect and here we have an issue with -payment being before plain edit.
But the overall highlight seems more intuitive.

The reason I write this is that 'scattered letters' was by far the main complains with classic fuzzaldrin and complains stated to reappears.

@winstliu
Copy link
Contributor

const unmatched = path.substring(lastIndex, matchIndex)

@jeancroy
Copy link
Contributor Author

jeancroy commented May 21, 2017

Ok the problem is a bit complicated because there's code in place to use the proper matcher.

const matches = this.alternateScoring ?
fuzzaldrinPlus.match(projectRelativePath, filterQuery) :
fuzzaldrin.match(projectRelativePath, filterQuery)

However I can replicate

v1.15
screenshot_1

v1.17 (1.18beta looks the same)
screenshot_3

Both have alternate scoring turned on.
Maybe something broke the propagation of settings in 1.17 ?

@winstliu
Copy link
Contributor

Looks like a regression with the jQuery removal then. Thanks for catching this.

/cc @as-cii

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants