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 user-facing variable for customization of symbol completion methods #333

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ramenbytes
Copy link

@ramenbytes ramenbytes commented Jul 12, 2020

This pull request seeks to make it possible to integrate ivy's dynamic completions feature with sly, bringing the incremental and auto-updating style of the company completion support to things like jumping to definitions. I'm not certain that the way I went about doing it was the best or not, and would appreciate any feedback or suggestions.

Concerns:

  • Expanding the meaning of sly-symbol-completion mode. If enabling it is supposed to mean that Sly's UI is used to complete symbols, this pull request breaks that. On the other hand if it is just supposed to mean that some special handling of completion takes place then this pull request just removes the hard-coding of Sly's UI.

  • Possibly working around an ivy problem. The majority of this change is working around the fact that ivy requires a one argument collection function for dynamic collections instead of the standard three argument function that completing-read uses. Since all that appears needed to translate from the standard function behavior to what ivy requires is using all-completions, I'm not sure why ivy enforces a single arg. It might be cleaner to try and submit a change to ivy instead.

  • Missing idiomatic Emacs way of doing things. I'm new to emacs software development, and may have missed an obvious way to do this non-intrusively. There may also be questionable stylistic choices.

I look forward to hearing your thoughts on this, and will try to attach a example gif of the customizations this enables.

EDIT: Animated gif
sly-test


* lib/sly-completion.el

(sly-symbol-completing-read-function, sly-read-symbol-name):
Add new var referenced in sly-read-symbol-name for customization of
symbol completion methods. This allows users to, say, integrate ivy's
dynamic completions support into sly. Default value is
\#'sly-default-symbol-completing-read.

(sly-default-symbol-completing-read):
Encapsulate the logic for completing-read with the special sly
minibuffer in a function. This is to provide a default value for the
added configuration variable, and allow users to reset it.
@joaotavora
Copy link
Owner

Expanding the meaning of sly-symbol-completion mode. If enabling it is supposed to mean that Sly's UI is used to complete symbols, this pull request breaks that.

Yes, I'm afraid that's exactly what it is. it also means that if you turn it off, SLY should behave and play along with Emacs's interfaces and hence other UI's. To my knowledge it already does this with bare completion-at-point, company-mode, and Helm (the latter was a big fight, but we came through in the end).

@ramenbytes
Copy link
Author

Ok, I was worried about that. What do you think about me removing the dependence of my code on that variable but leaving in the ability to change sly-read-symbol-name's completing-read function? I'll push a commit that does that in a couple minutes to illustrate (hopefully).

It is intended to mean that Sly's special UI will be used. If that is
not enabled, allow the user to customize the completing-read style
function used to complete symbols through a variable.
@joaotavora
Copy link
Owner

One thing I don't understand is why Emacs's existing indirection provided by completing-read-function isn't enough for your purposes.

That I don't understand it, doesnt' mean it isn't. You seem to be after flex completion, right (as seens from the gif). But you also seem to want that particular flex completion to be performed client side by Ivy, am I not right? If so, and in no way requesting that you read all of emacs-helm/helm#2165, are you at least familiar with the problem being presented there?

Inside that issue, this link contains a visual summary of the problems: emacs-helm/helm#2165 (comment)

In other words, I wonder if it isn't time that Ivy follows suit on Helm and also starts supporting completion-styles. It should, in principle start working immediately with SLY (and Eglot, and other things).

@ramenbytes
Copy link
Author

ramenbytes commented Jul 13, 2020

One thing I don't understand is why Emacs's existing indirection provided by completing-read-function isn't enough for your purposes.

I'm not certain it isn't, but there are a few things keeping me from using it at the moment. The main issue is that because of the way ivy does things, I think I would need to know too much about the implementations and run-time behavior of any Sly functions I wanted to use the special behavior. To use dynamic completions in ivy, the collection argument to completing-read needs to only take one argument. Because of this I preprocess the collection function passed in by Sly so that ivy can handle it, and I need to know that it is Sly doing symbol completion when choosing whether to preprocess the collection argument to avoid impacting other uses. If I understand correctly, I could use completing-read-function to bind to my special preprocessing function while Sly does completion. I figure this would be implemented either by small wrapper functions or maybe hooks (not sure). Either way though, binding it around Sly stuff will result in all uses of completing-read in the dynamic scope using the special function. For this to not break stuff, I need to know that the call chain of the Sly function being wrapped won't ever use completing-read with a function. That is the sticking point for me.

That I don't understand it, doesnt' mean it isn't. You seem to be after flex completion, right (as seens from the gif). But you also seem to want that particular flex completion to be performed client side by Ivy, am I not right? If so, and in no way requesting that you read all of emacs-helm/helm#2165, are you at least familiar with the problem being presented there?

Yes, I'm trying to integrate ivy and flex completion. I'm not quite sure I understand what you're saying though. By client-side, I assume you mean that I'm trying to have ivy perform the match search itself? If so, that isn't what I want. Unless I'm mistaken, Ivy is using the slynk backend for the flex completions. How this works is using the :dynamic-completions argument of ivy-read, which is called under the hood of ivy-completing-read. Link: https://oremacs.com/swiper/#optional-arguments-for-ivy-read. What happens is that each time the minibuffer input changes, ivy calls the collection function again with the new input so that completions are recalculated. If I understand correctly, this means that ivy is mostly just repeatedly requesting sly/slynk to flex complete a changing string and displaying the results. So ivy only really acts as a UI here (I think) and does no actual completion.

I remember reading through that a while ago when issue #214 was open, but I'm a bit fuzzy on things. To my rememberance (and after rereading most of it) the issue was that Helm wasn't updating the completions list when input merited it? Specifically, if you tried completing with initial input "multiple-value-b" then edited the minibuffer input to only be "m" Helm would not update the completions to reflect the added possibilities.

In other words, I wonder if it isn't time that Ivy follows suit on Helm and also starts supporting completion-styles. It should, in principle start working immediately with SLY (and Eglot, and other things).

Quite possibly. I'm not sure if that is required for what I (think I am) trying to do though. Namely, have ivy phone home to slynk every time input changes. I'm starting to become more convinced that a good solution for this will involve changes to ivy. Specifically, if ivy took a normal 3-arg collection function and just repeatedly called all-completions on that with the current prompt if :dynamic-collection was t, that would clean things up quite a bit at least in terms of having/not having to tip toe around functions with different arglists. It wouldn't solve the scoping issue though, since all uses of ivy-completing-read with collection functions in the dynamic scope of binding ivy-completing-read-dynamic-collection to t would lose ivy's completion algorithms and be expected to calculate the completions themselves. In other words, a way to restrict the effect to specific sly completions would still be needed. Would supporting completion-styles allow this sort of scoping of effects as well as phoning home to slynk?

EDIT: I should note that when I say I'm trying integrate ivy & flex completions, I mean I am trying to use ivy as an interface to sly/slynk's flex completions. Not write/give ivy flex completion in general.

@joaotavora
Copy link
Owner

If I understand correctly, this means that ivy is mostly just repeatedly requesting sly/slynk to flex complete a changing string and displaying the results. So ivy only really acts as a UI here (I think) and does no actual completion.

Indeed that answers my question: if there's a request every time you type a character that needs new information from the Slynk side, then Ivy is not doing the flex-matching itself, and the problem here is not the same as it was Helm's.

EDIT: I should note that when I say I'm trying integrate ivy & flex completions, I mean I am trying to use ivy as an interface to sly/slynk's flex completions. Not write/give ivy flex completion in general.
, if someone cumbersome
Yes, I understand that. What I'm saying is that Emacs already has a working, if admittedly somewhat cumbersome, protocol of negotiating completion with various backends, be they backends that return all possible completions so that the UI does the filtering, or, as is the case in SLY, backends that have to do the filtering itself. It's the completion-at-point-functions + completion-styles protocol.

From your description, it seems that Ivy is closer to supporting this, closer to bridging this gap, than Helm was at the time. If you invest your time in Ivy's support of this protocol, you will, in principle fix Ivy for both SLY's flex/etc/completions and other extensions such as Eglot, that also suffer from a very similar problem.

Now, where is "the protocol" documented. Here and there, in bits and pieces and examples mostly (or maybe I'm missing something and @monnier can point me to more consolidated documentation). Perhaps a decent place to start is to check how company-mode does it. You'll notice it uses a lot of all-completions, try-completions etc. Those functions, when used on "tables", should in principle navigate through most of the completion-styles cruft and present you the best result.

All this said, I'm not keen on adding more code in SLY just to support Ivy, since Emacs already has this abstraction. I reckon it would be a hack. If you need something quickly/urgently I reckon some advice-add or even a redefinition can do the trick.

@ramenbytes
Copy link
Author

Yes, I understand that. What I'm saying is that Emacs already has a working, if admittedly somewhat cumbersome, protocol of negotiating completion with various backends, be they backends that return all possible completions so that the UI does the filtering, or, as is the case in SLY, backends that have to do the filtering itself. It's the completion-at-point-functions + completion-styles protocol.

I'll have to look more into that then, I'd definitely prefer to use the existing interface than a hack for a specific backend's idiosyncrasies. Just so I understand better, does that system cover all completions? i.e, it covers completions when typing in the minibuffer as well as completions when typing into normal buffers (what company normally handles)? I think it does, given the minibuffer has a point too and the elisp docs don't seem to specify restrictions (from what I've read so far), but I'm rather inexperienced with this and unsure.

From your description, it seems that Ivy is closer to supporting this, closer to bridging this gap, than Helm was at the time. If you invest your time in Ivy's support of this protocol, you will, in principle fix Ivy for both SLY's flex/etc/completions and other extensions such as Eglot, that also suffer from a very similar problem.

If this is the way forward, I'll probably spend some time investigating. Might take a while, time is in short supply at the moment.

Now, where is "the protocol" documented. Here and there, in bits and pieces and examples mostly (or maybe I'm missing something and @monnier can point me to more consolidated documentation). Perhaps a decent place to start is to check how company-mode does it. You'll notice it uses a lot of all-completions, try-completions etc. Those functions, when used on "tables", should in principle navigate through most of the completion-styles cruft and present you the best result.

Alright, thanks for the pointers.

All this said, I'm not keen on adding more code in SLY just to support Ivy, since Emacs already has this abstraction. I reckon it would be a hack. If you need something quickly/urgently I reckon some advice-add or even a redefinition can do the trick.

Yes, I see your point. So in summary, all three concerns I had when I submitted this pull request were valid. However, now I think I have a (small) grasp on a better way forward. Thanks for that.

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