Separate utils.read_file and read_file_binary

See #4467

4 files changed, 35 insertions, 30 deletions

diff --git a/qutebrowser/browser/pdfjs.py b/qutebrowser/browser/pdfjs.py
index 3bde1b8a4..5656fbfc2 100644
--- a/qutebrowser/browser/pdfjs.py
+++ b/qutebrowser/browser/pdfjs.py
@@ -149,7 +149,7 @@ def get_pdfjs_res_and_path(path):
     if content is None:
         res_path = '3rdparty/pdfjs/{}'.format(path)
         try:
-            content = utils.read_file(res_path, binary=True)
+            content = utils.read_file_binary(res_path)
         except FileNotFoundError:
             raise PDFJSNotFound(path) from None
         except OSError as e:
diff --git a/qutebrowser/browser/qutescheme.py b/qutebrowser/browser/qutescheme.py
index 3f95dae32..612bccdc0 100644
--- a/qutebrowser/browser/qutescheme.py
+++ b/qutebrowser/browser/qutescheme.py
@@ -270,7 +270,7 @@ def qute_javascript(url: QUrl) -> _HandlerRet:
     path = url.path()
     if path:
         path = "javascript" + os.sep.join(path.split('/'))
-        return 'text/html', utils.read_file(path, binary=False)
+        return 'text/html', utils.read_file(path)
     else:
         raise UrlInvalidError("No file specified")
 
@@ -371,7 +371,7 @@ def qute_help(url: QUrl) -> _HandlerRet:
     path = 'html/doc/{}'.format(urlpath)
     if not urlpath.endswith('.html'):
         try:
-            bdata = utils.read_file(path, binary=True)
+            bdata = utils.read_file_binary(path)
         except OSError as e:
             raise SchemeOSError(e)
         mimetype = utils.guess_mimetype(urlpath)
@@ -574,7 +574,7 @@ def qute_resource(url: QUrl) -> _HandlerRet:
     path = url.path().lstrip('/')
     mimetype = utils.guess_mimetype(path, fallback=True)
     try:
-        data = utils.read_file(path, binary=True)
+        data = utils.read_file_binary(path)
     except FileNotFoundError as e:
         raise NotFoundError(str(e))
     return mimetype, data
diff --git a/qutebrowser/utils/jinja.py b/qutebrowser/utils/jinja.py
index 4eaa53ead..59e887126 100644
--- a/qutebrowser/utils/jinja.py
+++ b/qutebrowser/utils/jinja.py
@@ -120,7 +120,7 @@ class Environment(jinja2.Environment):
 
     def _data_url(self, path: str) -> str:
         """Get a data: url for the broken qutebrowser logo."""
-        data = utils.read_file(path, binary=True)
+        data = utils.read_file_binary(path)
         mimetype = utils.guess_mimetype(path)
         return urlutils.data_url(mimetype, data).toString()
 
diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py
index 483a9a6f3..18654749b 100644
--- a/qutebrowser/utils/utils.py
+++ b/qutebrowser/utils/utils.py
@@ -33,6 +33,7 @@ import contextlib
 import posixpath
 import shlex
 import mimetypes
+import pathlib
 import ctypes
 import ctypes.util
 from typing import (Any, Callable, IO, Iterator, Optional, Sequence, Tuple, Type, Union,
@@ -179,9 +180,22 @@ def compact_text(text: str, elidelength: int = None) -> str:
     return out
 
 
+def _resource_path(filename: str) -> pathlib.Path:
+    """Get a pathlib.Path object for a resource."""
+    assert not posixpath.isabs(filename), filename
+    assert os.path.pardir not in filename.split(posixpath.sep), filename
+
+    if hasattr(sys, 'frozen'):
+        # For PyInstaller, where we can't store resource files in a qutebrowser/ folder
+        # because the executable is already named "qutebrowser" (at least on macOS).
+        return pathlib.Path(sys.executable).parent / filename
+
+    return importlib_resources.files(qutebrowser) / filename
+
+
 def preload_resources() -> None:
     """Load resource files into the cache."""
-    resource_path = importlib_resources.files(qutebrowser)
+    resource_path = _resource_path('')
     for subdir, pattern in [('html', '*.html'), ('javascript', '*.js')]:
         path = resource_path / subdir
         for full_path in path.glob(pattern):
@@ -189,42 +203,33 @@ def preload_resources() -> None:
             _resource_cache[sub_path] = read_file(sub_path)
 
 
-# FIXME:typing Return value should be bytes/str
-def read_file(filename: str, binary: bool = False) -> Any:
+def read_file(filename: str) -> str:
     """Get the contents of a file contained with qutebrowser.
 
     Args:
         filename: The filename to open as string.
-        binary: Whether to return a binary string.
-                If False, the data is UTF-8-decoded.
 
     Return:
         The file contents as string.
     """
-    assert not posixpath.isabs(filename), filename
-    assert os.path.pardir not in filename.split(posixpath.sep), filename
-
-    if not binary and filename in _resource_cache:
+    if filename in _resource_cache:
         return _resource_cache[filename]
 
-    if hasattr(sys, 'frozen'):
-        # For PyInstaller, where we can't store resource files in a qutebrowser/ folder
-        # because the executable is already named "qutebrowser" (at least on macOS).
-        fn = os.path.join(os.path.dirname(sys.executable), filename)
-        if binary:
-            f: IO
-            with open(fn, 'rb') as f:
-                return f.read()
-        else:
-            with open(fn, 'r', encoding='utf-8') as f:
-                return f.read()
-    else:
-        p = importlib_resources.files(qutebrowser) / filename
+    path = _resource_path(filename)
+    return path.read_text(encoding='utf-8')
+
 
-        if binary:
-            return p.read_bytes()
+def read_file_binary(filename: str) -> bytes:
+    """Get the contents of a binary file contained with qutebrowser.
 
-        return p.read_text()
+    Args:
+        filename: The filename to open as string.
+
+    Return:
+        The file contents as a bytes object.
+    """
+    path = _resource_path(filename)
+    return path.read_bytes()
 
 
 def parse_version(version: str) -> VersionNumber: