emacs-helm / helm

Emacs incremental completion and selection narrowing framework
https://emacs-helm.github.io/helm/
GNU General Public License v3.0
3.36k stars 389 forks source link

Allow using `completion-styles` in Helm and implementation of own helm style #2165

Closed joaotavora closed 4 years ago

joaotavora commented 5 years ago

Expected behavior

Helm's completion engine can use SLY's completion functions as a backend. The original issue is here: https://github.com/joaotavora/sly/issues/214

Actual behavior (from emacs-helm.sh if possible, see note at the bottom)

The value for completion-at-point-functions is sly-complete-symbol which does the completion itself. This does not trigger the problem. It only happens in other situations such as when SLY's sly-read-symbol-name calls completing-read which Helm overrides via completing-read-function. Actually it used to happen because I've turned off that overriding in that particular context context.

But the root problem remains and I guess some users would like to use Helm's UI for completing SLY symbols. However, when trying

(helm-comp-read "foo " (sly--completion-function-wrapper sly-complete-symbol-function))

Nothing interesting happens. sly--completion-function-wrapper is supposed to produce a reasonably well-behaved completion-function/table. It works well with SLY's own sly-complete-symbol

But it probably isn't well behaved enough, or not to Helm's expectations. I want you to help me understand what's missing.

In particular SLY's lower-level sly-flex-completions and sly-simple-completions functions, which request completions over the network, will return an empty list of completions when the pattern is the empty string, i.e. they only start operating once you feed them at least a character.

Perhaps Helm expects the full completion list to be handed to it up front. But that is impractical because it's frequent that a running SLY session has many thousands of symbols available. Is there a way for Helm to behave more incrementally, a little bit like company does?

Also sly-flex-completions will perform "scatter-match" completion instead of "prefix" completion, which is what sly-simple-completions does.

Steps to reproduce (recipe)

./emacs-helm.sh
(add-to-list 'load-path "~/Source/to/sly")
(require 'sly-autoloads)
M-x sly ; this requires a `lisp` program somewhere in your path, possibly sbcl

Backtraces if any (M-x toggle-debug-on-error)

N/A

Describe versions of Helm, Emacs, operating system, etc.

MELPA helm-20190627.758 A reasonably fresh Emacs 27 Windows 10

Are you using emacs-helm.sh to reproduce this bug (yes/no):

yes

thierryvolpiatto commented 5 years ago

Thanks to report, I will install sly and try to understand what's going on ASAP.

thierryvolpiatto commented 5 years ago

Ok, I think I found what's wrong. What you expect is after starting Sly TAB trigger helm completion when helm-mode is enabled, right? It is not working because you override locally the value of completion-in-region-function, if you don't set it, helm completion is working fine. You have to comment the line number 340 in sly-completion.el and eval the function BEFORE starting the REPL. See screenshot below. Capture d’écran_2019-06-28_17-52-48

thierryvolpiatto commented 5 years ago

OTH.

thierryvolpiatto commented 5 years ago

Also I noticed that when I hit "," from the REPL I have an ido completion, you need completing-read as value for sly-completing-read-function to have helm completion. A helm user wanting ido completion somewhere have just to customize helm-completing-read-handlers-alist for the needed commands.

joaotavora commented 5 years ago

OTH

"Off the hook" Lol :-) OK mostly. I think there will be some complications somehow somewhere, I'll test with your advice.

Also I noticed that when I hit "," from the REPL I have an ido completion, you need completing-read as value for sly-completing-read-function to have helm completion. A helm user wanting ido completion somewhere have just to customize helm-completing-read-handlers-alist for the needed commands.

That's a different problem, but thanks for reminding me.

joaotavora commented 5 years ago

think there will be some complications somehow somewhere, I'll test with your advice.

As I feared, a deeper problem persists. First of all, thanks for promptly installing SLY and reproducing this, it is an effort not every package maintainer will make.

Now, the root problem is that Helm is caching whatever completions it gets from the completion-at-point function (capf). In the scenario where capf is and non-prefix is used, the completion UI cannot do that. If you have access to it, I would refer you to how company-mode deals with this.

But even before that, you can try this out: notice that once you complete "sly", you get a completion list of, say, 100 symbols. If you add a letter to "sly", such as "slyt" you now get 73 symbols. Fine so far. Delete the "t", you get the 100 again, now delete the "y" and notice that you stay at the very same 100, while you should now have other symbols at your disposal. (I understand the 100 is somehow a default cap in Helm, but it doesn't change the problem: the symbol list isn't refreshed).

In other words, the Lisp process needs to be contacted every time a character is added or removed from the pattern string. It is only in the reasonably simple "prefix"-matching situation that caching and client-side matching can be used. But even so, Helm must either get the full completion list upfront, or contact the process every time the pattern is widened.

Being such a popular package, surely there is some similar use-case in say, https://github.com/clojure-emacs/cider.

I hope I have managed to explain myself.

thierryvolpiatto commented 5 years ago

João Távora notifications@github.com writes:

think there will be some complications somehow somewhere, I'll test with your advice.

As I feared, a deeper problem persists. First of all, thanks for promptly installing SLY and reproducing this, it is an effort not every package maintainer will make.

Now, the root problem is that Helm is caching whatever completions it gets from the completion-at-point function (capf). In the scenario where capf is and non-prefix is used, the completion UI cannot do that. If you have access to it, I would refer you to how company-mode deals with this.

Company-mode doesn't have to deal with this as it uses as input the text at point in current-buffer i.e. it have no minibuffer.

The question is what is completion? Is it completing what is under cursor in current-buffer or is it completing what we have in minibuffer? Helm actually is doing the former, in our example it gives the completion on "sly-" and only "sly-" until you exit helm and change the text at point. You can try in any elisp buffer with e.g. "def" as text at point, you will have completion against "def" and nothing else until you change what is at point.

But even before that, you can try this out: notice that once you complete "sly", you get a completion list of, say, 100 symbols. If you add a letter to "sly", such as "slyt" you now get 73 symbols. Fine so far. Delete the "t", you get the 100 again, now delete the "y" and notice that you stay at the very same 100, while you should now have other symbols at your disposal. (I understand the 100 is somehow a default cap in Helm, but it doesn't change the problem: the symbol list isn't refreshed).

Yes, it is same as described above.

In other words, the Lisp process needs to be contacted every time a character is added or removed from the pattern string. It is only in the reasonably simple "prefix"-matching situation that caching and client-side matching can be used. But even so, Helm must either get the full completion list upfront, or contact the process every time the pattern is widened.

As said above this is questionable. Actually we have once in Helm to C-g, modify input in curent-buffer and hit TAB again, do we want something else? i.e. Taking in account what we have in minibuffer instead of what is at point in current-buffer once Helm is started.

Being such a popular package, surely there is some similar use-case in say, https://github.com/clojure-emacs/cider.

Depend of the UI they use i.e. a minibuffer or not.

I hope I have managed to explain myself.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.*

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

thierryvolpiatto commented 5 years ago

What may help is quitting helm as soon as minibuffer-contents length is < to initial input which would allow user modify its input and hit TAB again when done.

joaotavora commented 5 years ago

The question is what is completion? Is it completing what is under cursor in current-buffer or is it completing what we have in minibuffer?

So Helm assumes that whatever text it starts out with in the minibuffer is correct and immutable until the next time you invoke Helm? And likewise the the list of completions corresponding to that text. Is that set of (apparently 100 max by default) the largest set of completions that will ever be attained in that particular Helm invocation/session?

If what I'm asking is against Helm's philosophy, it's perfectly OK. Let me just ask, to clarify. Helm is an "incremental completion and narrowing framework", right? Do you think there there is any way that this framework can be used to present the user the choice of one among, say, 1 million symbols, especially given that those 1 million symbols do not reside in the same memory as the Emacs process?

Both SLY's native interface (very close to Emacs's and the company-mode interface allow this, without forcing the user to exit that interface. Does Helm, too? (notice that this is not the same as saying that 1 million symbols are visible or present in the interface at any given time).

joaotavora commented 5 years ago

What may help is quitting helm as soon as minibuffer-contents length is < to initial input which would allow user modify its input and hit TAB again when done.

But don't you think it's a reasonable expectation by the user that the set of possible completions is enlarged as he backspaces in the Helm interface, just as it does when the user does so after more characters have been entered beyond "initial input"?

thierryvolpiatto commented 5 years ago

At first yes, but after thinking about it no. Emacs completion-in-region expect completing something against initial input, adding the missing characters to what is at point. If you have at point "sly" completion-in-region will complete "sly" with anything else starting with "sly", if you change your mind and change the minibuffer contents to what is at point e.g. "def" and Helm behave as you are asking, completion in region mechanism would be confused and insert e.g. "slydefun". Remember that what we are talking about is a generic completion i.e. same function should handle sly, elisp completion etc... So for a generic completion, I am not sure we should do more than what we are doing actually (apart maybe quitting when minibuffer differs from input). But a specialized package may be created to complete in sly as you are expecting. What do you think?

thierryvolpiatto commented 5 years ago

Apart this SLY looks really cool!

thierryvolpiatto commented 5 years ago

João Távora notifications@github.com writes:

The question is what is completion? Is it completing what is under cursor in current-buffer or is it completing what we have in minibuffer?

So Helm assumes that whatever text it starts out with in the minibuffer is correct and immutable until the next time you invoke Helm?

Only in the context of completing text at point.

And likewise the the list of completions corresponding to that text. Is that set of (apparently 100 max by default) the largest set of completions that will ever be attained in that particular Helm invocation/session?

No, see helm-candidate-number-limit.

If what I'm asking is against Helm's philosophy, it's perfectly OK. Let me just ask, to clarify. Helm is an "incremental completion and narrowing framework", right?

Yes, but what we are speaking about here is generic completion which limits capabilities.

Do you think there there is any way that this framework can be used to present the user the choice of one among, say, 1 million symbols, especially given that those 1 million symbols do not reside in the same memory as the Emacs process?

Yes, but Helm will compute only the helm-candidate-number-limit of them matching helm-pattern.

Both SLY's native interface (very close to Emacs's and the company-mode interface allow this, without forcing the user to exit that interface.

As I said because there is no minibuffer involved. The interface is not accessed because your cursor is still in your buffer, not in a minibuffer.

company-mode is an auto-complete interface, while Emacs native completion is fully manual (needs to hit TAB at each time you change input) and Helm is partially auto completing as display change as you add characters to original input.

The difference with Helm and company is that Helm takes the initial input and compute candidates against it, while company compute the initial input in real time as it changes.

Does Helm, too? (notice that this is not the same as saying that 1 million symbols are visible or present in the interface at any given time).

Completing against million symbols ? Yes.

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

joaotavora commented 5 years ago

Completing against million symbols ? Yes.

But to do so without exiting the Helm interface, you need to fetch the million symbols upfront, yes?

thierryvolpiatto commented 5 years ago

João Távora notifications@github.com writes:

Completing against million symbols ? Yes.

But to do so without exiting the Helm interface, you need to fetch the million symbols upfront, yes?

Not if you complete against the prefix, which is completion, I have the impression you want to compute all candidates like if input were empty string.

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

joaotavora commented 5 years ago

Not if you complete against the prefix, which is completion, I have the impression you want to compute all candidates like if input were empty string.

No, it's very simple: I want to be able to explore the whole domain, the whole set/space of completions, in the following way:

  1. Without exiting the completion interface, or whatever you want to call it.
  2. Without ever having to compute the whole set upfront.

Prefix or non-prefix does not enter into it. I just found it very surprising that once you invoke helm on abc and then backspace back to ab you stay in the Helm interface but don't see the completions that start with abd. My simple question is: is this by design or am I doing something wrong? If it's by design just say so :) I don't use Helm but perhaps that is what your users expect/or how you think it should behave.

thierryvolpiatto commented 5 years ago

What you are asking (1.) is not completion, so I can't make this happen in Helm as it will no more fit with the requirements of a generic command. (2.) can be done by dynamically computing candidates with probably a function instead of a plain list of candidates precomputed.

thierryvolpiatto commented 5 years ago

With last Helm, now completion behave like this:

joaotavora commented 5 years ago

What you are asking (1.) is not completion

In your personal view of what completion is. company behaves exactly like this. Is company a completion engine or is it something else? Maybe it's a question that doesn't need an answer: let's call it a different UI.

with probably a function instead of a plain list of candidates precomputed

Great idea. But what do you mean "probably"? In fact that idea has already been had, and, what a coincidence ;-) company uses it!

With last Helm, now completion behave like this:

I think these are improvements on the current situation. I don't understand the use case of letting the user delete the initial pattern and suggesting he press TAB. In my opinion it should either be totally forbidden, or the re-fetch should be automatic. Don't you think this is a simpler interface?

Anyway remember that you shouldn't be doing these improvements motivated by Sly, which, for the time being now, uses either its own UI or company. Think about what your current users would expect (i'm not one of them ;-) )

thierryvolpiatto commented 5 years ago

João Távora notifications@github.com writes:

What you are asking (1.) is not completion

In your personal view of what completion is. company behaves exactly like this.

No, as explained before company behave the same and emacs vanilla completion too, the only difference is they have NO minibuffer, i.e. the input is fetched directly from what the user write in current buffer whereas helm is taking this same input and put it in minibuffer to provide completion.

Is company a completion engine or is it something else? Maybe it's a question that doesn't need an answer: let's call it a different UI.

Company, auto-complete => auto complete engine. Helm => semi auto complete engine. Emacs vanilla => Manual completion.

with probably a function instead of a plain list of candidates precomputed

Great idea. But what do you mean "probably"? In fact that idea has already been had, and, what a coincidence ;-) company uses it!

Helm could also do this but for a specialized command to complete dynamically, here we are speaking of generic completion for helm-mode.

With last Helm, now completion behave like this:

I think these are improvements on the current situation. I don't understand the use case of letting the user delete the initial pattern and suggesting he press TAB. In my opinion it should either be totally forbidden, or the re-fetch should be automatic. Don't you think this is a simpler interface?

You have yo understand that Helm is not company or auto-complete, (see above comparisons) and I could write something doing what you are asking but NOT in the context of a generic function.

Anyway remember that you shouldn't be doing these improvements motivated by Sly, which, for the time being now, uses either its own UI or company. Think about what your current users would expect (i'm not one of them ;-) )

Of course, I am not going to change the actual behavior which is IMO the right thing, eshell completion behave also like this among others and is happily used by helm users, helm-slime completion behave also like this. I suggest someone write a helm package for sly behaving like you expect if you think it is the right think to do.

To resume the situation for the initial purpose of this bug report:

To have a generic completion using Helm interface (using completion-at-point) all you have to do is not using the sly function for completion-in-region.

To have dynamic completion with Helm you have to write a specialized package providing a new command for TAB replacing completion-at-point.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.*

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

joaotavora commented 5 years ago

No, as explained before company behave the same and emacs vanilla completion too, the only difference is they have NO minibuffer, i.e. the input is fetched directly from what the user write in current buffer whereas helm is taking this same input and put it in minibuffer to provide completion.

Helm is taking this same input and put it in minibuffer to provide completion.

I don't understand why it makes a difference where typing happens. The end goal for the user is to complete the thing he started, presumably with as few keystrokes as possible, with as much visual feedback as possible, and as little visual intrusion as possible. Different UI's have different approaches. Helm happens uses the minibuffer, fine by me.

I also don't understand what semiauto/auto/manual mean in your case. Certainly none of these UI engines complete something "automatically", i.e. with some kind of IA engine. They all show behave slightly differently on certain keystrokes, and show information in different areas of the screen.

Helm could also do this but for a specialized command to complete dynamically, here we are speaking of generic completion for helm-mode.

A helm-sly (or company-sly, or x-sly) would be a waste of effort. I have another LSP extension, eglot that uses the same concepts, I'm not going to repeat that effort, and I don't think anyone should have to. You (and I) should be coding to a common interface, not to a specific backend/frontend.

The good news is that interface already exists, see info node 20.6.7 Programmed Completion. The bad news is it not the best (for historical/backward compatibility reasons), and not wonderfully documented either. I'm frequently in touch with Stefan Monnier about this. Let's bother him here again, @monnier. See also the variable completion-styles, it's another part of the interface that frontends should respect.

Now SLY doesn't yet respect these interfaces fully (and neither does company-capf by the way). But it respects parts of it and I am improving support for it. If Helm were to respect it a bit more, it would work with SLY and other backends that respect it.

To have dynamic completion with Helm you have to write a specialized package providing a new command for TAB replacing completion-at-point.

No, this is precisely what I want to avoid. No speciazlied nothings, life is too short.

joaotavora commented 5 years ago

Now SLY doesn't yet respect these interfaces fully

I've done some work in a side branch (https://github.com/joaotavora/sly/tree/scratch/fix-214-rework-completion) and it should be more compliant now. To illustrate (and to prove that it really does work) here are 4 scenarios:

(1) basic completion (what you call "manual" completion).

sly-basic

(2) company (what you call "fully auto") sly-company

(3) icomplete (an ido.el replacement in emacs core) sly-icomplete

(4) SLY's own UI sly-own-ui

(5) Helm, not quite working

sly-helm

thierryvolpiatto commented 5 years ago

You can try with this branch https://github.com/emacs-helm/helm/tree/completion_in_region_dynamic. Not sure it works everywhere.

joaotavora commented 5 years ago

You can try with this branch https://github.com/emacs-helm/helm/tree/completion_in_region_dynamic. Not sure it works everywhere.

Thanks very much for attempting this. Working properly with Helm is a big plus for any package.

Now, I tried it, but it didn't work. Perhaps I'm not testing correctly. I checked out your branch, git clean -fdx, make EMACS_COMMAND=..., then ./emacs-helm and did basically what I do in the last .gif. I get the same behaviour except, in the completion-at-point case, it doesn't even get to the Helm interface.

How are you testing?

thierryvolpiatto commented 5 years ago

João Távora notifications@github.com writes:

You can try with this branch https://github.com/emacs-helm/helm/tree/completion_in_region_dynamic.
Not sure it works everywhere.

Thanks very much for attempting this. Working properly with Helm is a big plus for any package.

Now, I tried it, but it didn't work. Perhaps I'm not testing correctly. I checked out your branch, git clean -fdx, make EMACS_COMMAND=..., then ./emacs-helm and did basically what I do in the last .gif. I get the same behaviour except, in the completion-at-point case, it doesn't even get to the Helm interface.

How are you testing?

Tested with completion-at-point in emacs-lisp buffers and completing-read-multiple, these are working.

With Sly I only have [no matches] for now, don't know why it is not working here.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.*

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

joaotavora commented 5 years ago

Tested with completion-at-point in emacs-lisp buffers and completing-read-multiple, these are working.

That's a good start. I will debug SLY a bit today.

thierryvolpiatto commented 5 years ago

João Távora notifications@github.com writes:

Tested with completion-at-point in emacs-lisp buffers and completing-read-multiple, these are working.

That's a good start. I will debug SLY a bit today.

I could get it working with Sly by passing explicitely pattern to dynamic function, but this is defeating helm matching functions (I can workaround multimatch but fuzzy is broken). I noticed also that flexmatching is not working with helm whereas it is working with sly-symbol-completion-mode.

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

thierryvolpiatto commented 5 years ago

As I expected, trying to pass pattern (minibuffer-contents) to a dynamic function is a regression in Helm because it defeat all helm matching functions. In the example you show above e.g. icomplete, it is simple because the completion is basic, and helm is working fine like this, but as soon as you enter a more complex pattern, the function doesn't understand it, we would need a super function handling alone all the helm features, which is not possible. So with the actual behavior of the Master branch, I think we have a good compromise, loosing only the ability of changing on the fly the initial input, but well when you start to complete on e.g. "def" I hardly see why you would change your mind as soon you hit TAB to complete on something else. Below a screenshot illustrating pattern than can be used with helm: Capture d’écran_2019-07-03_09-37-29

joaotavora commented 5 years ago

Yes, this isn't so simple, there is still a long way to go.

You must query the completion table's category to discover the completion style that the table allows. SLY's category is sly-completion and the style is backend, meaning that SLY in particular will probably never support those fancy patterns because filtering is done server-side (the only place where it is efficient). SLY only supports "prefix" and "flex/scatter" matching for now (I'm thinking of adding "regexp" somewhere down the line).

The good news, more or less, is that all this complexity, etc should be hidden behind Emacs functions.

icomplete, it is simple because the completion is basic

icomplete is simple indeed, wouldn't you rather Helm was simple too :-)? Simplicity is a wonderful thing. But it can use any completion strategy because of a clean separation between UI and that strategy. It can use basic, substring, partial-completion, initial, and some more exotic styles, like emacs22, whatever that is. I recently added a flex style. You could add an regexp style, or an exclamation-mark-before-some-words-style. All of this is withhin the completion-styles interface, which all compliant UI's adhere to. When icomplete targets SLY's completion table, it understands that it will do its own matching. It does this by simply calling completion-all-sorted-completions, which does all the style-related gritty work. So does the standard UI and so does company-capf.

thierryvolpiatto commented 5 years ago

We loose the original purpose of this bug report.

The original issue was about making helm working with Sly, seems it is working now by letting Helm completion-in-region working.

Then you asked for dynamic completion, the issue being completing against something else than the original input, this would defeat all Helm completion features much more powerful than what you are describing (I am not a fan of Emacs completion API), fixing this would require a huge amount of work for a very small benefit, introducing bugs and probably as said above having a strong regression, so I won't try to fix this sorry.

joaotavora commented 5 years ago

The original issue was about making helm working with Sly, seems it is working now by letting Helm completion-in-region working

It's not working, I'm afraid. Once you enter the interface, you can't access all symbols as you can in all the other 4 UIs.

Also, sooner or later I would ask you to have it work with Eglot, so the scope change is appropriate IMO.

fixing this would require a huge amount of work for a very small benefit, introducing bugs and probably as said above having a strong regression, so I won't try to fix this sorry.

That's ok. It's not easy, but it's easier than you think. I didn't do a good job of selling you the API. But if you used it, you would share that fancy exclamation mark pattern matching with all the other compliant UIs.

If you change your mind, you should code this alongside existing helm functionality, so no breakage occurs. Another option is, does helm have an API? It could take a similar approach to company and there could be a helm-compliant plugin or something.

You can close this if you want. Thanks for the attempt.

João

thierryvolpiatto commented 5 years ago

So to resume here, the initial issue have been found and we have completion with helm. The behavior you expect (dynamic function) break the helm matching capabilities. I am able to patch helm-mode to behave like you want but it would be a regression in helm unless beeing able to use the backend function provided by sly (which is not working with helm). I can't test anymore as sly seems broken actually (see warnings at compilation). With actual behavior of master, completion in sly is working fine (even with fuzzy matching) from my point of view, so I am closing now, feel free to reopen if needed. Thanks.

joaotavora commented 5 years ago

I can't test anymore as sly seems broken actually (see warnings at compilation).

What do you mean "broken"? What is happening that shouldn't be happening and/or what isn't happening that should be happening?

The compilation warnings in SLY are nothing to worry about (if I understand the warnings you are talking about).

With actual behavior of master, completion in sly is working fine (even with fuzzy matching) from my point of view, so I am closing now, feel free to reopen if needed.

It is certainly not "working well". The backend completion style expects the frontend to let it take care of the filtering, but Helm doesn't allow that.

On the other hand, if it's from "your point of view", then OK, if you close one eye and squint and look from far away it look okayish. :-)

I am able to patch helm-mode to behave like you want but it would be a regression in helm unless beeing able to use the backend function provided by sly (which is not working with helm).

I don't see how honouring completion-styles, etc, would be a regression in helm, it would be a progression. You could make some kind of indirection that keeps the current helm behaviour for all those other backends, thus not breaking any package's/user's behaviour.

But I agree that given the size and complexity of Helm, probably a hard effort.

You may not like Emacs's completion API, and that's acceptable, but in the end of the day Emacs is the software you are writing for and keeping your software adaptable and conformant will bring advantages.

feel free to reopen if needed.

I can't, only you can.

thierryvolpiatto commented 4 years ago

You can now use helm-completion-style to use emacs completion (use customize-set-variable or the customize interface, not setq). Then when you customize completion-styles to your need (e.g. add flex or backend), you should have what you expect.

thierryvolpiatto commented 4 years ago

Note that this is only in dev branch for now.

thierryvolpiatto commented 4 years ago

Now on Master.

joaotavora commented 4 years ago

Wonderful, I think. So everything works with Emacs -Q and sly and helm, provided the user turns off sly-symbol-completion-mode and sets helm-completion-style to emacs ,right?

thierryvolpiatto commented 4 years ago

João Távora notifications@github.com writes:

Wonderful, I think. So everything works with Emacs -Q and sly and helm, provided the user turns off sly-symbol-completion-mode and sets helm-completion-style to emacs ,right?

Yes, in addition you have to add backend to completion-styles.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or unsubscribe.*

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

joaotavora commented 4 years ago

Yes, in addition you have to add backend to completion-styles.

Oh so you don't support figuring out the style from the completion table's category, which in turn is indicated by its metadata?

joaotavora commented 4 years ago

I haven't looked at the code, but I think that should be relatively easy to do, right?

thierryvolpiatto commented 4 years ago

João Távora notifications@github.com writes:

Yes, in addition you have to add backend to completion-styles.

You so you don't support figuring out the style from the completion table's category, which in turn is indicated by its metadata?

Not yet because I didn't know it was possible, will add soon, thanks.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or unsubscribe.*

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

thierryvolpiatto commented 4 years ago

João Távora notifications@github.com writes:

I haven't looked at the code, but I think that should be relatively easy to do, right?

You are right, I don't have to add backend to completion-styles, it is added automatically, this I guess because I am passing metadata to completion-all-completions and probably you are setting locally completion-category-overrides in sly? So the good news is I have nothing to do :-). Thanks.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or unsubscribe.*

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

thierryvolpiatto commented 4 years ago

So to resume, with these settings I have (native) sly completion with helm:

(progn
  (add-to-list 'load-path "~/tmp/sly")
  (load "~/tmp/sly/sly-autoloads.el")
  (setq inferior-lisp-program "/usr/bin/sbcl")
  (customize-set-variable 'helm-completion-style 'emacs)
  (setq sly-completing-read-function 'completing-read)
  (add-hook 'sly-mode-hook (lambda () (sly-symbol-completion-mode -1))))
joaotavora commented 4 years ago

setting locally completion-category-overrides in sly?

No, but the completion table is returning a sly-completion category, which points to a globally-set completion-category-defaults:

So in sly-completion.el

;; TODO: this `basic' completion style is actually a `backend'
;; completion style, meaning a completion style where the filtering is
;; done entirely by the backend.
(when (boundp 'completion-category-defaults)
  (add-to-list 'completion-category-defaults
               '(sly-completion (styles . (backend)))))

This is quite complicated for now, but should be much easier when the backend completion style makes it into Emacs.

I think @monnier has signalled before that it could (pinging him here). The fact that such an massive package as Helm is now making correct use of this API is a very good argument for the evolution of Emacs completion.

So the good news is I have nothing to do :-).

Yep, for now everything appears dandy! Maybe, very slowly, you could start thinking about defaulting helm-completion-style to emacs and porting some of those fancy completion styles to its "styles" API, so that other frontends can use it. But baby steps :-)

joaotavora commented 4 years ago

sly-symbol-completion-mode is a global var so add-hook shouldn't be needed. And you don't need to set sly-completion-read-function, that variable is gone is the latest SLY master. And the autoloads line is also simpler.

(progn
  (add-to-list 'load-path "~/tmp/sly")
  (require 'sly-autoloads)
  (setq inferior-lisp-program "/usr/bin/sbcl")
  (customize-set-variable 'helm-completion-style 'emacs)
  (sly-symbol-completion-mode -1))
thierryvolpiatto commented 4 years ago

João Távora notifications@github.com writes:

setting locally completion-category-overrides in sly?

No, but the completion table is returning a sly-completion category, which points to a globally-set completion-category-defaults:

So in sly-completion.el

;; TODO: this basic' completion style is actually abackend' ;; completion style, meaning a completion style where the filtering is ;; done entirely by the backend. (when (boundp 'completion-category-defaults) (add-to-list 'completion-category-defaults '(sly-completion (styles . (backend)))))

Ah! Ok, thanks for explanation.

Yep, for now everything appears dandy! Maybe, very slowly, you could start thinking about defaulting helm-completion-style to emacs and porting some of those fancy completion styles to its "styles" API, so that other frontends can use it. But baby steps :-)

A lot of work, I doubt I will find the time for this...

-- Thierry

Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

joaotavora commented 4 years ago

A lot of work, I doubt I will find the time for this...

Yes, I suppose. You can do it incrementally. And maybe reuse some code. Or ask someone to do it for you :-). Anyway, this was a very good step.

thierryvolpiatto commented 4 years ago

Finally did it, I added a style for helm, so now emacs helm-completion style have also regular helm completion. @joaotavora What the backend style expected to do, predating all other styles and working on its own or allowing other styles to work along with it? I can make actually both flex style and helm style working happilly together, but in sly only the sly completion is working predating helm completion (and probably others?) Thanks.

thierryvolpiatto commented 4 years ago

So with this value for metadata: (metadata (display-sort-function . identity) (category . sly-completion)) and I transform it like this (metadata (display-sort-function . identity) (category helm-completion sly-completion)) I expect being able to use both styles, but actually only sly is working.

joaotavora commented 4 years ago
(metadata (display-sort-function . identity) (category helm-completion sly-completion))

SLY's completion table is very limited by the very nature. I don't think it can magically work with any other category that links to a style that is not backend.