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

Support clj-reload as well as tools.namespace reload since it's now built into nREPL #574

Open
Olical opened this issue May 6, 2024 · 0 comments
Labels
client-clojure enhancement New feature or request

Comments

@Olical
Copy link
Owner

Olical commented May 6, 2024

As requested by @dharrigan, we should support both methods. We should add an enum option that lets you swap to the new method easily. It should use the same mappings to control the other system.

https://github.com/tonsky/clj-reload

Original Discord message:

dharrigan — Yesterday at 09:16
Looking around the source, I seem to see that when doing rr, it invokes the nrepl command refresh, which ultimately invokes the clojure.tools.namespace.reload functionality (i.e.,

->
(defn- refresh-impl [op]
-> https://github.com/clojure-emacs/cider-nrepl/blob/b7aace8ad3f83872c0f1395ef2439a8572e13e3b/src/cider/nrepl/middleware/refresh.clj#L118 -> https://github.com/clojure-emacs/cider-nrepl/blob/b7aace8ad3f83872c0f1395ef2439a8572e13e3b/src/cider/nrepl/middleware/refresh.clj#L95 finally arriving to here https://github.com/clojure/tools.namespace/blob/05328ed700840165bcf1c5d34d5ef2e299b58c1a/src/main/clojure/clojure/tools/namespace/reload.clj#L43
There's another way which uses the https://github.com/tonsky/clj-reload library which has been added to nrepl which has the op of cider.clj-reload/reload-all (and a few others, seen here: https://github.com/clojure-emacs/cider-nrepl/blob/b7aace8ad3f83872c0f1395ef2439a8572e13e3b/src/cider/nrepl/middleware/reload.clj#L66). This library does a reload of all the namespaces, according to it's own words: Smarter way to reload Clojure code. Clj-Reload tracks namespace dependencies, unloads namespaces, and then loads them in the correct topological order.
My question here is, would it be nice to support this nrepl op in Conjure for those wishing to use the clj-reload functionality? Would this require a new mapping key? Or perhaps a way to change the behaviour of rr so that it can either use the refresh op or the cider.clj-reload/reload-all op depending upon a conjure setting.
Is this idea even something that might be useful, or not that useful? I'm simply pondering over the source code atm and having some thoughts.
(the support of clj-reload in nrepl came in nrepl 0.46.0, released on 2024-03-05, so it is a new feature of nrepl from when conjure first supported nrepl)

@Olical Olical added enhancement New feature or request client-clojure labels May 6, 2024
dharrigan added a commit to dharrigan/conjure that referenced this issue May 6, 2024
Since version 0.46.0, nREPL has supported the `clj-reload` library in
addition to the standard `tools.namespace` library for facilitating
reloading namespaces.

This commit introduces a new Clojure nREPL variable to allow switching
to the new `clj-reload` library.

The variable is:

`g:conjure#client#clojure#nrepl#refresh#backend`

The default is to continue to use `tools.namespace`, but if it is set to
`clj-reload`, i.e.,

`g:conjure#client#clojure#nrepl#refresh#backend = 'clj-reload`

It will then use the new backend.

fixes Olical#574

-=david=-
dharrigan added a commit to dharrigan/conjure that referenced this issue May 6, 2024
Since version 0.46.0, nREPL has supported the `clj-reload` library in
addition to the standard `tools.namespace` library for facilitating
reloading namespaces.

This commit introduces a new Clojure nREPL variable to allow switching
to the new `clj-reload` library.

The variable is:

`g:conjure#client#clojure#nrepl#refresh#backend`

The default is to continue to use `tools.namespace`, but if it is set to
`clj-reload`, i.e.,

`g:conjure#client#clojure#nrepl#refresh#backend = 'clj-reload'`

It will then use the new backend.

fixes Olical#574

-=david=-
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-clojure enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant