Skip to content
This repository has been archived by the owner on Apr 1, 2020. It is now read-only.

Quick Open Case Sensitivity #825

Closed
CrossR opened this issue Oct 24, 2017 · 3 comments
Closed

Quick Open Case Sensitivity #825

CrossR opened this issue Oct 24, 2017 · 3 comments

Comments

@CrossR
Copy link
Member

CrossR commented Oct 24, 2017

As far as I can tell, the quick open window is Case sensitive?

If that is the case, I suggest instead that Oni copy Vims config options for ignorecase and smartcase.

  • Ignore case is fairly self explanatory, we just always ignore the case, IE a returns A or a.

  • Smart case is slightly different, you ignore case when a lower case string is given, but if you include upper case letters then it swaps to case-sensitive search.

    • IE apple returns Apple, apple, APPLE or any other combo, but searching for aPPle will only return aPPle.

I'm quite used to using just lower case search terms as I have this set in Vim, so having the quick open window work similarly would be nice.

After having a quick look, it looks like we explicitly pass over a "--case-sensitive" flag to ripgrep, so it would look like I can add a config option or two to pass over "--ignore-case" or "--smart-case" instead?

Assuming that sounds okay, what config options would we want?

@badosu
Copy link
Collaborator

badosu commented Oct 25, 2017

@CrossR I think this is related to the reason I filed #747.

Ripgrep is the default fuzzy finder used for quick open and it's hardcoded as case-sensitive, a solution to this issue would be to enable configuration of the default arguments to ripgrep. Or at least the case-sensitive part.

@bryphe
Copy link
Member

bryphe commented Oct 25, 2017

Ah it's actually a bit confusing (and good for us to clarify). Thanks for calling this out.

We actually only every call ripgrep once (it's really fast!), so I don't have it set up to utilize the ${searchString}, which would call it as the text is changed. So the case sensitive option we passed to ripgrep is effectively a no-op, since it isn't even using search parameters.

The logic that is impacting this is our filtering of results here:
https://github.com/bryphe/oni/blob/ba6c932c15204e4cac3302b25b7d205976b062fa/browser/src/Services/Menu/MenuReducer.ts#L114

And there are two phases of filtering there (after we get the results from the finder process, which defaults to ripgrep):

  • A 'pre-filter' pass where we check that all letters in the search string are present in the items
  • A 'fuzzy-find' pass where we use the fuse library to find and rank results.

The pre-filter pass is used for performance reasons, because running fuse can get slow with a large amount of results, so we try and rule some out ahead of time.

So there's really three phases:
(1) get results from the running process (Line 158 in QuickOpen.ts) -> (2) do the pre-filter pass (Line 114 in MenuReducer.ts) -> (3) run the fuse ranking and get the highlighted letters (Line 132 in MenuReducer.ts)

(2) and (3) are always run after the finder process returns results, so that we can rank + do the highlighting in the menu.

I was looking through this and it seems like the pre-filter pass (2) is case-sensitive, whereas the (3) fuse actually isn't, which is a problem since the behavior is confusing today and hard to explain. (And as mentioned above, the ripgrep process just dumps all the results, so it actually doesn't care about the casing at the moment).

I'm thinking we could do the following:

  • Add an option editor.quickOpen.caseSensitive - it can be either false, true, or "smart", to control the case-sensitivity of steps (2) and (3) in the pipeline, and the "smart" setting would behave as described above - all lowercase would be case-insensitive, but if there are upper-case letters, it would be case-sensitive.
  • Change the pre-filter pass to always be case-insensitive
  • Pick the right casing strategy for fuse based on the quickOpen.caseSensitive setting and the search input.

If we had that setting, it would at least help clarify the behavior. The only downside is that, if a custom editor.quickOpen.execCommand is specified, it could have its own casing strategy, separate from the editor.quickOpen.caseSensitive setting, which applies to the post-result filtering logic in (2) and (3). But I think that is reasonable.

Thanks for logging the issue @CrossR and the ideas @badosu ! I'll give it a shot tomorrow morning - let me know if you have any feedback on the proposal.

@badosu
Copy link
Collaborator

badosu commented Oct 25, 2017

@bryphe Defaulting case sensitivity to 'smart' would be a good idea I think (it would be good enough for me, and probably for most people).

Also, if someone is configuring execCommand it should be pretty clear for the person that it is not following the 'standard' configuration for the quickOpen functionality. Thanks for the clarification @bryphe, really good job!

To be frank I am not sure why we have this complicated setup, shouldn't ripgrep be fast enough to be called each time? At least that's how most of the Vim world uses a fuzzy-finder, but I should test both configurations to be sure it's better than what we have (probably not).

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

3 participants