Broken current ns detection #781

Closed
danskarda opened this Issue Sep 9, 2014 · 13 comments

Projects

None yet

4 participants

@danskarda
Contributor

Since commit d07b7a0 I experience following problem:

(ns foo
   (:require [bar :as b]))

(b/baz)
  1. start Cider
  2. load a buffer using C-c C-k
  3. evaluate the expression (b/baz) inside the buffer using C-x C-e
  4. evaluate the same expression again using C-x C-e. It FAILS with exception no such namespace b.

Also C-x M-n does not change current namespace to current buffer, but to starting namespace.

It seems that cider uses the last ns and then forgets about it. I used git bisect to find commit d07b7a0 which causes the error. I use latest cider-nrepl, tools.nrepl etc.

Hope this helps.

@gar3thjon3s

I experienced the same problem. What version of Clojure are you using? It seems to have gone away for me when I went from 1.5.1 -> 1.6.0. Or it could have been one of the 300 other things I tried to fix it :/

@bbatsov
Member
bbatsov commented Sep 10, 2014

See http://dev.clojure.org/jira/browse/NREPL-59 for background regarding this. I think that the temp ns declaration I'm using for the interactive evaluation is causing this, but I don't have time to debug it now.

@gar3thjon3s

I take it back, it didn't go away. Thanks for the update.

@bbatsov
Member
bbatsov commented Sep 11, 2014

I can't reproduce this. Guess my theory regarding the problem in NREPL-59 is incorrect.

@vspinu
Contributor
vspinu commented Sep 16, 2014

I can confirm this. No evaluation works except of the load-current-buffer. It looks like messing with ns declaration is ridden with peril.

@bbatsov
Member
bbatsov commented Sep 16, 2014

We can always use the full ns declaration - that should work. I'm still waiting to hear back from @cemerick regarding NREPL-59. As I mentioned I can't reproduce this myself, otherwise I would have changed the code by now.

@bbatsov
Member
bbatsov commented Sep 16, 2014

@vitoshka In the mean time you can test whether using the full ns declaration solves the problem. Unfortunately we'll have to adjust the filling logic a bit to factor the ns declaration length (in terms of lines), but that should be pretty simple.

@vspinu
Contributor
vspinu commented Sep 16, 2014

Bozhidar Batsov on Tue, 16 Sep 2014 02:48:30 -0700 wrote:

@vitoshka In the mean time you can test whether using the full ns declaration
solves the problem. Unfortunately we'll have to adjust the filling logic a bit
to factor the ns declaration length (in terms of lines), but that should be
pretty simple.

I have just looked into it. It indeed solves the problem. Will make a PR
in a second.

@vspinu vspinu added a commit to vspinu/cider that referenced this issue Sep 16, 2014
@vspinu vspinu [Fix #781] Insert full ns-form on into dummy eval file f431130
@bbatsov bbatsov closed this in fb7e019 Sep 16, 2014
@vspinu
Contributor
vspinu commented Sep 18, 2014

Ok. I think I know what went wrong. It wasn't the full ns form thing. It's the wrong nrepl-buffer-ns in the buffer. For some reason it is not automatically detected and stays "user" all the time for me. As previous code was using it to build the (ns .. ) form it was screwing things up. So, @cemerick was completely right about ns form being additive.

@bbatsov
Member
bbatsov commented Sep 18, 2014

@vitoshka Sounds like you might be right, but cider-find-ns should be consulting the ns declaration of the source buffer as well. I'll reopen this ticket, so won't forget to investigate the problem further.

@vspinu
Contributor
vspinu commented Sep 18, 2014

I will investigate it too, but I am quite happy with the current status. I think sending the full form is not a bad idea as the user never needs to evaluate the ns form before eval. That is, the first C-c C-k or C-c C-n is not necessary anymore.

@bbatsov
Member
bbatsov commented Sep 18, 2014

@vitoshka Haven't thought of this - it's actually a pretty nice improvement over the previous workflow. But there's a small problem we need to solve - stripping :reload and :reload-all before we don't want those to affect interactive evaluations. Few people use them these days, but it's an actual problem never-the-less.

@vspinu
Contributor
vspinu commented Sep 18, 2014

But there's a small problem we need to solve - stripping :reload and :reload-all

We don't want to strip them on first evaluation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment