From 165edcecc8310a89dbeb444628a98b4d0533d386 Mon Sep 17 00:00:00 2001 From: Dev Dalia Date: Sat, 22 Aug 2026 21:37:13 +0530 Subject: [PATCH 1/2] Name the fzf command for the machine, not for macOS Both places that mention fzf assumed Homebrew. The person most likely to read them is on Linux without fzf, where 'brew install fzf' is simply wrong and 'see the fzf homepage' means going to read a page instead of running a line. install.sh and the runtime fallback now detect the package manager present and print the exact command; both also offer fzf's own installer for anyone without sudo. Printed, never run: installing system packages needs sudo, and a piped installer that sudos is the reason people distrust piped installers. The README's non-brew section lists the four commands rather than linking out. Preferring brew when both exist is deliberate: on a box with Linuxbrew and apt, brew installs without sudo. --- README.md | 14 ++++++++++-- agsearch | 20 +++++++++++++++++- install.sh | 19 ++++++++++++++--- tests/test_fzf_hint.py | 48 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 6 deletions(-) create mode 100644 tests/test_fzf_hint.py diff --git a/README.md b/README.md index 730f56f..1df1eb6 100644 --- a/README.md +++ b/README.md @@ -70,8 +70,18 @@ uv tool install agsearch pipx install agsearch ``` -Install [`fzf`](https://github.com/junegunn/fzf) separately for the interactive interface. -Without it, `agsearch -n "query"` still works. +These do not bring `fzf`, which the interactive interface needs: + +```sh +sudo apt install fzf # Debian, Ubuntu +sudo dnf install fzf # Fedora +sudo pacman -S fzf # Arch +brew install fzf # macOS, Linuxbrew +``` + +No sudo? `git clone --depth 1 https://github.com/junegunn/fzf.git ~/.fzf && ~/.fzf/install`. + +Without fzf, `agsearch -n "query"` still prints ranked matches. ### Install script diff --git a/agsearch b/agsearch index 04339bb..245987e 100755 --- a/agsearch +++ b/agsearch @@ -1331,10 +1331,28 @@ def cmd_filter(argv): sys.stdout.write("\n".join(out)) +def fzf_install_hint(): + """The command that installs fzf on *this* machine. + + "see the fzf homepage" means going and reading a page. Naming the exact + line means running it, and the person most likely to hit this is on Linux, + where a hardcoded `brew install fzf` is simply wrong. + """ + for tool, cmd in (("brew", "brew install fzf"), + ("apt-get", "sudo apt install fzf"), + ("dnf", "sudo dnf install fzf"), + ("pacman", "sudo pacman -S fzf"), + ("zypper", "sudo zypper install fzf"), + ("apk", "sudo apk add fzf")): + if shutil.which(tool): + return cmd + return "https://github.com/junegunn/fzf#installation" + + def run_fzf(lines, query, thinking=False, no_resume=False, fuzzy=False): if not shutil.which("fzf"): print("fzf not installed. Falling back to non-interactive output.\n" - "Install with: brew install fzf\n", file=sys.stderr) + f"Install with: {fzf_install_hint()}\n", file=sys.stderr) return print_matches(lines, query) self = os.path.abspath(__file__) diff --git a/install.sh b/install.sh index 31554d9..cb9bf38 100755 --- a/install.sh +++ b/install.sh @@ -71,10 +71,23 @@ case ":$PATH:" in echo " export PATH=\"$PREFIX:\$PATH\"" ;; esac +# Name the command for the machine this is running on. "see the fzf homepage" +# means going and reading a page; an exact line means running it. Printed, never +# run: installing system packages needs sudo, and a piped installer that sudos +# is the reason people distrust piped installers. if ! command -v fzf >/dev/null 2>&1; then - echo "note: fzf is not installed — the interactive TUI needs it." - echo " macOS: brew install fzf · Linux: see https://github.com/junegunn/fzf" - echo " (agsearch -n \"query\" works without fzf)" + if command -v brew >/dev/null 2>&1; then FZF_HINT="brew install fzf" + elif command -v apt-get >/dev/null 2>&1; then FZF_HINT="sudo apt install fzf" + elif command -v dnf >/dev/null 2>&1; then FZF_HINT="sudo dnf install fzf" + elif command -v pacman >/dev/null 2>&1; then FZF_HINT="sudo pacman -S fzf" + elif command -v zypper >/dev/null 2>&1; then FZF_HINT="sudo zypper install fzf" + elif command -v apk >/dev/null 2>&1; then FZF_HINT="sudo apk add fzf" + else FZF_HINT="" + fi + echo "note: fzf is not installed — the interactive interface needs it." + [ -n "$FZF_HINT" ] && echo " $FZF_HINT" + echo " no sudo: git clone --depth 1 https://github.com/junegunn/fzf.git ~/.fzf && ~/.fzf/install" + echo " or skip it: agsearch -n \"query\" works without fzf" fi echo "done. run: agsearch" diff --git a/tests/test_fzf_hint.py b/tests/test_fzf_hint.py new file mode 100644 index 0000000..1b4c48a --- /dev/null +++ b/tests/test_fzf_hint.py @@ -0,0 +1,48 @@ +"""The fzf hint has to name a command that works on the machine reading it. + +The person most likely to see this is on Linux without fzf, where the previous +hardcoded `brew install fzf` was simply wrong, and "see the fzf homepage" meant +going to read a page instead of running a line. +""" + +import unittest +from unittest import mock + +from load_agsearch import load_agsearch + + +class FzfHintTests(unittest.TestCase): + def setUp(self): + self.ag = load_agsearch() + + def _hint(self, present): + with mock.patch.object(self.ag.shutil, "which", + side_effect=lambda t: t if t in present else None): + return self.ag.fzf_install_hint() + + def test_names_the_package_manager_that_exists(self): + for tools, expected in ( + ({"brew"}, "brew install fzf"), + ({"apt-get"}, "sudo apt install fzf"), + ({"dnf"}, "sudo dnf install fzf"), + ({"pacman"}, "sudo pacman -S fzf"), + ({"zypper"}, "sudo zypper install fzf"), + ({"apk"}, "sudo apk add fzf"), + ): + with self.subTest(tools=tools): + self.assertEqual(self._hint(tools), expected) + + def test_does_not_suggest_brew_on_a_linux_box(self): + """The original bug: brew named on a machine that has never had it.""" + self.assertNotIn("brew", self._hint({"apt-get"})) + + def test_falls_back_to_the_homepage_when_nothing_is_recognised(self): + self.assertIn("github.com/junegunn/fzf", self._hint(set())) + + def test_prefers_brew_when_both_exist(self): + """Linuxbrew alongside apt: brew installs without sudo, so prefer it.""" + self.assertEqual(self._hint({"brew", "apt-get"}), "brew install fzf") + + +if __name__ == "__main__": + unittest.main() From 02aaa62b4f713b29cfb58f688be0be70fc59fdc9 Mon Sep 17 00:00:00 2001 From: Dev Dalia Date: Sat, 22 Aug 2026 21:41:37 +0530 Subject: [PATCH 2/2] Document the fzf version floor; stop naming brew on Linux Two messages told people to run 'brew install fzf'. On Linux, where the person without fzf most likely is, that is wrong information. Both now link to fzf's installation page, which is correct everywhere and does not go stale. More importantly, the floor was undocumented. agsearch binds fzf's 'start' event, added in fzf 0.35.0, so anything older fails with 'unknown event: start'. Some distributions still package below that. zoxide and forgit both link out and both state a minimum version. This does the same rather than carrying a per-distro table that ages. --- README.md | 16 ++++---------- agsearch | 21 ++---------------- install.sh | 19 +++-------------- tests/test_fzf_hint.py | 48 ------------------------------------------ 4 files changed, 9 insertions(+), 95 deletions(-) delete mode 100644 tests/test_fzf_hint.py diff --git a/README.md b/README.md index 1df1eb6..f3b4057 100644 --- a/README.md +++ b/README.md @@ -70,18 +70,10 @@ uv tool install agsearch pipx install agsearch ``` -These do not bring `fzf`, which the interactive interface needs: - -```sh -sudo apt install fzf # Debian, Ubuntu -sudo dnf install fzf # Fedora -sudo pacman -S fzf # Arch -brew install fzf # macOS, Linuxbrew -``` - -No sudo? `git clone --depth 1 https://github.com/junegunn/fzf.git ~/.fzf && ~/.fzf/install`. - -Without fzf, `agsearch -n "query"` still prints ranked matches. +The interactive interface needs [`fzf`](https://github.com/junegunn/fzf#installation) **0.35 or +newer** — that is the release which added the `start` event agsearch binds. Some distributions +package an older one; `fzf`'s own install script is the fallback. Without fzf, +`agsearch -n "query"` still prints ranked matches. ### Install script diff --git a/agsearch b/agsearch index 245987e..3daedc5 100755 --- a/agsearch +++ b/agsearch @@ -1331,28 +1331,11 @@ def cmd_filter(argv): sys.stdout.write("\n".join(out)) -def fzf_install_hint(): - """The command that installs fzf on *this* machine. - - "see the fzf homepage" means going and reading a page. Naming the exact - line means running it, and the person most likely to hit this is on Linux, - where a hardcoded `brew install fzf` is simply wrong. - """ - for tool, cmd in (("brew", "brew install fzf"), - ("apt-get", "sudo apt install fzf"), - ("dnf", "sudo dnf install fzf"), - ("pacman", "sudo pacman -S fzf"), - ("zypper", "sudo zypper install fzf"), - ("apk", "sudo apk add fzf")): - if shutil.which(tool): - return cmd - return "https://github.com/junegunn/fzf#installation" - - def run_fzf(lines, query, thinking=False, no_resume=False, fuzzy=False): if not shutil.which("fzf"): print("fzf not installed. Falling back to non-interactive output.\n" - f"Install with: {fzf_install_hint()}\n", file=sys.stderr) + "Install fzf 0.35+: https://github.com/junegunn/fzf#installation\n", + file=sys.stderr) return print_matches(lines, query) self = os.path.abspath(__file__) diff --git a/install.sh b/install.sh index cb9bf38..9657c63 100755 --- a/install.sh +++ b/install.sh @@ -71,23 +71,10 @@ case ":$PATH:" in echo " export PATH=\"$PREFIX:\$PATH\"" ;; esac -# Name the command for the machine this is running on. "see the fzf homepage" -# means going and reading a page; an exact line means running it. Printed, never -# run: installing system packages needs sudo, and a piped installer that sudos -# is the reason people distrust piped installers. if ! command -v fzf >/dev/null 2>&1; then - if command -v brew >/dev/null 2>&1; then FZF_HINT="brew install fzf" - elif command -v apt-get >/dev/null 2>&1; then FZF_HINT="sudo apt install fzf" - elif command -v dnf >/dev/null 2>&1; then FZF_HINT="sudo dnf install fzf" - elif command -v pacman >/dev/null 2>&1; then FZF_HINT="sudo pacman -S fzf" - elif command -v zypper >/dev/null 2>&1; then FZF_HINT="sudo zypper install fzf" - elif command -v apk >/dev/null 2>&1; then FZF_HINT="sudo apk add fzf" - else FZF_HINT="" - fi - echo "note: fzf is not installed — the interactive interface needs it." - [ -n "$FZF_HINT" ] && echo " $FZF_HINT" - echo " no sudo: git clone --depth 1 https://github.com/junegunn/fzf.git ~/.fzf && ~/.fzf/install" - echo " or skip it: agsearch -n \"query\" works without fzf" + echo "note: fzf is not installed — the interactive TUI needs it." + echo " fzf 0.35 or newer: https://github.com/junegunn/fzf#installation" + echo " (agsearch -n \"query\" works without fzf)" fi echo "done. run: agsearch" diff --git a/tests/test_fzf_hint.py b/tests/test_fzf_hint.py deleted file mode 100644 index 1b4c48a..0000000 --- a/tests/test_fzf_hint.py +++ /dev/null @@ -1,48 +0,0 @@ -"""The fzf hint has to name a command that works on the machine reading it. - -The person most likely to see this is on Linux without fzf, where the previous -hardcoded `brew install fzf` was simply wrong, and "see the fzf homepage" meant -going to read a page instead of running a line. -""" - -import unittest -from unittest import mock - -from load_agsearch import load_agsearch - - -class FzfHintTests(unittest.TestCase): - def setUp(self): - self.ag = load_agsearch() - - def _hint(self, present): - with mock.patch.object(self.ag.shutil, "which", - side_effect=lambda t: t if t in present else None): - return self.ag.fzf_install_hint() - - def test_names_the_package_manager_that_exists(self): - for tools, expected in ( - ({"brew"}, "brew install fzf"), - ({"apt-get"}, "sudo apt install fzf"), - ({"dnf"}, "sudo dnf install fzf"), - ({"pacman"}, "sudo pacman -S fzf"), - ({"zypper"}, "sudo zypper install fzf"), - ({"apk"}, "sudo apk add fzf"), - ): - with self.subTest(tools=tools): - self.assertEqual(self._hint(tools), expected) - - def test_does_not_suggest_brew_on_a_linux_box(self): - """The original bug: brew named on a machine that has never had it.""" - self.assertNotIn("brew", self._hint({"apt-get"})) - - def test_falls_back_to_the_homepage_when_nothing_is_recognised(self): - self.assertIn("github.com/junegunn/fzf", self._hint(set())) - - def test_prefers_brew_when_both_exist(self): - """Linuxbrew alongside apt: brew installs without sudo, so prefer it.""" - self.assertEqual(self._hint({"brew", "apt-get"}), "brew install fzf") - - -if __name__ == "__main__": - unittest.main()