-
Notifications
You must be signed in to change notification settings - Fork 38
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
Infer the class of string and coll literals #107
Infer the class of string and coll literals #107
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! Sure, it's no problem since the addition is minimal.
which indicates that the members are an exact match against the class of `[]`" | ||
(let [c (count (src/members-candidates "." (-ns) (ctx/cache-context | ||
"(__prefix__ [])")))] | ||
(< 17 c 21)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose testing for something exact. Let's say, check that when completing .in
, only .indexOf
is suggested when context is present, and more than one candidate are suggested without context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I finally did a bit of both, LMK what you think
expected (if (and (= 1 major) | ||
(< minor 12)) | ||
18 | ||
19)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, you are overdoing it. Your changes to Compliment don't depend on the exact public methods of PersistentVector or other classes, they transcend the exact versions of Clojure and Java. Just make a minimal test that proves that context performs the filtering, and that's it. There are plenty of tests like that already, take any as an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, I believe!
Perfection. |
Cheers. A release would make it into cider/cider-nrepl which are scheduled for a shiny big release next week. I've already tested this feature locally during yesterday. |
Pushed |
🍻! It's on its way now. Btw, I'll be excited to work on clojure-emacs/cider-nrepl#812 someday soon - I find it fairly important for making our other Java-related work shine. |
Fixes #106
Hopefully the inclusion of colls won't be a nuisance - as you can see, luckily it turned out to be a minimal addition.
Most pragmatically, it would be used in clojure-emacs/cider-nrepl#812 for instance.
Cheers - V