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

FR: filename prioritization, similar to zf #3608

Closed
5 of 10 tasks
chrisgrieser opened this issue Feb 1, 2024 · 11 comments
Closed
5 of 10 tasks

FR: filename prioritization, similar to zf #3608

chrisgrieser opened this issue Feb 1, 2024 · 11 comments

Comments

@chrisgrieser
Copy link

  • I have read through the manual page (man fzf)
  • I have the latest version of fzf
  • I have searched through the existing issues

Info

  • OS
    • Linux
    • Mac OS X
    • Windows
    • Etc.
  • Shell
    • bash
    • zsh
    • fish

Problem / Steps to reproduce

I recently stumbled upon zf, which prioritizes matches in the filename over matches in the filepath, which I found to be a subtle, but really useful improvement.

I think rather than using a separate fuzzy finder for filepaths (which also has far less features), it would be really useful to have this kind of sorting behavior added to fzf.

Currently, the one method to accomplish something similar would be --delimiter="/" --nth=-1, which restricts matching to the filename. This has the downside of ignoring matches in the directory path, instead of just deprioritizing them.

I could see something like adding an option --scheme=filename to opt-in this kind of behavior.

@junegunn
Copy link
Owner

junegunn commented Feb 2, 2024

  • A common trick is to prefix your query with /s
    • e.g. /foo or //foo instead of foo
    • fzf --filepath-word --bind 'ctrl-/:backward-word+put(/)+forward-word'
  • Also, you might want to see if --tiebreak end helps in your case.

@chrisgrieser
Copy link
Author

Both help a little bit, but still leave various cases where it does not work.

  • /foo does not work to find files in the root, but includes files with folder/foo/filename
  • --tiebreak end does not seem to help that much, when searching for "foo", foo/bar/filename still gets prioritized over folder/foobar for example

@junegunn
Copy link
Owner

junegunn commented Feb 2, 2024

  • A common trick is to prefix your query with /s

Oh, I think this was not the right advice I had in mind. Ignore that. I usually tell users to append . to the query.

# Append `.` to the query to give a higher score to the `f.` in the name part of the path
fzf --query f. << EOF
src/foo/bar.go
src/bar/foo.go
EOF
  • --tiebreak end does not seem to help that much, when searching for "foo", foo/bar/filename still gets prioritized over folder/foobar for example

That's because fzf by default gives a greater bonus point to the character at the very beginning. --scheme path is an option that makes the bonus points equal, so it helps in this case.

fzf --scheme path --tiebreak end --query foo << EOF
foo/bar/baz
baz/foo/bar
bar/baz/foo
foo/bar/filename
folder/foobar
EOF

@chrisgrieser
Copy link
Author

ah yes, indeed, appending . does work much better. --scheme=path works slightly better, but not too much though.

Would it maybe make sense to modify --scheme=path so that it automatically uses --tiebreak=end and acts as if there is a trailing . in the query?

The . trick is not discoverable for users, and something like filename prioritization is something I'd have expected to happen with --scheme=path.

@junegunn
Copy link
Owner

junegunn commented Feb 2, 2024

--scheme=path works slightly better, but not too much though.

The second example shows that it works pretty well when combined with --tiebreak=end even without the trailing .. Can you explain what you mean by "not too much"?

Would it maybe make sense to modify --scheme=path so that it automatically uses --tiebreak=end and acts as if there is a trailing . in the query?

Setting --tiebreak=end is possible though I'm not sure if I agree with that decision probably because I've never really felt the need for it. I don't use it. Most of the time the default tiebreak of length happens to work well for me with a list of files in a project. And if you're familiar with the search syntax of fzf (using multiple out-of-order terms), and some tricks like . suffix, it's not really hard to pinpoint the item you have in mind.

The second proposal is not feasible because it requires making a non-trivial exception to the scoring mechanism of fzf.

@chrisgrieser
Copy link
Author

And if you're familiar with the search syntax of fzf (using multiple out-of-order terms), and some tricks like . suffix, it's not really hard to pinpoint the item you have in mind.

yeah, I am familiar with the syntax (just not using the correct terms, since I am working with too many selector-implementations which all use different terminology). While you can achieve the desired behavior by appending ., I personally think that filename prioritization is something you would always want to have on if you prefer this behavior.

Having a flag that one could add to FZF_DEFAULT_OPTS would make sense to me. And the fact that zf exists indicates to me that there are probably more users who think that filename prioritization is a sensible default.

@junegunn
Copy link
Owner

junegunn commented Feb 2, 2024

Unfortunately, we don't have an objective measure to evaluate the quality of the results. There are so many different cases and different expectations from different users with different needs.

I'm not familiar with zf, but the example is not really convincing. Is /docs/MappingProtocol.md a better match than /src/app/main.c for the query app because app is in the file name? I don't agree.

The problem with --tiebreak=end is similar. It doesn't always yield better results.

fzf --tiebreak end --query re << EOF
docs/README.md
vendor/some/directory/too/far/from/the/root/that/you're/not/interested/regex.c
EOF

This makes me think the default --tiebreak=length (with . trick) is a better choice.

Having a flag that one could add to FZF_DEFAULT_OPTS would make sense to me.

I don't think it's a good idea to put anything that affects the scoring mechanism in FZF_DEFAULT_OPTS because fzf is a generic text filter. A set of options that works well with a specific type of input, such as file paths, is not guaranteed to work well with different types of input, such as a list of processes (ps -ef | fzf), command history (history | fzf), commit hashes, etc. In the same vein, I discourage users from putting --preview option in FZF_DEFAULT_OPTS.

@chrisgrieser
Copy link
Author

chrisgrieser commented Feb 3, 2024

Unfortunately, we don't have an objective measure to evaluate the quality of the results. There are so many different cases and different expectations from different users with different needs.

Agree, which is why I suggested an option – users who prefer a certain kind of behavior can enable the options, and those who don't, leave it disabled. The fact hat zf exists and seems to have a decent number of users indicates that at least some people prefer this kind of matching behavior.

I don't think it's a good idea to put anything that affects the scoring mechanism in FZF_DEFAULT_OPTS because fzf is a generic text filter.

Sorry, FZF_DEFAULT_OPTS was a bad example. Nevertheless, having an option would still be beneficial, since you could add the option to your aliases/functions that use fzf.

@junegunn
Copy link
Owner

junegunn commented Feb 4, 2024

Agree, which is why I suggested an option

My point is that they will not benefit much from the new option contrary to their belief, because the option will not universally yield better results than the default. So I think they will benefit more in the long run by getting used to the default matching/scoring mechanism of fzf and learning tricks to make better use of it. I can understand their desire to tweak the settings, but as the maintainer of the project, I have to be careful not to add options that have minor benefits and have the potential to make the program harder to understand and confuse the users. Also, it would be easier for users to understand the matching/scoring scheme of fzf, if it worked consistently across different invocations.

@junegunn
Copy link
Owner

junegunn commented Feb 4, 2024

Having said that, here's a PoC patch, you can experiment with it.

diff --git a/src/options.go b/src/options.go
index dcb70dc..b643c26 100644
--- a/src/options.go
+++ b/src/options.go
@@ -153,6 +153,7 @@ const (
 	byLength
 	byBegin
 	byEnd
+	byPathname
 )
 
 type heightSpec struct {
@@ -547,8 +548,11 @@ func processScheme(opts *Options) {
 	if !algo.Init(opts.Scheme) {
 		errorExit("invalid scoring scheme (expected: default|path|history)")
 	}
-	if opts.Scheme == "history" {
+	switch opts.Scheme {
+	case "history":
 		opts.Criteria = []criterion{byScore}
+	case "path":
+		opts.Criteria = []criterion{byScore, byPathname, byLength}
 	}
 }
 
@@ -775,6 +779,7 @@ func parseTiebreak(str string) []criterion {
 	hasLength := false
 	hasBegin := false
 	hasEnd := false
+	hasPathname := false
 	check := func(notExpected *bool, name string) {
 		if *notExpected {
 			errorExit("duplicate sort criteria: " + name)
@@ -791,6 +796,9 @@ func parseTiebreak(str string) []criterion {
 		case "chunk":
 			check(&hasChunk, "chunk")
 			criteria = append(criteria, byChunk)
+		case "pathname":
+			check(&hasPathname, "pathname")
+			criteria = append(criteria, byPathname)
 		case "length":
 			check(&hasLength, "length")
 			criteria = append(criteria, byLength)
diff --git a/src/result.go b/src/result.go
index b042829..76774a1 100644
--- a/src/result.go
+++ b/src/result.go
@@ -3,6 +3,7 @@ package fzf
 import (
 	"math"
 	"sort"
+	"strings"
 	"unicode"
 
 	"github.com/junegunn/fzf/src/tui"
@@ -67,6 +68,15 @@ func buildResult(item *Item, offsets []Offset, score int) Result {
 			}
 		case byLength:
 			val = item.TrimLength()
+		case byPathname:
+			if validOffsetFound {
+				lastDelim := strings.LastIndexByte(item.text.ToString(), '/')
+				if lastDelim < 0 {
+					val = 0
+				} else if lastDelim < minBegin {
+					val = 1
+				}
+			}
 		case byBegin, byEnd:
 			if validOffsetFound {
 				whitePrefixLen := 0

@chrisgrieser
Copy link
Author

I can understand their desire to tweak the settings, but as the maintainer of the project, I have to be careful not to add options that have minor benefits and have the potential to make the program harder to understand and confuse the users.

I see. Yeah, that I can understand.

@chrisgrieser chrisgrieser closed this as not planned Won't fix, can't repro, duplicate, stale Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants