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

use nrepl, not eval #14

Merged
merged 3 commits into from
Feb 19, 2016
Merged

use nrepl, not eval #14

merged 3 commits into from
Feb 19, 2016

Conversation

pdenno
Copy link
Collaborator

@pdenno pdenno commented Feb 17, 2016

As the subject suggests, this is a major change. Writing a reliable REPL, which is where this code seemed to be heading, is a lot of work. tools.nrepl is a REPL solution made just for this sort of problem, and it is tested (used by emacs/cider etc.) and solid. See the section why nREPL? at tools.nrepl github.

I have not experienced any problems changing namespaces in this fork. I implemented stacktraces using code from cider-nrepl.

stub-in is-complete-reply

nrepl instead of eval
@pdenno
Copy link
Collaborator Author

pdenno commented Feb 17, 2016

Apparently I still don't quite understand how squashing commits works. Thus the 3 commits...Sorry.

roryk added a commit that referenced this pull request Feb 19, 2016
@roryk roryk merged commit 89ed047 into clojupyter:master Feb 19, 2016
@roryk
Copy link
Collaborator

roryk commented Feb 19, 2016

!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

@roryk
Copy link
Collaborator

roryk commented Feb 19, 2016

Thank you so so so so so so so much, Peter. This is incredible. I am so happy, it would have taken me forever to figure that out. I gave you push access to the repository, as far as I am concerned you should own it.

@pdenno
Copy link
Collaborator Author

pdenno commented Feb 19, 2016

Well thanks for all the work on Zero MQ and the interface with the the
notebook! That would have taken me ages to figure out, and I probably
wouldn't have taken on the work had I not seen something "close enough"
to usable in clojupyter.

On 2016-02-18 9:58 PM, Rory Kirchner wrote:

Thank you so so so so so so so much, Peter. This is incredible. I am
so happy, it would have taken me forever to figure that out. I gave
you push access to the repository, as far as I am concerned you should
own it.


Reply to this email directly or view it on GitHub
#14 (comment).

@roryk
Copy link
Collaborator

roryk commented Feb 19, 2016

Sweet, I have been walking around happy today. These bugs were weighing on me and I didn't realize it. Thanks so much!

@thomasjm
Copy link
Collaborator

@pdenno , it seems this PR actually broke something basic: you can no longer print to stdout. I.e. something like (prn "HI") doesn't show any output other than the nil result.

I think the reason is that stdout used to be captured by the (binding [*out* s#] ...) currently at core.clj:282, back when this kernel worked using eval.

In the world of nrepl-eval, I'm not sure how stdout capturing should work. Any chance you know how to add it back? :)

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