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 support for refresh middleware #1041

Merged
merged 1 commit into from
Apr 28, 2015
Merged

Conversation

cichli
Copy link
Member

@cichli cichli commented Mar 22, 2015

See clojure-emacs/cider-nrepl#173.

Currently this just works like an interactive eval - the printed results from calling refresh are sent to the REPL buffer and the result (either :ok or the exception) are printed in the echo area.

refresh actually returns the last exception if any are thrown. We can add a custom transport to the middleware if we want to rethrow it and display the error buffer/compilation error highlighting. Will do that as a separate enhancement if you like.

@cichli
Copy link
Member Author

cichli commented Mar 22, 2015

btw, this leaves cider-load-fn-into-repl-buffer as the only feature still relying on the tooling session. I'll log a separate issue regarding what I think should be done about it :-).

@bbatsov
Copy link
Member

bbatsov commented Apr 25, 2015

btw, this leaves cider-load-fn-into-repl-buffer as the only feature still relying on the tooling session. I'll log a separate issue regarding what I think should be done about it :-).

We deleted it a few days ago. This was functionality with little value that relied on a lot of legacy code (which we deleted as well).

@bbatsov
Copy link
Member

bbatsov commented Apr 27, 2015

Btw, this is in a mergeable state, right? You mentioned a fancy UI in the matching cider-nrepl PR, but I guess it's a work in progress.

@cichli
Copy link
Member Author

cichli commented Apr 27, 2015

Yeah, so I've got the error conveyance working now (a stacktrace will display in the error buffer if reloading fails), and both this PR + the middleware PR are synced and could be merged now.

But I'm still thinking about how to handle the error recovery (i.e. which namespaces, if any, we should attempt to restore aliases in). Maybe for now we should do no recovery (ala tools.namespace < 0.2.8) and come back to it later.

Regarding the fancy UI, I have the necessary middleware op implemented in a stash locally, but haven't looked into how the elisp-side will work at all yet. I guess ewoc would be a good candidate to use?

If this is blocking the 0.9 release - feel free to merge as is now, and we can add the fancy UI later, or hold off and I will get it all done with the new UI for 0.10. Sorry for the delay around these PRs - it's been a busy few weeks :-).

@bbatsov
Copy link
Member

bbatsov commented Apr 27, 2015

But I'm still thinking about how to handle the error recovery (i.e. which namespaces, if any, we should attempt to restore aliases in). Maybe for now we should do no recovery (ala tools.namespace < 0.2.8) and come back to it later.

Fine by me.

Regarding the fancy UI, I have the necessary middleware op implemented in a stash locally, but haven't looked into how the elisp-side will work at all yet. I guess ewoc would be a good candidate to use?

Guess so. We use it for the connection management and it works fine.

If this is blocking the 0.9 release - feel free to merge as is now, and we can add the fancy UI later, or hold off and I will get it all done with the new UI for 0.10. Sorry for the delay around these PRs - it's been a busy few weeks :-).

No worries. I've been super busy myself lately. I do feel that the code is a good shape for a release now, so I'll probably do it pretty soon. 6 months without a new release are way too many. :-)

@cichli
Copy link
Member Author

cichli commented Apr 27, 2015

Cool - the descriptor doc needs to be tweaked slightly (doesn't have the error slot in the response or the print-length/print-level slots in the request documented yet), which I'll do next time I'm at my laptop, but feel free to merge in the meantime if you want and I can fix it up afterwards.

bbatsov added a commit that referenced this pull request Apr 28, 2015
Add support for refresh middleware
@bbatsov bbatsov merged commit c85d5ae into clojure-emacs:master Apr 28, 2015
@bbatsov
Copy link
Member

bbatsov commented Apr 28, 2015

👍 There a few other things blocking the release - e.g. wrapping the completion changes. Fixing #1059 and #1061 is also on my radar for the release, although we can definitely do it without them.

@cichli cichli deleted the refresh branch April 28, 2015 08:38
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.

2 participants