summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-05-16 03:55:49 -0500
committerCraig Jennings <c@cjennings.net>2026-05-16 03:55:49 -0500
commitd84aa4374af5e3447445377a836c66cc07d7a223 (patch)
tree0d438568f866feef0a036de7a59192b469e231bc
parenta005c7636f30b710e27a6812ca989506d5df7531 (diff)
downloaddotemacs-d84aa4374af5e3447445377a836c66cc07d7a223.tar.gz
dotemacs-d84aa4374af5e3447445377a836c66cc07d7a223.zip
refactor(prog): six programming-track hygiene fixes from re-review
- prog-lsp.el: rename `cj/lsp--remove-eldoc-provider' → `cj/lsp--remove-eldoc-provider-global' and call it once from the lsp-mode `:config' block instead of attaching it per-buffer via `lsp-managed-mode-hook'. The previous per-buffer remove with the buffer-local flag raced lsp-mode's own population of the local hook; removing the provider from the global default before any LSP buffer attaches makes the absence stick. Two existing tests updated to the new contract (remove-from-default + idempotent re-run). - prog-webdev.el / prog-python.el: warn at load time when `prettier' or `pyright' is missing on PATH via `cj/executable-find-or-warn'. Both modules now `(require 'system-lib)' to expose the helper. Missing dependencies surface up front instead of mid-edit at first format/LSP attach. - keyboard-compat.el: document existing idempotence. The hook install uses a named function so `add-hook' deduplicates, and the hook body only calls `define-key' (latest binding wins, same value) -- adding a comment so future readers don't re-question. - dev-fkeys.el: add a `typescript' clause to `cj/--f6-test-runner-cmd-for'. F6 now runs `npx --no-install vitest <path>' when vitest is on PATH, otherwise `npx --no-install jest <path>'. Updates the matching test from "returns nil" to cover both code paths; the impl-level test now asserts the routed command instead of expecting a user-error. - flycheck-config.el: build the LanguageTool wrapper path with `(expand-file-name "scripts/languagetool-flycheck" user-emacs-directory)' instead of a hardcoded `~/.emacs.d/...'. Survives a non-standard `user-emacs-directory'. - latex-config.el: replace the hardcoded Zathura viewer with `cj/--latex-select-pdf-viewer', which walks `cj/--latex-pdf-viewer-candidates' (zathura → evince → okular → SumatraPDF → xdg-open) and falls back to "PDF Tools" when nothing is on PATH. Each entry maps an executable to the matching TeX-view-program-list name so AUCTeX's defaults handle the actual viewer invocation.
-rw-r--r--modules/dev-fkeys.el9
-rw-r--r--modules/flycheck-config.el3
-rw-r--r--modules/keyboard-compat.el6
-rw-r--r--modules/latex-config.el33
-rw-r--r--modules/prog-lsp.el18
-rw-r--r--modules/prog-python.el6
-rw-r--r--modules/prog-webdev.el6
-rw-r--r--tests/test-dev-fkeys--f6-current-file-tests-impl.el18
-rw-r--r--tests/test-dev-fkeys--f6-test-runner-cmd-for.el23
-rw-r--r--tests/test-prog-lsp--add-file-watch-ignored-extras.el29
-rw-r--r--todo.org12
11 files changed, 125 insertions, 38 deletions
diff --git a/modules/dev-fkeys.el b/modules/dev-fkeys.el
index 170e70b9..3b0a818c 100644
--- a/modules/dev-fkeys.el
+++ b/modules/dev-fkeys.el
@@ -370,6 +370,15 @@ TypeScript / JavaScript and unknown languages return nil."
(if (string-empty-p rel-dir)
"./"
(format "./%s" rel-dir)))))
+ ('typescript
+ ;; Prefer vitest when present on PATH, fall back to jest otherwise.
+ ;; Both runners take a path argument and accept relative paths.
+ (let ((runner (or (and (executable-find "vitest") "vitest")
+ (and (executable-find "jest") "jest")
+ "jest"))) ; reasonable default for stack traces
+ (format "npx --no-install %s %s"
+ runner
+ (cj/shell-quote-argument-readable rel-path))))
(_ nil)))
;; ---------- F6 current-file orchestrator ----------
diff --git a/modules/flycheck-config.el b/modules/flycheck-config.el
index e6dfb25e..ba408b52 100644
--- a/modules/flycheck-config.el
+++ b/modules/flycheck-config.el
@@ -70,7 +70,8 @@
(flycheck-define-checker languagetool
"A grammar checker using LanguageTool.
Uses a wrapper script to format output for flycheck."
- :command ("~/.emacs.d/scripts/languagetool-flycheck"
+ :command ((eval (expand-file-name "scripts/languagetool-flycheck"
+ user-emacs-directory))
source-inplace)
:error-patterns
((warning line-start (file-name) ":" line ":" column ": "
diff --git a/modules/keyboard-compat.el b/modules/keyboard-compat.el
index b8e91dd8..22263e9c 100644
--- a/modules/keyboard-compat.el
+++ b/modules/keyboard-compat.el
@@ -169,6 +169,12 @@ Meta+Shift+letter triggers M-S-letter keybindings."
;; In daemon mode, no frame exists at startup so env-gui-p returns nil.
;; Use server-after-make-frame-hook to set up translations when the first
;; GUI client connects. In non-daemon mode, run at startup as before.
+;;
+;; `add-hook' is idempotent for named functions (re-adding the same
+;; symbol is a no-op), and `cj/keyboard-compat-gui-setup' itself only
+;; calls `define-key' -- each key has one binding regardless of how many
+;; times the function fires -- so this block is safe under repeated
+;; module loads.
(if (daemonp)
(add-hook 'server-after-make-frame-hook #'cj/keyboard-compat-gui-setup)
(add-hook 'emacs-startup-hook #'cj/keyboard-compat-gui-setup))
diff --git a/modules/latex-config.el b/modules/latex-config.el
index bb4cf510..a0af2a1a 100644
--- a/modules/latex-config.el
+++ b/modules/latex-config.el
@@ -15,6 +15,37 @@
;;
;;; Code:
+;; ----------------------------- PDF Viewer Selection --------------------------
+;;
+;; Pick whichever PDF viewer is available rather than hard-coding Zathura.
+;; The selected viewer is pushed onto `TeX-view-program-selection' for
+;; output-pdf so `C-c C-v' opens the compiled PDF.
+
+(defconst cj/--latex-pdf-viewer-candidates
+ ;; (PROGRAM . TeX-VIEWER-NAME) in preference order. TeX-view-program-list
+ ;; has built-in entries for "Zathura", "Evince", "Okular", "Skim",
+ ;; "PDF Tools", and platform openers; we match against those names.
+ '(("zathura" . "Zathura")
+ ("evince" . "Evince")
+ ("okular" . "Okular")
+ ("SumatraPDF" . "Sumatra PDF")
+ ("xdg-open" . "xdg-open"))
+ "Ordered (EXECUTABLE . TEX-VIEWER-NAME) pairs for PDF preview selection.")
+
+(defvar TeX-view-program-selection)
+(declare-function pdf-view-mode "pdf-view")
+
+(defun cj/--latex-select-pdf-viewer ()
+ "Push the first available external PDF viewer onto `TeX-view-program-selection'.
+Falls back to PDF Tools when no external viewer is on PATH. The new
+selection is consed onto the head of the alist so it wins over any
+default. Idempotent: re-running picks the same viewer."
+ (let* ((found (cl-find-if (lambda (entry)
+ (executable-find (car entry)))
+ cj/--latex-pdf-viewer-candidates))
+ (viewer-name (if found (cdr found) "PDF Tools")))
+ (push (list 'output-pdf viewer-name) TeX-view-program-selection)))
+
;; ----------------------------- Auctex And Related ----------------------------
(use-package tex
@@ -25,7 +56,7 @@
(LaTeX-mode . (lambda () (TeX-fold-mode 1))) ; automatically activate TeX-fold-mode.
(LaTeX-mode . flyspell-mode) ; turn on flyspell-mode by default
(LaTeX-mode . TeX-PDF-mode)
- (LaTeX-mode . (lambda () (push (list 'output-pdf "Zathura") TeX-view-program-selection)))
+ (LaTeX-mode . cj/--latex-select-pdf-viewer)
:mode
("\\.tex\\'" . latex-mode)
:config
diff --git a/modules/prog-lsp.el b/modules/prog-lsp.el
index e03a475d..045dda24 100644
--- a/modules/prog-lsp.el
+++ b/modules/prog-lsp.el
@@ -48,10 +48,15 @@ Idempotent — `add-to-list' skips patterns already present."
(dolist (pattern cj/lsp-file-watch-ignored-extras)
(add-to-list 'lsp-file-watch-ignored-directories pattern)))
-(defun cj/lsp--remove-eldoc-provider ()
- "Remove lsp-mode's provider from `eldoc-documentation-functions' here.
-Buffer-local — the global hook value is untouched."
- (remove-hook 'eldoc-documentation-functions #'lsp-eldoc-function t))
+(defun cj/lsp--remove-eldoc-provider-global ()
+ "Remove lsp-mode's provider from the global `eldoc-documentation-functions'.
+Run once after lsp-mode loads. The previous per-buffer removal raced
+lsp's own buffer-local add: the buffer-local remove fired before lsp
+populated the buffer-local hook (lsp inherits the global default and
+mutates from there), so the buffer-local hook ended up holding the
+provider anyway. Removing globally before lsp ever attaches a buffer
+makes the absence stick for every subsequent lsp-managed buffer."
+ (remove-hook 'eldoc-documentation-functions #'lsp-eldoc-function))
;;;;; ---------------------------- LSP Mode ---------------------------
@@ -73,7 +78,10 @@ Buffer-local — the global hook value is untouched."
(setq lsp-enable-on-type-formatting nil)
(setq lsp-signature-auto-activate nil)
(setq lsp-signature-render-documentation nil)
- (add-hook 'lsp-managed-mode-hook #'cj/lsp--remove-eldoc-provider)
+ ;; Strip lsp-mode's eldoc provider from the GLOBAL hook value once,
+ ;; not per buffer. See `cj/lsp--remove-eldoc-provider-global' for
+ ;; why per-buffer racing didn't stick.
+ (cj/lsp--remove-eldoc-provider-global)
(setq lsp-modeline-code-actions-enable nil)
(setq lsp-modeline-diagnostics-enable nil)
(setq lsp-headerline-breadcrumb-enable nil)
diff --git a/modules/prog-python.el b/modules/prog-python.el
index a4c662e4..5f9a4175 100644
--- a/modules/prog-python.el
+++ b/modules/prog-python.el
@@ -21,6 +21,8 @@
(defvar python-ts-mode-map)
+(require 'system-lib) ; for cj/executable-find-or-warn
+
;; Forward declarations for LSP
(declare-function lsp-deferred "lsp-mode")
@@ -38,6 +40,10 @@ Install with: pip install pyright")
"Path to mypy static type checker.
Install with: pip install mypy")
+;; Warn at load time if pyright is missing rather than waiting for the
+;; first LSP attach to fail with a confusing connection error.
+(cj/executable-find-or-warn pyright-path "pyright LSP" 'prog-python)
+
;; -------------------------------- Python Setup -------------------------------
;; preferences for Python programming
diff --git a/modules/prog-webdev.el b/modules/prog-webdev.el
index 9d69d7fb..fbe3825c 100644
--- a/modules/prog-webdev.el
+++ b/modules/prog-webdev.el
@@ -21,6 +21,8 @@
;;; Code:
+(require 'system-lib) ; for cj/executable-find-or-warn
+
(defvar typescript-ts-mode-map)
(defvar tsx-ts-mode-map)
(defvar js-ts-mode-map)
@@ -39,6 +41,10 @@ Install with: sudo pacman -S typescript-language-server")
"Path to prettier executable.
Install with: sudo pacman -S prettier")
+;; Warn at load time if prettier is missing rather than waiting for the
+;; first format-on-save to fail mid-edit.
+(cj/executable-find-or-warn prettier-path "prettier formatter" 'prog-webdev)
+
;; ------------------------------ Web Dev Setup --------------------------------
;; shared setup for TypeScript, JavaScript, and TSX modes
diff --git a/tests/test-dev-fkeys--f6-current-file-tests-impl.el b/tests/test-dev-fkeys--f6-current-file-tests-impl.el
index 318a78db..1cf22230 100644
--- a/tests/test-dev-fkeys--f6-current-file-tests-impl.el
+++ b/tests/test-dev-fkeys--f6-current-file-tests-impl.el
@@ -105,13 +105,17 @@ just that file's tests run, not the whole module's prefix."
"/home/u/proj/modules/foo.el" nil)
:type 'user-error)))
-(ert-deftest test-dev-fkeys-f6-current-file-tests-impl-unsupported-language-errors ()
- "Error: a file with no language-specific runner signals a user-error
-naming the language."
- (cl-letf (((symbol-function 'compile) (lambda (_cmd) nil)))
- (should-error (cj/--f6-current-file-tests-impl
- "/home/u/proj/src/foo.test.ts" "/home/u/proj/")
- :type 'user-error)))
+(ert-deftest test-dev-fkeys-f6-current-file-tests-impl-typescript-runs-jest ()
+ "TypeScript now routes to the `npx --no-install jest|vitest <path>'
+runner instead of erroring as unsupported."
+ (let ((compile-called nil))
+ (cl-letf (((symbol-function 'compile)
+ (lambda (cmd) (setq compile-called cmd)))
+ ((symbol-function 'executable-find) (lambda (_) nil)))
+ (cj/--f6-current-file-tests-impl
+ "/home/u/proj/src/foo.test.ts" "/home/u/proj/")
+ (should (stringp compile-called))
+ (should (string-match-p "jest src/foo.test.ts" compile-called)))))
(ert-deftest test-dev-fkeys-f6-current-file-tests-impl-unknown-language-errors ()
"Error: an unknown extension signals a user-error rather than running
diff --git a/tests/test-dev-fkeys--f6-test-runner-cmd-for.el b/tests/test-dev-fkeys--f6-test-runner-cmd-for.el
index 36f97548..9a552612 100644
--- a/tests/test-dev-fkeys--f6-test-runner-cmd-for.el
+++ b/tests/test-dev-fkeys--f6-test-runner-cmd-for.el
@@ -118,10 +118,25 @@ would over-match. Pass just the basename — the Makefile re-prepends
;;; Error Cases
-(ert-deftest test-dev-fkeys-f6-cmd-for-typescript-returns-nil ()
- "Error: TypeScript is punted for v1 and returns nil."
- (should (null (cj/--f6-test-runner-cmd-for
- 'typescript t "src/foo.test.ts" "foo" "src"))))
+(ert-deftest test-dev-fkeys-f6-cmd-for-typescript-uses-jest-or-vitest ()
+ "Normal: TypeScript builds an `npx vitest|jest <path>' command.
+Picks vitest when on PATH, jest otherwise. Falls back to jest as the
+default runner so the command shape is always inspectable -- even if
+neither tool is present, the user gets a clear runner-not-found error
+rather than a silent nil that F6's outer wrapper interprets as
+\"language unsupported.\""
+ (cl-letf (((symbol-function 'executable-find)
+ (lambda (_) nil)))
+ (should (equal
+ (cj/--f6-test-runner-cmd-for
+ 'typescript t "src/foo.test.ts" "foo" "src")
+ "npx --no-install jest src/foo.test.ts")))
+ (cl-letf (((symbol-function 'executable-find)
+ (lambda (p) (when (equal p "vitest") "/usr/bin/vitest"))))
+ (should (equal
+ (cj/--f6-test-runner-cmd-for
+ 'typescript t "src/foo.test.ts" "foo" "src")
+ "npx --no-install vitest src/foo.test.ts"))))
(ert-deftest test-dev-fkeys-f6-cmd-for-javascript-returns-nil ()
"Error: JavaScript is punted for v1 and returns nil."
diff --git a/tests/test-prog-lsp--add-file-watch-ignored-extras.el b/tests/test-prog-lsp--add-file-watch-ignored-extras.el
index 142cd2ea..9b71cab8 100644
--- a/tests/test-prog-lsp--add-file-watch-ignored-extras.el
+++ b/tests/test-prog-lsp--add-file-watch-ignored-extras.el
@@ -87,23 +87,24 @@
(should-error (cj/lsp--add-file-watch-ignored-extras)
:type 'wrong-type-argument)))
-(ert-deftest test-prog-lsp--remove-eldoc-provider-removes-lsp-provider-locally ()
- "Normal: remove lsp-mode's Eldoc provider from the buffer-local hook."
- (with-temp-buffer
- (setq-local eldoc-documentation-functions
- '(lsp-eldoc-function eldoc-documentation-default))
- (cj/lsp--remove-eldoc-provider)
+(ert-deftest test-prog-lsp--remove-eldoc-provider-global-removes-from-default ()
+ "Normal: remove lsp-mode's Eldoc provider from the global hook value.
+The per-buffer removal that this replaced raced lsp-mode's own buffer-
+local hook population; removing globally before any LSP buffer attaches
+makes the absence stick for every subsequent lsp-managed buffer."
+ (let ((eldoc-documentation-functions
+ '(lsp-eldoc-function eldoc-documentation-default)))
+ (cj/lsp--remove-eldoc-provider-global)
(should-not (memq #'lsp-eldoc-function eldoc-documentation-functions))
(should (memq 'eldoc-documentation-default eldoc-documentation-functions))))
-(ert-deftest test-prog-lsp--remove-eldoc-provider-does-not-touch-default-value ()
- "Boundary: removing the LSP provider in one buffer leaves the default hook alone."
- (let ((eldoc-documentation-functions '(lsp-eldoc-function)))
- (with-temp-buffer
- (setq-local eldoc-documentation-functions '(lsp-eldoc-function))
- (cj/lsp--remove-eldoc-provider)
- (should-not (memq #'lsp-eldoc-function eldoc-documentation-functions)))
- (should (memq #'lsp-eldoc-function eldoc-documentation-functions))))
+(ert-deftest test-prog-lsp--remove-eldoc-provider-global-is-idempotent ()
+ "Boundary: re-running the removal after the provider is gone is a no-op."
+ (let ((eldoc-documentation-functions '(eldoc-documentation-default)))
+ (cj/lsp--remove-eldoc-provider-global)
+ (cj/lsp--remove-eldoc-provider-global)
+ (should (equal eldoc-documentation-functions
+ '(eldoc-documentation-default)))))
(ert-deftest test-prog-lsp--module-no-obsolete-lsp-eldoc-hook-reference ()
"Regression: prog-lsp should not reference obsolete `lsp-eldoc-hook'."
diff --git a/todo.org b/todo.org
index 1bcab75b..1c215c3e 100644
--- a/todo.org
+++ b/todo.org
@@ -1934,7 +1934,7 @@ Expected outcome:
- Keep existing formatter wiring tests and add command-construction tests if a
helper is extracted.
-**** TODO [#C] Add =executable-find= checks for =prettier= and =pyright= at load time :safety:
+**** 2026-05-16 Sat @ 03:54:56 -0500 Added cj/executable-find-or-warn checks for prettier and pyright at load time
=modules/prog-webdev.el:34-40= declares =prettier-path= and
=modules/prog-python.el:33-40= declares =pyright-path= as string
@@ -1944,7 +1944,7 @@ mid-edit, then the user has to discover why. Wrap with
=cj/executable-find-or-warn= (already in =system-lib.el=) at module
load time so the missing dependency is reported up front.
-**** TODO [#C] Make =keyboard-compat= =server-after-make-frame-hook= idempotent :safety:
+**** 2026-05-16 Sat @ 03:54:56 -0500 Documented keyboard-compat hook idempotence (already correct)
=modules/keyboard-compat.el:169-174= adds the frame-setup hook
unconditionally. If the module is required twice (e.g. via two
@@ -1953,7 +1953,7 @@ twice per new frame and installs duplicate =key-translation-map=
entries. Wrap the =add-hook= in a guard, or use a named function
and rely on =add-hook='s own duplicate-check.
-**** TODO [#C] Wire =dev-fkeys= F6 test-runner clause for typescript / tsx :refactor:
+**** 2026-05-16 Sat @ 03:54:56 -0500 Wired F6 TypeScript clause to npx vitest|jest
=modules/dev-fkeys.el:261-269= maps =tsx= to =typescript= in the
language detection table. =modules/dev-fkeys.el:347-349=
@@ -1962,7 +1962,7 @@ the catch-all =(_)= returns nil, so F6 errors instead of routing to
a real runner. Either add a =typescript= → =jest=/=vitest= clause
or remove the =tsx= mapping until the runner side is implemented.
-**** TODO [#B] Fix =prog-lsp= eldoc-provider removal scope :bug:
+**** 2026-05-16 Sat @ 03:54:56 -0500 Fixed prog-lsp eldoc-provider removal to act on the global hook
=modules/prog-lsp.el:51-54,76= attaches
=cj/lsp--remove-eldoc-provider= globally to =lsp-managed-mode-hook=
@@ -1975,7 +1975,7 @@ the hook itself buffer-local-friendly (add it inside
=lsp-managed-mode-hook= per-buffer) or remove from the global
provider list once instead of per-buffer.
-**** TODO [#C] Externalize hardcoded LanguageTool script path in =flycheck-config= :cleanup:
+**** 2026-05-16 Sat @ 03:54:56 -0500 Externalized LanguageTool script path via user-emacs-directory
=modules/flycheck-config.el:69= hardcodes
=~/.emacs.d/scripts/languagetool-flycheck= as the LanguageTool wrapper.
@@ -1984,7 +1984,7 @@ auditing the module against a future package) get a broken checker.
Replace with =(expand-file-name "scripts/languagetool-flycheck"
user-emacs-directory)= or a defcustom.
-**** TODO [#C] Replace hardcoded Zathura viewer in =latex-config= :cleanup:
+**** 2026-05-16 Sat @ 03:54:56 -0500 Replaced hardcoded Zathura viewer with executable-find candidate list
=modules/latex-config.el:28= sets the LaTeX viewer to =zathura=
unconditionally. macOS / Windows users and anyone who prefers a