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

cider-find-var: jump to namespace-qualified keywords #2112

Closed
vemv opened this issue Nov 12, 2017 · 13 comments
Closed

cider-find-var: jump to namespace-qualified keywords #2112

vemv opened this issue Nov 12, 2017 · 13 comments

Comments

@vemv
Copy link
Member

vemv commented Nov 12, 2017

Hi there!

I have a feature request, hopefully a reasonable one.

When using cider-find-var over a ns-qualified keyword such as e.g. ::other.namespace/foo, CIDER won't jump to anything.

This is partly justified, as keywords are literal values, and as such don't belong anywhere in particular.

OTOH, it'd be intuitively useful that it jumps to the first occurrence of ::foo in other.namespace.

Maybe other.namespace has more occurrences of ::foo but we pick the first one because that's the best-effort thing to do.

If other.namespace had no occurrences of ::foo, we'd still jump to that namespace, it's very probably what the user wants. The user could have a look in the namespace and check himself that there are no occurrences of ::foo - that's still a useful activity when programming.

Cheers - Victor

@oliyh
Copy link

oliyh commented Nov 20, 2017

Hi,

I wanted this too, specifically for re-frame registrations. My solution (which is re-frame specific) is here: https://github.com/oliyh/re-jump.el

It lets you jump from ::model/foo to something that looks like (re-frame/reg-sub ::foo) in the model ns. It works very well for my use case!

@vemv
Copy link
Member Author

vemv commented Nov 20, 2017

Nice!

I took a look - is it actually re-frame specific? The code looks clean although there might be an implicit assumption I fail to see.

@oliyh
Copy link

oliyh commented Nov 20, 2017

Hi @vemv,

The one assumption is on this line: https://github.com/oliyh/re-jump.el/blob/master/re-jump.el#L22

It's looking for something that starts with "reg-" which is the pattern that re-frame uses. I found I needed this because the first instance in a file wouldn't necessarily be the registration, but that's the only bit you need to change if you want a much more generic case.

@bbatsov
Copy link
Member

bbatsov commented Dec 9, 2017

I guessing this can be done relatively easy:

  • we can check if the thing at point is a namespaced symbol
  • extract the namespace
  • do find ns for the namespace
  • search forward in its buffer for the first mention of the symbol

@bbatsov
Copy link
Member

bbatsov commented Dec 9, 2017

I see @oliyh did something along those lines but for some reason decided not to use cider-find-ns, which would simplify things a lot IMO.

@oliyh
Copy link

oliyh commented Dec 9, 2017

Thanks for the tip @bbatsov :)

The problem with the first mention is that it may still be a usage rather than a declaration. re-jump searches for the registration. I could do the same for (def ::xyz) if the search for reg failed and then it should handle this use case too.

@bbatsov
Copy link
Member

bbatsov commented Dec 10, 2017

Of you can do it configurable in the spirit of Emacs and people would be able to just set different values for different projects. :-)

Anyways, feel free to submit this to CIDER if you want.

@vspinu
Copy link
Contributor

vspinu commented Dec 10, 2017

I wanted this too, specifically for re-frame registrations.

My solution for such case is to add custom regexp to imenu and then use imenu-anywhere to jump around.

Specifically, for re-frame it's:

(add-to-list 'imenu-generic-expression
               '("re-frame" "(register-\\(handler\\|sub\\)[ \n]+\\([^\t \n]+\\)" 2)
               t)

@bbatsov
Copy link
Member

bbatsov commented Dec 16, 2017

That's a very cool idea!

@bbatsov
Copy link
Member

bbatsov commented Dec 19, 2017

I've added something similar to @oliyh's command. @vemv Feel free to play with it.

@vemv
Copy link
Member Author

vemv commented Dec 30, 2017

Tried out!

Unfortunately the functionality won't work for me: it will print Can't find namespace xxx.

The culprit seems to be this call: (cider-resolve-alias (cider-current-ns) ns-qualifier)

If you see its implementation:

(defun cider-resolve-alias (ns alias)
  "Return the namespace that ALIAS refers to in namespace NS.
If it doesn't point anywhere, returns ALIAS."
  (or (cider-resolve--get-in ns "aliases" alias)
      alias))

You'll see that it does a somewhat dubious or, so the fn returns either a useful result, or a result that will cause cider-find-keyword to fail.

So the root cause (at least in my case) is that cider-resolve-alias is not working. In any given ns with an active conection, (cider-resolve-alias (cider-current-ns) "set") will return "set" rather than "clojure.set".

Using clojurescript. CIDER works great for me (no setup issues) so IDK why cider-resolve-alias isn't working.

@vemv
Copy link
Member Author

vemv commented Dec 30, 2017

I just checked, and this functionality and cider-resolve-alias both work well in clojure, but not in cljs for me

@bbatsov
Copy link
Member

bbatsov commented Dec 30, 2017

Maybe something is broken with the ns caching on ClojureScript. Run this with the Elisp debugger and see what's going on exactly there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants