Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve palette "Fuzzy" matching ordering. #1182

Closed
Carreau opened this issue Oct 31, 2016 · 10 comments
Closed

Improve palette "Fuzzy" matching ordering. #1182

Carreau opened this issue Oct 31, 2016 · 10 comments

Comments

@Carreau
Copy link
Contributor

@Carreau Carreau commented Oct 31, 2016

When opening the command palette the first element is (for me) "Clear Cells".

I expect that if I were to Type "Cells" this would still be the first one (contiguous perfect match).
Though while typing the following happen:

  • Type character | First Element
  • C | Clear Cells
  • e | Clear Cells
  • l | Clear All Outputs
  • l | Cut Cell(s)
  • s | Cut Cell(s)

Having stability of element position through typing would be of great help to build a mental model of what is where, and how fuzzy-matching works. Here I'm getting the example of the first element, but I guess that would apply to any.

Also I'm unsure about how the virtual dom works. but having transition as well where element that don't match get filtered out and fave quickly (50 ms) away while the other one get a bit animated would be great.

@sccolbert
Copy link
Contributor

@sccolbert sccolbert commented May 8, 2017

This would be a change to Phosphor, not JupyterLab. I also believe we improved the fuzzy matcher logic since this issue was filed. So it may no longer be relevant.

In general, the problem with looking at only a single case for "what should happen" is that those same rules make other cases not work very well. We currently have something which we think works well over a wide-variety of use cases, while still remaining fast and taking into account the category labels.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented May 9, 2017

Now (around 0.20.3), I get:

  • C: Clear cells
  • E: Clear cells
  • L: Run Cell
  • L: Run Cell
  • S: Cut Cell(s)

There is a Clear All Outputs that is not getting picked up with "CEL" anymore.

@sccolbert
Copy link
Contributor

@sccolbert sccolbert commented May 9, 2017

@jasongrout can you post a screenshot for your last case?

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented May 9, 2017

screen shot 2017-05-09 at 2 50 03 pm

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented May 9, 2017

I think it makes a lot of sense to prioritize substrings at the top. I can imagine it is quite frustrating to type exactly "Cells" and have the first instance of that actual word show up at the bottom of the list, like above.

@sccolbert
Copy link
Contributor

@sccolbert sccolbert commented May 9, 2017

Well, results are still grouped by category. So in the above, Cut cell(s) is a stronger match than Run all Cells because the match appears earlier in the string. This means that any matches under Notebook Cell Operations also show up earlier, because they are in the same category with the best match.

To do it otherwise, would require duplicating categories, or removing them when searching.

@sccolbert
Copy link
Contributor

@sccolbert sccolbert commented May 9, 2017

Maybe we could have a "Best Matches..." synthetic category that holds the top-five across all categories or something...

@sccolbert
Copy link
Contributor

@sccolbert sccolbert commented May 9, 2017

Also keep in mind, that we match in category names too (we may want to change this), but that means those items like "Markdown Header 1" actually matched in the category, which is why they are included, but there's not real way to indicate that since we group by category.

@sccolbert
Copy link
Contributor

@sccolbert sccolbert commented May 9, 2017

I'm definitely open to improving the search algo, but I'd like people to propose concrete alternatives for the search algorithm, rather than just saying they don't like the current behavior. We did spend a good bit of time thinking about the current algo across a bunch of use cases, while also keeping in mind performance needs and implementation complexity.

@sccolbert
Copy link
Contributor

@sccolbert sccolbert commented May 15, 2017

This should be fixed in @phosphor/widgets version 1.2.0, which is available now.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants