Support for custom `cider-ns-refresh-fn`

[filipesilva]

I'd like to use https://tonsky.me/blog/clj-reload/ instead of clojure.tools.namespace to refresh my code. The main thing I like it in is how defonce just works, and I don't have to fiddle around with clojure.tools.namespace in my source code.

I see there's cider-ns-refresh-before-fn and cider-ns-refresh-after-fn in the docs, but there's no configurable cider-ns-refresh-fn at least in those docs. So that seemed like a nice hook point.

I'd be interested in making a PR for this approach, or some other approach you feel is better. Thanks for all the good work!

[vemv]

Thanks!

Giving it some thought, I wouldn't want to add conditionals to https://github.com/clojure-emacs/cider-nrepl/blob/9aea519b87430c3a26cd146417dd84c33bb14676/src/cider/nrepl/middleware/refresh.clj .

It's also, unfortunately, not as simple as adding a -fn opt since we also have to take care of dep management.

So I'd suggest that you implement a similar namespace based on the new lib.

What users would customize would be the nREPL ops.

Currently we accept:

• "refresh"
• "refresh-all"
• "refresh-clear"

So users in cider.el would instead specify:

• "clj-reload/refresh"
• "clj-reload/refresh-all"
• "clj-reload/refresh-clear"

...that's a small change that would make a nice small PR into cider.el.

You could then have the custom middleware in your classpath for a while (as small bugs are reasonable to expect in the integration or clj-reload).

After a couple weeks, we could make it official and integrate it here.

SGTY?

Cheers - V

[bbatsov]

"clj-reload/refresh"

I'm not sure if that's prudent, as there's also #710 and I was thinking of a prefix like cider/... for all the ops. I'm guessing the ops can be named differently (e.g. clj-reload-whatever), but in general I agree this should just be a separate (but similar middleware). We can figure this out once there's some proposed code to review.

[vemv]

I'm not sure if that's prudent, as there's also https://github.com/clojure-emacs/cider-nrepl/issues/710 and I was thinking of a prefix like cider/... for all the ops.

Yes good call, cider.clj-reload/refresh would be better naming as it's less global.

[bbatsov]

That'd be fine by me as well. Depends on what we agree to do for the global package prefix, as I think we already have ops namespaced with cider/.

[filipesilva]

I'm working on this and hope to have a PR we can look at soon.

A questions please: Is there a way already in cider-nrepl codebase to get the src-dirs that clojure is using?

[vemv]

That's great to hear!

We use https://github.com/clojure-emacs/cider-nrepl/blob/9aea519b87430c3a26cd146417dd84c33bb14676/src/cider/nrepl/middleware/refresh.clj#L22 since users are expected to set that dir explicitly via clojure.tools.namespace.repl/set-refresh-dirs (there are tools.namespace users that don't, but then again they easily get into trouble so it's not a supported workflow)

[filipesilva]

So is the expectation for tools.namespace that users set those dirs in their project directly?

I was testing cld-reload and it requires the dirs to be set in advance, otherwise reload won't work.

(require '[clj-reload.core :as reload])

(reload/init
  {:dirs ["src" "dev" "test"]})

;; fails if reload/init wasn't called yet
(reload/reload)

[vemv]

So is the expectation for tools.namespace that users set those dirs in their project directly?

Yes, there's normally a set-refresh-dirs call in user.clj or dev.clj

The pattern seems similar in clj-reload

[bbatsov]

I'm guessing those can be inferred on CIDER's end. I have to check the code for c.t.n., but I don't see in the user docs we wrote anything about a manual setup needed by the users https://docs.cider.mx/cider/usage/misc_features.html#reloading-code

[vemv]

Maybe we should! I've saved the item

(without explicit refresh dirs, it/we can try to load undesired code e.g. stuff under resources/)

[bbatsov]

Yeah, maybe the docs need some improvement. I haven't used this functionality in a while, so my memory there is a bit fuzzy.

[filipesilva]

I was actually surprised about the source mentioning the dirs needed to be set for tools.namespace. I've been using the refresh functionality for a while in a couple of different projects and I've never set the dirs.

Which I guess leads me to the next point: this repo carries its own tools.namespace dependency. I understand mranderson takes cares of scoped dependencies.

But it also means that the refresh functionality work without the user installing tools.namespace. A CIDER user right now is able to use refresh without any change to their project.

But since clj-reload need the init, if cider-nrepl does not carry its own clj-reload dep, and does not infer the src dirs necessary for init, then the clj-reload functionality will not work without the user changing their project.

[vemv]

then the clj-reload functionality will not work without the user changing their project.

I wouldn't characterize it as changing it as much as "setting it up".

It's not different in that regard from tools.namespace. I understand that it can work without the setup, but it's something I know to be problematic (I can say this as a long-time t.n user)

Btw, we'd be ok with shipping a mranderson-ized clj-refresh. It states to be a dep-free lib.

[filipesilva]

Proposed a change to cli-reload that would mimic the tools.namespace behaviour: https://github.com/tonsky/clj-reload/pull/4. Can be done in cider-nrepl instead, but I thought maybe it'd be desirable overall.

[filipesilva]

Opened #850 as a draft implementation just so we can start talking about it and sort out the names.