joaotavora / sly

Sylvester the Cat's Common Lisp IDE
1.24k stars 140 forks source link

sly-completing-read and Vertico mode #473

Closed andreyorst closed 2 years ago

andreyorst commented 2 years ago

Hi. This is more a question, rather than an issue, although maybe it should be fixed in Sly, or maybe elsewhere. I'm using the Vertico package, which provides vertical completion UI for everything in Emacs. I've noticed, however, that Sly uses IDO mode when I press , in the REPL buffer. Inspecting the code I see that the sly-completing-read function checks exactly for two conditions - whether completing-read-function equals to 'completing-read-default, and that icomplete-mode is disabled. If both are true, ido-completing-read is used:

https://github.com/joaotavora/sly/blob/540a8c5b9a04af0a6907e07cb070f1fed8a76f48/lib/sly-messages.el#L95-L101

This prevents Vertico from working here, as it doesn't change the completing-read-function, and there's no check for vertico-mode either.

Now, I think it would be not the best approach to include every single completion UI mode into this function, so I'm not proposing adding a check for vertico-mode here. However, I'd like to use Vertico for this menu, for consistency's sake. I can use the advice system to make it work, but it seems kinda hacky for this particular case:

(define-advice sly-completing-read (:around (fn &rest args))
  (let ((icomplete-mode t))
    (apply fn args)))

Any suggestions or thoughts? Feel free to close it if you think that Sly does everything correctly here, and this is a user configuration/Vertico problem.

joaotavora commented 2 years ago

Yes you are right.

I think as time progresses, more people will become aware of things better than Ido and then we can probably kill that block completely. You can probably just set sly-completing-read to completing-read...

btw if you like vertico-mode (it's nice!) , then give fido-vertical-mode (in emacs 28) a try and tell me what you think.

andreyorst commented 2 years ago

btw if you like vertico-mode (it's nice!) , then give fido-vertical-mode (in emacs 28) a try and tell me what you think.

I've tried fido-vertical-mode, and its performance is not as good as I'd like it to be. The completion candidate list appears after a slight delay, which also can be noticed after each keypress. This actually happens in fido-mode without the vertical UI, so I guess, it's an unrelated problem, probably caused by matching in any place in the substring. E.g. typing complete in M-x yields:

image

Whereas Verctico only matches at the start of the string:

image

And only shows the rest if I move the cursor to the start of the prompt:

image

I'm not sure if fido-mode does its own matching, but Vertico uses completion styles, and some of the above trickery. There's also icomplete-mode, and the icomplete-vertical-mode is a bit more performant than fido-vertical-mode, but I don't understand icomplete completion semantics.

I think as time progresses, more people will become aware of things better than Ido and then we can probably kill that block completely. You can probably just set sly-completing-read to completing-read...

Yes, seems that doing (fset 'sly-completing-read 'completing-read) works fine

joaotavora commented 2 years ago

fido-vertical-modeis doing always the flex completion style, which is a bit slower. vertico-mode seems to use prefix in the beginning, for speed, then change to flex if you move the cursor elsewhere. Not sure I'd like that tbh. But that's probably what causes the speed difference.

andreyorst commented 2 years ago

I don't have flex in my completion styles: (partial-completion basic), and Vertico only uses completion styles for filtering, so I guess it's using partial completion in the screenshot above.

joaotavora commented 2 years ago

fido-vertical-mode is overriding your completion styles :-) It's a flex-lover because it's "ido" emulation, and ido used to use flex. You can re-override them if you want.

andreyorst commented 2 years ago

fido-vertical-mode is overriding your completion styles :-) It's a flex-lover because it's "ido" emulation, and ido used flex. You can re-override them if you want.

Ah, I only meant that Vertico doesn't use flex when I move the cursor, as changing completion styles affects the resulting completion list when I move the cursor. For example, here's the behavior with completion-styles set to (emacs21):

image

No flex matching or anything special happening here.


Anyway, this is off-topic of the original problem. I think the issue is fixed for me with (fset 'sly-completing-read 'completing-read), but you've said that you may remove this function in the future, so I'm not sure if I should close this issue, or you'd want to leave it open until the change.

monnier commented 2 years ago

I think as time progresses, more people will become aware of things better than Ido and then we can probably kill that block completely. You can probably just set sly-completing-read to completing-read...

Or maybe check for ido-mode (i.e. check for positive signs that the user wants IDO, rather than positive signs that the user wants something else).

Or just don't push IDO at all, and let users who want it use ido-everywhere, ido-ubiquitous, or something like that.

    Stefan
joaotavora commented 2 years ago

Or maybe check for ido-mode (i.e. check for positive signs that the user wants IDO, rather than positive signs that the user wants something else).

That code was done when people (for people that) weren't even aware of IDO.

This showed that Emacs had IDE-like, visually responsive, capabilities to lots of old-timer SLIME users/switchers for example.

THe code is obsolete now, it should probably just be deleted.

andreyorst commented 2 years ago

Thanks!