From bbd1b73cbaf1716734b1193467564a0c7122dec7 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Mon, 11 May 2026 17:17:54 -0500 Subject: fix(slack): harden and curate the C-; S ! reaction workflow Two problems with `C-; S !'. First, emacs-slack's `slack-reaction-echo-description' runs in a buffer-local `post-command-hook' and can error on every keystroke when a reaction widget's text properties are malformed, which makes it hard to leave the Slack buffer or recover with C-g. It's now wrapped in `condition-case' that, on error, removes the hook from that buffer's local `post-command-hook' and messages once. Second, without emojify the upstream reaction picker is a flat completing-read over 1600+ names. `cj/slack-message-add-reaction' now offers a curated common list (`thumbsup', `pray', `eyes', `white_check_mark', `heart', `joy', `thinking_face', `rocket', `tada') with "Other..." delegating back to `slack-message-reaction-input' for the full set. Tests cover the hook hardening, the curated picker, and the `C-; S !' rebinding. --- modules/slack-config.el | 95 +++++++++++++++++++++++++++++++++++- tests/test-slack-config-reactions.el | 91 ++++++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 tests/test-slack-config-reactions.el diff --git a/modules/slack-config.el b/modules/slack-config.el index d8fd3f83..14d15ecf 100644 --- a/modules/slack-config.el +++ b/modules/slack-config.el @@ -35,6 +35,30 @@ ;;; Code: (require 'auth-source) +(require 'cl-lib) + +(defvar slack-current-buffer) +(defvar slack-message-compose-buffer-mode-map) +(defvar slack-message-custom-notifier) +(defvar slack-teams) + +(declare-function slack-buffer-add-reaction-to-message "slack-buffer") +(declare-function slack-buffer-latest-ts "slack-buffer") +(declare-function slack-buffer-team "slack-buffer") +(declare-function slack-buffer-update-mark-request "slack-buffer") +(declare-function slack-get-ts "slack-util") +(declare-function slack-im-p "slack-im") +(declare-function slack-message-body "slack-message") +(declare-function slack-message-embed-channel "slack-message-buffer") +(declare-function slack-message-embed-mention "slack-message-buffer") +(declare-function slack-message-mentioned-p "slack-message") +(declare-function slack-message-minep "slack-message") +(declare-function slack-message-reaction-input "slack-message-reaction") +(declare-function slack-message-send-from-buffer "slack-message-sender") +(declare-function slack-message-write-another-buffer "slack-message-buffer") +(declare-function slack-reaction-echo-description "slack-buffer") +(declare-function slack-room-display-name "slack-room") +(declare-function slack-ws-close "slack") (defvar cj/slack-workspace "deepsatworkspace.slack.com" "Slack workspace domain for auth-source lookup.") @@ -91,6 +115,75 @@ :config (setq slack-message-custom-notifier #'cj/slack-notify)) +;; ------------------------------ Reactions ------------------------------------ + +(defvar cj/slack-common-reactions + '(("thumbs up" . "+1") + ("thumbsup" . "thumbsup") + ("thumbs down" . "-1") + ("pray" . "pray") + ("raised hands" . "raised_hands") + ("eyes" . "eyes") + ("white check mark" . "white_check_mark") + ("heavy check mark" . "heavy_check_mark") + ("plus one" . "+1") + ("heart" . "heart") + ("heart eyes" . "heart_eyes") + ("joy" . "joy") + ("laughing" . "laughing") + ("smile" . "smile") + ("thinking face" . "thinking_face") + ("rocket" . "rocket") + ("fire" . "fire") + ("party" . "tada") + ("clap" . "clap") + ("ok hand" . "ok_hand")) + "Curated common Slack reaction labels mapped to Slack emoji names.") + +(defun cj/slack--safe-reaction-echo-description (orig-fun &rest args) + "Call ORIG-FUN safely from `post-command-hook'. +If emacs-slack sees a malformed reaction text property, remove the local hook +so the Slack buffer stays usable." + (condition-case err + (apply orig-fun args) + (error + (remove-hook 'post-command-hook #'slack-reaction-echo-description t) + (message "Slack reaction hover disabled in this buffer: %s" + (error-message-string err))))) + +(defun cj/slack--reaction-candidates () + "Return display candidates for `cj/slack-common-reactions'." + (append + (mapcar (lambda (entry) + (let ((label (car entry)) + (name (cdr entry))) + (cons (format "%-18s :%s:" label name) name))) + cj/slack-common-reactions) + '(("Other..." . :other)))) + +(defun cj/slack-select-reaction (team) + "Select a Slack reaction for TEAM, preferring a short common list." + (let* ((candidates (cj/slack--reaction-candidates)) + (choice (completing-read "Reaction: " candidates nil t)) + (reaction (cdr (assoc choice candidates)))) + (if (eq reaction :other) + (slack-message-reaction-input team) + reaction))) + +(defun cj/slack-message-add-reaction () + "Add a reaction to the current Slack message using a curated shortlist." + (interactive) + (when-let* ((buf slack-current-buffer) + (team (slack-buffer-team buf)) + (reaction (cj/slack-select-reaction team))) + (slack-buffer-add-reaction-to-message buf + reaction + (slack-get-ts)))) + +(with-eval-after-load 'slack-buffer + (advice-add 'slack-reaction-echo-description + :around #'cj/slack--safe-reaction-echo-description)) + ;; ----------------------------- Notifications --------------------------------- (defun cj/slack-notify (message room team) @@ -153,7 +246,7 @@ swallows exceptions via `websocket-try-callback'." (define-key cj/slack-keymap (kbd "w") #'slack-message-write-another-buffer) (define-key cj/slack-keymap (kbd "r") #'slack-thread-show-or-create) (define-key cj/slack-keymap (kbd "e") #'slack-insert-emoji) -(define-key cj/slack-keymap (kbd "!") #'slack-message-add-reaction) +(define-key cj/slack-keymap (kbd "!") #'cj/slack-message-add-reaction) (define-key cj/slack-keymap (kbd "@") #'slack-message-embed-mention) (define-key cj/slack-keymap (kbd "#") #'slack-message-embed-channel) (define-key cj/slack-keymap (kbd "q") #'cj/slack-mark-read-and-bury) diff --git a/tests/test-slack-config-reactions.el b/tests/test-slack-config-reactions.el new file mode 100644 index 00000000..a5755a49 --- /dev/null +++ b/tests/test-slack-config-reactions.el @@ -0,0 +1,91 @@ +;;; test-slack-config-reactions.el --- Tests for Slack reaction helpers -*- lexical-binding: t; -*- + +;;; Commentary: +;; Tests for local Slack reaction workflow hardening and shortlist selection. + +;;; Code: + +(require 'ert) +(require 'cl-lib) + +(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory)) +(require 'slack-config) + +(defvar slack-current-buffer) + +(ert-deftest test-slack-config-reaction-echo-error-removes-local-hook () + "Error: malformed reaction hover errors remove the local post-command hook." + (with-temp-buffer + (add-hook 'post-command-hook #'slack-reaction-echo-description nil t) + (cl-letf (((symbol-function 'message) (lambda (&rest _args) nil))) + (cj/slack--safe-reaction-echo-description + (lambda (&rest _args) (error "bad reaction")))) + (should-not (memq #'slack-reaction-echo-description post-command-hook)))) + +(ert-deftest test-slack-config-reaction-echo-success-keeps-local-hook () + "Normal: successful reaction hover leaves the local hook installed." + (with-temp-buffer + (add-hook 'post-command-hook #'slack-reaction-echo-description nil t) + (cj/slack--safe-reaction-echo-description (lambda (&rest _args) :ok)) + (should (memq #'slack-reaction-echo-description post-command-hook)))) + +(ert-deftest test-slack-config-reaction-candidates-include-common-and-fallback () + "Normal: common reaction candidates include frequent choices and Other." + (let ((candidates (cj/slack--reaction-candidates))) + (should (member '("Other..." . :other) candidates)) + (should (cl-some (lambda (candidate) + (and (string-match-p "thumbsup" (car candidate)) + (equal "thumbsup" (cdr candidate)))) + candidates)) + (should (cl-some (lambda (candidate) + (and (string-match-p "pray" (car candidate)) + (equal "pray" (cdr candidate)))) + candidates)))) + +(ert-deftest test-slack-config-select-reaction-common-choice () + "Normal: selecting a common display candidate returns its Slack emoji name." + (let* ((candidates (cj/slack--reaction-candidates)) + (choice (car (cl-find-if (lambda (candidate) + (equal "pray" (cdr candidate))) + candidates)))) + (cl-letf (((symbol-function 'completing-read) + (lambda (&rest _args) choice))) + (should (equal "pray" (cj/slack-select-reaction :team)))))) + +(ert-deftest test-slack-config-select-reaction-other-falls-back-to-upstream () + "Boundary: Other delegates to emacs-slack's full emoji input." + (cl-letf (((symbol-function 'completing-read) + (lambda (&rest _args) "Other...")) + ((symbol-function 'slack-message-reaction-input) + (lambda (team) + (should (eq team :team)) + "custom_emoji"))) + (should (equal "custom_emoji" (cj/slack-select-reaction :team))))) + +(ert-deftest test-slack-config-reaction-key-uses-local-command () + "Normal: C-; S ! uses the local safer reaction command." + (should (eq (keymap-lookup cj/slack-keymap "!") + #'cj/slack-message-add-reaction))) + +(ert-deftest test-slack-config-message-add-reaction-dispatches-selected-reaction () + "Normal: local reaction command sends selected reaction to Slack." + (let ((slack-current-buffer :buffer) + called) + (cl-letf (((symbol-function 'slack-buffer-team) + (lambda (buffer) + (should (eq buffer :buffer)) + :team)) + ((symbol-function 'cj/slack-select-reaction) + (lambda (team) + (should (eq team :team)) + "pray")) + ((symbol-function 'slack-get-ts) + (lambda () "123.456")) + ((symbol-function 'slack-buffer-add-reaction-to-message) + (lambda (buffer reaction ts) + (setq called (list buffer reaction ts))))) + (cj/slack-message-add-reaction) + (should (equal called '(:buffer "pray" "123.456")))))) + +(provide 'test-slack-config-reactions) +;;; test-slack-config-reactions.el ends here -- cgit v1.2.3