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

re #462 - jump-to-def via 'info' op #465

Merged
merged 2 commits into from
Feb 7, 2014
Merged

Conversation

gtrak
Copy link
Contributor

@gtrak gtrak commented Jan 31, 2014

I couldn't make an async impl work via cider-send-op, but sync seems to work.

tested on clj code. Addresses #462

@@ -489,8 +489,7 @@ Uses `find-file'."
(lambda (buffer err) (message err))
nil))

(defun cider-jump-to-def (var)
"Jump to the definition of the VAR at point."
(defun cider-jump-to-def-eval-fn (var)
Copy link
Member

Choose a reason for hiding this comment

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

Seems you removed the method docstring here.

@gtrak
Copy link
Contributor Author

gtrak commented Jan 31, 2014

Just a note, my emacs seems to have destabilized in some way from these changes.

I have no idea what I'm doing in elisp :-).

But, in general, seems like if there's an exception on the jvm side, then emacs gets hosed.

@@ -530,6 +529,28 @@ Uses `find-file'."
(cider-jump-to-def-handler (current-buffer))
nrepl-buffer-ns)))

(defun cider-jump-to-def-op-fn (var)
Copy link
Member

Choose a reason for hiding this comment

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

We should probably denote this function as private by naming it cider--jump.... Same goes for the other one as well.

@bbatsov
Copy link
Member

bbatsov commented Jan 31, 2014

Might be some problem in the new middleware? Btw, is commit messages when you want to refer to an issue the convention is [#NNN] ... (or [Fix #NNN] ....).

@bbatsov
Copy link
Member

bbatsov commented Feb 5, 2014

@gtrak Please, address my comments and mention this in the changelog so we can have this merged.

(let* ((val (plist-get (nrepl-send-request-sync
(list "op" "info"
"session" (nrepl-current-session)
"ns" nrepl-buffer-ns
Copy link
Member

Choose a reason for hiding this comment

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

Use (cider-current-ns) instead of nrepl-buffer-ns.

@gtrak
Copy link
Contributor Author

gtrak commented Feb 7, 2014

I fixed the issues I was having, seems to be working sanely now.

bbatsov added a commit that referenced this pull request Feb 7, 2014
re #462 - jump-to-def via 'info' op
@bbatsov bbatsov merged commit 56dc504 into clojure-emacs:master Feb 7, 2014
@bbatsov
Copy link
Member

bbatsov commented Feb 7, 2014

👍

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.

2 participants