Resync the load-state indicators after namespace reloads

The per-form eval fringe and the namespace load-state marker only ever got
refreshed by the load-file handler (C-c C-k). `cider-ns-reload`, `-reload-all`
and `cider-ns-refresh` went around it, so after a reload a buffer's markers
stayed stale even though its namespace was current again.

Extract the load handler's resync into `cider--mark-loaded` (re-mark the fringe
across the buffer + run `cider-file-loaded-hook`) and call it from the reload
paths for the namespaces actually reloaded - but only for buffers with no
unsaved edits, since `:reload` loads from disk and an unsaved buffer is
genuinely out of sync. `cider-ns-refresh` threads the reloaded-namespace list
through a per-request cell, so concurrent refreshes across REPLs can't clobber
each other.

Since this routes through `cider-file-loaded-hook', `cider-auto-test-mode' now
re-runs tests on reload too, consistent with `cider-load-buffer'.

Author: bbatsov

CHANGELOG.md

@@ -21,6 +21,7 @@
 
 ### Changes
 
+- [#3980](https://github.com/clojure-emacs/cider/pull/3980): Keep the load-state indicators in sync across `cider-ns-reload`/`cider-ns-reload-all` and `cider-ns-refresh`: the reloaded namespaces' (unsaved-edit-free) buffers get their fringe and mode-line markers refreshed, instead of staying stale until the next `cider-load-buffer`. These commands now run `cider-file-loaded-hook` for the reloaded buffers, so `cider-auto-test-mode` re-runs their tests too (matching what `cider-load-buffer` already does).
 - [#3967](https://github.com/clojure-emacs/cider/pull/3967): Refresh the embedded Clojure cheatsheet with functions added since Clojure 1.11 (`partitionv`, `partitionv-all`, `splitv-at`, `clojure.repl.deps`, `clojure.java.process`, the `clojure.core` Java-stream helpers, and more `clojure.math` members).
 - [#3967](https://github.com/clojure-emacs/cider/pull/3967): Give the cheatsheet buffer a dedicated `cider-cheatsheet-mode` with fontified section headers and `TAB`/`S-TAB` navigation between entries.
 - [#3971](https://github.com/clojure-emacs/cider/pull/3971): Add `n` and `t` keys in the macroexpansion buffer to cycle namespace display (`cider-macroexpansion-display-namespaces`) and toggle metadata (`cider-macroexpansion-print-metadata`), re-expanding in place.

lisp/cider-eval.el

@@ -300,6 +300,16 @@ REPL buffer.  This is controlled via
             (cider--make-fringe-overlay (point)))
         (scan-error nil)))))
 
+(defun cider--mark-loaded (&optional buffer)
+  "Mark BUFFER's content as loaded into the REPL and in sync.
+Refreshes the evaluation fringe indicators across BUFFER and runs
+`cider-file-loaded-hook' (which the namespace load-state indicator hooks
+into).  BUFFER defaults to the current buffer.  Used by the load-file flow
+and by the namespace reloading commands once they finish."
+  (with-current-buffer (or buffer (current-buffer))
+    (cider--make-fringe-overlays-for-region (point-min) (point-max))
+    (run-hooks 'cider-file-loaded-hook)))
+
 
 (declare-function cider-inspect-last-result "cider-inspector")
 (defun cider-interactive-eval-handler (&optional buffer place)
@@ -359,9 +369,7 @@ Optional argument DONE-HANDLER lambda will be run once load is complete."
                  (when cider-eval-register
                    (setq res (concat res value)))
                  (when (buffer-live-p target)
-                   (with-current-buffer target
-                     (cider--make-fringe-overlays-for-region (point-min) (point-max))
-                     (run-hooks 'cider-file-loaded-hook))))
+                   (cider--mark-loaded target)))
      :on-stdout #'cider-emit-interactive-eval-output
      :on-stderr (lambda (err)
                   (cider-emit-interactive-eval-err-output err)

lisp/cider-ns.el

@@ -161,8 +161,31 @@ Present the error message as an overlay."
     (when buf
       (select-window (get-buffer-window buf)))))
 
-(defun cider-ns-refresh--handle-response (response log-buffer)
-  "Refresh LOG-BUFFER with RESPONSE."
+(defun cider-ns--mark-reloaded (namespaces)
+  "Resync the load-state indicators of open buffers in NAMESPACES.
+For each live Clojure buffer whose namespace is in NAMESPACES (a list of
+name strings) and that has no unsaved edits, refresh the evaluation fringe
+indicators and the namespace load-state marker.  Modified buffers are left
+alone, since reloading happens from disk and an unsaved buffer is genuinely
+out of sync with what was loaded.
+
+This goes through `cider-file-loaded-hook' (like `cider-load-buffer' does),
+so its other subscribers also run for the reloaded buffers - notably
+`cider-auto-test-mode' will re-run their tests."
+  (when namespaces
+    (dolist (buf (buffer-list))
+      (with-current-buffer buf
+        (when (and (derived-mode-p 'clojure-mode 'clojure-ts-mode)
+                   (not (buffer-modified-p))
+                   (member (cider-current-ns 'no-default) namespaces))
+          (cider--mark-loaded))))))
+
+(defun cider-ns-refresh--handle-response (response log-buffer &optional reloading-cell)
+  "Refresh LOG-BUFFER with RESPONSE.
+RELOADING-CELL, when given, is a one-element list (a mutable cell, private to
+a single refresh request) used to carry the set of namespaces being reloaded
+from the `reloading' response to the final `ok' response, so that concurrent
+refreshes across REPLs don't clobber each other."
   (nrepl-dbind-response response (out err reloading progress status error error-ns after before)
     (cl-flet* ((log (message &optional face)
                     (cider-emit-into-popup-buffer log-buffer message face t))
@@ -189,6 +212,7 @@ Present the error message as an overlay."
         (log-echo "Could not resolve refresh function\n" 'font-lock-string-face))
 
        (reloading
+        (when reloading-cell (setcar reloading-cell reloading))
         (log-echo (format "Reloading %s\n" reloading) 'font-lock-string-face))
 
        (progress
@@ -198,7 +222,8 @@ Present the error message as an overlay."
         (log-echo "Nothing to reload\n" 'font-lock-string-face))
 
        ((member "ok" status)
-        (log-echo "Reloading successful\n" 'font-lock-string-face))
+        (log-echo "Reloading successful\n" 'font-lock-string-face)
+        (cider-ns--mark-reloaded (car reloading-cell)))
 
        (error-ns
         (log-echo (format "Error reloading %s\n" error-ns) 'font-lock-warning-face))
@@ -245,6 +270,21 @@ Based on OP-NAME and the value of `cider-ns-code-reload-tool'."
           ((string= op-name "reload-all")   "cider.clj-reload/reload-all")
           ((string= op-name "reload-clear") "cider.clj-reload/reload-clear"))))
 
+(defun cider-ns--reload-callback (ns)
+  "Return an eval handler that resyncs NS's buffers after a successful reload.
+Wraps the standard interactive-eval handler so results and errors display as
+usual, and additionally marks NS's buffers loaded once evaluation finishes
+without an error."
+  (let ((base (cider-interactive-eval-handler))
+        (errored nil))
+    (lambda (response)
+      (funcall base response)
+      (let ((status (nrepl-dict-get response "status")))
+        (when (member "eval-error" status)
+          (setq errored t))
+        (when (and (not errored) (member "done" status))
+          (cider-ns--mark-reloaded (list ns)))))))
+
 ;;;###autoload
 (defun cider-ns-reload (&optional prompt)
   "Send a (require \\='ns :reload) to the REPL.
@@ -258,7 +298,8 @@ identified libs even if they are already loaded\"."
   (when-let* ((ns (if prompt
                       (string-remove-prefix "'" (read-from-minibuffer "Namespace: " (cider-get-ns-name)))
                     (cider-get-ns-name))))
-    (cider-interactive-eval (format "(require '%s :reload)" ns))))
+    (cider-interactive-eval (format "(require '%s :reload)" ns)
+                            (cider-ns--reload-callback ns))))
 
 ;;;###autoload
 (defun cider-ns-reload-all (&optional prompt)
@@ -274,7 +315,8 @@ indirectly load via require\"."
   (when-let* ((ns (if prompt
                       (string-remove-prefix "'" (read-from-minibuffer "Namespace: " (cider-get-ns-name)))
                     (cider-get-ns-name))))
-    (cider-interactive-eval (format "(require '%s :reload-all)" ns))))
+    (cider-interactive-eval (format "(require '%s :reload-all)" ns)
+                            (cider-ns--reload-callback ns))))
 
 ;;;###autoload
 (defun cider-ns-refresh (&optional mode)
@@ -314,19 +356,20 @@ refresh functions (defined in `cider-ns-refresh-before-fn' and
                                           t))
           (when clear?
             (cider-nrepl-send-sync-request `("op" ,(cider-ns--reload-op "reload-clear")) conn))
-          (cider-nrepl-send-request
-           `("op" ,(cider-ns--reload-op (if all? "reload-all" "reload"))
-             ,@(cider--nrepl-print-request-plist fill-column)
-             ,@(when (and (not inhibit-refresh-fns) cider-ns-refresh-before-fn)
-                 `("before" ,cider-ns-refresh-before-fn))
-             ,@(when (and (not inhibit-refresh-fns) cider-ns-refresh-after-fn)
-                 `("after" ,cider-ns-refresh-after-fn)))
-           (lambda (response)
-             (cider-ns-refresh--handle-response response log-buffer)
-             (nrepl-dbind-response response (status id)
-               (when (member "done" status)
-                 (nrepl--mark-id-completed id))))
-           conn))))))
+          (let ((reloading (list nil)))
+            (cider-nrepl-send-request
+             `("op" ,(cider-ns--reload-op (if all? "reload-all" "reload"))
+               ,@(cider--nrepl-print-request-plist fill-column)
+               ,@(when (and (not inhibit-refresh-fns) cider-ns-refresh-before-fn)
+                   `("before" ,cider-ns-refresh-before-fn))
+               ,@(when (and (not inhibit-refresh-fns) cider-ns-refresh-after-fn)
+                   `("after" ,cider-ns-refresh-after-fn)))
+             (lambda (response)
+               (cider-ns-refresh--handle-response response log-buffer reloading)
+               (nrepl-dbind-response response (status id)
+                 (when (member "done" status)
+                   (nrepl--mark-id-completed id))))
+             conn)))))))
 
 (provide 'cider-ns)
 ;;; cider-ns.el ends here

test/cider-ns-tests.el

@@ -204,3 +204,38 @@
       (cider-ns--present-error cider-ns-tests--sample-causes)
       (when-let ((b (get-buffer "*cider-error*"))) ;; Clean it up for other tests
         (kill-buffer b)))))
+
+(describe "cider-ns--mark-reloaded"
+  (it "resyncs an unmodified buffer whose namespace was reloaded"
+    (spy-on 'cider--mark-loaded)
+    (with-clojure-buffer "(ns foo)"
+      (set-buffer-modified-p nil)
+      (cider-ns--mark-reloaded '("foo"))
+      (expect 'cider--mark-loaded :to-have-been-called)))
+  (it "skips a buffer whose namespace was not reloaded"
+    (spy-on 'cider--mark-loaded)
+    (with-clojure-buffer "(ns bar)"
+      (set-buffer-modified-p nil)
+      (cider-ns--mark-reloaded '("foo"))
+      (expect 'cider--mark-loaded :not :to-have-been-called)))
+  (it "skips a buffer with unsaved edits (out of sync with what was reloaded)"
+    (spy-on 'cider--mark-loaded)
+    (with-clojure-buffer "(ns foo)"
+      (set-buffer-modified-p t)
+      (cider-ns--mark-reloaded '("foo"))
+      (expect 'cider--mark-loaded :not :to-have-been-called))))
+
+(describe "cider-ns--reload-callback"
+  (it "resyncs the namespace once evaluation finishes without error"
+    (spy-on 'cider-interactive-eval-handler :and-return-value #'ignore)
+    (spy-on 'cider-ns--mark-reloaded)
+    (let ((cb (cider-ns--reload-callback "foo")))
+      (funcall cb (nrepl-dict "status" '("done")))
+      (expect 'cider-ns--mark-reloaded :to-have-been-called-with '("foo"))))
+  (it "does not resync when evaluation errored"
+    (spy-on 'cider-interactive-eval-handler :and-return-value #'ignore)
+    (spy-on 'cider-ns--mark-reloaded)
+    (let ((cb (cider-ns--reload-callback "foo")))
+      (funcall cb (nrepl-dict "status" '("eval-error")))
+      (funcall cb (nrepl-dict "status" '("done")))
+      (expect 'cider-ns--mark-reloaded :not :to-have-been-called))))