From 618bc7813b9acfcf1dfccc9c6590f6f5aece86cf Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Sun, 10 May 2026 15:37:36 -0500 Subject: refactor(external-open): extract external-open-lib for shared helpers Same shared-helpers split-pattern that ai-vterm/vterm-config use through cj-window-toggle-lib and that calendar-sync uses through cj-org-text-lib. Pull the two pure dispatch helpers out of the external-open feature module into a sibling library so consumers that only need the dispatch don't have to require the whole feature. New `modules/external-open-lib.el' carries: - `cj/external-open-command' - `cj/external-open-launcher-p' `modules/external-open.el' stays as the feature module: the `default-open-extensions' defcustom, the `find-file' advice (`cj/find-file-auto'), and the interactive commands (`cj/xdg-open', `cj/open-this-file-with'). It now requires external-open-lib for the dispatch helpers. Migrate consumers: - system-utils.el used to require `external-open' for `cj/external-open-launcher-p' alone -- now requires `external-open-lib' directly. - dirvish-config.el calls `cj/external-open-command' from `cj/dirvish-open-file-manager-here' -- add an explicit `(require \='external-open-lib)'. Test files renamed to match the system-lib naming pattern (test--.el): - test-external-open-command.el -> test-external-open-lib-command.el - test-external-open-launcher-p.el -> test-external-open-lib-launcher-p.el No behavior change. --- modules/dirvish-config.el | 1 + modules/external-open-lib.el | 42 +++++++++++++++++++ modules/external-open.el | 23 +---------- modules/system-utils.el | 2 +- tests/test-external-open-command.el | 65 ------------------------------ tests/test-external-open-launcher-p.el | 55 ------------------------- tests/test-external-open-lib-command.el | 65 ++++++++++++++++++++++++++++++ tests/test-external-open-lib-launcher-p.el | 55 +++++++++++++++++++++++++ 8 files changed, 165 insertions(+), 143 deletions(-) create mode 100644 modules/external-open-lib.el delete mode 100644 tests/test-external-open-command.el delete mode 100644 tests/test-external-open-launcher-p.el create mode 100644 tests/test-external-open-lib-command.el create mode 100644 tests/test-external-open-lib-launcher-p.el diff --git a/modules/dirvish-config.el b/modules/dirvish-config.el index d5834dac..bf91ae2e 100644 --- a/modules/dirvish-config.el +++ b/modules/dirvish-config.el @@ -27,6 +27,7 @@ (eval-when-compile (require 'system-utils)) (require 'host-environment) (require 'system-lib) +(require 'external-open-lib) ;; mark files in dirvish, attach in mu4e (add-hook 'dired-mode-hook 'turn-on-gnus-dired-mode) diff --git a/modules/external-open-lib.el b/modules/external-open-lib.el new file mode 100644 index 00000000..aa90eb67 --- /dev/null +++ b/modules/external-open-lib.el @@ -0,0 +1,42 @@ +;;; external-open-lib.el --- Pure helpers for OS open-with dispatch -*- lexical-binding: t; -*- + +;; Author: Craig Jennings + +;;; Commentary: + +;; Pure helpers for resolving the OS-default "open" command and +;; recognizing desktop launchers. No side effects, no state. The +;; feature module (`external-open.el') uses these helpers; consumers +;; that only need the dispatch (system-utils' "open with command", +;; dirvish's "open file manager here") require this library directly +;; instead of the feature module. +;; +;; Pulled out of `external-open.el' as part of utility-consolidation +;; Phase 4. See `docs/design/utility-consolidation.org'. + +;;; Code: + +(require 'host-environment) + +(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)) + +(provide 'external-open-lib) +;;; external-open-lib.el ends here diff --git a/modules/external-open.el b/modules/external-open.el index c9b5f1f6..0d6ec520 100644 --- a/modules/external-open.el +++ b/modules/external-open.el @@ -23,6 +23,7 @@ (require 'host-environment) ;; environment information functions (require 'system-lib) ;; for cj/file-from-context +(require 'external-open-lib) ;; pure dispatch helpers (require 'cl-lib) ;; Declare platform-specific functions @@ -89,28 +90,6 @@ :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*." diff --git a/modules/system-utils.el b/modules/system-utils.el index 008a5396..43200403 100644 --- a/modules/system-utils.el +++ b/modules/system-utils.el @@ -24,7 +24,7 @@ ;;; Code: (require 'system-lib) -(require 'external-open) +(require 'external-open-lib) (declare-function dired-get-file-for-visit "dired" ()) (declare-function dired-file-name-at-point "dired" ()) diff --git a/tests/test-external-open-command.el b/tests/test-external-open-command.el deleted file mode 100644 index b408b305..00000000 --- a/tests/test-external-open-command.el +++ /dev/null @@ -1,65 +0,0 @@ -;;; 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 deleted file mode 100644 index 4aca3a55..00000000 --- a/tests/test-external-open-launcher-p.el +++ /dev/null @@ -1,55 +0,0 @@ -;;; 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-external-open-lib-command.el b/tests/test-external-open-lib-command.el new file mode 100644 index 00000000..7f54442c --- /dev/null +++ b/tests/test-external-open-lib-command.el @@ -0,0 +1,65 @@ +;;; test-external-open-lib-command.el --- Tests for cj/external-open-command -*- lexical-binding: t; -*- + +;;; Commentary: +;; Unit tests for `cj/external-open-command' in external-open-lib.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-lib) + +(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-lib-command) +;;; test-external-open-lib-command.el ends here diff --git a/tests/test-external-open-lib-launcher-p.el b/tests/test-external-open-lib-launcher-p.el new file mode 100644 index 00000000..928293b4 --- /dev/null +++ b/tests/test-external-open-lib-launcher-p.el @@ -0,0 +1,55 @@ +;;; test-external-open-lib-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-lib.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-lib) + +;;; 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-lib-launcher-p) +;;; test-external-open-lib-launcher-p.el ends here -- cgit v1.2.3