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 eldoc datomic query operation #395

Merged
merged 1 commit into from
Feb 7, 2017

Conversation

mallt
Copy link
Contributor

@mallt mallt commented Feb 3, 2017

  • 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)

This PR adds an operation to supply eldoc information for a datomic query. The operation returns a list of the expected inputs of the supplied query.

The basic idea here is that if in the client the regular eldoc operation has determined the function call at point resolves to datomic.api/q, the query is passed to this new operation that will return the expected inputs so these can be shown as eldoc info (as described in the cider feature request 1832).

I've been testing calls from cider already, but before finalizing the cider code I thought it would be useful to already submit this pull request for review.

Thanks!

@bbatsov
Copy link
Member

bbatsov commented Feb 7, 2017

The code looks good to me.

@bbatsov
Copy link
Member

bbatsov commented Feb 7, 2017

Just one small remark, I'm not sure we really need a separate eldoc op for this, but I'm fine with the current approach. (alternatively we could have just made the existing op a bit smarter)

@bbatsov bbatsov merged commit 0b859a6 into clojure-emacs:master Feb 7, 2017
@mallt
Copy link
Contributor Author

mallt commented Feb 7, 2017

Thanks!

@bbatsov
Copy link
Member

bbatsov commented Mar 10, 2017

@mallt Any progress on the client-side of things?

@mallt
Copy link
Contributor Author

mallt commented Mar 10, 2017

Yes, I hope to have a PR ready within the next couple of days.

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