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

Remove eval'd clojure code in favor of cider's cider-dispatch-complete-symbol #49

Open
gtrak opened this issue Feb 16, 2014 · 9 comments

Comments

@gtrak
Copy link

gtrak commented Feb 16, 2014

https://github.com/clojure-emacs/cider/blob/master/cider-interaction.el#L582

This falls back to eval if not present, but will now rely on the presence of the 'complete' nrepl op, which also works with clojurescript repls.

Backend impl is here:
https://github.com/gtrak/cider-nrepl/blob/master/src/cider/nrepl/middleware/complete.clj

Company mode is doing something similar:
https://github.com/clojure-emacs/company-cider/blob/master/company-cider.el#

Seems sensible to me since ac-nrepl's already relying on cider.

@purcell
Copy link
Member

purcell commented Feb 18, 2014

Thanks: this is certainly planned, but will have to wait until I have some time, or until a corresponding pull request comes in.

@gtrak
Copy link
Author

gtrak commented Feb 18, 2014

I'm willing to do it.

The primary motivator is cljs support, which means I have to remove direct references to complete.core. Since there are multiple ac-sources, I think I'd have to switch off the others in the presences of cider's implementation. Do you have any thoughts on that?

@purcell
Copy link
Member

purcell commented Feb 18, 2014

The only real advantage of the multiple sources is that the flavour of the completion candidate is indicated, e.g. with "c" or "v". But it would be a shame to lose that if we could avoid it... do the candidates returned by nrepl have any meta info indicating their type?

@gtrak
Copy link
Author

gtrak commented Feb 18, 2014

No, but it would be better than nothing in the case that the op is
available. I could reconsider the implementations, but it's not a small
amount of work.

Alternatively, there exists an 'info' op that returns detailed information
about a particular symbol. That could be used in some way to enhance the
output.
(Edit: I see now that it could be used for the tooltip)

On Tue, Feb 18, 2014 at 10:31 AM, Steve Purcell [email protected]:

The only real advantage of the multiple sources is that the flavour of the
completion candidate is indicated, e.g. with "c" or "v". But it would be a
shame to lose that if we could avoid it... do the candidates returned by
nrepl have any meta info indicating their type?

Reply to this email directly or view it on GitHubhttps://github.com//issues/49#issuecomment-35395368
.

@purcell
Copy link
Member

purcell commented Feb 18, 2014

Yeah, we probably don't want to do extra backend calls to grab info for the completed symbols. I'd vote in favour of just merging the current multiple sources into one source backed by nrepl's complete op, thereby losing the "v", "f" symbols etc. I don't think we should retain all the code which uses core.complete just to keep those symbols there. Let's ping @samaaron here and see what he thinks.

@purcell
Copy link
Member

purcell commented Feb 18, 2014

@gtrak How recent must nrepl and/or cider be for the complete op to be available?

@gtrak
Copy link
Author

gtrak commented Feb 18, 2014

Maybe an easy compromise is to add a filter param to the 'complete' op.
That would be less intrusive than changing the return structure.

EDIT 4/10/2014: I think I'm going with this.

3 calls is not much worse than 1.

@gtrak
Copy link
Author

gtrak commented Feb 18, 2014

Cider-nrepl middlewares must be loaded in your project for this to work,
it's still pre-release. The cider function we'd be calling has existed for
some time already, not sure how recent.

On Tuesday, February 18, 2014, Steve Purcell [email protected]
wrote:

@gtrak https://github.com/gtrak How recent must nrepl and/or cider be
for the complete op to be available?

Reply to this email directly or view it on GitHubhttps://github.com//issues/49#issuecomment-35397265
.

@MalloZup
Copy link

MalloZup commented Aug 5, 2019

autogenerated with https://github.com/MalloZup/doghub: issue inactive since 450 days. Please update the issue or close it

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

No branches or pull requests

3 participants