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

Add before and after options to refresh middleware #233

Merged
merged 2 commits into from Jul 20, 2015

Conversation

cichli
Copy link
Member

@cichli cichli commented Jul 12, 2015

I'm still refining the elisp bits for this, but am posting the middleware changes for review. The options are passed in as namespace-qualified function names, which must have zero arity. before is invoked before refreshing, and refreshing does not proceed if it throws an exception. after is invoked after refreshing.

refresh-tracker is now an agent, and the logic for refreshing dispatched using send-off - this means that only one refresh request will be being processed at any given time, so you don't have to worry about your before and after functions running concurrently or being retried. This is also means they won't lock up an nREPL thread if they block for a long time.

*out* and *err* are both captured and streamed back to the client while invoking both before and after, and if an exception is thrown either during their execution or while reloading, it is printed to *out* as it would be at the REPL (using clojure.main/repl-caught) and sent back to the client in stacktrace format for rendering in the error buffer.

Here's the client UI so far (a log buffer that prints the output of before and after, the list of namespaces being refreshed, and whether or not it was successful):

screenshot 2015-07-12 20 02 04

@bbatsov
Copy link
Member

bbatsov commented Jul 13, 2015

The changes look good to me. Btw, this log buffer reminds me that it might be nice to add some generic "server-side" logging to cider-nrepl.

@cichli
Copy link
Member Author

cichli commented Jul 19, 2015

Pushed a slight tweak to send a message both prior and subsequent to calling before/after - otherwise, if they don't emit any output, the client has no indication of them being called.

Btw, this log buffer reminds me that it might be nice to add some generic "server-side" logging to cider-nrepl.

Do you mean something that user programs hook into, or some logging performed by cider-nrepl itself (enabled in the client by some debug mode flag, presumably)?

@bbatsov
Copy link
Member

bbatsov commented Jul 19, 2015

Do you mean something that user programs hook into, or some logging performed by cider-nrepl itself (enabled in the client by some debug mode flag, presumably)?

The latter. This would be valuable info when trying to pin point where a problem originates (or so I think).

(defn- reloading-reply
[tracker {:keys [transport] :as msg}]
[{reloading ::track/load}
Copy link
Member

Choose a reason for hiding this comment

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

Nice, I didn't know about this syntax.

bbatsov added a commit that referenced this pull request Jul 20, 2015
Add `before` and `after` options to refresh middleware
@bbatsov bbatsov merged commit 3fc9b4a into clojure-emacs:master Jul 20, 2015
@cichli cichli deleted the refresh-before-after branch August 4, 2015 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants