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

Add a middleware to find docs from ClojureDocs #628

Merged
merged 2 commits into from
Jul 25, 2019

Conversation

liquidz
Copy link
Member

@liquidz liquidz commented Jul 23, 2019

Introduce a middleware to find docs from not Grimoire but ClojureDocs.
See also clojure-emacs/cider#2663 and clojure-emacs/orchard#64

  • The commits are consistent with our contribution guidelines
  • You've added tests to cover your change(s)
  • All tests are passing
  • The new code is not generating reflection warnings
  • You've updated the README (if adding/changing middleware)

@liquidz liquidz force-pushed the feature/clojuredocs branch 3 times, most recently from 77c98b2 to ececb7d Compare July 23, 2019 14:58
@@ -478,12 +478,32 @@
:returns {"fn-deps" "A list of function deps."
"status" "done"}}}})

(def-wrapper wrap-clojuredocs cider.nrepl.middleware.clojuredocs/handle-clojuredocs
{:doc "Middleware to find a documents from ClojureDocs."
:handles {"load-clojuredocs-cache!"
Copy link
Member

Choose a reason for hiding this comment

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

I don't think clients will need to call this explicitly, probably they'll just need the ability to reload the cache.

Copy link
Member Author

Choose a reason for hiding this comment

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

Apart from refreshing cache, I thought it could be used for cache loading in the background (e.g. when connecting nREPL).
But once the cache file is generated, loading of the cache is fast enough, so it seems that just refreshing is good.

:requires {}
:optional {"export-edn-url" "EDN file URL exported from ClojureDocs. Defaults to \"https://clojuredocs-edn.netlify.com/export.edn\"."}
:returns {"status" "\"ok\" if loading was successful"}}
"clean-clojuredocs-cache!"
Copy link
Member

Choose a reason for hiding this comment

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

Same with this one - probably we can expose a single "clojuredocs-refresh-cache" op instead of two.

{:doc "Clean a cached file and documents"
:requires {}
:returns {"status" "\"ok\" if cleaning was successful"}}
"clojuredocs"
Copy link
Member

Choose a reason for hiding this comment

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

I think clojuredocs-lookup is a more informative name for this one.

Makefile Outdated
.inline-deps:
lein inline-deps
touch .inline-deps

inline-deps: .inline-deps

test: .inline-deps
test: .inline-deps test/resources/clojuredocs/export.edn
Copy link
Member

Choose a reason for hiding this comment

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

It's best to put resources under the cider/nrepl package for consistency with the existing resources.

@liquidz
Copy link
Member Author

liquidz commented Jul 24, 2019

@bbatsov Thank you for your reviewing!
I had forgotten to update README, so I fixed them including README!

@bbatsov bbatsov merged commit a0190ec into clojure-emacs:master Jul 25, 2019
@bbatsov
Copy link
Member

bbatsov commented Jul 25, 2019

Excellent! I'll cut a new beta now.

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