diff options
| author | Craig Jennings <c@cjennings.net> | 2026-05-16 04:01:04 -0500 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-05-16 04:01:04 -0500 |
| commit | 500687f8d7d5b87ceb33fd959e545746ec9db1ba (patch) | |
| tree | 9959d3641259a7299893c99db0c5da55d3b8e942 | |
| parent | d84aa4374af5e3447445377a836c66cc07d7a223 (diff) | |
| download | dotemacs-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.el | 12 | ||||
| -rw-r--r-- | modules/eshell-config.el | 13 | ||||
| -rw-r--r-- | modules/markdown-config.el | 24 | ||||
| -rw-r--r-- | modules/slack-config.el | 34 | ||||
| -rw-r--r-- | tests/test-ai-vterm--launch-command.el | 22 | ||||
| -rw-r--r-- | todo.org | 10 |
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") @@ -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 |
