Translate paths from CIDER to nREPL and vice-versa #2897
Conversation
|
The changes look good to me overall, but if this is going to be public API used by other packages the functions shouldn't have |
|
Hi @bbatsov I didn't intend any of the new functions to be public. I saw them as just internal implementation details for Regarding the documentation expansion, do you mean the functions documentation or other kind of documentation? |
cider-common.el
Outdated
| Looks at `cider-path-translations' for (container . host) alist of path | ||
| prefixes and translates PATH from container to host or viceversa depending on | ||
| whether DIRECTION is :from-nrepl or :to-nrepl" | ||
| (seq-let [from-fn to-fn path-fn] (cond ((eq direction :from-nrepl) '(car cdr identity)) |
There was a problem hiding this comment.
In Emacs Lisp we normally use symbols instead of keywords - e.g. 'from-nrepl.
cider-common.el
Outdated
| "Attempt to translate the PATH in the given DIRECTION. | ||
| Looks at `cider-path-translations' for (container . host) alist of path | ||
| prefixes and translates PATH from container to host or viceversa depending on | ||
| whether DIRECTION is :from-nrepl or :to-nrepl" |
|
My bad. I misread the implementation. |
When `cider-path-translations` is in use (e.g., when CIDER is running on the host, but the nREPL is running in a virtual machine or a container) we need to translate from the host "CIDER-based" paths to the container "nREPL-based" paths and vice-versa, depending on the operation. Translations from "nREPL-based" paths to "CIDER-based" paths were already implemented, but not the other way around. Some operations in clj-refactor need the "CIDER-based"to "nREPL-based" path translations to work correctly (e.g., `cljr-rename-file-or-dir`, `cljr-rename-symbol`, etc.). This patch implements the missing path translations, and is a pre-requisite for the complementary clj-refactor patch that is proposed separately.
|
Thanks for tackling this! |
When
cider-path-translationsis in use (e.g., when CIDER is runningon the host, but the nREPL is running in a virtual machine or a
container) we need to translate from the host "CIDER-based" paths to
the container "nREPL-based" paths and vice-versa, depending on the
operation.
Translations from "nREPL-based" paths to "CIDER-based" paths were
already implemented, but not the other way around. Some operations in
clj-refactor need the "CIDER-based"to "nREPL-based" path translations
to work correctly (e.g.,
cljr-rename-file-or-dir,cljr-rename-symbol, etc.).This patch implements the missing path translations, and is a
pre-requisite for the complementary clj-refactor patch that is
proposed separately.
Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):
eldev test)eldev lint) which is based onelisp-lintand includescheckdoc, check-declare, packaging metadata, indentation, and trailing whitespace checks.Thanks!
If you're just starting out to hack on CIDER you might find this section of its
manual extremely useful.