Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recompute namespace info on each shadow-cljs recompilation or evaluation #3396

Merged
merged 2 commits into from
Aug 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
- Fix the `xref-find-definitions` CIDER backend to return correct filenames.
- Fix the `cider-xref-fn-deps` buttons to direct to the right file.

- [#3393](https://github.com/clojure-emacs/cider/issues/3393): recompute namespace info on each shadow-cljs recompilation or evaluation.

### Changes

Expand Down
44 changes: 43 additions & 1 deletion cider-repl.el
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,44 @@ t, as the content-type response is (currently) an alternative to the
value response. However for handlers which themselves issue subsequent
nREPL ops, it may be convenient to prevent inserting a prompt.")

(defun cider--maybe-get-state-for-shadow-cljs (buffer &optional err)
"Refresh the changed namespaces metadata given BUFFER and ERR (stderr string).
This is particularly necessary for shadow-cljs because:
* it has a particular nREPL implementation; and
* one may have saved files (which triggers recompilation,
and therefore the need for recomputing changed namespaces)
without sending a nREPL message (this can particularly happen
if the file was edited outside Emacs)."
(with-current-buffer buffer
(when (and (eq cider-repl-type 'cljs)
(eq cider-cljs-repl-type 'shadow)
(not cider-repl-cljs-upgrade-pending)
(if err
(string-match-p "Build completed\\." err)
t))
(when-let ((conn (cider-current-repl 'cljs)))
(when (nrepl-op-supported-p "cider/get-state" conn)
(nrepl-send-request '("op" "cider/get-state") nil conn))))))

(defun cider--shadow-cljs-handle-stderr (buffer err)
"Refresh the changed namespaces metadata given BUFFER and ERR."
(cider--maybe-get-state-for-shadow-cljs buffer err))

(defun cider--shadow-cljs-handle-done (buffer)
"Refresh the changed namespaces metadata given BUFFER."
(cider--maybe-get-state-for-shadow-cljs buffer))

(defvar cider--repl-stderr-functions (list #'cider--shadow-cljs-handle-stderr)
"Functions to be invoked each time new stderr is received on a repl buffer.
Good for, for instance, monitoring specific strings that may be logged,
and responding to them.")

(defvar cider--repl-done-functions (list #'cider--shadow-cljs-handle-done)
"Functions to be invoked each time a given REPL interaction is complete.")

(defun cider-repl-handler (buffer)
"Make an nREPL evaluation handler for the REPL BUFFER."
(let ((show-prompt t))
Expand All @@ -996,12 +1034,16 @@ nREPL ops, it may be convenient to prevent inserting a prompt.")
(lambda (buffer out)
(cider-repl-emit-stdout buffer out))
(lambda (buffer err)
(dolist (f cider--repl-stderr-functions)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's one more thing to consider here (for output in general and for values) - as this can be triggered a ton of times for something generating chunked output it can degrade performance significantly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the guards are carefully ordered:

    (when (and (eq cider-repl-type 'cljs)
               (eq cider-cljs-repl-type 'shadow)
               (not cider-repl-cljs-upgrade-pending)
               (if err
                   (string-match-p "Build completed\\." err)
                 t))

no matter how large the output we're receiving is, for most cases we're not even inspecting it.

WIth that said, yesterday I created thheller/shadow-cljs#1139 which is going in a good direction. A few features could be built on top of it, and then this could be discarded.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, yeah - I'm aware of this. Just a point of concern, as we have to be careful about this. Hopefully we'll come up with a better solution down the road.

(funcall f buffer err))
(cider-repl-emit-stderr buffer err))
(lambda (buffer)
(when show-prompt
(cider-repl-emit-prompt buffer))
(when cider-repl-buffer-size-limit
(cider-repl-maybe-trim-buffer buffer)))
(cider-repl-maybe-trim-buffer buffer))
(dolist (f cider--repl-done-functions)
(funcall f buffer)))
nrepl-err-handler
(lambda (buffer value content-type)
(if-let* ((content-attrs (cadr content-type))
Expand Down
Loading