nREPL support #72

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
@jarohen
Contributor

jarohen commented Apr 9, 2013

Hi James - as discussed, have added in nREPL support to the lein ring server etc commands.

Have updated the docs, and verified that the code is correct with:

(server-task (-> my-project
                 (assoc-in [:ring :start-repl?] true)
                 (assoc-in [:ring :repl-port] 7000))
             {})

where my-project was a project.clj map that I pulled from another project with leinjacker.

Let me know if this is the right place to put this functionality, or if there's any other changes you'd like me to make!

James

James Henderson
Added nREPL support
Can now start an nREPL server in the same process as the Ring server
by adding a [:start-repl? true] entry in the :ring map (and optionally
:repl-port)
@cch1

This comment has been minimized.

Show comment Hide comment
@cch1

cch1 Apr 9, 2013

Should lein-ring be coupled with nrepl? It's not hard to wire up yourself. Perhaps I'm missing something...

cch1 commented Apr 9, 2013

Should lein-ring be coupled with nrepl? It's not hard to wire up yourself. Perhaps I'm missing something...

@weavejester

This comment has been minimized.

Show comment Hide comment
@weavejester

weavejester Apr 10, 2013

Owner

nREPL can be wired up manually, for instance by adding it to the :init function. However, because there's only one init function, and because starting nREPL is typically a debugging task, it seems like it could be convenient to provide an option for lein-ring to start it in a way that doesn't contaminate the source code.

However, assuming we do add this, nREPL should only be added as a dependency if the option to use it is set in the project file. If the option is not set, then there should be no additional dependencies injected. The dependency on nREPL in the plugin project.clj file is not necessary, and should be removed.

I also think the config options should look something more like:

:ring {:nrepl {:start? true :port 7000}}
Owner

weavejester commented Apr 10, 2013

nREPL can be wired up manually, for instance by adding it to the :init function. However, because there's only one init function, and because starting nREPL is typically a debugging task, it seems like it could be convenient to provide an option for lein-ring to start it in a way that doesn't contaminate the source code.

However, assuming we do add this, nREPL should only be added as a dependency if the option to use it is set in the project file. If the option is not set, then there should be no additional dependencies injected. The dependency on nREPL in the plugin project.clj file is not necessary, and should be removed.

I also think the config options should look something more like:

:ring {:nrepl {:start? true :port 7000}}
James Henderson
A couple of changes as recommended by James:
Removed nREPL dependency from the lein plugin
Only add the nREPL dependency to the project if it requires it
Change the configuration to use a single map
@jarohen

This comment has been minimized.

Show comment Hide comment
@jarohen

jarohen Apr 10, 2013

Contributor

Thanks for the advice - have made the changes as suggested.

Contributor

jarohen commented Apr 10, 2013

Thanks for the advice - have made the changes as suggested.

weavejester added a commit that referenced this pull request Apr 22, 2013

@weavejester

This comment has been minimized.

Show comment Hide comment
@weavejester

weavejester Apr 22, 2013

Owner

Merged.

Owner

weavejester commented Apr 22, 2013

Merged.

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