joaotavora / sly

Sylvester the Cat's Common Lisp IDE
1.23k stars 139 forks source link

Sly completion UI issue: small window size and inconsistent highlighting #597

Open Adomas-beep-boop-maggots opened 1 year ago

Adomas-beep-boop-maggots commented 1 year ago

Hello, I've tried various settings and the completion UI window is sometimes too small (see 2nd picture) and the highlighting does not match the text that we are trying to complete (see the 1st picture). Note that the 3nd picture is not sly-mode, but i added to illustrate how default emacs completion highlighting should look like

1. 20230509_08h31m52s_grim

(setq sly-complete-symbol-function 'sly-simple-completions
      temp-buffer-resize-mode nil
      sly-symbol-completion-mode t)

2. 20230509_08h44m46s_grim

(setq sly-complete-symbol-function 'sly-simple-completions
      temp-buffer-resize-mode t
      sly-symbol-completion-mode t)

3. 20230509_08h48m06s_grim

(setq temp-buffer-resize-mode t)
Adomas-beep-boop-maggots commented 1 year ago

I just noticed that completion highlighting syntax problem only occurs with sly-simple-completions, which i also observed that with this setting i cannot complete a symbol, of there is single match available. I believe this is not intended behavior. Also i wish there was a function for sly-complete-symbol-function which uses completion-styles set by user, instead of using pre-defined completion functions like sly-complete-symbol-function and sly-flex-symbol-function if that makes sense.

Adomas-beep-boop-maggots commented 1 year ago

im not that good at elisp, but i at tried to fix the highlighting: sly-flex-completions also needs a bit of tweaking

(defun sly-simple-completions (prefix)
  "Return (COMPLETIONS COMMON) where COMPLETIONS complete the PREFIX.
COMPLETIONS is a list of propertized strings.
COMMON a string, the common prefix."
  (cl-loop with first-difference-pos = (length prefix)
           with (completions common) =
           (sly--completion-request-completions prefix 'slynk-completion:simple-completions)
           for completion in completions
           do (let ((diff-pos (1+ first-difference-pos)))
                (put-text-property 0
                                   (- diff-pos 1)
                                   'face 'completions-common-part
                                   completion))
       do (let ((diff-pos (1+ first-difference-pos)))
                (put-text-property first-difference-pos 
                                   (min diff-pos (1- (length completion)))
                                   'face 'completions-first-difference
                                   completion))
           collect completion into formatted
           finally return (list formatted common)))

20230510_20h12m57s_grim

joaotavora commented 1 year ago

Submit a patch for the highlighting. Read CONTRIBUTING.md first.

As for the temp buffer resize mode, it doesn't seem to make anything useful in my testing, so my advice is not to use it. But you could try (setq sly--completion-explanation "") to see if that improves things. If it does, I guess we can export that variable.

Adomas-beep-boop-maggots commented 1 year ago

Submit a patch for the highlighting. Read CONTRIBUTING.md first.

As for the temp buffer resize mode, it doesn't seem to make anything useful in my testing, so my advice is not to use it. But you could try (setq sly--completion-explanation "") to see if that improves things. If it does, I guess we can export that variable.

When ill have time ill submit complete fix for sly-simple-completions and sly-flex-completions highlighting. also i want to add separate patch for when using default ui (without sly-symbol-completion-mode). the code should be added which handles completion of longest common denominator, cause right now the completion ui pops up, but you have to type everything manually. Sure completion frameworks that i tried, like corfu, handles the latter case, but i think default completion ui should do it too