ccw-ide / ccw

Counterclockwise is an Eclipse plugin helping developers write Clojure code
https://github.com/laurentpetit/ccw/wiki/GoogleCodeHome
Eclipse Public License 1.0
220 stars 50 forks source link

Clojure 1.7: REPL printing #813

Closed casperc closed 9 years ago

casperc commented 9 years ago

The namespace object is not printed right when using "Switch REPL to file's namespace" when using Clojure 1.7. It is printed as

object[clojure.lang.Namespace 0x23dbd676 "microservice.core"]

Edit: Same goes when deffing a var in the REPL:

(def something [1 2 3 4]) prints #<Var@6957c8f3: [1 2 3 4]>

(defn somefn [] (prn "something")) prints #<Var@415b78a2: #object[microservice.cassandra$somefn 0x222b9f0e "microservice.cassandra$somefn@222b9f0e"]>

laurentpetit commented 9 years ago

This has indeed to be improved. We need to detect the clojure version, and when 1.7 or higher, set the right dynamic var for both the printing of objects and the error reporting so it's reported as before.

arichiardi commented 9 years ago

Yes Clojure 1.7 has changed some things about printing exceptions and objs in general. There has been some fuss around it :)

arichiardi commented 9 years ago

The problem is not as easy at it seems... Here some info.

arichiardi commented 9 years ago

Maybe we can just specify a print-method for Namespace if >1.7...or drop printing the namespace...I guess we don't need/want to change too much at the REPL...

laurentpetit commented 9 years ago

Yes we can probably check if there's a print-method for namespaces, and if not, add one. Chad published the code on Twitter / github snippets.

arichiardi commented 9 years ago

Harrington? I checked but could not find it...

arichiardi commented 9 years ago

Ok, I wrote, very quickly, my own...now I just need to find where/how to put it:

(defmethod print-method clojure.lang.Namespace [v ^java.io.Writer w]
  (.write w (str v)))

Basically I hope there was a method to execute it just when ccw switches but not for everything. For instance, let's say somebody uses *ns*, that should print according to clojure's printing (tagged object in 1.7...).

arichiardi commented 9 years ago

Found! fixing as we speak,

arichiardi commented 9 years ago

Nope, the problem is that every execution prints out something, therefore I cannot do:

(defmethod...)
(in-ns ...)
(remove-method)

Maybe there is a way to avoid printing the result of an evaluation?

laurentpetit commented 9 years ago

I don't understand, sorry 

— Envoyé avec Mailbox

On Sat, Jul 18, 2015 at 6:02 PM, Andrea Richiardi notifications@github.com wrote:

Nope, the problem is that every execution prints out something, therefore I cannot do:

(defmethod...)
(in-ns ...)
(remove-method)

Maybe there is a way to avoid printing the result of an evaluation?

Reply to this email directly or view it on GitHub: https://github.com/laurentpetit/ccw/issues/813#issuecomment-122558728

arichiardi commented 9 years ago

No problem, basically on namespace switching I can execute

(defmethod...)       ;; sets the new print-method
(in-ns ...)               ;; performs the namespace change 
(remove-method)  ;; removes the multi method so that we don't interfere with the user repl session

The problem is that each evaluation prints a result, that in case of the multi method addition/removal is not what we wanted. If there were a way to mute the evaluation result just for some instruction...I would have solved.

I am browsing the code base to see if it is feasible, and I suggestions are more than welcome :smile:

arichiardi commented 9 years ago

BTW I have found the gist but I think my problem persist, and my solution less invasive for the user.

laurentpetit commented 9 years ago

Don't we control the final printing to the log area of the repl view? Maybe we could just check the content with a regex like

"object[clojure.lang.Namespace[^\"]+\"([\"]+)\"]" and if it matches then

replace it with the matched group?

I'm a little bit concerned about creating then removing a print-method on every evaluation. It's not thread safe.

Another option would be to install once and for all a print method for the namespace, which would either delegate to the print-object method, or own repl-custom print code. The choice would be driven by a new dyn bar we would also introduce once and for all in clojure.core. We could then thread-safely bond this var during evaluation of user code sent via the REPLView.

Le samedi 18 juillet 2015, Andrea Richiardi notifications@github.com a écrit :

BTW I have found gist https://gist.github.com/cemerick/c8ed5730b0055f41cbef but I think my problem persist, and my solution less invasive for the user.

— Reply to this email directly or view it on GitHub https://github.com/laurentpetit/ccw/issues/813#issuecomment-122587425.

Laurent Petit

arichiardi commented 9 years ago

I did not know that defmethod is not thread safe, thanks! Maybe the first option is easier to implement...

laurentpetit commented 9 years ago

No it's not the fact that defmethod is not thread safe (which is also right indeed as far as I know), but the fact that it's a global change that will be visible by any thread while the repl expression is evaluating. 

— Envoyé avec Mailbox

On Sun, Jul 19, 2015 at 10:03 AM, Andrea Richiardi notifications@github.com wrote:

I did not know that defmethod is not thread safe, thanks!

Maybe the first option is easier to implement...

Reply to this email directly or view it on GitHub: https://github.com/laurentpetit/ccw/issues/813#issuecomment-122637751

arichiardi commented 9 years ago

Well, that particular nrepl instance will evaluate the three instructions in sequence and the problem might be that, if someone else evaluates between defmethod and in-ns, she will see a different printing of Namespace. Am I completely on the wrong path? In any case filtering the output of the evaluation feels cleaner...

laurentpetit commented 9 years ago

Yes. That's it. 

Filtering the output could be something proposed as a post processing that can be overrides by user plugins, by the way. 

— Envoyé avec Mailbox

On Sun, Jul 19, 2015 at 11:21 AM, Andrea Richiardi notifications@github.com wrote:

Well, that particular nrepl instance will evaluate the three instructions in sequence and the problem might be that, if someone else evaluates between defmethod and in-ns, she will see a different printing of Namespace. Am I completely on the wrong path?

In any case filtering the output of the evaluation feels cleaner...

Reply to this email directly or view it on GitHub: https://github.com/laurentpetit/ccw/issues/813#issuecomment-122641656

casperc commented 9 years ago

I updated the top post with two more examples of things that are not printed correctly: def and defn

arichiardi commented 9 years ago

As said, we cannot change the behavior of clojure.core, only workaround the commands that are "ccw's", like the namespace. If you want to change all the printing to 1.6, follow this gist. I will provide a way to override the display through user plugin, but it is an advanced feature that not everybody would want to have.

arichiardi commented 9 years ago

I am working on this, taking the time for the tests as well.

laurentpetit commented 9 years ago

I'm making my mind on this issue as well

laurentpetit commented 9 years ago

Well, maybe the current cider-nrepl version is tricking us, at least for the def defn parts. Seems like it tried to stick to ClojureScript way of printing, rather than Clojure? Will do some tests without cider-nrepl enabled.

laurentpetit commented 9 years ago

OK, so as far as defn and def are concerned, it seems like it's an issue with cider-nrepl's pprint middleware, which is now active by default. I contacted the cider team. For the ns, the initial mentioned problem remains. I think we should split this issue in two.

laurentpetit commented 9 years ago

WRT pprint: I posted the problem with pprint to the clojure-dev ml: https://groups.google.com/d/msg/clojure-dev/pUG0cDkyK_s/Rsnb4_HGyQwJ

laurentpetit commented 9 years ago

@casperc in order not to see the wrong display for defs, the workaround is to disable the usage of pprint in the Clojure > REPL View preferences. The issue exists since at least Clojure 1.6, and lies within clojure.pprint/pprint.

laurentpetit commented 9 years ago

Ok, so for the issue with pprinted vars, I have "backported" the solution from clojure 1.8.0-alpha2 dynamically.

For the initial problem, ns not displayed correctly (as well as *e), there's no clean solution without patching nrepl heavily (replacing interruptible-eval middleware). So I will apply the not-so-clean solution of reverting back to 1.6.0 print-object for the whole process JVM.

laurentpetit commented 9 years ago

Maybe I'll add a preference to revert back to this behavior, set to true by default.

arichiardi commented 9 years ago

Yes i was goint to suggest a preference

laurentpetit commented 9 years ago

Done. Both problems are solved.One with a hack which will stay for at least as long as some people use clojure 1.7 or below (for pprint). the other which is enabled by default, but has a preference for disabling it (ns and *e printing).