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). --- modules/dirvish-config.el | 20 ++------------------ modules/external-open.el | 47 ++++++++++++++++++++++++++++++++++++++++++++++- modules/system-utils.el | 40 ++-------------------------------------- 3 files changed, 50 insertions(+), 57 deletions(-) (limited to 'modules') 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 ------------------------------ -- cgit v1.2.3