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

Allow _ and \ to match path separators #6

Merged
merged 1 commit into from
Jul 2, 2019

Conversation

rafeca
Copy link

@rafeca rafeca commented Jul 1, 2019

This PR addresses the feedback from @Fedexyz (comment) and @Songworks (comment) around matching full paths using _ or \\.

This is a common pattern in PHP, where in order to create namespaces, people either use underscores or \\ (for real namespacing) on the class names.

Usually, these namespaces match the folder structure, so a class named App_Controllers_MyController or \\App\\Controllers\\MyController will be located in src/App/Controllers/MyController.php.

For convenience, before the fast mode, people used to copy the classname from the editor and paste it on the fuzzy finder, which returned the result src/App/Controllers/MyController.php that the user expected (even if the matching logic was not very clear).

When we implemented the fast mode, this functionality was lost, so this PR adds the relevant logic to fuzzy-native (the library that the fast mode uses to match paths) to add it back.

Alternate designs

I've considered making this mode configurable, but this would add extra complexity on the logic, and since the previous fuzzyfinder matcher didn't have it configurable I thought it's not worth it to add an extra config option.

Copy link

@nathansobo nathansobo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a weirdly-worded comment that I noted, but the code looks great. Nice solution. When I looked briefly last week, concerns about exact matches involving _ getting buried gave me pause, but it looks like you've addressed that.

src/score_match.cpp Outdated Show resolved Hide resolved
src/score_match.cpp Show resolved Hide resolved
spec/fuzzy-native-spec.js Show resolved Hide resolved
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants