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

Limit search by delimiter (e.g. slashes between directories) #11

Closed
Pistos opened this issue Dec 7, 2022 · 15 comments
Closed

Limit search by delimiter (e.g. slashes between directories) #11

Pistos opened this issue Dec 7, 2022 · 15 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@Pistos
Copy link

Pistos commented Dec 7, 2022

junegunn/fzf#1706

My feature request is probably best explained by an example. Given the following lines:

app/models/foo/bar/baz.rb
app/models/foo/bar-baz.rb
app/models/foo-bar-baz.rb
app/monsters/dungeon/foo/bar/baz.rb

if I type app/mod/fo/ba/baz, I want fzf to filter down to app/models/foo/bar/baz.rb, and not include any of the others.

In other words, if I know in my head the dir structure of my project, and I want to get at a file that is under a specific subtree in the repo, I want to be able to use slashes to trim fzf's result set.

As fzf is now, when I type app/mod, it matches both app/models and app/monsters/dungeon because the letters mo are in monsters and d is in dungeon.

@natecraddock
Copy link
Owner

I just released zf 0.6.0 (updates to using zf as a library) and am now looking into implementing this. Here's some of my thoughts

The current function scans byte-by-byte for each token, and already stores state like whether or not the last match was a sequential character.

I think this feature would work like this

  • if the current query token character is a / then enable strict path matching for this token
  • scanning through the string for each token char continues as normal
  • if a string char equals / before the next / is found in the token then reject the match.
    • this indicates characters after a / or between /.../ that didn't match in a path

so with query /foo

/foo/bar - match
/fo/o    - reject
/f/oo    - reject
/a/foo   - match

I will experiment with this next week. I'm excited for this feature, and I think it complements zf's focus on filename matching. Thanks for the suggestion!

@natecraddock natecraddock added this to the 0.8.0 milestone Jan 14, 2023
@ratfactor
Copy link
Contributor

I really like this suggestion. Typing a / character "feels" different from typing letters fo to match foo. I think it signals a more specific intent.

This reminds me of the excellent smartcase option in Vim (and the default behavior of Ack and The Silver Searcher where a search that contains a capital letter causes the search to become case-sensitive. It's simple and intuitive because my intent to match that capital letter is pretty obvious.

Example of smartcase:

Searching for "foo" matches:
  foo
  Foobar
  MyFooBar
  myfoobar

But searching for "Foo" only matches:
  Foobar
  MyFooBar

@natecraddock
Copy link
Owner

@ratfactor thanks for calling out the intent here. That's an excellent way of describing it!

I really want to add this now, but adding unicode support will require changing some of the fuzzy finding algorithm, so I want to do that first.

@natecraddock natecraddock modified the milestones: 0.8.0, 0.7.0 Feb 5, 2023
@natecraddock natecraddock added the enhancement New feature or request label Feb 7, 2023
@natecraddock natecraddock self-assigned this Feb 7, 2023
@natecraddock
Copy link
Owner

natecraddock commented Feb 9, 2023

@Pistos I implemented this feature today and I have it in a feature branch. It was about as straightforward as I expected which makes me think I might have not considered all cases. But tests are passing so maybe its good!

Do you want to test it and give feedback? The branch is strict-path-matching https://github.com/natecraddock/zf/tree/strict-path-matching.

If you don't have a zig dev environment set up I can compile an executable for you if you want.

I also wouldn't be against any feedback from @ratfactor either!

@ratfactor
Copy link
Contributor

Oh my gosh, it's amazing! 😲 I understood what this was going to be in theory...but it's even better than I would have imagined in practice.

Because zf is interactive, once I match a directory, typing / visibly locks it in so I can keep searching deeper without worrying about the ancestor path accidentally matching stuff I add. Being able to incrementally build up a path like that is actually a huge gain.

I'm switching to this immediately. 😆

@natecraddock
Copy link
Owner

I love the enthusiasm! I agree, this is much more powerful than I first anticipated.

I just released version 0.7.0 with this feature included! Thanks again @Pistos for suggesting it!

@Pistos
Copy link
Author

Pistos commented Feb 12, 2023

I haven't tried it yet, but I will. Glad I could contribute.

@Pistos
Copy link
Author

Pistos commented Feb 13, 2023

So I gave this a test drive. Good work.

However, I see that it behaves slightly differently than what I'm used to. That's my fault for not being even more specific with my feature request. An addendum (or unwritten test 😉):

Given these files:

app/models/foo/bar/baz.rb
app/models/foo/bar-baz.rb
app/models/foo-bar-baz.rb
app/monsters/dungeon/foo/bar/baz.rb

I would expect that typing model/barbaz would filter down to:

app/models/foo/bar-baz.rb
app/models/foo-bar-baz.rb

In other words: I don't want to have to know every segment of the dir ancestry of the file I want to get to. If I know I've got a bar-baz file somewhere under the models/ dir tree (at some unknown depth), I'd like to be able to get at it just by typing something to filter to the models/ tree, then something for the filename.

Apply the same concept generally for any arbitrary subdir segments. For example, typing dung/bar/ (note trailing slash) should filter to

app/monsters/dungeon/foo/bar/baz.rb

I guess, programmatically, this means that / should match a lone /, but also any arbitrary string between slashes. (possible regexp: /\/.+\//)

If you want, I can make a separate github issue for this request.

@ratfactor
Copy link
Contributor

@Pistos

I would expect that typing model/barbaz would filter down to...

It's true that I do get three results rather than your expected two.

> model/barbaz 
app/models/foo-bar-baz.rb
app/models/foo/bar-baz.rb
app/models/foo/bar/baz.rb

But I'm not understanding why app/models/foo/bar/baz.rb should not match. I would expect that it would.

Apply the same concept generally for any arbitrary subdir segments. For example, typing dung/bar/ (note trailing slash) should filter to... app/monsters/dungeon/foo/bar/baz.rb

It does for me. Is this not what you're getting?

> dung/bar/
app/monsters/dungeon/foo/bar/baz.rb

@Pistos
Copy link
Author

Pistos commented Feb 13, 2023

No, I get this:

> model/barbaz
./app/models/foo-bar-baz.rb

and

> dung/bar

has no matches at all, whether I put a trailing / or not.

Maybe one of us is not using a binary built from the strict-path-matching branch (though I think that I am).

@Pistos
Copy link
Author

Pistos commented Feb 13, 2023

To be clear, here is the tree of the local files I've made for experimenting with:

% tree app
app
├── models
│   ├── foo
│   │   ├── bar
│   │   │   └── baz.rb
│   │   └── bar-baz.rb
│   └── foo-bar-baz.rb
└── monsters
    └── dungeon
        └── foo
            └── bar
                └── baz.rb

@Pistos
Copy link
Author

Pistos commented Feb 13, 2023

I'm not understanding why app/models/foo/bar/baz.rb should not match

Because, for a search input of model/barbaz, I'm expecting that barbaz should match only within one level of depth in the dir tree. foo/bar/baz.rb has "bar/baz" split across two levels.

If we split the input string into segments delimited by slashes, I want each segment only to match within one level of depth.

@ratfactor
Copy link
Contributor

Maybe one of us is not using a binary built from the strict-path-matching branch (though I think that I am).

Yup, it was me! I see how I accidentally reverted to a previous zf. Oops!

And you're right. I get the same results you do. And I agree, the path matching strictness before the slash I typed is exactly what I'd want. But it's too strict after the slash I typed.

It's like each slash should "lock" a portion of the path to the left, but still leave the portion to the right for fuzzier matching until the next slash and so on...

Search:
> dung/bar/
  | 1 | 2 |  3 (fuzzy)...

Match:
app/monsters/dungeon/foo/bar/baz.rb
|        1          |   2   |  3 (fuzzy)...
                    ^       ^
                 "lock"  "lock"

Does this sound right and/or make sense?

@natecraddock
Copy link
Owner

@Pistos thanks for taking the time to test and give feedback! Sorry I misunderstood the initial request. My solution worked for your original test case so I went ahead with it.

But when testing I had already identified a few cases in which I thought it could work better. I think the changes you are proposing here are really good. But they will be a bit more tricky to implement. I'm going to take some time to make sure I really understand this :) and think of a good way to fit it in the framework of the current matching algorithm.

If you want, I can make a separate github issue for this request.

I'll go ahead and make one.

@natecraddock
Copy link
Owner

natecraddock commented Feb 23, 2023

I spent a long time looking over this and I think I found a good solution. I wrote (a lot) about it in the new issue: #24

Again, y'all aren't required to, but I would appreciate some feedback! I think my new proposal handles all the cases and I really like its intended behavior. But I'm open to suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants