Query with slashes.

[chamini2]

As instructed in atom/fuzzaldrin#27, opened this issue where it's more relevant:

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:

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"

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.

[jeancroy]

After some test, appcon works, con appcon works. con/appcon doesnt.

Basically there is a importance boost for matching file-name, and when query contain slashes, I promote the end of path as if it where the file name. (A query with n slashes will promote n folder from the end of the path)

It often serves us well but here it doesn't. Two things I can see:

1. config/application.rb is promoted as a result that is 100% filename.
2. It is also shorter. This is important because from other user report a length difference in filename is more important that one in full path.

[jeancroy]

Changed the title of the issue because this looks like it would require quite a bit of change, and I'd like to see if there's other issue related to slashes.

One thing I consider is removing the end-of-path bonus that come with con/appcon vs con appcon.
Some of the reason it was here:

1. When there is a slash we get extra insight on the structure of the match, we may as well use it.
   1. The score rely a lot on consecutive character. When query contain a space, the consecutive count reset folder then file. When query contain a slash it traverse to to include both folder/file. A end of path bonus that contain only the file-name will miss this, so a middle of path match can over-rank a end of path one.

[chamini2]

I think a way to maybe fix this is not promote the idea of n slashes looks for paths with exactly n slashes in them. I do understand the idea behind this, but sometimes we don't remember the full path, just some of the key directories to match what we want. So maybe keep matching shorter paths, in this case the fact that con matches consecutive characters, may give application_controller.rb the needed extra points.

It's just an idea, though, if the current behaviour works best in most scenarios, we could just leave it as it is.

[jeancroy]

Yea I tried to remove it, some test are failing, but they are design test, instead of user report tests.
Here's one I find useful, IE explicitly tell you want the folder.

it "allows to select between full query and basename using path.sep", ->

      candidate = [
        path.join('models', 'user.rb'),
        path.join('migrate', 'model_users.rb')
      ]

      expect(bestMatch(candidate, "modeluser")).toBe candidate[1]
      expect(bestMatch(candidate, "model user")).toBe candidate[1]
      expect(bestMatch(candidate, path.join("model","user"))).toBe candidate[0]

[chamini2]

That test probably should be passing, did you completely remove the path separator check?

Maybe if given an input with n slashes, instead of "find paths with exactly n slashes", do "find paths with _at least_ n slashes". Or that was what you did?

The slash and what was before and after it is important, maybe not the number of shashes.

[jeancroy]

The code in question is here

Basically there is two scoring being done.

1. One scoring pass on the full path.
2. And one scoring pass on the end-of-path.

So when the query is con appcon we compare

1. con appcon against the full app/controllers/application_controller
2. con appcon against the application_controller

By default the end-of-path scoring is resilient to error. It cannot find the first part of the query con, but we basically have a no point / no penalty way to handle that missing part.

So when the query is con/appcon the n folder backward kick in. we compare

1. con/appcon against the full app/controllers/application_controller
2. con/appcon against the controllers/application_controller

You see that it's doing it's job in a logical way. Matching is improved and we don't have to rely on the resilient mechanism for the end-of-path to works.

If you don't know the correct number of folder. Say path is control/test/app_con.

So when the query is con/appcon we do comparison:

1. con/appcon against the full control/test/app_con
2. con/appcon against the test/app_con

Now we cannot find con inside test. However the resilient mechanism of filename only case (n=0) kick in. I'll add the the comparison (1) will be better because we prefer match at the start of the string.

So basically going back n folder + resiliency when not finding part of query match you at-least-n expectation. application_controller benefit from going backward n folder. Unfortunately right now the config/application benefit more.

Part of the story is that people expect foo/bar/baz/fizzbuzz/application to win against foo/bar/baz/application-spec : length penalty is more severe in the zommed-in filename.

[jeancroy]

I'll close this issue. Mostly because it require a lot of change and the problem is one character away from solving itself.

If anyone see problem with slash in query in a more realist scenario i'll reopen