Fixes and improvements for the CLI and TUI

CHANGELOG.md

@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 ### Fixed
 - [lib] Directly adjusting image seek position no longer affects iteration with `ImageIterator` ([#42]).
+- [cli] Handling of `SIGINT` while processing sources ([#56]).
 - [tui] Intensive performance drop while populating large image grids ([#41]).
 - [tui] Navigation across animated images ([#42]).
   - No more waiting for the first frame to be rendered before moving on.
@@ -56,7 +57,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - `TermImageException` to `TermImageError`.
   - `InvalidSize` to `InvalidSizError`.
 - [cli] `-S` from `--scroll` to `--style` ([#44]).
+- [cli] CLI mode is now forced when output is not a TTY ([#56]).
 - [cli,tui] Changed default value of `font ratio` config option to `null` ([#45]).
+- [cli,tui] Improved startup speed and source processing ([#56]).
+- [cli,tui] Improved config error handling ([#56]).
 
 ### Deprecated
 - [lib] `term_image.image.TermImage` ([#46]).
@@ -77,6 +81,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#49]: https://github.com/AnonymouX47/term-image/pull/49
 [#50]: https://github.com/AnonymouX47/term-image/pull/50
 [#51]: https://github.com/AnonymouX47/term-image/pull/51
+[#56]: https://github.com/AnonymouX47/term-image/pull/56
 [3b658f3]: https://github.com/AnonymouX47/term-image/commit/3b658f388db8e36bc8f4d42c77375cd7c3593d4b
 
 

docs/source/viewer/index.rst

@@ -33,13 +33,20 @@ The viewer can be used in two modes:
 
 1. **CLI mode**
 
-   | In this mode, images are directly printed to standard output.
-   | This mode is used whenever there is only a single image source or when the ``--cli`` option is specified.
+   In this mode, images are directly printed to standard output. It is used when:
+
+   * output is not a terminal
+   * there is only a single image source
+   * the ``--cli`` option is specified
 
 2. **TUI mode**
 
-   | In this mode, a Terminal/Text-based User Interface is launched, within which images and directories can be browsed and viewed in different ways.
-   | This mode is used whenever there are multiple image sources or at least one directory source, or when the ``--tui`` option is specified.
+   In this mode, a Terminal/Text-based User Interface is launched, within which images
+   and directories can be browsed and viewed in different ways. It is used when:
+
+   * there is at least one non-empty directory source
+   * there are multiple image sources
+   * the ``--tui`` option is specified
 
 
 Usage

term_image/__main__.py

@@ -92,7 +92,7 @@ def finish_multi_logging():
     finally:
         # Explicit cleanup is neccessary since the top-level `Image` widgets
         # will still hold references to the `BaseImage` instances
-        if cli.url_images is not None:
+        if cli.url_images:
             for _, value in cli.url_images:
                 value._ti_image.close()
 

term_image/cli.py

@@ -38,7 +38,14 @@
 from .logging import Thread, init_log, log, log_exception
 from .logging_multi import Process
 from .tui.widgets import Image
-from .utils import CSI, OS_IS_UNIX, QUERY_TIMEOUT, get_terminal_size, write_tty
+from .utils import (
+    CSI,
+    OS_IS_UNIX,
+    QUERY_TIMEOUT,
+    clear_queue,
+    get_terminal_size,
+    write_tty,
+)
 
 
 def check_dir(
@@ -94,6 +101,8 @@ def check_dir(
     empty = True
     content = {}
     for entry in entries:
+        if interrupted and interrupted.is_set():
+            break
         if not SHOW_HIDDEN and entry.name.startswith("."):
             continue
         try:
@@ -349,7 +358,7 @@ def process_result(
             setitem(checks_in_progress, *progress_queue.get())
 
             while not (
-                interrupted.is_set()  # MainThread has been interruped
+                interrupted.is_set()  # MainThread has been interrupted
                 or not any(checks_in_progress)  # All checkers are dead
                 # All checks are done
                 or (
@@ -391,6 +400,9 @@ def process_result(
                 sleep(0.01)  # Allow queue sizes to be updated
         finally:
             if interrupted.is_set():
+                clear_queue(dir_queue)
+                clear_queue(content_queue)
+                clear_queue(progress_queue)
                 return
 
             if not any(checks_in_progress):
@@ -418,7 +430,7 @@ def process_result(
         current_thread.name = "Checker"
 
         _, links, source, _depth = dir_queue.get()
-        while source:
+        while not interrupted.is_set() and source:
             log(f"Checking {source!r}", logger, verbose=True)
             if islink(source):
                 links.append((source, realpath(source)))
@@ -432,10 +444,13 @@ def process_result(
                     source = abspath(source)
                     contents[source] = result
                     images.append((source, ...))
-                elif result is None:
+                elif not interrupted.is_set() and result is None:
                     log(f"{source!r} is empty", logger)
             _, links, source, _depth = dir_queue.get()
 
+        if interrupted.is_set():
+            clear_queue(dir_queue)
+
 
 def update_contents(
     dir: str,
@@ -497,6 +512,9 @@ def get_urls(
             log(f"Done getting {source!r}", logger, verbose=True)
         source = url_queue.get()
 
+    if interrupted.is_set():
+        clear_queue(url_queue)
+
 
 def open_files(
     file_queue: Queue,
@@ -516,6 +534,9 @@ def open_files(
             log_exception(f"Opening {source!r} failed", logger, direct=True)
         source = file_queue.get()
 
+    if interrupted.is_set():
+        clear_queue(file_queue)
+
 
 def main() -> None:
     """CLI execution sub-entry-point"""
@@ -1137,6 +1158,10 @@ def check_arg(
         store_config(default=True)
         sys.exit(SUCCESS)
 
+    force_cli_mode = not sys.stdout.isatty() and not args.cli
+    if force_cli_mode:
+        args.cli = True
+
     init_log(
         (
             args.log_file
@@ -1245,6 +1270,13 @@ def check_arg(
     # non-supporting terminal emulators
     write_tty(f"{CSI}1K\r".encode())
 
+    if force_cli_mode:
+        log(
+            "Output is not a terminal, forcing CLI mode!",
+            logger,
+            level=_logging.WARNING,
+        )
+
     log("Processing sources", logger, loading=True)
 
     file_images, url_images, dir_images = [], [], []
@@ -1260,21 +1292,18 @@ def check_arg(
             target=get_urls,
             args=(url_queue, url_images, ImageClass),
             name=f"Getter-{n}",
-            daemon=True,
         )
         for n in range(1, args.getters + 1)
     ]
-    for getter in getters:
-        getter.start()
+    getters_started = False
 
     file_queue = Queue()
     opener = Thread(
         target=open_files,
         args=(file_queue, file_images, ImageClass),
         name="Opener",
-        daemon=True,
     )
-    opener.start()
+    opener_started = False
 
     if OS_IS_UNIX and not args.cli:
         dir_queue = mp_Queue() if logging.MULTI and args.checkers > 1 else Queue()
@@ -1283,9 +1312,8 @@ def check_arg(
             target=manage_checkers,
             args=(dir_queue, contents, dir_images),
             name="CheckManager",
-            daemon=True,
         )
-        check_manager.start()
+    checkers_started = False
 
     for source in sources:
         if source in unique_sources:
@@ -1294,8 +1322,15 @@ def check_arg(
         unique_sources.add(source)
 
         if all(urlparse(source)[:3]):  # Is valid URL
+            if not getters_started:
+                for getter in getters:
+                    getter.start()
+                getters_started = True
             url_queue.put(source)
         elif isfile(source):
+            if not opener_started:
+                opener.start()
+                opener_started = True
             file_queue.put(source)
         elif isdir(source):
             if args.cli:
@@ -1304,25 +1339,42 @@ def check_arg(
             if not OS_IS_UNIX:
                 dir_images = True
                 continue
+            if not checkers_started:
+                check_manager.start()
+                checkers_started = True
             dir_queue.put(("", [], source, 0))
         else:
             log(f"{source!r} is invalid or does not exist", logger, _logging.ERROR)
 
     # Signal end of sources
-    for _ in range(args.getters):
-        url_queue.put(None)
-    file_queue.put(None)
-    if OS_IS_UNIX and not args.cli:
+    if getters_started:
+        for _ in range(args.getters):
+            url_queue.put(None)
+    if opener_started:
+        file_queue.put(None)
+    if checkers_started:
         if logging.MULTI and args.checkers > 1:
             dir_queue.sources_finished = True
         else:
             dir_queue.put((None,) * 4)
 
-    for getter in getters:
-        getter.join()
-    opener.join()
-    if OS_IS_UNIX and not args.cli:
-        check_manager.join()
+    interrupt = None
+    while True:
+        try:
+            if getters_started:
+                for getter in getters:
+                    getter.join()
+            if opener_started:
+                opener.join()
+            if checkers_started:
+                check_manager.join()
+            break
+        except KeyboardInterrupt as e:  # Ensure logs are in correct order
+            if not interrupt:  # keep the first
+                interrupted.set()
+                interrupt = e
+    if interrupt:
+        raise interrupt from None
 
     notify.stop_loading()
     while notify.is_loading():

term_image/config.py

@@ -32,11 +32,11 @@ def info(msg: str) -> None:
 
 
 def error(msg: str) -> None:
-    print(f"{CSI}34mconfig: {CSI}33m{msg}{COLOR_RESET}", file=sys.stderr)
+    print(f"{CSI}34mconfig: {CSI}31m{msg}{COLOR_RESET}", file=sys.stderr)
 
 
 def fatal(msg: str) -> None:
-    print(f"{CSI}34mconfig: {CSI}31m{msg}{COLOR_RESET}", file=sys.stderr)
+    print(f"{CSI}34mconfig: {CSI}39m{CSI}41m{msg}{COLOR_RESET}", file=sys.stderr)
 
 
 def init_config() -> None:
@@ -50,13 +50,14 @@ def init_config() -> None:
 
     if os.path.exists(user_dir):
         if not os.path.isdir(user_dir):
-            fatal("Please rename or remove the file {user_dir!r}.")
+            fatal(f"Please rename or remove the file {user_dir!r}.")
             sys.exit(CONFIG_ERROR)
     else:
         os.mkdir(user_dir)
 
     if os.path.isfile(config_file):
         if load_config():
+            # Stored at this point in order to put missing values in place.
             store_config()
             info("... Successfully updated user config.")
     else:
@@ -88,17 +89,20 @@ def load_config() -> bool:
     try:
         with open(config_file) as f:
             config = json.load(f)
-    except json.JSONDecodeError:
-        error("Error loading user config... Using defaults.")
+    except Exception as e:
+        error(
+            f"Failed to load user config ({type(e).__name__}: {e})... Using defaults."
+        )
         update_context_nav_keys(context_keys, nav, nav)
         return updated
 
     try:
         c_version = config["version"]
         if gt(*[(*map(int, v.split(".")),) for v in (version, c_version)]):
             info("Updating user config...")
-            update_config(config, c_version)
-            updated = True
+            updated = update_config(config, c_version)
+            if not updated:
+                error("... Failed to update user config.")
     except KeyError:
         error("Config version not found... Please correct this manually.")
 
@@ -160,23 +164,30 @@ def store_config(*, default: bool = False) -> None:
         if keys:
             stored_keys[context] = keys
 
-    with open(config_file, "w") as f:
-        json.dump(
-            {
-                "version": version,
-                **{
-                    name: globals()["_" * default + f"{name.replace(' ', '_')}"]
-                    for name in config_options
+    try:
+        with open(config_file, "w") as f:
+            json.dump(
+                {
+                    "version": version,
+                    **{
+                        name: globals()["_" * default + f"{name.replace(' ', '_')}"]
+                        for name in config_options
+                    },
+                    "keys": stored_keys,
                 },
-                "keys": stored_keys,
-            },
-            f,
-            indent=4,
-        )
+                f,
+                indent=4,
+            )
+    except Exception as e:
+        error(f"Failed to write user config ({type(e).__name__}: {e}).")
 
 
-def update_config(config: Dict[str, Any], old_version: str):
-    """Updates the user config to latest version"""
+def update_config(config: Dict[str, Any], old_version: str) -> bool:
+    """Updates the user config to latest version
+
+    Returns:
+        ``True``, if successful. Otherwise, ``False``.
+    """
     # Must use the values directly, never reference the corresponding global variables,
     # as those might change later and will break updating since it's incremental
     #
@@ -220,11 +231,16 @@ def update_config(config: Dict[str, Any], old_version: str):
                         "the new default will be put in place."
                     )
 
-    os.replace(config_file, f"{config_file}.old")
-    info(f"Previous config file moved to '{config_file}.old'.")
     config["version"] = version
-    with open(config_file, "w") as f:
-        json.dump(config, f, indent=4)
+    try:
+        os.replace(config_file, f"{config_file}.old")
+    except OSError as e:
+        error(f"Failed to backup previous config file ({type(e).__name__}: {e})")
+        return False
+    else:
+        info(f"Previous config file has been moved to '{config_file}.old'.")
+
+    return True
 
 
 def update_context(name: str, keyset: Dict[str, list], update: Dict[str, list]) -> None: