Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Route all non-repl evals through nrepl-current-buffer function. #51

Closed
wants to merge 2 commits into from

Conversation

technomancy
Copy link
Contributor

This implements the fixes discussed on the mailing list:
https://groups.google.com/group/nrepl-el/browse_thread/thread/26971a4e4717189a

The nrepl-current-ns function now checks the major mode and looks for an ns form if the mode isn't nrepl-mode.

@kingtim
Copy link
Member

kingtim commented Aug 2, 2012

Thank you for putting this together. I will do some testing on it and get it merged in.

@kingtim
Copy link
Member

kingtim commented Aug 4, 2012

Maybe I'm missing something but this still doesn't work for me. When I first visit a buffer the ns has not been initialized yet. Any forms I try to eval will use the ns of the buffer, and fail. If I try to eval the ns form, using either C-c C-n or C-x C-e it also fails. I have to load the current buffer to things up and running.

@technomancy
Copy link
Contributor Author

Right, the regional commands must be preceded by a full buffer load to work. But this isn't caused by my change; these commands won't work anyway without a full load if the needed requires haven't happeded.

The one exception is that trivial code that doesn't need any requires would have allowed C-x C-e before and break now. But I think it's pretty reasonable to require that C-c C-k precede any other evaluation commands.

@kingtim
Copy link
Member

kingtim commented Aug 4, 2012

Hmm... I work this way all the time by using C-c C-n or C-x C-e to eval the ns and then continue on happily developing and evaluating the other forms. It hasn't ever been a problem for me. But with this change I can't do that.

On my load-file-current-ns branch, I spiked together an alternative experiment which forces the buffer ns to be set after a load file, as I suspect that is the source of most of the problems. If you do a load-file, the response does not contain the ns, so the nrepl-buffer-ns is never set.

Anyway, I'll try to connect up with you on irc and we can figure this out.

@technomancy
Copy link
Contributor Author

So on the one hand, I'm not sure why we have C-c C-n since I can't think of any reason you'd want to eval the ns form on its own without the rest of the namespace. So I don't see the point of a workflow that goes out of its way to avoid C-c C-k.

But on the other hand, you're right that it's kind of lame that you can't eval things like simple arithmetic calls straight away. So that probably means this approach isn't going to fly.

I just tested the "load-file-current-ns" branch briefly, and it seems to address the problems I was having with the master branch. It does still call nrepl-current-ns in the nrepl-eldoc command, which should probably change. In fact, that function could probably just be replaced with clojure-find-ns. But I think we can close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants