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

Make jump-to-definition work in projects needing cider-path-translations #3414

Merged
merged 4 commits into from
Aug 20, 2023

Conversation

vemv
Copy link
Member

@vemv vemv commented Aug 16, 2023

Fixes #3413

The gist of it is:

  • during friendly session calculation, we translate paths in one direction; and
  • during jump-to-definition (i.e. the cider--file-path, as invoked from cider-find-file), we translate paths in the opposite direction.

Cheers - V

@vemv vemv requested a review from bbatsov August 16, 2023 06:03
@@ -0,0 +1,9 @@
FROM clojure:temurin-17-lein-bullseye
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This is a different Dockerfile from that of #3412)

@vemv
Copy link
Member Author

vemv commented Aug 18, 2023

@bbatsov rebased and green now

(when debug
(list file "was not determined to belong to classpath:" classpath "or classpath-roots:" classpath-roots)))))))

(defun cider-debug-sesman-friendly-session-p ()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this named as a predicate?

You might want to mention it somewhere in the troubleshooting/sesman docs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this named as a predicate?

Funny you ask, I intended it to read debug(sesman-friendly-session-p) i.e. the -p is for the thing being debugged.

Alternative welcome.

Good call on adding it to the manual!

@@ -286,22 +286,38 @@ in the container, the alist would be `((\"/src\" \"~/projects/foo/src\"))."
:group 'cider
:package-version '(cider . "0.23.0"))

(defun cider--translate-path (path direction)
"Attempt to translate the PATH in the given DIRECTION.
(defun cider--translate-path (path direction &optional return-all)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remind me why we can have multiple mappings for the same path? I never used TRAMP, so I don't remember very well how this was supposed to work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally we don't expect that to be the case, however I'm using the new cider--all-path-translations function (it has a comment justifying its existence) which gets the buffer-local cider-path-translations value from all buffers.

I imagine it's possible that some unrelated buffer's cider-path-translations can unexpectedly work for a given path.

So by specifying return-all, we get all possible matches - good and bad ones. Later, in the 'friendly session' computation, we compare them against the classpath. The bad ones won't matter - they won't be found in the classpath, so nothing is changed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems overly complicated to me, but given that I don't use or care about this functionality I'll assume you know what you're doing. :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish it could be less complicated. 😄

@vemv
Copy link
Member Author

vemv commented Aug 20, 2023

Thanks!

@vemv vemv merged commit 9c588af into master Aug 20, 2023
26 checks passed
@vemv vemv deleted the docker-2 branch August 20, 2023 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

M-. - possible Docker issue
2 participants