clojure-emacs / cider-nrepl

A collection of nREPL middleware to enhance Clojure editors with common functionality like definition lookup, code completion, etc.
https://docs.cider.mx/cider-nrepl
685 stars 176 forks source link

Remove inlining of cider/orchard dep? #662

Closed yuhan0 closed 4 years ago

yuhan0 commented 4 years ago

The Orchard library is closely linked to cider-nrepl and is unlikely to collide with the project deps.

Not inlining it make it easier for users of cider-nrepl to interactively extend some features in Orchard, eg. overriding or adding new multimethods or protocol implementations.

SevereOverfl0w commented 4 years ago

I use orchard in my projects. It will collide with me there. I don't want to field bug reports from conflicts from tooling. It's just a total pain.

yuhan0 commented 4 years ago

Here was my motivation for asking in the first place:

I regularly deal with multi-line strings in large nested data structures, which I use the Cider inspector to explore.

However, the default implementation in orchard.inspect uses pr-str, displaying the entire string as a single line with \n, \" control characters. For my purposes a multi line representation is much more readable.

Orchard uses multimethods for exactly this purpose, allowing me to override the behaviour with:

(defmethod orchard.inspect/inspect :string 
   ...)

But because of cider-nrepl's inlining, the namespace is mangled and one has to refer to the (non-obvious) location:

(require '[cider.nrepl.inlined-deps.orchard.v0v5v5.orchard.inspect :as orchard.inspect])

And recently cider-nrepl bumped its dependencies to orchard 0.5.6, breaking the above code because the inlined ns is coupled to the version number.

I recognize that some users have dependencies on a specific version of Orchard that might cause conflict, but I think the above use-case is also valid. Another example could be extending the inspect-value multimethod for a user-defined type to show salient information up front.

Ideally a project should be able to apply these extensions in its :dev profile without needing an exact-version-number requirement on Cider and its dependencies, which may be injected at jack-in.

Maybe there are other workarounds that could be considered (inline at a specific non-versioned namespace?)

expez commented 4 years ago

Maybe there are other workarounds that could be considered (inline at a specific non-versioned namespace?)

Instead of having a declarative import like that, you can query the runtime for all the namespaces, find the ns you're looking for, using e.g. a substring match without the version number, and then require that ns with the desired alias.

yuhan0 commented 4 years ago

@expez I suppose that's an reasonable solution if a little bit of a hack:

(doseq [ns (all-ns)]
  (when (re-find #"cider\.nrepl\.inlined-deps.*orchard\.inspect" (str ns))
    (require [(symbol (str ns)) :as 'inspect])))

Hopefully this will be of help to others - will close the issue now, thanks :)

bbatsov commented 4 years ago

Might be a good idea to document this somewhere.