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

Make max query length configurable #4292

Closed
5 of 10 tasks
alex-huff opened this issue Feb 26, 2025 · 10 comments
Closed
5 of 10 tasks

Make max query length configurable #4292

alex-huff opened this issue Feb 26, 2025 · 10 comments

Comments

@alex-huff
Copy link
Contributor

Checklist

  • I have read through the manual page (man fzf)
  • I have searched through the existing issues
  • For bug reports, I have checked if the bug is reproducible in the latest version of fzf

Output of fzf --version

0.60 (devel)

OS

  • Linux
  • macOS
  • Windows
  • Etc.

Shell

  • bash
  • zsh
  • fish

Problem / Steps to reproduce

I realize that a similar issue was already opened and closed ~7 years ago in #1312. At the time, the reasoning for not allowing queries longer than 300 made sense.

I don't think it's worth the effort. What would be the point of allowing exceptionally long query strings? Also, the time complexity of the search algorithm of fzf is proportional to the length of the query, so having some limit makes sense in that regard as well.

From what I can see the --disabled option did not exist at this time. Because all queries were used for fuzzy finding having super long queries did not make much sense. Now that queries can be used for basically anything, this limit seems arbitrary in some scenarios.

For example when using fzfrepl as a live preview for an ffmpeg filtergraph: #1312 (comment)

Recently, I also hit the 300 character limit when trying to search through all the files owned by a package on my system. I use fzf as a front-end to ripgrep like many others. With my rg-fzf script I specify the files to be searched as command-line arguments but I can edit the files being searched along with ripgrep options from the fzf query.

For example if I run:

rg-fzf src man

the following query is generated:

Image
Everything before ;; is a python expression that builds the ripgrep command to be run. It's a bit of a hack but it allows me to easily modify everything about the ripgrep command without needing to close fzf.

While trying to debug my system I tried to search through all the files owned by xdg-desktop-portal with this command:

for file in $(pacman -Ql xdg-desktop-portal | cut -d " " -f 2-); do [ -f "$file" ] && echo "$file"; done | xargs rg-fzf

But the query got cut off at 300 characters.

Image
I propose any of the following solutions:

  • Add an option for manually specifying the limit.
  • Disable the limit if search is disabled. Since the limit only makes sense when search is enabled, only use it then. What would happen if the user had a very long query and then used the enable-search action? Should the query be truncated? That does not seem desirable.
  • Remove the limit. Does the user really need to be protected from a long search time?
@junegunn
Copy link
Owner

I get that longer queries are sometimes needed, but adding an option feels unnecessary since it's a niche case. Removing the limit entirely isn't ideal either—users might accidentally paste megabytes of data, leading to fzf becoming unresponsive or consuming excessive CPU. So I'll just increase the limit. I'm thinking 10000, because it should be sufficient even for such niche use cases, and it won't make fzf grind to a halt. What do you think?

@alex-huff
Copy link
Contributor Author

I get that longer queries are sometimes needed, but adding an option feels unnecessary since it's a niche case

This is fair.

Removing the limit entirely isn't ideal either—users might accidentally paste megabytes of data, leading to fzf becoming unresponsive or consuming excessive CPU

This is a good point I did not think about accidentally pasting a large clipboard.

So I'll just increase the limit. I'm thinking 10000, because it should be sufficient even for such niche use cases, and it won't make fzf grind to a halt. What do you think?

I certainly don't see myself reaching anywhere near 10000 and I can't think of any case where a query string could be usefully edited at such sizes.

@junegunn
Copy link
Owner

I certainly don't see myself reaching anywhere near 10000 and I can't think of any case where a query string could be usefully edited at such sizes.

Thanks. Then how about starting with 1000 and seeing if anyone hits the limit?

@alex-huff
Copy link
Contributor Author

Thanks. Then how about starting with 1000 and seeing if anyone hits the limit?

I just tried pasting a 10000 character long query and it took ~20 seconds to paste in.
1000 was much more reasonable.

@junegunn
Copy link
Owner

I see, I'll adjust the limit to 1000.

@alex-huff
Copy link
Contributor Author

alex-huff commented Feb 27, 2025

for file in $(pacman -Ql xdg-desktop-portal | cut -d " " -f 2-); do [ -f "$file" ] && echo "$file"; done | xargs rg-fzf

Sadly I still hit the query limit with this command.

From my testing I think any limit well beyond ~1000 would result in significant hangs while pasting in a large query.

However, even when using a 100_000 long query with the --query option fzf started up instantly and editing worked without any perceivable latency.

fzf --disabled --query "$(python3 -c 'print("a"*100_000)')"

Would it be feasible to only apply the limit to queries that are being pasted in? In the case that the query is being set by an action or a command-line argument, there is much less need for any safeguards especially since these cases would likely have search disabled.

Still, as any query this big is niche and arguably a misuse of fzf, I understand if you feel this isn't worth the effort and/or the added complexity.

Edit: Thinking about this I realize that there probably wouldn't be any reliable way to differentiate between somebody pasting in a query versus manually typing a query. I will just maintain a fork with a much higher limit for my own (stupid) use.

@junegunn
Copy link
Owner

From my testing I think any limit well beyond ~1000 would result in significant hangs while pasting in a large query.

I can confirm, and that's because fzf processes the event loop for every character inserted, checking if the character has bindings (e.g. --bind a:something,b:something,...), triggering change event, restarting search for the updated query etc. We could somehow optimize the code to process a bulk of characters at once, but it's not a priority and probably not worth the effort.

@junegunn
Copy link
Owner

By the way, you can easily multiply the query length inside fzf without pasting by pressing a series of CTRL-U, CTRL-Y, CTRL-Y, CTRL-Y, CTRL-U, CTRL-Y, CTRL-Y, CTRL-Y, ...

@junegunn
Copy link
Owner

I can confirm, and that's because fzf processes the event loop for every character inserted,

Turns out the reason fzf particularly having a hard time processing longer queries is that truncateQuery has an O(n) time complexity.

fzf/src/terminal.go

Lines 2288 to 2291 in e771c5d

func (t *Terminal) truncateQuery() {
t.input, _ = t.trimRight(t.input, maxPatternLength)
t.cx = util.Constrain(t.cx, 0, len(t.input))
}

We're not really interested in the exact display width of the query, so we can simplify it and make it run faster.

junegunn added a commit that referenced this issue Feb 27, 2025
@alex-huff
Copy link
Contributor Author

alex-huff commented Feb 27, 2025

We're not really interested in the exact display width of the query, so we can simplify it and make it run faster.

I can confirm that 3ba82b6 has no noticeable delay when editing a ~100_000k long query while the previous commit does. There is definitely still some seriously delay when pasting large clipboards but that must be from the overhead of the event loop since - if my understanding of go slices is correct - the new truncateQuery has very little cost.

tmeijn pushed a commit to tmeijn/dotfiles that referenced this issue Mar 4, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [junegunn/fzf](https://github.com/junegunn/fzf) | patch | `v0.60.2` -> `v0.60.3` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>junegunn/fzf (junegunn/fzf)</summary>

### [`v0.60.3`](https://github.com/junegunn/fzf/releases/tag/v0.60.3): 0.60.3

[Compare Source](junegunn/fzf@v0.60.2...v0.60.3)

-   Bug fixes and improvements
    -   \[fish] Enable multiple history commands insertion ([#&#8203;4280](junegunn/fzf#4280)) ([@&#8203;bitraid](https://github.com/bitraid))
    -   \[walker] Append '/' to directory entries on MSYS2 ([#&#8203;4281](junegunn/fzf#4281))
    -   Trim trailing whitespaces after processing ANSI sequences ([#&#8203;4282](junegunn/fzf#4282))
    -   Remove temp files before `become` when using `--tmux` option ([#&#8203;4283](junegunn/fzf#4283))
    -   Fix condition for using item numlines cache ([#&#8203;4285](junegunn/fzf#4285)) ([@&#8203;alex-huff](https://github.com/alex-huff))
    -   Make `--accept-nth` compatible with `--select-1` ([#&#8203;4287](junegunn/fzf#4287))
    -   Increase the query length limit from 300 to 1000 ([#&#8203;4292](junegunn/fzf#4292))
    -   \[windows] Prevent fzf from consuming user input while paused ([#&#8203;4260](junegunn/fzf#4260))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xODUuMCIsInVwZGF0ZWRJblZlciI6IjM5LjE4NS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
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