From 16396d25c2795bd7f8822a695de111d07f588b26 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Sun, 10 May 2026 14:42:04 -0500 Subject: refactor(external-open): consolidate OS-open dispatch in external-open.el Phase 4 of utility-consolidation. Three previously-overlapping helpers (system-utils' `cj/identify-external-open-command' and `cj/--open-with-is-launcher-p', plus the dirvish-only `cj/--file-manager-program-for' shipped earlier today) all answered "which OS-open program should I run?". Pull the answer into one place: external-open.el. Move and rename: - `cj/--open-with-is-launcher-p' (system-utils) -> `cj/external-open-launcher-p' (external-open). Public name now matches its module. - `cj/identify-external-open-command' (system-utils) -> `cj/external-open-command' (external-open). Returns nil for unsupported hosts instead of signaling -- callers that need a command must handle nil explicitly. The wrapper `cj/xdg-open' (also moved into external-open) converts nil to a `user-error' with a clear message, preserving the user-facing failure shape. - Delete dirvish's `cj/--file-manager-program-for' helper. `cj/dirvish-open-file-manager-here' now calls `cj/external-open-command' directly. The shell-command fallback for nil-program preserves the previous escape hatch. Break the system-utils <-> external-open recursive require by moving `cj/xdg-open' (the only system-utils function that external-open used) into external-open along with the dispatch. Tests reorganized to match the move. Two new test files (`test-external-open-command.el', `test-external-open-launcher-p.el') replace the two system-utils-named test files. The dirvish file-manager-program test goes away with the helper. 11 tests covering Normal/Boundary/Error for the dispatch (plus the new "unsupported host returns nil" contract). Add `(require \='external-open)' to system-utils.el and `(require \='system-lib)' to external-open.el (for `cj/file-from-context' which xdg-open uses). --- tests/test-dirvish-config-file-manager-program.el | 54 ------------------ tests/test-external-open-command.el | 65 ++++++++++++++++++++++ tests/test-external-open-launcher-p.el | 55 ++++++++++++++++++ .../test-system-utils--open-with-is-launcher-p.el | 52 ----------------- ...-system-utils-identify-external-open-command.el | 59 -------------------- 5 files changed, 120 insertions(+), 165 deletions(-) delete mode 100644 tests/test-dirvish-config-file-manager-program.el create mode 100644 tests/test-external-open-command.el create mode 100644 tests/test-external-open-launcher-p.el delete mode 100644 tests/test-system-utils--open-with-is-launcher-p.el delete mode 100644 tests/test-system-utils-identify-external-open-command.el (limited to 'tests') diff --git a/tests/test-dirvish-config-file-manager-program.el b/tests/test-dirvish-config-file-manager-program.el deleted file mode 100644 index bfd4cad9..00000000 --- a/tests/test-dirvish-config-file-manager-program.el +++ /dev/null @@ -1,54 +0,0 @@ -;;; test-dirvish-config-file-manager-program.el --- Tests for the file-manager dispatch -*- lexical-binding: t; -*- - -;;; Commentary: -;; `cj/--file-manager-program-for' is the pure dispatch behind -;; `cj/dirvish-open-file-manager-here'. Given whether xdg-open is -;; present and the running `system-type', it returns the program name -;; the wrapper should call -- or nil to signal the wrapper should fall -;; back to a shell-command. Keeping `executable-find' and `system-type' -;; outside lets the helper be tested without faking the live machine. - -;;; Code: - -(require 'ert) -(require 'package) - -(setq package-user-dir (expand-file-name "elpa" user-emacs-directory)) -(package-initialize) -(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory)) -(add-to-list 'load-path (expand-file-name "elpa/dirvish-2.3.0/extensions" - user-emacs-directory)) -(require 'user-constants) -(require 'keybindings) -(require 'dirvish-config) - -(ert-deftest test-cj--file-manager-program-for-xdg-open-on-linux () - "Normal: xdg-open present on Linux returns xdg-open." - (should (equal (cj/--file-manager-program-for t 'gnu/linux) - "xdg-open"))) - -(ert-deftest test-cj--file-manager-program-for-xdg-open-wins-on-macos () - "Boundary: xdg-open present even on macOS returns xdg-open (Linux-isms ported)." - (should (equal (cj/--file-manager-program-for t 'darwin) - "xdg-open"))) - -(ert-deftest test-cj--file-manager-program-for-darwin-no-xdg () - "Normal: macOS without xdg-open returns open." - (should (equal (cj/--file-manager-program-for nil 'darwin) - "open"))) - -(ert-deftest test-cj--file-manager-program-for-windows-no-xdg () - "Normal: Windows without xdg-open returns explorer." - (should (equal (cj/--file-manager-program-for nil 'windows-nt) - "explorer"))) - -(ert-deftest test-cj--file-manager-program-for-linux-without-xdg-falls-back () - "Boundary: Linux without xdg-open returns nil so the wrapper shells out." - (should-not (cj/--file-manager-program-for nil 'gnu/linux))) - -(ert-deftest test-cj--file-manager-program-for-unknown-system-falls-back () - "Boundary: an unknown `system-type' with no xdg-open returns nil." - (should-not (cj/--file-manager-program-for nil 'haiku))) - -(provide 'test-dirvish-config-file-manager-program) -;;; test-dirvish-config-file-manager-program.el ends here diff --git a/tests/test-external-open-command.el b/tests/test-external-open-command.el new file mode 100644 index 00000000..b408b305 --- /dev/null +++ b/tests/test-external-open-command.el @@ -0,0 +1,65 @@ +;;; test-external-open-command.el --- Tests for cj/external-open-command -*- lexical-binding: t; -*- + +;;; Commentary: +;; Unit tests for `cj/external-open-command' in external-open.el. The +;; function dispatches on host-environment predicates to return the +;; appropriate "open" command: xdg-open on Linux, open on macOS, +;; start on Windows. Returns nil for unsupported hosts (callers that +;; require a command should error on nil with a contextual message). + +;;; Code: + +(require 'ert) +(require 'cl-lib) +(require 'package) + +(setq package-user-dir (expand-file-name "elpa" user-emacs-directory)) +(package-initialize) +(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory)) +(require 'external-open) + +(defmacro test-eoc--with-host (linux macos windows &rest body) + "Run BODY with env-*-p predicates stubbed to LINUX, MACOS, WINDOWS." + (declare (indent 3)) + `(cl-letf (((symbol-function 'env-linux-p) (lambda () ,linux)) + ((symbol-function 'env-macos-p) (lambda () ,macos)) + ((symbol-function 'env-windows-p) (lambda () ,windows))) + ,@body)) + +;;; Normal cases + +(ert-deftest test-eoc-linux-returns-xdg-open () + "Normal: Linux host returns \"xdg-open\"." + (test-eoc--with-host t nil nil + (should (string= "xdg-open" (cj/external-open-command))))) + +(ert-deftest test-eoc-macos-returns-open () + "Normal: macOS host returns \"open\"." + (test-eoc--with-host nil t nil + (should (string= "open" (cj/external-open-command))))) + +(ert-deftest test-eoc-windows-returns-start () + "Normal: Windows host returns \"start\"." + (test-eoc--with-host nil nil t + (should (string= "start" (cj/external-open-command))))) + +;;; Boundary cases + +(ert-deftest test-eoc-dispatch-order-linux-wins () + "Boundary: Linux check runs first; wins when predicates disagree. +Documents the dispatch order. A real host only returns t from one +of these anyway -- but if something goes wrong, Linux takes priority." + (test-eoc--with-host t t t + (should (string= "xdg-open" (cj/external-open-command))))) + +(ert-deftest test-eoc-unsupported-host-returns-nil () + "Boundary: when no platform predicate returns non-nil, returns nil. +Callers requiring a command must handle the nil case explicitly -- +this is a behavior change from the prior `cj/identify-external-open-command' +which signaled an error. The wrapper `cj/xdg-open' converts nil to a +user-error with a clear message." + (test-eoc--with-host nil nil nil + (should-not (cj/external-open-command)))) + +(provide 'test-external-open-command) +;;; test-external-open-command.el ends here diff --git a/tests/test-external-open-launcher-p.el b/tests/test-external-open-launcher-p.el new file mode 100644 index 00000000..4aca3a55 --- /dev/null +++ b/tests/test-external-open-launcher-p.el @@ -0,0 +1,55 @@ +;;; test-external-open-launcher-p.el --- Tests for cj/external-open-launcher-p -*- lexical-binding: t; -*- + +;;; Commentary: +;; Unit tests for `cj/external-open-launcher-p' in external-open.el. +;; The predicate returns t for desktop launcher commands (xdg-open, +;; open, start) that need `call-process' with a zero buffer argument +;; to fully detach from Emacs. Anything else returns nil. + +;;; Code: + +(require 'ert) +(require 'package) + +(setq package-user-dir (expand-file-name "elpa" user-emacs-directory)) +(package-initialize) +(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory)) +(require 'external-open) + +;;; Normal cases + +(ert-deftest test-eolp-xdg-open-is-launcher () + "Normal: \"xdg-open\" (Linux launcher) returns t." + (should (eq t (cj/external-open-launcher-p "xdg-open")))) + +(ert-deftest test-eolp-open-is-launcher () + "Normal: \"open\" (macOS launcher) returns t." + (should (eq t (cj/external-open-launcher-p "open")))) + +(ert-deftest test-eolp-start-is-launcher () + "Normal: \"start\" (Windows launcher) returns t." + (should (eq t (cj/external-open-launcher-p "start")))) + +;;; Boundary cases + +(ert-deftest test-eolp-non-launcher-command-returns-nil () + "Boundary: a non-launcher command (e.g. gimp) returns nil." + (should-not (cj/external-open-launcher-p "gimp"))) + +(ert-deftest test-eolp-empty-string-returns-nil () + "Boundary: empty string is not a launcher." + (should-not (cj/external-open-launcher-p ""))) + +(ert-deftest test-eolp-case-sensitive () + "Boundary: launcher check is case-sensitive (\"Open\" is not \"open\")." + (should-not (cj/external-open-launcher-p "Open")) + (should-not (cj/external-open-launcher-p "XDG-OPEN"))) + +;;; Error cases + +(ert-deftest test-eolp-nil-argument-returns-nil () + "Error: nil input is handled gracefully (not in the launcher list)." + (should-not (cj/external-open-launcher-p nil))) + +(provide 'test-external-open-launcher-p) +;;; test-external-open-launcher-p.el ends here diff --git a/tests/test-system-utils--open-with-is-launcher-p.el b/tests/test-system-utils--open-with-is-launcher-p.el deleted file mode 100644 index 64e9a4b6..00000000 --- a/tests/test-system-utils--open-with-is-launcher-p.el +++ /dev/null @@ -1,52 +0,0 @@ -;;; test-system-utils--open-with-is-launcher-p.el --- Tests for cj/--open-with-is-launcher-p -*- lexical-binding: t; -*- - -;;; Commentary: -;; Unit tests for `cj/--open-with-is-launcher-p' in system-utils.el. -;; The predicate returns t for desktop launcher commands (xdg-open, -;; open, start) that need `call-process' with a zero buffer argument -;; to fully detach from Emacs. Anything else returns nil. - -;;; Code: - -(require 'ert) - -(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory)) -(require 'system-utils) - -;;; Normal cases - -(ert-deftest test-owilp-xdg-open-is-launcher () - "Normal: \"xdg-open\" (Linux launcher) returns t." - (should (eq t (cj/--open-with-is-launcher-p "xdg-open")))) - -(ert-deftest test-owilp-open-is-launcher () - "Normal: \"open\" (macOS launcher) returns t." - (should (eq t (cj/--open-with-is-launcher-p "open")))) - -(ert-deftest test-owilp-start-is-launcher () - "Normal: \"start\" (Windows launcher) returns t." - (should (eq t (cj/--open-with-is-launcher-p "start")))) - -;;; Boundary cases - -(ert-deftest test-owilp-non-launcher-command-returns-nil () - "Boundary: a non-launcher command (e.g. gimp) returns nil." - (should-not (cj/--open-with-is-launcher-p "gimp"))) - -(ert-deftest test-owilp-empty-string-returns-nil () - "Boundary: empty string is not a launcher." - (should-not (cj/--open-with-is-launcher-p ""))) - -(ert-deftest test-owilp-case-sensitive () - "Boundary: launcher check is case-sensitive (\"Open\" is not \"open\")." - (should-not (cj/--open-with-is-launcher-p "Open")) - (should-not (cj/--open-with-is-launcher-p "XDG-OPEN"))) - -;;; Error cases - -(ert-deftest test-owilp-nil-argument-returns-nil () - "Error: nil input is handled gracefully (not in the launcher list)." - (should-not (cj/--open-with-is-launcher-p nil))) - -(provide 'test-system-utils--open-with-is-launcher-p) -;;; test-system-utils--open-with-is-launcher-p.el ends here diff --git a/tests/test-system-utils-identify-external-open-command.el b/tests/test-system-utils-identify-external-open-command.el deleted file mode 100644 index dc272570..00000000 --- a/tests/test-system-utils-identify-external-open-command.el +++ /dev/null @@ -1,59 +0,0 @@ -;;; test-system-utils-identify-external-open-command.el --- Tests for cj/identify-external-open-command -*- lexical-binding: t; -*- - -;;; Commentary: -;; Unit tests for `cj/identify-external-open-command' in system-utils.el. -;; The function dispatches on host-environment predicates to return the -;; appropriate "open" command: xdg-open on Linux, open on macOS, -;; start on Windows. Anything else is a fatal error. - -;;; Code: - -(require 'ert) -(require 'cl-lib) - -(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory)) -(require 'system-utils) - -(defmacro test-siuec--with-host (linux macos windows &rest body) - "Run BODY with env-*-p predicates stubbed to LINUX, MACOS, WINDOWS." - (declare (indent 3)) - `(cl-letf (((symbol-function 'env-linux-p) (lambda () ,linux)) - ((symbol-function 'env-macos-p) (lambda () ,macos)) - ((symbol-function 'env-windows-p) (lambda () ,windows))) - ,@body)) - -;;; Normal cases - -(ert-deftest test-siuec-linux-returns-xdg-open () - "Normal: Linux host returns \"xdg-open\"." - (test-siuec--with-host t nil nil - (should (string= "xdg-open" (cj/identify-external-open-command))))) - -(ert-deftest test-siuec-macos-returns-open () - "Normal: macOS host returns \"open\"." - (test-siuec--with-host nil t nil - (should (string= "open" (cj/identify-external-open-command))))) - -(ert-deftest test-siuec-windows-returns-start () - "Normal: Windows host returns \"start\"." - (test-siuec--with-host nil nil t - (should (string= "start" (cj/identify-external-open-command))))) - -;;; Boundary cases - -(ert-deftest test-siuec-dispatch-order-linux-wins () - "Boundary: Linux check runs first; wins when predicates disagree. -Documents the dispatch order. A real host only returns t from one -of these anyway — but if something goes wrong, Linux takes priority." - (test-siuec--with-host t t t - (should (string= "xdg-open" (cj/identify-external-open-command))))) - -;;; Error cases - -(ert-deftest test-siuec-unsupported-host-signals-error () - "Error: when no platform predicate returns non-nil, signals an error." - (test-siuec--with-host nil nil nil - (should-error (cj/identify-external-open-command)))) - -(provide 'test-system-utils-identify-external-open-command) -;;; test-system-utils-identify-external-open-command.el ends here -- cgit v1.2.3