New middleware: out.clj #257
Conversation
| (defn eval-messages-watch [_ _ _ new-state] | ||
| (let [o (fork-out (vals new-state))] | ||
| ;; FIXME: This won't apply to Java loggers unless we also setOut. | ||
| ;; (System/setOut (PrintStream. o)) |
There was a problem hiding this comment.
What's the reason for postponing this part?
There was a problem hiding this comment.
Because you can't directly convert a PrintWriter to a PrintStream.
|
The code looks good to me. You'll have to mention this middleware in the README I guess, even though it doesn't provide any new ops. And when squashing the commits you should update the text of the first commit. |
|
One more thing - some tests for this would be nice to have. Maybe something printing output from a future or something. Not sure what's the best way to test it. |
| (defmacro do-with-out-messages | ||
| "Run body with v bound to the output stream of each msg in msg-seq. | ||
| Also run body with v bound to `original-out`." | ||
| [[v msg-seq] & body] |
|
Where are we with this? |
|
Readme and tests missing. Haven't started on them yet. |
| [messages] | ||
| (PrintWriter. (proxy [Writer] [] | ||
| (close [] (.flush ^Writer this)) | ||
| (write [& [x ^Integer off ^Integer len]] |
There was a problem hiding this comment.
This & + destructuring seems extraneous and I think is responsible for a stacktrace. Well, removing & and the destructuring makes things work :-) :
ERROR [2015-09-18 22:03:06,794] com.yammer.dropwizard.jersey.LoggingExceptionMapper: Error handling a request: 8cf72e1a7bae9e6a
! java.lang.IllegalArgumentException: No matching method found: write for class java.io.OutputStreamWriter
! at clojure.lang.Reflector.invokeMatchingMethod(Reflector.java:80)
! at clojure.lang.Reflector.invokeInstanceMethod(Reflector.java:28)
! at cider.nrepl.middleware.out$fork_out$fn__20648.doInvoke(out.clj:45) ~[na:na]
! at clojure.lang.RestFn.invoke(RestFn.java:423)
! at cider.nrepl.middleware.out.proxy$java.io.Writer$ff19274a.write(Unknown Source) ~[na:na]
! at java.io.PrintWriter.write(PrintWriter.java:405) ~[na:1.8.0_45-internal]
! at java.io.PrintWriter.append(PrintWriter.java:1063) ~[na:1.8.0_45-internal]
! at java.io.PrintWriter.append(PrintWriter.java:56) ~[na:1.8.0_45-internal]
! at clojure.core$pr.doInvoke(core.clj:3563)
! at clojure.lang.RestFn.applyTo(RestFn.java:139)
! at clojure.core$apply.invoke(core.clj:630)
! at clojure.core$prn.doInvoke(core.clj:3593)
! at clojure.lang.RestFn.applyTo(RestFn.java:137)
! at clojure.core$apply.invoke(core.clj:630)
I'm using this impl of fork-out:
(defn fork-out
"Returns a PrintWriter suitable for binding as *out* or *err*. All
operations are forwarded to all output bindings in the sessions of
messages in addition to the server's usual PrintWriter (saved in
`original-out`)."
[messages]
(PrintWriter. (proxy [Writer] []
(close [] (.flush ^Writer this))
(write
([c]
(do-with-out-messages [out messages] (.write out c)))
([s ^Integer off ^Integer len]
(do-with-out-messages [out messages] (.write out s off len))))
(flush []
(do-with-out-messages [out messages] (.flush out))))
true))
There was a problem hiding this comment.
I think it also explains all these build failures: https://travis-ci.org/clojure-emacs/cider-nrepl/builds/81060333
There was a problem hiding this comment.
hm... I guess it's not safe to call (.write out s off len) on the system's standard out. I'll fix it, thanks.
There was a problem hiding this comment.
The reflector can't find the right java method when off/len are null in the case that the 1-ary version of .write was called. I think that's because they're primitive arguments, even if it could find it, it would fail to cast a null to a primitive. It might be jvm-version-dependent as well.
|
Thanks @gtrak, I believe the exception is fixed now. I've added a readme entry and a test as well. The test is terribly superficial, but my knowledge of this whole Writer/Print/Stream family of Java inheritance is very limited, so I can't do much better. |
|
Guess this will do for now. Squash the commits and we'll refine the code down the road. |
| ([x] | ||
| (with-out-binding [out messages] | ||
| (.write out x))) | ||
| ([x ^Integer off & [^Integer len]] |
There was a problem hiding this comment.
I still don't get why you need this argument as optional, there's only arity 1 and 3 .write methods in http://docs.oracle.com/javase/7/docs/api/java/io/Writer.html, so len will never be null if this arity gets called (since it's also a primitive int).
There was a problem hiding this comment.
I still don't get why you need this argument as optional, there's only arity 1 and 3 .write methods in Writer
Because I didn't know that. :)
Automatically changes the root binding of *out* to print to any active connections. An active connection is a session that has sent at least one "eval" op and hasn't been closed. We use an eval message, instead of the clone op, because there's no guarantee that the message that sent the clone op will properly handle output replies.
|
Ok. I believe the failing tests are the same ones that fail on master |
Automatically changes the root binding of out to print to any active
connections. An active connection is a session that has sent at least
one "eval" op and hasn't been closed.
We use an eval message, instead of the clone op, because there's no
guarantee that the message that sent the clone op will properly handle
output replies.
clojure-emacs/cider#1328