summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-05-16 04:01:04 -0500
committerCraig Jennings <c@cjennings.net>2026-05-16 04:01:04 -0500
commit500687f8d7d5b87ceb33fd959e545746ec9db1ba (patch)
tree9959d3641259a7299893c99db0c5da55d3b8e942
parentd84aa4374af5e3447445377a836c66cc07d7a223 (diff)
downloaddotemacs-500687f8d7d5b87ceb33fd959e545746ec9db1ba.tar.gz
dotemacs-500687f8d7d5b87ceb33fd959e545746ec9db1ba.zip
refactor(integrations): five hygiene fixes from the module-by-module re-review
- markdown-config.el: two related fixes on `markdown-preview'. First, the URL was `https://localhost:8080/imp' but simple-httpd serves plaintext on port 8080 -- the browser hit a TLS handshake against a non-TLS listener and the preview never rendered. Changed to `http://' and switched from `browse-url-generic' to plain `browse-url' so the user's default protocol handler picks the browser. Second, the function used to start the network listener as a side effect of opening a preview; that's split into a separate `cj/markdown-preview-server-start' command and `markdown-preview' now signals a `user-error' (with the recovery command in the message) when the server isn't running. - slack-config.el: wrap the `which-key-add-keymap-based-replacements' call in `with-eval-after-load 'which-key'. Matches the pattern other config modules use and means a slow / missing which-key load won't block requiring slack-config. - ai-vterm.el: pass the inner shell-command-string through `shell-quote-argument' before wrapping in the tmux invocation. The default value with embedded double quotes was safe under the prior literal-single-quote wrap, but a user-customized `cj/ai-vterm-agent-command' containing a single quote silently broke the shell parse. Two existing tests updated to tolerate the post-quote escape shape; new regression test asserts a single-quote-bearing custom command survives. - eshell-config.el: scope the `TERM=xterm-256color' override to eshell-spawned processes only via an `eshell-mode' hook that prepends to a buffer-local `process-environment'. The previous global `setenv' at config-time changed `TERM' for every subsequent `start-process' across the Emacs session, so any subprocess (not just eshell pipelines) inherited `xterm-256color' regardless of whether the receiver could interpret the escapes.
-rw-r--r--modules/ai-vterm.el12
-rw-r--r--modules/eshell-config.el13
-rw-r--r--modules/markdown-config.el24
-rw-r--r--modules/slack-config.el34
-rw-r--r--tests/test-ai-vterm--launch-command.el22
-rw-r--r--todo.org10
6 files changed, 84 insertions, 31 deletions
diff --git a/modules/ai-vterm.el b/modules/ai-vterm.el
index 18320eee..4a8f7e0e 100644
--- a/modules/ai-vterm.el
+++ b/modules/ai-vterm.el
@@ -233,11 +233,19 @@ so the tmux window survives the AI command exiting -- the session stays
alive with a bare bash prompt for recovery, and reattach works the same way."
(let ((session (cj/--ai-vterm-tmux-session-name dir))
(start-dir (expand-file-name dir)))
- (format "tmux new-session -A -s %s -n %s -c %s '%s'"
+ ;; Pass the inner shell-command-string through `shell-quote-argument'
+ ;; so any single quotes embedded in a user-customized
+ ;; `cj/ai-vterm-agent-command' don't break the literal single-quote
+ ;; wrap below. The default value carries embedded double quotes
+ ;; (\"Read .ai/protocols.org and follow all instructions.\") which
+ ;; was safe in the prior shape but a single-quoted custom value
+ ;; silently broke the shell parse.
+ (format "tmux new-session -A -s %s -n %s -c %s %s"
(shell-quote-argument session)
(shell-quote-argument cj/ai-vterm-tmux-window-name)
(shell-quote-argument start-dir)
- (concat cj/ai-vterm-agent-command "; exec bash"))))
+ (shell-quote-argument
+ (concat cj/ai-vterm-agent-command "; exec bash")))))
(defun cj/--ai-vterm-has-marker-p (dir)
"Return non-nil when DIR contains .ai/protocols.org."
diff --git a/modules/eshell-config.el b/modules/eshell-config.el
index 62b3a7ab..4d180d1c 100644
--- a/modules/eshell-config.el
+++ b/modules/eshell-config.el
@@ -128,8 +128,17 @@
:hook
(eshell-before-prompt-hook . (lambda ()
(setq xterm-color-preserve-properties t)))
- :config
- (setenv "TERM" "xterm-256color"))
+ ;; Scope `TERM=xterm-256color' to eshell-spawned processes only by
+ ;; binding the env var on the eshell mode hook. The previous global
+ ;; `setenv' at config-time changed `process-environment' for the
+ ;; whole Emacs process, so every subsequent `start-process' inherited
+ ;; `xterm-256color' regardless of whether the receiver was a terminal
+ ;; that could actually interpret the escapes.
+ :hook
+ (eshell-mode . (lambda ()
+ (setq-local process-environment
+ (cons "TERM=xterm-256color"
+ process-environment)))))
(use-package eshell-syntax-highlighting
:after esh-mode
diff --git a/modules/markdown-config.el b/modules/markdown-config.el
index 2536904a..1fc4cb0e 100644
--- a/modules/markdown-config.el
+++ b/modules/markdown-config.el
@@ -33,16 +33,34 @@
;;;; --------------------- WIP: Markdown-Preview ---------------------
+(defun cj/markdown-preview-server-start ()
+ "Start the simple-httpd listener that serves the live markdown preview.
+Idempotent: re-running while the server is already up is a no-op."
+ (interactive)
+ (require 'simple-httpd)
+ (httpd-start)
+ (message "markdown preview server running on http://localhost:8080/imp"))
+
;; the filter to apply to markdown before impatient-mode pushes it to the server
(defun markdown-preview ()
+ "Open the current buffer as a live HTML preview at http://localhost:8080/imp.
+The simple-httpd listener must already be running -- see
+`cj/markdown-preview-server-start'. Starting a network listener as a
+side effect of opening a preview is surprising, so the server start
+lives in a separate command."
(interactive)
- (httpd-start)
+ (unless (and (boundp 'httpd-process) httpd-process)
+ (user-error "markdown preview server not running; run `M-x cj/markdown-preview-server-start' first"))
(impatient-mode 1)
(setq imp-user-filter #'cj/markdown-html)
(cl-incf imp-last-state)
(imp--notify-clients)
- ;; (browse-url-generic-function 'browse-url-xdg-open)
- (browse-url-generic "https://localhost:8080/imp" 1))
+ ;; Use plain `browse-url' (not `browse-url-generic') so the user's
+ ;; default protocol handler picks the browser, and use `http://' --
+ ;; simple-httpd serves plaintext; the previous `https://' URL caused
+ ;; a TLS handshake against a non-TLS listener and the preview never
+ ;; rendered.
+ (browse-url "http://localhost:8080/imp"))
(defun cj/markdown-html (buffer)
(princ (with-current-buffer buffer
diff --git a/modules/slack-config.el b/modules/slack-config.el
index 3e9ae283..b51db444 100644
--- a/modules/slack-config.el
+++ b/modules/slack-config.el
@@ -253,21 +253,25 @@ swallows exceptions via `websocket-try-callback'."
(define-key cj/slack-keymap (kbd "Q") #'cj/slack-close-all-buffers)
(define-key cj/slack-keymap (kbd "S") #'cj/slack-stop)
-(which-key-add-keymap-based-replacements cj/slack-keymap
- "" "slack menu"
- "s" "start slack"
- "c" "unread rooms"
- "C" "select channel"
- "d" "direct message"
- "w" "compose message"
- "r" "reply / thread"
- "e" "insert emoji"
- "!" "add reaction"
- "@" "embed @mention"
- "#" "embed #channel"
- "q" "mark read & bury"
- "Q" "close all slack"
- "S" "disconnect")
+;; Register which-key labels lazily so this module's require doesn't
+;; depend on which-key being loaded. Other config modules use the same
+;; pattern.
+(with-eval-after-load 'which-key
+ (which-key-add-keymap-based-replacements cj/slack-keymap
+ "" "slack menu"
+ "s" "start slack"
+ "c" "unread rooms"
+ "C" "select channel"
+ "d" "direct message"
+ "w" "compose message"
+ "r" "reply / thread"
+ "e" "insert emoji"
+ "!" "add reaction"
+ "@" "embed @mention"
+ "#" "embed #channel"
+ "q" "mark read & bury"
+ "Q" "close all slack"
+ "S" "disconnect"))
;; Send from compose buffer with C-<return>
(with-eval-after-load 'slack-message-compose-buffer
diff --git a/tests/test-ai-vterm--launch-command.el b/tests/test-ai-vterm--launch-command.el
index 7e455a8b..bac36d4e 100644
--- a/tests/test-ai-vterm--launch-command.el
+++ b/tests/test-ai-vterm--launch-command.el
@@ -55,19 +55,33 @@
(cj/--ai-vterm-launch-command "/code/foo")))))
(ert-deftest test-ai-vterm--launch-command-includes-agent-command ()
- "Normal: the configured agent command is in the launched shell command."
+ "Normal: the configured agent command is in the launched shell command.
+The inner command is passed through `shell-quote-argument', so spaces
+are escaped (`\\\\ ') -- the regex below accepts either form."
(let ((cj/ai-vterm-agent-command "agent --some-flag"))
(should (string-match-p
- "agent --some-flag"
+ "agent\\(\\\\\\)? --some-flag"
(cj/--ai-vterm-launch-command "/code/foo")))))
(ert-deftest test-ai-vterm--launch-command-tails-with-exec-bash ()
- "Boundary: `exec bash' tails so the tmux window survives the agent exiting."
+ "Boundary: `exec bash' tails so the tmux window survives the agent exiting.
+Accepts the post-`shell-quote-argument' shape (`exec\\\\ bash')."
(let ((cj/ai-vterm-agent-command "agent"))
(should (string-match-p
- "exec bash"
+ "exec\\(\\\\\\)? bash"
(cj/--ai-vterm-launch-command "/code/foo")))))
+(ert-deftest test-ai-vterm--launch-command-survives-single-quote-in-agent ()
+ "Normal: a user-customized agent command containing a single quote
+shouldn't break the shell parse. `shell-quote-argument' produces a
+valid shell token regardless of the embedded quote shape -- the
+escaping is implementation-detail, so we assert the literal words
+\"hi\" and \"there\" both appear (the space between them may be
+escaped as \\\\ )."
+ (let ((cj/ai-vterm-agent-command "agent --say 'hi there'"))
+ (let ((cmd (cj/--ai-vterm-launch-command "/code/foo")))
+ (should (string-match-p "hi\\(\\\\\\)? there" cmd)))))
+
(ert-deftest test-ai-vterm--launch-command-handles-spaces-in-basename ()
"Boundary: a basename with whitespace becomes hyphenated before quoting."
(let ((cj/ai-vterm-agent-command "agent")
diff --git a/todo.org b/todo.org
index 1c215c3e..8c04c955 100644
--- a/todo.org
+++ b/todo.org
@@ -2356,7 +2356,7 @@ Expected outcome:
- Add a small test or manual checklist for the EWW user-agent advice so it does
not affect package.el or non-EWW URL callers.
-**** TODO [#B] Move Slack which-key registration behind =with-eval-after-load= :cleanup:quick:
+**** 2026-05-16 Sat @ 04:00:00 -0500 Moved Slack which-key registration behind with-eval-after-load
=slack-config.el= calls =which-key-add-keymap-based-replacements= at top level,
while most modules defer which-key registration. If which-key is not loaded or
@@ -2366,7 +2366,7 @@ Expected outcome:
- Wrap the registration in =with-eval-after-load 'which-key=.
- Add a module-load smoke test or byte-compile check if easy.
-**** TODO [#C] Remove =httpd-start= side effect from =markdown-preview= :safety:
+**** 2026-05-16 Sat @ 04:00:00 -0500 Removed httpd-start side effect from markdown-preview
=modules/markdown-config.el:37-51= starts =simple-httpd= inside an
interactive command, then opens a browser at
@@ -2387,7 +2387,7 @@ config (a defcustom, an alist read from disk, or
=host-environment.el='s machine-table) so the module itself is
portable.
-**** TODO [#B] Fix HTTPS scheme in =markdown-preview= URL :bug:
+**** 2026-05-16 Sat @ 04:00:00 -0500 Fixed https→http in markdown-preview + extracted server start
=modules/markdown-config.el:45= opens
=https://localhost:8080/imp= via =browse-url-generic=, but the
@@ -2409,7 +2409,7 @@ disappears. Document the dependency in the module commentary at
minimum; better, vendor a copy of =strapdown.js= under
=assets/= and serve it from the local =simple-httpd= root.
-**** TODO [#C] Reconsider global =TERM=xterm-256color= setenv in =eshell-config= :startup:
+**** 2026-05-16 Sat @ 04:00:00 -0500 Scoped TERM=xterm-256color to eshell buffers via process-environment
=modules/eshell-config.el:131= calls =(setenv "TERM" "xterm-256color")=
at config time inside the =xterm-color= use-package block. That
@@ -2421,7 +2421,7 @@ in shells that aren't interpreting them. Move the setenv into the
eshell entry hook (=eshell-mode-hook= or =eshell-before-prompt-hook=)
so it scopes to eshell processes only.
-**** TODO [#C] Quote agent command in =cj/--ai-vterm-launch-command= :safety:
+**** 2026-05-16 Sat @ 04:00:00 -0500 Quoted agent command in cj/--ai-vterm-launch-command via shell-quote-argument
=modules/ai-vterm.el:236-240= builds the tmux launch command by
wrapping =(concat cj/ai-vterm-agent-command "; exec bash")= in