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).

8 files changed, 170 insertions, 222 deletions

diff --git a/modules/dirvish-config.el b/modules/dirvish-config.el
index 89f6c397..d5834dac 100644
--- a/modules/dirvish-config.el
+++ b/modules/dirvish-config.el
@@ -246,21 +246,6 @@ Examples:
 
 ;;; ----------------------- Dirvish Open File Manager Here ----------------------
 
-(defun cj/--file-manager-program-for (has-xdg-open-p system-type)
-  "Return the file-manager command for HAS-XDG-OPEN-P + SYSTEM-TYPE, or nil.
-
-Pure helper used by `cj/dirvish-open-file-manager-here'.  When
-HAS-XDG-OPEN-P is non-nil, returns \"xdg-open\" regardless of
-SYSTEM-TYPE -- xdg-open works on Linux and many ported environments.
-Without xdg-open, falls back to `darwin' -> \"open\", `windows-nt' ->
-\"explorer\", everything else -> nil so the caller can shell-command
-its way out."
-  (cond
-   (has-xdg-open-p "xdg-open")
-   ((eq system-type 'darwin) "open")
-   ((eq system-type 'windows-nt) "explorer")
-   (t nil)))
-
 (defun cj/dirvish-open-file-manager-here ()
   "Open system's default file manager in the current dired/dirvish directory.
 Always opens the file manager in the directory currently being displayed,
@@ -271,9 +256,8 @@ regardless of what file or subdirectory the point is on."
         (progn
           (message "Opening file manager in %s..." current-dir)
           ;; Use pipe instead of pty for the async call-process below.
-          (let* ((process-connection-type nil)
-                 (program (cj/--file-manager-program-for
-                           (executable-find "xdg-open") system-type)))
+          (let ((process-connection-type nil)
+                (program (cj/external-open-command)))
             (if program
                 (call-process program nil 0 nil current-dir)
               (shell-command (format "xdg-open %s &"
diff --git a/modules/external-open.el b/modules/external-open.el
index 9eddf352..c9b5f1f6 100644
--- a/modules/external-open.el
+++ b/modules/external-open.el
@@ -21,8 +21,8 @@
 ;;
 ;;; Code:
 
-(require 'system-utils) ;; for xdg-open and others
 (require 'host-environment) ;; environment information functions
+(require 'system-lib) ;; for cj/file-from-context
 (require 'cl-lib)
 
 ;; Declare platform-specific functions
@@ -89,6 +89,51 @@
   :type '(repeat (regexp :tag "File extension regexp"))
   :group 'external-open)
 
+;; ----------------------- External-Open Command Resolution -------------------
+
+(defun cj/external-open-command ()
+  "Return the OS-default \"open\" command for this host, or nil if unsupported.
+Returns one of \"xdg-open\" (Linux), \"open\" (macOS), \"start\" (Windows).
+Callers that require a command should error on nil with a contextual
+message so the user sees what feature is unavailable."
+  (cond
+   ((env-linux-p)   "xdg-open")
+   ((env-macos-p)   "open")
+   ((env-windows-p) "start")
+   (t nil)))
+
+(defun cj/external-open-launcher-p (command)
+  "Return non-nil when COMMAND is a desktop launcher.
+Launchers (xdg-open, open, start) need to be called with `call-process'
+and a zero BUFFER argument so they fully detach from Emacs.  Other
+commands get `start-process-shell-command' so their output is visible."
+  (and (stringp command)
+       (member command '("xdg-open" "open" "start"))
+       t))
+
+(defun cj/xdg-open (&optional filename)
+  "Open FILENAME (or the file at point) with the OS default handler.
+Logs output and exit code to buffer *external-open.log*."
+  (interactive)
+  (let* ((file  (expand-file-name
+                 (or (cj/file-from-context filename)
+                     (user-error "No file associated with this buffer"))))
+         (cmd   (or (cj/external-open-command)
+                    (user-error "External-open: unsupported host environment")))
+         (logbuf (get-buffer-create "*external-open.log*")))
+    (with-current-buffer logbuf
+      (goto-char (point-max))
+      (insert (format-time-string "[%Y-%m-%d %H:%M:%S] "))
+      (insert (format "Opening: %s\n" file)))
+    (cond
+     ((env-windows-p)
+      (w32-shell-execute "open" file))
+     (t
+      (call-process cmd nil 0 nil file)
+      (with-current-buffer logbuf
+        (insert "  → Launched asynchronously\n"))))
+    nil))
+
 ;; ------------------------------- Open File With ------------------------------
 
 (defun cj/open-this-file-with (command)
diff --git a/modules/system-utils.el b/modules/system-utils.el
index e266cd15..008a5396 100644
--- a/modules/system-utils.el
+++ b/modules/system-utils.el
@@ -24,6 +24,7 @@
 ;;; Code:
 
 (require 'system-lib)
+(require 'external-open)
 
 (declare-function dired-get-file-for-visit "dired" ())
 (declare-function dired-file-name-at-point "dired" ())
@@ -57,13 +58,6 @@
 ;;; ------------------------------- Open File With ------------------------------
 ;; TASK: Favor this method over cj/open-this-file-with and add to custom buffer funcs
 
-(defun cj/--open-with-is-launcher-p (command)
-  "Return non-nil if COMMAND is a desktop launcher.
-Launchers (xdg-open, open, start) need to be called with `call-process'
-and a zero BUFFER argument so they fully detach from Emacs.  Other
-commands get `start-process-shell-command' so their output is visible."
-  (and (member command '("xdg-open" "open" "start")) t))
-
 (defun cj/open-file-with-command (command)
   "Open the current file with COMMAND.
 Works in both Dired buffers and regular file buffers.  Prompts for a
@@ -74,7 +68,7 @@ detached from Emacs."
 				   (read-file-name "File to open: "))))
 	(unless (and file (file-exists-p file))
 	  (error "No valid file found or selected"))
-	(if (cj/--open-with-is-launcher-p command)
+	(if (cj/external-open-launcher-p command)
 		(progn
 		  (call-process command nil 0 nil file)
 		  (message "Opening %s with %s..."
@@ -90,36 +84,6 @@ detached from Emacs."
 		(message "Running %s on %s..."
 				 (file-name-nondirectory file) command)))))
 
-(defun cj/identify-external-open-command ()
-  "Return the OS-default \"open\" command for this host.
-Signals an error if the host is unsupported."
-  (cond
-   ((env-linux-p)   "xdg-open")
-   ((env-macos-p)   "open")
-   ((env-windows-p) "start")
-   (t (error "External-open: unsupported host environment"))))
-
-(defun cj/xdg-open (&optional filename)
-  "Open FILENAME (or the file at point) with the OS default handler.
-Logs output and exit code to buffer *external-open.log*."
-  (interactive)
-  (let* ((file  (expand-file-name
-				 (or (cj/file-from-context filename)
-					 (user-error "No file associated with this buffer"))))
-		 (cmd   (cj/identify-external-open-command))
-		 (logbuf (get-buffer-create "*external-open.log*")))
-	(with-current-buffer logbuf
-	  (goto-char (point-max))
-	  (insert (format-time-string "[%Y-%m-%d %H:%M:%S] "))
-	  (insert (format "Opening: %s\n" file)))
-	(cond
-	 ((env-windows-p)
-	  (w32-shell-execute "open" file))
-	 (t
-	  (call-process cmd nil 0 nil file)
-	  (with-current-buffer logbuf
-		(insert "  → Launched asynchronously\n"))))
-	nil))
 
 ;;; ------------------------------ Server Shutdown ------------------------------
 
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