ritz re-uses some of nrepl's private functions to avoid duplication. The uses are listed below.

Would it be possible to make these functions public? More subjectively, it might also be worth considering factoring out the session functionality into it's own namespace (including msg and possibly queue-eval), so the functionality is not split across the session middleware and the interruptible-eval middleware.

When specifying middleware, it would be much easier for the user to be able to override a default middleware without having to specify a handler.

For example, if there is a default middleware providing the "complete" operation, the user should be able to just specify their preferred completion middleware, without having to specify all middleware as a handler.

One way to do this might be to check metadata for the provided operations of the specified metadata, and ensure that either the default middleware for that operation is removed, or that the specified middleware takes precendence (which may be simpler when a middleware provides multiple operations).

Just making sure that the order in which additional middlewares are provided is taken as a default stack order will suffice for most use cases. Transforming middlewares (either in full or in part) would need another metadata slot, :replace perhaps, though it seems like that would be much more difficult to get right.

Basically, (send-off some-agent println) & co. should get what's printed to out in that agent's thread back to the nREPL client — not silently let it dump out to System/out.

Portal ostensibly does this well. Examine their approach, see if it is compatible with nREPL's objectives.

Ill-formed brain dump:

multiplex new out's to System/out

(still won't solve clojure.test/test-out content will disappearing into the ether when it's loaded when out is bound to an nREPL out; maybe we should ensure out is bound to System/out while code is being loaded?)

Most users want to put an nREPL port on their app, and that should always be on a typical port. Auto-selection of a port is only desirable in tooling scenarios, and tooling authors can pass an argument to start-server.

Hi Chas, do you have ideas on how to expose middlewares?
Should it be through a new op, mimicking what `describe` does, but for installed middlewares?
Should it leverage the existing `describe` middleware but with an additional (backwards-compatible) key (e.g. key = "type" value = "op" or "middleware") ?

I've thought about this a bit. I would like to avoid exposing transient implementation details (e.g. the namespace/var name of particular middlewares) to clients like this. Doing so just makes it easy for complications to occur later (e.g. what happens when that name changes due to a project reorg, or when there's N pprint middlewares, rather than just one?).

I think it's reasonable to expect middlewares to participate in the reporting of available operations in the describe response. Even things like a pprint middleware should offer an operation or two to toggle whether or not it's active. I'd be interested in any counterexamples to this general expectation…?

I agree that middlewares should be able to participate, and that it should be declarative.
Short of having special semantics for reporting middlewares instead of ops, what I did for `cider-nrepl` is to declare a dummy op named `pprint-middleware`, even though currently there's no pprint dedicated op.

Dev tools writers using nREPL's eval op face the following problem - using the `eval` op you cannot pass information regarding the filename and the file position of the form you're evaluating and therefore afterwards you can't use find its definition. Because of this most CIDER users reload their source buffers all the time with `load-file`. Another implication of this problem is that exception backtraces have meaningless information about the position of the problem (usually something like `NO_FILE:1`). I've noticed that LightTable has a custom middleware for this, ritz used to have one as well, probably others too. Now I'm contemplating doing something similar for CIDER, but it seems to me this is basic functionality that should be supported out-of-the-box.

I'd like to propose that the default eval middleware be augmented to support some form of source form tracking to spare tool writers from having to reinvent the wheel.

I agree that this should be baked in. `:file`, `:line`, and `:column` are some optional slots eval could accept (but should not require), with the obvious effect on the reader passed along to the compiler. This may be trickier than it sounds, especially if you're aiming to keep things at parity for both Clojure and ClojureScript (as code to be eval'd is read well before it hits interruptible_eval, in piggieback).

`:filepath` should be another optional slot. I don't do much ClojureScript (and I don't know much about piggieback) so getting something working for Clojure would be a good enough start for me. I'll try to cook some patch soon.

I've only skimmed Ritz's implementation, but it does a lot of clever things, and is tied to Clojure. Perhaps there's reasons why my (perhaps hacky?) approach isn't suitable in certain situations, but it should be pretty future-proof, and should work transparently with ClojureScript/piggieback. I'd rather dummy up "files" to be loaded than overriding LineNumberingPushbackReader method impls, etc.

All of the source-file-aware stuff is in load-file, so I'm inclined to put the burden on the client. Even if an op accepted the necessary info, the client would still need to provide it (target namespace + line and column information); going from there to a dummied-up ns form and the corresponding whitespace is so straightforward, I'd rather not complicate the op interface(s).

Btw, looking at this ticket https://github.com/clojure-emacs/cider/issues/781 I'm not sure we can use just (ns ns-name) at the start of the dummy files as it seems this overrides the namespace definition. Perhaps we should always insert the complete ns declaration from the source file in question? Or is there some other smart trick I'm not aware of that'll help avoid this ns clobbering.

Another solution is to allow the `eval` middleware to accept a token that will be appended to the temp file that eval uses for evaluation. Then the client can split the file path and retrive the metadata back. You can even allow for a full path of a temp file as an argument. This should be trivial to implement.

The problem with the solution you propose is that *eval* is implemented in terms of Clojure's built *clojure.main/repl* and we obviously can't change it to suit our needs. Don't know about you, but I definitely don't want *eval* to have to be implemented in terms of some custom evaluation logic developed specially for nREPL.

The current solution works well enough and requires zero middleware changes. The only question is does the temp file need the complete ns declaration of the original source file or not.

I couldn't reproduce the problem some people are reporting, so I'm inclined to believe you. I've just merged a patch that uses the full ns declarations all the time, but I may revert it in light of your comment. I wonder if this behaviour changes in different Clojure versions.

Using the full ns declaration isn't necessarily bad, but there is the edge case of people who add :reload and/or :reload-all options to their :require, etc clauses. In such cases, sending a single top-level expr to be loaded will then cause other namespaces to be reloaded, perhaps with side effects (if those other namespaces have impolite top-level forms themselves).

So, we've finally tracked down the problem - load-file returns for "ns" either "user" or ":main" for some reason, regardless of the namespace of the file you're loading. This sounds like a bug to me; I'll find another ticket.

We found one more related problem. Seems that load-file in piggieback discards the return value of the evaluation. More details can be found here. I think the behaviour for Clojure and ClojureScript load-file should be unified.

The attached patch, 0001-NREPL-59-Tracking-source-form-positions-in-eval.patch, adds optional file, line and column slots to the eval op. As discussed in the above-mentioned gist, reflection is used to set the line and column numbers of LineNumberingPushbackReader.

The version-sniffing in the test is because support for column number metadata was only added in Clojure 1.5.

The reflection stuff is clearly non-idiomatic, but doing it this way lets us retain compatibility back to Clojure 1.2. FWIW the below crude benchmark suggests the performance impact is negligible, but we could presumably do some of it (resolving the _columnNumber and in fields) at load-time instead to further mitigate it.

Btw, I'll once again mention here that it's probably time to drop compatibility with Clojure pre-1.5. Supporting old (and pretty much unused) versions forever is bad for both the project and the community (sometimes people have to be nudged in the right direction).

> The role of those vars is actually the same across all Clojure REPLs.

Do you know if this behavior of all Clojure REPLs is documented anywhere? And yes, it would be nice if the nREPL documentation linked to this doc and/or it printed a short summary and/or link when starting (in addition to the currently provided info about (source) etc.)

REPL-bound vars are documented in a variety of places, though nowhere "official" AFAIK. We talked about it in Chapter 10 of Clojure Programming FWIW (I'm certain other books and online resources cover these vars as well, but the CP citation is the only one I have close at hand.)

FYI, the "currently-provided info" you mention is emitted by Leiningen/Reply, not nREPL.

The top hit for "Clojure REPL" seems to be http://clojure.org/repl_and_main, so it perhaps should be documented there or it should link to a more detailed documentation. Not sure how to make that happen I have also checked http://clojure-doc.org/ but there doesn't seem to be a suitable place to add this info either. Perhaps it should be mentioned in the docstring of clojure.main/repl and the Clojure page should link to it?

Not sure whether this ticket belongs here or in the Clojure-proper JIRA, so feel free to close if this is an inappropriate location. clojure.main/repl creates a new DynamicClassLoader on every execution, so it looks like the stack of classloaders grows without bounds. Seems a bit similar to http://dev.clojure.org/jira/browse/NREPL-31 in that clojure.main/repl has another assumption about when clojure.main/repl will run.

Yes, this should be fixed upstream; a new DynamicClassLoader should only be set as the thread-context classloader if one is not already in place.

My first impulse upon seeing this was that this was "last straw" territory w.r.t. using clojure.main/repl (as recorded in the thread linked above), but the work necessary to stop depending upon it would be considerable (not because repl does much, but because interruptible-eval/evaluate is structured to cater to it). Some years on, and it's clear that this fundamentally a minor problem (insofar as hardly anyone has complained AFAIK), so a fix that requires e.g. Clojure 1.7.0 would be fine.

The current nREPL version is driven by reading the resource at /clojure/tools/nrepl/version.txt. If it's not found (perhaps because of an android packaging / classloader detail I'm not familiar with?), then no nREPL version will be returned.

FWIW, the version indicator is meant to be strictly advisory, and not really depended upon for e.g. exposing tooling capabilities. What are you needing the version indicator for?

Currently, nREPL's interruptible-eval middleware hardcodes a dependency on pr-values. It would be nicer if the user could either specify alternate rendering middleware or if nREPL's built-in middleware accepted an arbitrary function to render values with. Specifically, this would enable pretty-printing REPL output.

Currently it is possible to replace the default middleware, but it involves some ugly runtime metadata manipulation which reaches into the nREPL internals. Addressing this would be another step towards simplifying pretty-printing/color integration in the REPL.

"Once a handler has completely processed a message, a response
containing a `:status` of `:done` must be sent."

I am assuming this includes messages that have resulted in an error. Some messages to an nREPL server never respond with such a status possibly leaving some clients waiting for follow up messages forever.

So far I have found two instances of this problem.

1. The interruptible-eval middleware may result in a :no-code error that does not include the :done status.

2. The session middleware may result in a :unknown-session error that does not include the :done status.

I'm including a patch for the above two which are just one word fixes. There might be other situations in which the problem arises.

In a development environment, one often has many nREPL servers running, sometimes more than one per project. This means that it's sometimes difficult to keep track of which ports are associated with which projects / processes, and existing per-project REPL tracking mechanisms (e.g. Leiningen/reply's repl-port file) are generally not up the job.

It would be great if nREPL exposed a JMX MBean that provided information on each active nREPL server, what URL(s) they'll respond to, and perhaps other details (what middleware is installed on each, etc). Clients could then use JMX to discover all of the nREPL servers on a given host, and present that information in tool-appropriate 'connect' dialogs, etc.

How do you envision clients discover which processes have nREPL servers? How would it work in practice from the client's point of view? Would the client be responsible for knowing the JMX port & associated configuration? Do you envision this feature applying mainly to clients on the local machine or to any number of hosts?

When I've worked with JMX, part of the battle is knowing the proper port and associated JMX configuration for each process so a client knows how to connect, especially with a variety of processes on a given host. It is even more problematic in the cloud with dozens of servers. And then there is the complication that locally one can sometimes use the Attach API, a mechanism that differs from how JMX is accessed remotely. At the risk of thinking too much, is there any other standard discovery mechanism that might be appropriate to share the JMX port & configuration with potential clients? Would some sort of multicast/cluster mechanism work here?