Small improvments I would like to make to the pREPL (first contribution)

190 views
Skip to first unread message

Oliver Caldwell

unread,
Dec 10, 2018, 8:49:42 AM12/10/18
to Clojure Dev
Hi there, I haven't contributed to Clojure or ClojureScript before so I thought I'd get some input from the mailing list before I went and made any changes or raised any more issues on JIRA.

For context, I'm currently working on a pREPL based development environment for Neovim, it's pre-pre-alpha so I'm not documenting and sharing it just yet but I'm using it for day to day work and it's been enjoyable so far.

I discovered an issue with the CLJS pREPL where exceptions are returned as data instead of a string, unlike all other :val responses. I've reported that issue here and will be submitting a patch soon: https://dev.clojure.org/jira/browse/CLJS-2994

I was just going to catch exceptions and ensure they're run through the val fn and formatted as a string, shouldn't be anything too controversial there.

The other issue I discovered but haven't reported yet was that the CLJS pREPL supports reader conditionals but the CLJ one doesn't. I worked around this by wrapping my CLJ code in read-strings and evals that _do_ allow reader conditionals. Is this by design or would allowing reader conditionals in the CLJ pREPL be desirable?

I also think I managed to get the CLJ pREPL to return data under :val for some exceptions but I need to look into that. In my opinion the pREPLs for both languages should:

* ALWAYS return a string under :val, no matter what.
* Accept reader conditionals.

Anyone disagree with those statements? If not I'll submit some patches to align the two which will allow me to delete some workarounds from my tooling :)

Thanks a lot,

Oliver Caldwell

(oh, and here's my pREPL tooling if you want to keep an eye on it for the future although I'll be tweeting about it when it's ready https://github.com/Olical/conjure)

Alex Miller

unread,
Dec 10, 2018, 10:17:35 AM12/10/18
to cloju...@googlegroups.com
There are a couple layers here so it would be useful to be more precise. The lowest layer is prepl, which is returning data messages, not strings, which it sends to out-fn. io-prepl wraps prepl and binds it to streams. It provides an out-fn that pr's all messages, converting to strings so they can be safely sent over the io streams. I believe CLJ prepl is doing this properly. 

I'm in the process of writing up some docs about prepl, but for the moment I uploaded the design picture which you might find helpful as you're digging into this:  https://clojure.org/images/content/reference/prepl/prepl.png

The read conditional thing in prepl is a good catch and I think it would make sense to do this. The place where this happens in prepl is the read+string call at https://github.com/clojure/clojure/blob/master/src/clj/clojure/core/server.clj#L229. Would be happy to have a ticket+patch for this (not going to be 1.10 though). 

FYI, there are a couple of old Clojure projects named conjure - https://github.com/macourtney/Conjure and https://github.com/amitrathore/conjure. :)


--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev...@googlegroups.com.
To post to this group, send email to cloju...@googlegroups.com.
Visit this group at https://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/d/optout.

Oliver Caldwell

unread,
Dec 10, 2018, 11:02:06 AM12/10/18
to cloju...@googlegroups.com
Ah, thank you for the clarification and diagram, that's extremely helpful. I know pREPL is still alpha and I am completely okay with dealing with changes / lack of documentation. I'm jumping on the train early to build my own tooling (which I hope others will find useful eventually) and help spot issues like those I've already mentioned.

For the reader conditionals it looks like I'll just need to mirror the CLJS version: https://github.com/clojure/clojurescript/blob/848e10a9dac539b9271d83577fc1266f18e949da/src/main/clojure/cljs/core/server.clj#L99-L100 - should I raise an issue for this in JIRA? (I'm guessing yes)

I'm not sure where best to solve the CLJS errors, I'm not sure why they're not being pr-str-ed. I guess there needs to be another try/catch somewhere that performs the `(Throwable->map ex)` dance, but it looks like it's all already there.

Also, prepl or pREPL? What does the p stand for? :) hopefully I will get these two patches submitted over the next week or so, is it worth sharing them here first before sending them to JIRA? Just want to work out what the best practice is for contributions before I start messing with code.

I was also wondering what remote-prepl existed for, is it to allow you to have one Clojure program connect to another through a pREPL? It's not a socket server + pREPL, it's a pREPL somewhere else hosted on a socket? I hope that question makes sense, I have a feeling it doesn't.

You received this message because you are subscribed to a topic in the Google Groups "Clojure Dev" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/clojure-dev/VhB2NETau8U/unsubscribe.
To unsubscribe from this group and all its topics, send an email to clojure-dev...@googlegroups.com.

Oliver Caldwell

unread,
Dec 10, 2018, 11:21:46 AM12/10/18
to cloju...@googlegroups.com
And thank you but I'm aware of the existing projects under the name, I just really like it. I think it's a very apt name for REPL tooling:

cause (a spirit or ghost) to appear by means of a magic ritual.
[archaic] implore (someone) to do something.

 I could still change it but I think it works too well. I'm actually surprised it's not far more common!

Alex Miller

unread,
Dec 10, 2018, 11:36:46 AM12/10/18
to cloju...@googlegroups.com
On Mon, Dec 10, 2018 at 10:02 AM Oliver Caldwell <olli...@gmail.com> wrote:
Ah, thank you for the clarification and diagram, that's extremely helpful. I know pREPL is still alpha and I am completely okay with dealing with changes / lack of documentation. I'm jumping on the train early to build my own tooling (which I hope others will find useful eventually) and help spot issues like those I've already mentioned.

Yeah, cool stuff! Glad you're using it.
 
For the reader conditionals it looks like I'll just need to mirror the CLJS version: https://github.com/clojure/clojurescript/blob/848e10a9dac539b9271d83577fc1266f18e949da/src/main/clojure/cljs/core/server.clj#L99-L100 - should I raise an issue for this in JIRA? (I'm guessing yes)

Yes, please. Basically just adding :read-cond :allow (although that may change the call arity you make. No need to inject the feature like cljs is doing; the Clojure reader injects it automatically.
 
I'm not sure where best to solve the CLJS errors, I'm not sure why they're not being pr-str-ed. I guess there needs to be another try/catch somewhere that performs the `(Throwable->map ex)` dance, but it looks like it's all already there.

Also, prepl or pREPL?

prepl (pronounced like preppy, not p-repl)
 
What does the p stand for? :)

programs (or programmatic access) - as in, this is not a repl you would use interactively as a user, but a repl that you can build tooling on top of (which might include a user repl). 
 
hopefully I will get these two patches submitted over the next week or so, is it worth sharing them here first before sending them to JIRA?

No, please just move to jira and I can review/discuss there.
 
Just want to work out what the best practice is for contributions before I start messing with code.

I was also wondering what remote-prepl existed for, is it to allow you to have one Clojure program connect to another through a pREPL? It's not a socket server + pREPL, it's a pREPL somewhere else hosted on a socket? I hope that question makes sense, I have a feeling it doesn't.

io-prepl is designed to work as a socket server acceptor (like clojure.core.server/repl). So you would start your server with an io-prepl running on a socket server, then connect as a remote client with remote-prepl.

Oliver Caldwell

unread,
Dec 19, 2018, 4:43:57 AM12/19/18
to cloju...@googlegroups.com
I've submitted the patch for Clojure prepl reader conditionals (https://dev.clojure.org/jira/browse/CLJ-2453), I am unable to edit the ticket to mark it as containing code though. I think I need permissions on JIRA for that.

I'll submit the ClojureScript prepl change today too hopefully, I think the process is essentially identical.

I hope you're having a good run up to the holiday season!

Thanks,

Oliver Caldwell

Alex Miller

unread,
Dec 19, 2018, 8:14:27 AM12/19/18
to cloju...@googlegroups.com
Thanks! I updated your permissions.
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev...@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages