The Wayback Machine - https://web.archive.org/web/20201130034342/https://github.com/nrepl/nrepl/pull/202
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

Ensure distinct static member completion candidates #201 #202

Merged
merged 3 commits into from Jun 25, 2020

Conversation

@eerohele
Copy link
Contributor

@eerohele eerohele commented Jun 22, 2020

Static methods such as String/valueOf have many overloads. Prior to this commit, the static-members function returned one completion candidate for each overload.

Static methods such as String/valueOf have many overloads. Prior to this
commit, the static-members function returned one completion candidate
for each overload.
@bbatsov
Copy link
Contributor

@bbatsov bbatsov commented Jun 22, 2020

I'm guessing there might be a similar problem for instance methods as well.

@arichiardi
Copy link
Contributor

@arichiardi arichiardi commented Jun 22, 2020

Hey folks, I am checking exactly that part in orchard now and a lot of work went into polishing that.

I know this implementation is different but probably it is worth having a look there 😁

@bbatsov
Copy link
Contributor

@bbatsov bbatsov commented Jun 22, 2020

@arichiardi The code completion is actually in compliment and it's way more sophisticated than anything I'd like to bundle with nREPL. I didn't want to duplicate a great library, so instead I've opted for the simplest thing that adds some value. I've started with clojure-complete and I've cleaned it up and improved a bit.

@bbatsov
Copy link
Contributor

@bbatsov bbatsov commented Jun 23, 2020

I'm guessing there might be a similar problem for instance methods as well.

Forget about this. I saw your comment on #201 just now.

(for [member (concat (.getMethods class) (.getDeclaredFields class)) :when (static? member)]
(.getName ^Member member)))
(distinct
(for [member (concat (.getMethods class) (.getDeclaredFields class)) :when (static? member)]

This comment has been minimized.

@bbatsov

bbatsov Jun 23, 2020
Contributor

Probably it's best to convert this to some ->> pipeline. Seems the author of the original code was quite fond of for, but I'm definitely more fond of map, filter and friends. :-)

This comment has been minimized.

@eerohele

eerohele Jun 24, 2020
Author Contributor

Yeah, I very rarely use for, too. I can certainly rewrite the function with ->>. Would you prefer that I do it in a separate commit?

@@ -45,6 +45,7 @@
(candidates "java.lang.System/out")))

(is (some #{"String/valueOf"} (candidates "String/")))
(is (every? #(= 1 %) (vals (frequencies (candidates "String/")))))

This comment has been minimized.

@bbatsov

bbatsov Jun 23, 2020
Contributor

Might be a good idea to add some helper named unique-candidates? and use it in the other completion tests as well.

This comment has been minimized.

@eerohele

eerohele Jun 24, 2020
Author Contributor

I can do that. I realized that I could just do (apply distinct? (candidates "String/")) instead, though.

@eerohele
Copy link
Contributor Author

@eerohele eerohele commented Jun 24, 2020

I pushed the changes you proposed as separate commits. I can squash them prior to the possible merge if necessary.

(Fixed typos in commit messages afterwards.)

eerohele added a commit to eerohele/nrepl that referenced this pull request Jun 24, 2020
eerohele added a commit to eerohele/nrepl that referenced this pull request Jun 24, 2020
@eerohele eerohele force-pushed the eerohele:patch/completions-distinct branch from 7f221ec to 6725957 Jun 24, 2020
@eerohele eerohele force-pushed the eerohele:patch/completions-distinct branch from 6725957 to 375dd8c Jun 24, 2020
@bbatsov bbatsov merged commit 44dd210 into nrepl:master Jun 25, 2020
17 checks passed
17 checks passed
ci/circleci: Checking Cljdoc Your tests passed on CircleCI!
Details
ci/circleci: Code Linting Your tests passed on CircleCI!
Details
ci/circleci: Java 11, Clojure 1.10 Your tests passed on CircleCI!
Details
ci/circleci: Java 11, Clojure 1.7 Your tests passed on CircleCI!
Details
ci/circleci: Java 11, Clojure 1.8 Your tests passed on CircleCI!
Details
ci/circleci: Java 11, Clojure 1.9 Your tests passed on CircleCI!
Details
ci/circleci: Java 11, Clojure master Your tests passed on CircleCI!
Details
ci/circleci: Java 14, Clojure 1.10 Your tests passed on CircleCI!
Details
ci/circleci: Java 14, Clojure 1.7 Your tests passed on CircleCI!
Details
ci/circleci: Java 14, Clojure 1.8 Your tests passed on CircleCI!
Details
ci/circleci: Java 14, Clojure 1.9 Your tests passed on CircleCI!
Details
ci/circleci: Java 14, Clojure master Your tests passed on CircleCI!
Details
ci/circleci: Java 8, Clojure 1.10 Your tests passed on CircleCI!
Details
ci/circleci: Java 8, Clojure 1.7 Your tests passed on CircleCI!
Details
ci/circleci: Java 8, Clojure 1.8 Your tests passed on CircleCI!
Details
ci/circleci: Java 8, Clojure 1.9 Your tests passed on CircleCI!
Details
ci/circleci: Java 8, Clojure master Your tests passed on CircleCI!
Details
@bbatsov
Copy link
Contributor

@bbatsov bbatsov commented Jun 25, 2020

Thanks! I'll push a new alpha now.

@eerohele eerohele deleted the eerohele:patch/completions-distinct branch Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.