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

Elpy-doc completion breaks with ivy #1084

Closed
jmercouris opened this issue Feb 17, 2017 · 18 comments
Closed

Elpy-doc completion breaks with ivy #1084

jmercouris opened this issue Feb 17, 2017 · 18 comments
Labels
Milestone

Comments

@jmercouris
Copy link

jmercouris commented Feb 17, 2017

To replicate:

M-x ivy-mode
M-x elpy-doc

type: 'datetime' + TAB, or try to expand to datetime.datetime, and you will find that you cannot. You can still type the full string 'datetime.datetime', and retrieve results, but somehow elpy-doc is not giving the appropriate readline info to ivy for completion after the first chunk of text. Similar behavior has been observed for helm as well.

Possible solutions:

  • hook to enable and disable ivy-mode, or helm when calling elpy-doc
  • force ido-mode in elpy-doc somehow
@jmercouris jmercouris changed the title Elpy-doc breaks with ivy Elpy-doc completion breaks with ivy Feb 17, 2017
@drym3r
Copy link

drym3r commented Feb 17, 2017

I'll just add that it happens to me, as well.

@jorgenschaefer
Copy link
Owner

jorgenschaefer commented Feb 17, 2017

Hello, and thanks for the report! Why do you think this is a problem that Elpy should fix, and not ivy-mode? Elpy seems to work fine?

@jorgenschaefer jorgenschaefer added this to the v1.15 milestone Feb 17, 2017
@jmercouris
Copy link
Author

jmercouris commented Feb 17, 2017

I believe it to be that helm and ivy are using standard interfaces for interacting with completion, so I guess elpy-doc should be utilizing these, I can't be sure. One of the other motivations for reporting here was due to your responsiveness

@drym3r
Copy link

drym3r commented Feb 17, 2017

I also don't know if helm and ivy use standard interfaces, but none of them work, so if is not an elpy related issue, it is related to both helm and ivy.

@jorgenschaefer
Copy link
Owner

elpy-doc uses completing-read with dynamic completion tables, a standard interface in Emacs (see elpy-doc--read-identifier-from-minibuffer). Both ivy and helm interfere with this standard interface to provide a different completion interface. I'm quite happy to add code to improve the use of the standard interface of Emacs, but I'm not willing to add code to cater to non-standard completion packages implementing standard facilities badly …

@jmercouris
Copy link
Author

well, I wouldn't say the are implementing standard facilities badly, almost literally every other function in emacs works with completion via ivy. though I understand your hesitation to include anything not standard/core to emacs. In the interim, do you know of a workaround to wrap elpy-doc? e.g. force a completion mechanism just for elpy-doc? maybe I should write a mini function that turns ivy on and off before/after calling elpy-doc?

@jorgenschaefer
Copy link
Owner

You could try an around-advice? I'm afraid I do not know ivy, and only use helm for very specific functions, so I do not know how to disable them for the parts of Emacs that they do not work with.

@jmercouris
Copy link
Author

I'll keep you updated if I come up with anything, thanks for your help

@jmercouris
Copy link
Author

issue opened in ivy: abo-abo/swiper#892

@abo-abo
Copy link

abo-abo commented Feb 17, 2017

elpy-doc uses completing-read with dynamic completion tables, a standard interface in Emacs

In my opinion, dynamic completion tables are a goofy interface that add an unnecessary layer of indirection.

Similar behavior has been observed for helm as well.

Both helm and ivy sometimes have difficulty with this interface, because it often assumes that the default completion is being used.

I suggest to migrate to the collection as a function interface:

(defun elpy-doc--read-identifier-from-minibuffer (initial)
  "Read a pydoc-able identifier from the minibuffer."
  (completing-read "Pydoc for: "
                   (lambda (str pred _)
                     (elpy-rpc-get-pydoc-completions str))
                   nil nil initial 'elpy-doc-history))

The above approach is better for introspection as well, provided elpy-rpc-get-pydoc-completions can be called in place of the lambda wrapper.

@jmercouris
Copy link
Author

I'm not sure I understood all of your response, but I put it in a buffer and eval'd it, it did substitute the function, but no improvement in behavior was observed

@jorgenschaefer
Copy link
Owner

No, elpy-rpc-get-pydoc-completion does not follow the function interface of completing-read because that requires also knowing about buffer positions. That's what completion tables are for, goofy or not.

If you do not implement the full interface of the functionality you replace in core Emacs, I would suggest you to provide a way for your users to disable your mode where it breaks.

@jorgenschaefer
Copy link
Owner

(completion-table-dynamic returns a function, incidentally.)

@abo-abo
Copy link

abo-abo commented Feb 17, 2017

No, elpy-rpc-get-pydoc-completion does not follow the function interface of completing-read because that requires also knowing about buffer positions. That's what completion tables are for, goofy or not.

I prefer the completion-at-point-functions interface, along with complete-symbol for this. Works great with helm and ivy since you can scroll through the candidates and each time you scroll, the old code is removed from the buffer and the new one is inserted.

@jorgenschaefer
Copy link
Owner

I have to admit, "adheres to the interface preferred by the author of ivy" is not exactly high up on my list of reasons for choosing certain APIs over others.

@jmercouris
Copy link
Author

There's no need to escalate this, let's all remain calm :D
we don't need these kinds of wars within the community itself :)
we're on the same team here

@abo-abo
Copy link

abo-abo commented Feb 17, 2017

@jorgenschaefer Just making a suggestion to improve your package. The alternative for your users is to employ crutches that turn off their preferred completion type, just because you're being suborn.

@jorgenschaefer
Copy link
Owner

Suggested solution to this problem: Use helm instead of ivy, as helm allows disabling the package where it does not implement the APIs it replaced correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants