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

Question: Can the cmdline.-run functionality be broken up into smaller functions for reuse? #108

Closed
eccentric-j opened this issue Dec 18, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@eccentric-j
Copy link
Contributor

commented Dec 18, 2018

Problem

We're going to be creating a few sibling Clojure projects for work. I'm setting up a small library we can reuse that provides a suped-up-repl experience combining nREPL, REBL, Rebel-Readline, and some middleware I am confident we'll always need\want to be productive.

To achieve some of the autonomy desired, I have to implement something like the cmdline file to setup the nREPL server and automatically start other services.

The problem is that that -run function looks particular meaty to me. It parses arguments, starts the repl server, prints help info, prints the start message, writes the port file, and a few other beneficial behaviors that make nREPL a dream to use that can't be reused.

https://github.com/nrepl/nrepl/blob/0.5.3/src/clojure/nrepl/cmdline.clj#L240

Solution

Could we split up that behavior into several functions? This way others can create more advanced REPLs while incorporating the features we love from stock nREPL.

I'm happy to submit a PR to split the behaviors up myself but to summarize what I'm thinking:

  • Create a function for parsing the arguments
  • Create a function for directing the parsed arguments into specific behaviors
  • Create a function for starting the server (already exists)
  • Create a function for printing the parseable header
  • Create a function for printing the current Clojure version and current user info

Value

  • Allow users to develop richer REPL experiences using nREPL's cmdline functions as base
  • More beginner friendly options can be more readily available

Alternatives

  • Using snippets from cmdline I can get the functionality I want, but I am duplicating some code
  • Interested in recommended alternatives if a better means to accomplish this exists

Notes

I'm by no means a Clojure expert so there may be very good reasons for the cmdline.-run function to be as meaty as it is be it performance or other reasons unknown to me at this time. From my perspective however, I think we can make that function a bit more idiomatic with a bit of time to get a lot more from it.

I am interested in your thoughts on this as I want to know if creating a PR for this is worth your time.

@eccentric-j eccentric-j changed the title Can the cmdline.-run functionality be broken up into smaller functions for reuse? Question: Can the cmdline.-run functionality be broken up into smaller functions for reuse? Dec 18, 2018

@bbatsov

This comment has been minimized.

Copy link
Contributor

commented Dec 22, 2018

Yeah, certainly we can do this. The main reason the code's monolithic right now is that we didn't really foresee someone willing to reuse it. That being said - we have to be careful with a public API, as once you commit to some API it's tricky to change it afterwards.

Create a function for parsing the arguments

I think we already have this one.

Create a function for starting the server

And this one.

@eccentric-j

This comment has been minimized.

Copy link
Contributor Author

commented Dec 22, 2018

That's a good point, the API should be thoughtfully done so it can stay consistent as time goes on. What would you recommend to facilitate that? My thought is just to draft it in the PR then discuss and refine it until it's in a place you're on board with.

You're right about those functions already existing, I may have conflated the idea of arg parsing with logic to test args and select defaults such as the logic around this line https://github.com/nrepl/nrepl/blob/0.5.3/src/clojure/nrepl/cmdline.clj#L264. What would you call that kind of logic? Arg resolution?

Anyway, I'm going to get started to get more insight if more discussion is desired up front for the API functions.

@bbatsov

This comment has been minimized.

Copy link
Contributor

commented Dec 22, 2018

Yeah, I think that a discussion around a PR would be a great starting point.

eccentric-j added a commit to eccentric-j/nrepl that referenced this issue Dec 30, 2018

eccentric-j added a commit to eccentric-j/nrepl that referenced this issue Jan 6, 2019

eccentric-j added a commit to eccentric-j/nrepl that referenced this issue Jan 6, 2019

@eccentric-j

This comment has been minimized.

Copy link
Contributor Author

commented Jan 6, 2019

First cut is drafted here #116 please direct this conversation as you see fit 🤘

eccentric-j added a commit to eccentric-j/nrepl that referenced this issue Jan 30, 2019

eccentric-j added a commit to eccentric-j/nrepl that referenced this issue Jan 30, 2019

@bbatsov bbatsov closed this in 176bde8 Feb 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.