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

Allow fuzzy searching in Quick Switch modal #204

Merged
merged 1 commit into from Jul 17, 2017

Conversation

Projects
None yet
2 participants
@dmlittle
Contributor

dmlittle commented May 13, 2017

What

  • Allow fuzzy search in request Quick Switch modal

Why

  • Makes searchString a little more forgiving
  • Allows matching of multiple ordered substrings in request names without having to type everything between substrings.
    • For example, if you have a request called (Payments) Refund charges for specific account you may search for p refund account to find such request.

Before

screen shot 2017-05-13 at 10 56 09 am

After

screen shot 2017-05-13 at 10 55 42 am

Details

N/A

Notes

I originally tried a more algorithmic approach of traversing both string to figure out if the searchString characters exist in order in the matchedRequests strings but this was significantly slower than constructing and using a regular expression (18ms vs. 1ms per matchedRequest)

Related Issue: N/A

@dmlittle

This comment has been minimized.

Contributor

dmlittle commented May 13, 2017

I'm not sure if we want it, but we could also add a checkbox in the modal to enable/disable fuzzy searching.

@gschier

This comment has been minimized.

Collaborator

gschier commented May 13, 2017

@dmlittle I like this. Is the same fuzzy behavior as text editors like Atom or Sublime implement? I know that IntelliJ has a slightly less fuzzy (more advanced?) matching algorithm.

It would also be good to see some tests around this. Currently, no code within React components is tested. You could extract the matching function into app/common/misc.js and add tests there. Should we also apply this fuzzy matching to the sidebar filter?

As for the checkbox. It's probably fine to leave it out for now. We can add it if it becomes a problem. I just checked the analytics and less than 2% of users have used this feature.

image

@dmlittle

This comment has been minimized.

Contributor

dmlittle commented May 13, 2017

@gschier I just took a quick look with Sublime and Atom and it's very similar with the exception that Atom also ignores whitespace (searching for p refund and prefund would yield the same results). I'm not sure how IntelliJ works. I don't have any preference as to ignore whitespace or not so I'll leave that decision to you.

As for the sidebar, I think it makes sense to also add this functionality there.

Let me know what you think of the whitespace issue and I'll refactor the changes to extract the fuzzy functionality into a separate file where we can test it's functionality.

I'm not surprised many people don't know about this feature as I only learned about it while reading some of the source code. But now that I know it exists, I'll definitely be using it regularly.

@dmlittle

This comment has been minimized.

Contributor

dmlittle commented May 13, 2017

Also, we could change the UI to show the fuzzy searching in action if we don't make each request name bold and we only bold the matching letters.

@gschier

This comment has been minimized.

Collaborator

gschier commented May 13, 2017

The implementation you have now is probably good enough. Better to ship it now and improve it later, if needed.

Highlighting matches in the sidebar would be awesome but could probably be done afterwards (unless you're feeling ambitious). Similar behavior is already done in the autocomplete code if you want to take a look at that. (In environment-autocomplete.js I think.)

image

@dmlittle

This comment has been minimized.

Contributor

dmlittle commented May 13, 2017

Currently filtering works if you either have a substring that is in a request name or if you provide a matching id. Is there a reason for the id matching clause? As far as I can tell these ids are hidden from the user so I'm not sure why the logic was implemented. I think we can remove the id logic but I want to make sure it was for a specific reason I don't know about.

// Match substring of name
const matchesName = name.indexOf(toMatch) >= 0;
// Match exact Id
const matchesId = id === toMatch;
return matchesName || matchesId;

@gschier

This comment has been minimized.

Collaborator

gschier commented May 14, 2017

@dmlittle I can't remember why the ID match is there but, you're right, it can safely be removed. 👍

I think I may have used it early on when debugging database things.

@gschier gschier merged commit da90cc3 into getinsomnia:develop Jul 17, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@gschier

This comment has been minimized.

Collaborator

gschier commented Jul 17, 2017

@dmlittle merging this in for now. Can be improved later if desired 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment