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

Allow nrepl clients to respect the initial namespace #955

Closed
mtopolnik opened this issue Jan 24, 2013 · 12 comments
Closed

Allow nrepl clients to respect the initial namespace #955

mtopolnik opened this issue Jan 24, 2013 · 12 comments
Milestone

Comments

@mtopolnik
Copy link
Collaborator

Currently only the built-in REPLy respects the initial namespace as set in project.clj (either with :init-ns or :main). My proposition is to make the repl task set the root binding of *ns* to the initial namespace, thereby allowing any client to evaluate (set! *ns* (.getRawRoot (var *ns*))) upon initialization.

This approach is not 100% ideal because it requires specific action from the client, but the current design of nrepl would make it very hard to have the client observe the initial namespace automatically. The nrepl server maintains a session for each connection, which contains all the dynamic bindings in effect, and is lazily initialized while handling the first message from the client. Only after that point would it be possible to influence the session's *ns* binding.

Furthermore, REPLy assumes user namespace while performing default initialization. At least with the current design, the nrepl server is not allowed to set the current namespace on its own.

All these things considered, the approach with the root *ns* binding seems like a solid compromise, generating the least friction against the current state of the design.

@technomancy
Copy link
Owner

Pinging @cemerick since he has probably thought about this issue.

@mtopolnik
Copy link
Collaborator Author

An issue with changing the root *ns* binding is that it can interfere with the behavior of some code. This could be quite serious. Maybe a separate var should be introduced.

@cemerick
Copy link
Collaborator

Why is (set! *ns* (.getRawRoot (var *ns*))) simpler/easier/better than (in-ns 'foo)? Perhaps I'm not understanding the use case, but this seems like an odd way to propagate a single project.clj setting (:init-ns).

I vaguely recall suggesting previously that the project map should be pprinted to a stable location, so runtime code could easily access it; a plugin that does this (and more) is configleaf, though I've not used it.

@mtopolnik
Copy link
Collaborator Author

The precedent to specifically support :init and :init-ns in the repl task has already happened; my proposal was to improve that support. If I understand correctly, your suggestion is to rework the current approach by providing a standard way to retrieve the whole project map from nrepl and then implement support for :init and :init-ns on top of that.

How about adding such a middleware that implements a command to retrieve the project map?

To my eyes, the cleanest approach would be to have the already existing session store that, but since sessions are lazily initialized, I haven't found a way to push the project map into it.

If we can agree on an approach, I can put effort into implementing that.

@cemerick
Copy link
Collaborator

I don't think :init-ns (and definitely not :init) was ever intended to carry over to secondary REPLs. I'm not suggesting anything re: the implementation of those options at the moment — again, I'm certain I don't grok the motivation for the use case.

Middleware isn't needed to access the project map; configleaf appears to make it readily available in a pre-defined namespace.

@mtopolnik
Copy link
Collaborator Author

The use case which :init-ns and :init look to support is the same irrespective of the repl client used. Let me describe how every Clojure development session starts in Emacs:

  1. $ lein repl
  2. M-x nrepl
  3. C-x C-f ...find main .clj file...
  4. C-c C-k (compile and load file)
  5. C-c M-n (transfer ns to the nrepl buffer)

With :init-ns respected by nrepl.el the steps 4 and 5 would be done automatically and step 3 could also be automated with a bit more elisp. That is a huge boost to convenience.

Now, if this was a minority concern, useful to only some developers, maybe requiring a separate plugin to make it work wouldn't be a bad idea, but I think we can agree that this is not the case here. Another point is that the standard leiningen features, which :init-ns and :init are, shouldn't require an extra plugin to work properly.

I am also not quite sure that configleaf's intrusive approach of adding a file into a source code directory is the best way to support this. configleaf's goal are persistent profiles, visible even in production code; this is not our goal.

@mtopolnik
Copy link
Collaborator Author

As far as intentions are concerned, these facts should be relevant:

  1. as of leiningen 2.0.0 all repl clients observe the effects of the :init form having been evaluated (the most recent fix to make this happen was Issue add :repl-options :init into headless startup #818);
  2. in the discussion on Google Groups @technomancy acknowledged that he would like to support :init-ns for all repl clients as well.

@hugoduncan
Copy link
Contributor

nREPL servers could be run in a variety of contexts. For example, a web project may be run in development locally and in production in a container. Does the same initialisation (even for a single project) make sense in every one of these contexts?

Not all nREPL servers will have been started by leiningen, so any client that assumes lein specific initialisation has occured is likely to break.

Not all nREPL servers will be running projects built by lein.

I think this leads to the conclusion that any initialisation should be explicitly initiated by the client, and should be executed by a middleware in the server.

Maybe a discussion on the clojure tooling group would be of use?

@mtopolnik
Copy link
Collaborator Author

Started a topic on the tooling group.

trptcolin added a commit to trptcolin/reply that referenced this issue Feb 2, 2013
trptcolin added a commit that referenced this issue Feb 15, 2013
@technomancy
Copy link
Owner

Sorry, I kind of lost track of the status of this. Do we have something that's working now? If not, could we get it in by ClojureWest (the 17th)?

@mtopolnik
Copy link
Collaborator Author

The Pull request #963 takes care of this in lein repl and @trptcolin has updated REPLy to work with it. I posted on nrepl.el's Google Group regarding support for the initial namespace, but there has been no feedback.

@technomancy
Copy link
Owner

Thanks. I'll go ahead and close this if we're done on the Leiningen side. Unfortunately nrepl.el seems a bit short-handed; there's a huge backlog of open issues there, so development proceeds a little more slowly.

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

No branches or pull requests

4 participants