From fb2593ad55d0523dc211019f7ec856d5898d7c99 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Thu, 23 Apr 2026 01:47:52 -0500 Subject: refactor(system-utils): extract testable open-file helpers Extracts two pure helpers from cj/open-file-with-command and cj/xdg-open so the file-resolution and launcher-detection logic becomes testable without mocking process launchers. New helpers: - cj/--file-from-context returns a file path from the current context, resolving in priority order (explicit arg, buffer-file-name, dired file at point). Returns nil when none apply. - cj/--open-with-is-launcher-p is a predicate for whether a command is a desktop launcher (xdg-open, open, start) that needs call-process detachment. Both commands now delegate. cj/open-file-with-command uses cj/--file-from-context with read-file-name as the final fallback, plus cj/--open-with-is-launcher-p for the launcher dispatch. cj/xdg-open uses cj/--file-from-context with user-error as the "no file" fallback. Behavior preserved. The existing system-utils test suites still pass, and the shape of each command's final effect is identical. New tests, 14 cases across two per-function files: - tests/test-system-utils--file-from-context.el covers: explicit wins over buffer-file, explicit wins over dired, buffer-file fallback, dired fallback, all-nil returns nil, explicit-nil uses chain, dired-mode-but-no-file-at-point. - tests/test-system-utils--open-with-is-launcher-p.el covers: each of the three launcher names returns t, non-launcher returns nil, empty string returns nil, case-sensitive check, nil input returns nil. Coverage: system-utils.el went from 10/52 (19.2%) to 15/52 (28.8%). The remaining uncovered lines are mostly in the process-launching paths of cj/open-file-with-command and cj/xdg-open. Those are testability-blocked. Mocking call-process, start-process-shell-command, and generate-new-buffer would give a lot of mock surface for low value. cj/server-shutdown is not meaningfully testable because it kills Emacs. --- modules/system-utils.el | 111 +++++++++++---------- tests/test-system-utils--file-from-context.el | 76 ++++++++++++++ .../test-system-utils--open-with-is-launcher-p.el | 52 ++++++++++ 3 files changed, 185 insertions(+), 54 deletions(-) create mode 100644 tests/test-system-utils--file-from-context.el create mode 100644 tests/test-system-utils--open-with-is-launcher-p.el diff --git a/modules/system-utils.el b/modules/system-utils.el index 29076f6d..86c2ae16 100644 --- a/modules/system-utils.el +++ b/modules/system-utils.el @@ -55,43 +55,50 @@ ;;; ------------------------------- Open File With ------------------------------ ;; TASK: Favor this method over cj/open-this-file-with and add to custom buffer funcs +(defun cj/--file-from-context (&optional explicit-filename) + "Return a file path from the current context, or nil. +Resolves in priority order: + 1. EXPLICIT-FILENAME, if non-nil. + 2. `buffer-file-name' of the current buffer. + 3. The file at point if the current buffer is in dired-mode. +Returns nil when none of these yield a file." + (or explicit-filename + buffer-file-name + (and (derived-mode-p 'dired-mode) + (dired-file-name-at-point)))) + +(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. The command runs -fully detached from Emacs." +Works in both Dired buffers and regular file buffers. Prompts for a +file only when neither context yields one. The command runs fully +detached from Emacs." (interactive "MOpen with command: ") - (let* ((file (cond - ;; In dired/dirvish mode, get file at point - ((derived-mode-p 'dired-mode) - (dired-get-file-for-visit)) - ;; In a regular file buffer - (buffer-file-name - buffer-file-name) - ;; Fallback - prompt for file - (t - (read-file-name "File to open: ")))) - ;; For xdg-open and similar launchers, we need special handling - (is-launcher (member command '("xdg-open" "open" "start")))) - ;; Validate file exists - (unless (and file (file-exists-p file)) - (error "No valid file found or selected")) - ;; Use different approaches for launchers vs regular commands - (if is-launcher - ;; For launchers, use call-process with 0 to fully detach - (progn - (call-process command nil 0 nil file) - (message "Opening %s with %s..." (file-name-nondirectory file) command)) - ;; For other commands, use start-process-shell-command for potential output - (let* ((output-buffer-name (format "*Open with %s: %s*" - command - (file-name-nondirectory file))) - (output-buffer (generate-new-buffer output-buffer-name))) - (start-process-shell-command - command - output-buffer - (format "%s %s" command (shell-quote-argument file))) - (message "Running %s on %s..." command (file-name-nondirectory file)))))) - + (let* ((file (or (cj/--file-from-context) + (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) + (progn + (call-process command nil 0 nil file) + (message "Opening %s with %s..." + (file-name-nondirectory file) command)) + (let* ((output-buffer-name (format "*Open with %s: %s*" + command + (file-name-nondirectory file))) + (output-buffer (generate-new-buffer output-buffer-name))) + (start-process-shell-command + command + output-buffer + (format "%s %s" command (shell-quote-argument file))) + (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. @@ -107,26 +114,22 @@ Signals an error if the host is unsupported." Logs output and exit code to buffer *external-open.log*." (interactive) (let* ((file (expand-file-name - (or filename - buffer-file-name - (and (derived-mode-p 'dired-mode) (dired-file-name-at-point)) - (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 - ;; Windows: let the shell handle association; fully detached. - ((env-windows-p) - (w32-shell-execute "open" file)) - ;; macOS/Linux: run the opener synchronously; it returns immediately. - (t - (call-process cmd nil 0 nil file) - (with-current-buffer logbuf - (insert " → Launched asynchronously\n")))) - nil)) + (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-system-utils--file-from-context.el b/tests/test-system-utils--file-from-context.el new file mode 100644 index 00000000..af4b0f82 --- /dev/null +++ b/tests/test-system-utils--file-from-context.el @@ -0,0 +1,76 @@ +;;; test-system-utils--file-from-context.el --- Tests for cj/--file-from-context -*- lexical-binding: t; -*- + +;;; Commentary: +;; Unit tests for `cj/--file-from-context' in system-utils.el. The +;; helper returns a file path from the current context, resolving in +;; priority order: explicit argument, `buffer-file-name', dired file +;; at point. Returns nil when none of these yield a file. + +;;; Code: + +(require 'ert) +(require 'cl-lib) + +(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory)) +(require 'system-utils) + +(defmacro test-ffc--with-context (buffer-file in-dired dired-file &rest body) + "Run BODY with context stubbed for testing `cj/--file-from-context'. +BUFFER-FILE becomes the value of `buffer-file-name'. +IN-DIRED controls `derived-mode-p' (t to simulate dired-mode). +DIRED-FILE becomes the return value of `dired-file-name-at-point'." + (declare (indent 3)) + `(let ((buffer-file-name ,buffer-file)) + (cl-letf (((symbol-function 'derived-mode-p) + (lambda (&rest modes) + (and ,in-dired (memq 'dired-mode modes)))) + ((symbol-function 'dired-file-name-at-point) + (lambda () ,dired-file))) + ,@body))) + +;;; Normal cases + +(ert-deftest test-ffc-explicit-wins-over-buffer-file () + "Normal: an explicit filename argument wins over `buffer-file-name'." + (test-ffc--with-context "/from-buffer.el" nil nil + (should (string= "/explicit.el" + (cj/--file-from-context "/explicit.el"))))) + +(ert-deftest test-ffc-explicit-wins-over-dired () + "Normal: an explicit filename argument wins over dired file at point." + (test-ffc--with-context nil t "/from-dired.el" + (should (string= "/explicit.el" + (cj/--file-from-context "/explicit.el"))))) + +(ert-deftest test-ffc-buffer-file-used-when-no-explicit () + "Normal: falls back to `buffer-file-name' when no explicit arg." + (test-ffc--with-context "/from-buffer.el" nil nil + (should (string= "/from-buffer.el" + (cj/--file-from-context))))) + +(ert-deftest test-ffc-dired-used-when-no-explicit-no-buffer-file () + "Normal: in a dired buffer, falls back to dired file at point." + (test-ffc--with-context nil t "/from-dired.el" + (should (string= "/from-dired.el" + (cj/--file-from-context))))) + +;;; Boundary cases + +(ert-deftest test-ffc-all-sources-nil-returns-nil () + "Boundary: no explicit, no buffer-file, not in dired → nil." + (test-ffc--with-context nil nil nil + (should-not (cj/--file-from-context)))) + +(ert-deftest test-ffc-explicit-nil-uses-fallback-chain () + "Boundary: explicitly passing nil as the arg still uses the fallback chain." + (test-ffc--with-context "/from-buffer.el" nil nil + (should (string= "/from-buffer.el" + (cj/--file-from-context nil))))) + +(ert-deftest test-ffc-dired-mode-but-no-file-at-point () + "Boundary: in dired but nothing at point returns nil (buffer-file also nil)." + (test-ffc--with-context nil t nil + (should-not (cj/--file-from-context)))) + +(provide 'test-system-utils--file-from-context) +;;; test-system-utils--file-from-context.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 new file mode 100644 index 00000000..64e9a4b6 --- /dev/null +++ b/tests/test-system-utils--open-with-is-launcher-p.el @@ -0,0 +1,52 @@ +;;; 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 -- cgit v1.2.3