[Fix #814] Use cider-current-ns instead of nrepl-buffer-ns

[vspinu]

The recent interactive eval improvements essentially broke eldoc, completion and all other stuff that relies directly or indirectly on nrepl-buffer-ns. This has to do with the fact that :load-file op returns :main or "user" ns and cider's default handler eagerly sets it everywhere it can. Therefore, after each interactive evaluation nrepl-buffer-ns has a wrong value.

The usage of nrepl-buffer-ns is unavoidable in special buffers (repl, scratch, macroexpanion, doc etc). It also might be used as a cache in clojure buffers, but then it should be reset to nil in buffer modification hook. It should be fine for now, but to fully set this issue straight one would need to set we need to rewrite the response handling first.

---

Other changes:

• Don't reset nrepl-buffer-ns in default response handler.
• Remove cider-find-ns (it's confusing to have two functions doing almost the same thing).
• Use nil global value of nrepl-buffer-ns. This will allow easy detection of cases when this variable is used but shouldn't.

[bbatsov]

This has to do with the fact that :load-file op returns :main or "user" ns and cider's default handler eagerly sets it everywhere it can.

Yeah, I just noticed this. @cemerick Is this a bug or a feature? Seems pretty not to return the namespace of the file just loaded.

[bbatsov]

The usage of nrepl-buffer-ns is unavoidable in special buffers (repl, scratch, macroexpanion, doc etc). It also might be used as a cache in clojure buffers, but then it should be reset to nil in buffer modification hook. It should be fine for now, but to fully set this issue straight one would need to set we need to rewrite the response handling first.

Yeah, the fact that the same response handler is used for both eval and load-file is quite confusing. We should break up nrepl-make-response-handler into separate functions, as it's pretty strange to want to handle unrelated requests with the same huge piece of logic. Nasty legacy...

Btw, you have a typo in the commit message.

[bbatsov]

I wrote it, so I know what it does - basically we needed to make sure we're not doing interactive evaluation for non-evaluated namespaces and it that case evaluted the namespace form first. Given the way interactive evaluation works now this is redundant.

[bbatsov]

Apart from my tiny remarks the proposed changes look good.

[vspinu]

Done!

[bbatsov]

Btw, it seems this introduced a regression. From the mailing list:

I have faced a strange behavior after updating cider and clojure-mode to the latest.
I ran cider-jack-in and then I used in-ns however it didn't move to the target ns.

; CIDER 0.8.0alpha (package: 20140921.2312) (Java nil, Clojure 1.6.0, nREPL 0.2.3)
user> (in-ns 'test)
#<Namespace test>
user> 

 The project I have tested is created by 'lein new test' and no source was added.

[vspinu]

Indeed. The fix is to set the ns in the repl handler, but I wonder if this statement is meaningful from a source file?

I already experienced a couple of ns related issues with the new evaluation mechanism. If the error is in ns-form and you want to fix it with something like (ns-unalias 'foo) you won't be able to do it from the source directly because the erroneous ns-form is always executed. So you will need to jump to repl to fix it.

[bbatsov]

Indeed. The fix is to set the ns in the repl handler, but I wonder if this statement is meaningful from a source file?

Don't think so. Doesn't really make sense to do in-ns is a source buffer.

I already experienced a couple of ns related issues with the new evaluation mechanism. If the error is in ns-form and you want to fix it with something like (ns-unalias 'foo) you won't be able to do it from the source directly because the erroneous ns-form is always executed. So you will need to jump to repl to fix it.

Small price to pay for all the benefits gained. And we can always add a an interactive command based on the eval op for situations like these.