Translate docker paths when cider-docker-translations alist is set

[dpsutton]

If you connect to a running docker image, often the source is mounted
in the docker image. This breaks navigation as rather than navigating
to "/home/me/projects/foo/src/ns.clj" it believes the file is
"/src/ns.clj". This allows rewriting of these prefixes so the
locations can map back to the local filesystem.

• The commits are consistent with our contribution guidelines
• You've added tests (if possible) to cover your change(s)
• All tests are passing (make test)
• All code passes the linter (make lint) which is based on elisp-lint and includes
  • byte-compilation, checkdoc, check-declare, packaging metadata, indentation, and trailing whitespace checks.
• You've updated the changelog (if adding/changing user-visible functionality)
• You've updated the user manual (if adding/changing user-visible functionality)

[dpsutton]

At work we have four projects that are run inside of docker container. The local source and m2 directories are mounted inside so navigation is broken in almost all files as the source directory is reported to be the mounted directories inside of the docker container.

Please let me know if i've poorly reinvented TRAMP as I don't know how to use it to connect to the docker container, but it seemed straight forward to swap the prefixes on filenames from docker to host so navigation works again. Evaluation and navigation are about 98% of my CIDER workflow so this works well for me.

[bbatsov]

I'll have to think a bit about this. I agree we need some solution, but I wonder if we can come up with something more generic.

[dpsutton]

I appreciate your thoughts @bbatsov . I actually have limited docker experience and this was born out of frustration at the new job. I can keep prototyping other things as well. I am not married to this implementation, this just helped me get back to working.

[bbatsov]

Understood. I've never used docker containers for development, so I can't really provide any insight (I've used them only for deployment and testing purposes). I do think that maybe we can have some list of functions for the filename translation, instead of hardcoding all sorts of possible behaviours, as we currently have been doing. Maybe we can just have some list where people can add more functions and they would be looped over until some match is found. Sounds like something reasonable to me.

[Malabarba]

If this function is going to be called translate-docker then I think it doesn't need the second argument, and it can just use the cider-docker-translations variable directly.

[dpsutton]

I agree and originally wrote it that way. The reason I changed is because the tests. Buttercup does not allow let bindings to span multiple expect forms it seems so i just made it an argument to aid in testing.

(let [cider-docker-translations ((...)))
  (expect ..
  (expect ..

[dpsutton]

or perhaps it forms. It would work when interactively evaluating the buttercup tests but make test would not work correctly.

[Malabarba]

:value-type directory (or was it dir?) might be a little more appropriate. :key-type probably should stay as string since it's not an actual directory in the user's system.

[Malabarba]

file-in-directory-p is probably what you're looking for.

[Malabarba]

do we know that translation and path are in the same format? For instance, one might be written as ~/project and the other as /home/project. If that's the case, expand-file-name might be more appropriate than file-name-as-directory.

[Malabarba]

Don't forget to regexp-quote :-)

[Malabarba]

I left a few comments on the elisp code, but now I wonder if can't make this feature a little more generic. Maybe there are other situations in which the server's code path is different from the code path that emacs is editing.

It probably wouldn't require any other code changes, just change the variable and function name from "docker translation" to "path translation".

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[bbatsov]

It probably wouldn't require any other code changes, just change the variable and function name from "docker translation" to "path translation".

My point exactly. @alexander-yakushev also reminded me today that SLIME has a very similar generic feature named slime-filename-translations. I thought we had this already, but it seems we don't.

[wandersoncferreira]

hi, only to ease the life of other people running around solutions to this problem, I just used the following code (compilation of this PR) to solve this problem at my local config.

(eval-after-load "cider"
  '(progn
     (defcustom cider-docker-translations nil
       "Translate docker endpoints to home dirs."
       :type '(alist :key-type string :value-type string)
       :group 'cider)

     (defun cider--translate-docker (path)
       "Attempt to translate the PATH.
Looks at `cider-docker-translations' for (docker . host) alist of path
prefixes.  TRANSLATIONS is an alist of docker prefix to host prefix."
       (hack-dir-local-variables)
       (seq-some (lambda (translation)
                   (when (string-prefix-p (car translation) path)
                     (replace-regexp-in-string (format "^%s" (file-name-as-directory (car translation)))
                                               (file-name-as-directory (cdr translation))
                                               path)))
                 cider-docker-translations))

     (defun cider--file-path (path)
       "Return PATH's local or tramp path using `cider-prefer-local-resources'.
If no local or remote file exists, return nil."
       (let* ((local-path (funcall cider-from-nrepl-filename-function path))
              (tramp-path (and local-path (cider--client-tramp-filename local-path)))
              (reverse-docker-path (cider--translate-docker local-path)))
         (cond ((equal local-path "") "")
               ((and reverse-docker-path (file-exists-p reverse-docker-path)) reverse-docker-path)
               ((and cider-prefer-local-resources (file-exists-p local-path))
                local-path)
               ((and tramp-path (file-exists-p tramp-path))
                tramp-path)
               ((and local-path (file-exists-p local-path))
                local-path))))))

Inside the folder of each project I have a .dir-locals.el file with the following content:

((nil (cider-docker-translations ("/app/src" . "/home/wand/platform/register/src"))))

Where /app/src is the volume mapped in my docker container and /home/wand/platform/register/src/ happens to be my pwd path inside the directory of the project at my machine.

It's not clear to me which points do we need to work on to proceed with this PR, could you help me on this?

[bbatsov]

@dpsutton What's the state of this PR? I see you're rebasing it from time to time, but I don't see changes to the original code.

[dpsutton]

@bbatsov no changes, i just rebase it onto master when i pull CIDER on my work machine. I just got back from strange loop and will finish this this week

[bbatsov]

@dpsutton Great! It always feels good to wrap a long running PR. :-)

[dpsutton]

test failing due to melpa issues but i'll kick them off again in a bit

  - Installing [ 3/11] spinner (1.7)... not available
  - Installing [ 4/11] queue (0.2)... not available

[bbatsov]

@dpsutton Seems you forgot to update the docs.

[bbatsov]

Just noticed you've added this to wrong manual. See the folder name. ;-) The real one is under doc.

[bbatsov]

You'll also have to rebase due to some conflicts in the changelog.

[bbatsov]

@dpsutton I plan to release the next CIDER on Monday. I hope we'll manage to squeeze this PR there.

I'm just summarize here was needs to be done:

• rebase on master (this should fix the broken build)
• Move the documentation to the new AsciiDoc manual

[dpsutton]

Ok i think docs and rebasing should be fixed. I'm getting several failed tests relating to the figwheel-main init form unfortunately. I'll see what i can get done.

[bbatsov]

It seems the other failure is unrelated, although I'm curious where this breakage originated. Likely something to do with #2698.