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

Namespace alias support for spec-list and spec-form #42

Merged
merged 4 commits into from Mar 21, 2019

Conversation

tatut
Copy link
Contributor

@tatut tatut commented Jan 25, 2019

Added support for resolving namespace aliased keywords for the current namespace.

Changes:

  • spec-list will return "::some-ns-alias/some-keyword" completions in addition to the fully qualified names for specs in namespaces that have an alias in the given ns.
  • spec-list will return "::some-keyword" completions in addition to the fully qualified names for specs in the given ns.
  • spec-form will expand both ns aliases before resolving the spec
  • added tests for ns alias functionality

I will add a separate PR for CIDER to add (cider-current-ns) for spec-list and spec-form requests in cider-client.el if this is accepted.

@bbatsov
Copy link
Member

bbatsov commented Mar 20, 2019

Sorry for the slow turnaround here! (I was away from a computer for quite a while) Can you rebase on master, so I can see the tests will pass with new CI?

The changes look good to me, but before merging this I wanted to ping @alexander-yakushev about this. It might make sense to have similar behavior for keyword in compliment for consistency's sake.

@alexander-yakushev
Copy link
Member

@bbatsov I honestly didn't quite get what is this all about (I don't use spec much, let alone in CIDER). Could you please describe what sort of behavior would you expect from Compliment regarding this?

@bbatsov
Copy link
Member

bbatsov commented Mar 20, 2019

I think the idea is provide completion candidates with non-expanded keywords as well as those with expanded ones, when doing something ::some-keyword and :some.ns/keyword, or ::ns-alias/keyword and :real.ns/keyword. Frankly, I rarely use namespaced keywords, so I'm not 100% certain how useful this would be, but it kind of makes sense to me.

@tatut
Copy link
Contributor Author

tatut commented Mar 20, 2019

Yes, the point is to be able to lookup keywords with ns aliases.

I use them alot, so that I will have a require like

(:require [mything.foo-domain :as foo])

And later use keywords like ::foo/bar. This will (with the upcoming CIDER) change allow you to lookup keyword under point and it will correctly understand that it means :mything.foo-domain/bar and get the correct spec for it.

@bbatsov
Copy link
Member

bbatsov commented Mar 20, 2019

Well, apart from my small remark about spec2, I'm ready to merge this.

@tatut
Copy link
Contributor Author

tatut commented Mar 20, 2019

I'll touch this up this evening and make sure the tests still pass.

@alexander-yakushev
Copy link
Member

alexander-yakushev commented Mar 20, 2019 via email

@tatut
Copy link
Contributor Author

tatut commented Mar 21, 2019

I think this should be ready for merge.

@bbatsov bbatsov merged commit 3caca64 into clojure-emacs:master Mar 21, 2019
@bbatsov
Copy link
Member

bbatsov commented Mar 21, 2019

Thanks for working on this!

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.

None yet

3 participants