Repository: tfeldmann/organize

Issue:
`filecontent` filter fails for PDFs when `pdftotext` isn't installed instead of falling back to `pdfminer`
Technical cause: `filecontent._pdftotext_available` catches the wrong exception when testing if an executable exist when it doesn't.

The `filecontent` filter tests if `pdftotext` exists. When it doesn't (or isn't on the PATH), it expects `subprocess.CalledProcessError` to be raised. However (at least on Python 3.9 and macOS), it raises a `FileNotFoundError`. The filter then behaves as if it didn't match, thus skipping all actions.

Environment: Python 3.9, organize v3.3.0

Also,`shutil.which` is a faster and safer way to test if an executable exists IMO.

@tfeldmann, I'll be adding a PR soon that should fix this.

Hints from issue comments:

Solution patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index eba485b..a9dc937 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## [Unreleased]
 
+- Fixed #438 (`filecontent` filter fails for PDFs when `pdftotext` isn't installed, instead of falling back to `pdfminer`)
+
 ## v3.3.0 (2024-11-25)
 
 - Added a new conflict mode `deduplicate` which skips duplicate files and renames
diff --git a/organize/filters/filecontent.py b/organize/filters/filecontent.py
index 3701f5c..16a14dd 100644
--- a/organize/filters/filecontent.py
+++ b/organize/filters/filecontent.py
@@ -1,4 +1,5 @@
 import re
+import shutil
 import subprocess
 from functools import lru_cache
 from pathlib import Path
@@ -40,16 +41,14 @@ def extract_txt(path: Path) -> str:
 @lru_cache(maxsize=1)
 def _pdftotext_available() -> bool:
     # check whether the given path is executable
-    try:
-        subprocess.check_call(
-            ["pdftotext", "-v"],
-            stdout=subprocess.DEVNULL,
-            stderr=subprocess.STDOUT,
-        )
-        return True
-    except subprocess.CalledProcessError:
+    ok = _is_executable("pdftotext")
+    if not ok:
         logger.warning("pdftotext not available. Falling back to pdfminer library.")
-        return False
+    return ok
+
+
+def _is_executable(name: str) -> bool:
+    return shutil.which(name) is not None
 
 
 def _extract_with_pdftotext(path: Path, keep_layout: bool) -> str:


Test patch:
diff --git a/tests/filters/test_filecontent.py b/tests/filters/test_filecontent.py
index 5a23625..65565c1 100644
--- a/tests/filters/test_filecontent.py
+++ b/tests/filters/test_filecontent.py
@@ -3,6 +3,19 @@ from conftest import make_files, read_files
 from organize import Config
 
 
+def test_is_executable():
+    import sys
+    from uuid import uuid1 as uuid
+
+    from organize.filters.filecontent import _is_executable as is_executable
+
+    present_exe = "dir" if hasattr(sys, "getwindowsversion") else "ls"
+    assert is_executable(present_exe)
+
+    absent_exe = f"no-such-executable-{uuid()}"  # random name that won't exist
+    assert not is_executable(absent_exe)
+
+
 def test_filecontent(fs):
     # inspired by https://github.com/tfeldmann/organize/issues/43
     files = {