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

Complete common part of candidates #170

Closed
bertulli opened this issue May 17, 2022 · 10 comments
Closed

Complete common part of candidates #170

bertulli opened this issue May 17, 2022 · 10 comments

Comments

@bertulli
Copy link

Hi!

First of all, thank you for your work. I am setting up vertico and Corfu, and I like them both.
However, there's something I miss from Company, that is the possibility to complete the common part of the candidates (company-complete-common). With vertico it's easy, there's minibuffer-complete (which is not part of vertico, but alas). I couldn't find in docstrings if it's possible to do the same also for Corfu. Is there a function to do so? Or should I define a custom function, taking inspiration from Company? Fwiw, I can submit a PR if it works.

Thank you for your time

@minad
Copy link
Owner

minad commented May 17, 2022

Thanks! If you set corfu-preselect-first=nil and use the basic completion style then corfu-complete will complete the common part. As soon as you select a candidate, common prefix completion does not make much sense anymore in my opinion. Does this solve your question?

@minad minad closed this as completed May 17, 2022
@bertulli
Copy link
Author

Thank you.
Indeed, this is the workaround I did for now. So, short answer: yes it works, thank you!

Longer answer:
It is ok, but I was wondering if it could be tuned this way:

  • let's say I start typing, and the child buffer pops up
  • the first candidate is already selected and inserted into the buffer
  • pressing <tab> completes the common part
  • pressing RET confirms the choice

That is, having the both of "common completion" and "automatic first candidate insertion".

I find imho that completing the common part may be useful, and I don't bother having it set to <tab>, since I can always confirm the inserted candidate with RET.
I don't want to force a philosophy shift in your package, of course. Still, I'm struggling to define this behavior in my init.el. See, looking at how Company solves the issue:

(defun company-complete ()
  "Insert the common part of all candidates or the current selection.
The first time this is called, the common part is inserted, the second
time, or when the selection has been changed, the selected candidate is
inserted."
  (interactive)
  (when (company-manual-begin)
    (if (or company-selection-changed
            (and (eq real-last-command 'company-complete)
                 (eq last-command 'company-complete-common)))
        (call-interactively 'company-complete-selection)
      (call-interactively 'company-complete-common)
      (when company-candidates
        (setq this-command 'company-complete-common)))))

(defun company-complete-common ()
  "Insert the common part of all candidates."
  (interactive)
  (when (company-manual-begin)
    (if (and (not (cdr company-candidates))
             (equal company-common (car company-candidates)))
        (company-complete-selection)
      (company--insert-candidate company-common))))

seems not too difficult, but I would need to access the completion table (in this case, company-candidates). Is it possible to retrieve it in Corfu?

@minad
Copy link
Owner

minad commented May 18, 2022

The answer lies in the code of corfu-complete:

corfu/corfu.el

Lines 990 to 1009 in 1481b67

;; Try to complete the current input string
(let* ((pt (max 0 (- (point) beg)))
(str (buffer-substring-no-properties beg end))
(metadata (completion-metadata (substring str 0 pt) table pred)))
(pcase (completion-try-completion str table pred pt metadata)
('t
(goto-char end)
(corfu--done str 'finished))
(`(,newstr . ,newpt)
(unless (equal str newstr)
;; bug#55205: completion--replace removes properties!
(completion--replace beg end (concat newstr)))
(goto-char (+ beg newpt))
;; Exit with status 'finished if input is a valid match
;; and no further completion is possible.
(when (and (test-completion newstr table pred)
(not (consp (completion-try-completion
newstr table pred newpt
(completion-metadata newstr table pred)))))
(corfu--done newstr 'finished))))))))

Please read that function.

In principle I am open to changes in behavior of tab completion. But it is not easy to make those without affecting the overall ui. So any kind of proposal which changes the behavior must be well-thought out and take all the configuration variables into account, such that we get somewhat reasonable interactions between the features. (EDIT: Regarding the interactions between different features, we have the configuration variables corfu-preselect-first, corfu-preview-current, corfu-auto, corfu-cycle and more. Also see different configuration modes like TNG. Furthermore it is important that the proposed Corfu behavior is useful across all completion contexts, not only in programming buffers, but for example also in the Eshell.)

@bertulli
Copy link
Author

bertulli commented May 18, 2022

Thanks, you gave me a lot to work on. I managed to write a custom function:

(defun corfu-custom-try-complete ()
  "Try to complete current input as much as possible"
  (interactive)
  (pcase-let ((`(,beg ,end ,table ,pred) completion-in-region--data))
      ;; Try to complete the current input string
      (let* ((pt (max 0 (- (point) beg)))
             (str (buffer-substring-no-properties beg end))
             (metadata (completion-metadata (substring str 0 pt) table pred)))
        (pcase (completion-try-completion str table pred pt metadata)
          ('t
           (goto-char end)
           (corfu--done str 'finished))
          (`(,newstr . ,newpt)
           (unless (equal str newstr)
             ;; bug#55205: completion--replace removes properties!
             (completion--replace beg end (concat newstr)))
           (goto-char (+ beg newpt))
           ;; Exit with status 'finished if input is a valid match
           ;; and no further completion is possible.
           (when (and (test-completion newstr table pred)
                      (not (consp (completion-try-completion
                                   newstr table pred newpt
                                   (completion-metadata newstr table pred)))))
             (corfu--done newstr 'finished)))))))

(define-key corfu-map (kbd "TAB") #'corfu-custom-try-complete)
(define-key corfu-map (kbd "<tab>") #'corfu-custom-try-complete)

I only take corfu-complete, removed the if condition and the then branch, so to keep only the "complete-common" part. It worked in my *scratch* buffer, but not in other ones. Then I added it in my init.el, restarted Emacs, and now it doesn't work in any buffer. I don't know why.
Anyway, you were super kind! Thanks a lot! 👍

EDIT
🎉🎊This works:

(defun complete-test ()
  "Test function."
  (interactive)
  (let* (
	 (current-symbol (thing-at-point 'symbol :no-properties))
	 (bounds (bounds-of-thing-at-point 'symbol))
	 (start (car bounds))
	 (end (cdr bounds))
	 (common-part (try-completion current-symbol corfu--candidates)))
    (if (and (not (cdr corfu--candidates))
             (equal common-part (car corfu--candidates)))
        (corfu-insert)
      (progn
      (delete-region start end)
      (insert common-part)))))

(setq corfu-preselect-first t)
(define-key corfu-map (kbd "TAB") #'complete-test)
(define-key corfu-map (kbd "<tab>") #'complete-test)

For credit, I have taken inspiration from this post by Xah Lee, specifically from "Solution 2". The variable I was looking for was corfu--candidates.
Now I got precisely what I described above, except that the current candidate is not automatically inserted (i.e., it must be confirmed with RET, and it's not inderted if I quit the completion). But actually, this is even better, because having exactly as I said would have been distracting.
Thank you so much for your support!

@liuyinz
Copy link

liuyinz commented Jun 15, 2022

@bertulli , I have a simpler one, like company-complete-common-or-next

  (defun corfu-complete-common-or-next ()
    "Complete common prefix or go to next candidate."
    (interactive)
    (if (= corfu--total 1)
        (progn
          (corfu--goto 1)
          (corfu-insert))
      (let* ((str (car corfu--input))
             (pt (cdr corfu--input))
             (common (try-completion str corfu--candidates)))
        (if (and (stringp common)
                 (not (string= str common)))
            (insert (substring common pt))
          (corfu-next)))))
  (put 'corfu-complete-common-or-next 'completion-predicate #'ignore)
  (define-key corfu-map (kbd "C-n") #'corfu-complete-common-or-next)

@bertulli
Copy link
Author

bertulli commented Jun 16, 2022

Thanks! Yes, your solution better leverages the Corfu variables, so I think it should be preferable. Actually, my function doesn't cycle the completions, but since your code is very clean it's just a matter of commenting/uncommenting the call to (corfu-next)!

There's just one thing I don't get it. Do you mind explaining what the second to last line ((put ...)) does? I'm sorry, but I'm still learning Elisp and I haven't encountered propnames.

EDIT: moreover, strangely enough the common completion doesn't complete punctuation, i.e., typing (wit and then <tab>, it completes to with, even if looking at the overlay all candidates starts with with- (note the dash). But this happens also with my function, so I suppose it's part of what try-completion considers part of the common substring.

@liuyinz
Copy link

liuyinz commented Jun 16, 2022

corfu/corfu.el

Lines 1277 to 1281 in a1638df

;; Emacs 28: Do not show Corfu commands with M-X
(dolist (sym '(corfu-next corfu-previous corfu-first corfu-last corfu-quit corfu-reset
corfu-complete corfu-insert corfu-scroll-up corfu-scroll-down
corfu-insert-separator))
(put sym 'completion-predicate #'ignore))

Hiding corfu-complete-common-or-next from the M-x commandline so that you would not call it accidentally from minibuffer.

for the with- problem, maybe it's your emacs version or some personal setting problem. I could complete to with- at once.

To debug:

(defun corfu-complete-common-or-next ()
    "Complete common prefix or go to next candidate."
    (interactive)
    (let ((c-before corfu--total))
      (if (= corfu--total 1)
          (progn
            (corfu--goto 1)
            (corfu-insert))
        (let* ((str (car corfu--input))
               (pt (cdr corfu--input))
               (common (try-completion str corfu--candidates)))

          (message "str: %s, common: %s" str common)  ;; print var values to check whether it's try-completion problem

          (if (and (stringp common)
                   (not (string= str common)))
              (progn
                (insert (substring common pt))

                (message "candidates---before: %d, after: %d" c-before corfu--total))  ;; check if corfu--total changes

            (corfu-next))))))

@TxGVNN
Copy link

TxGVNN commented Aug 19, 2022

Thank you guys, I'm looking for this feature when migrate from company to corfu.
But I see the corfu-complete-common-or-next doesn't work with file completion.
So I try to copy some code from corfu-complete to mix with corfu-complete-common-or-next and it works.
I actually don't understand too much how it works. You can see I use 2 common variable to see which var is longer.

(defun corfu-complete-common-or-next ()
    "Complete common prefix or go to next candidate."
    (interactive)
    (if (= corfu--total 1)
        (progn
          (corfu--goto 1)
          (corfu-insert))
      (let* ((str (car corfu--input))
             (pt (cdr corfu--input)))
        (pcase-let ((`(,beg ,end ,table ,pred) completion-in-region--data))
          (let* ((metadata (completion-metadata (substring str 0 pt) table pred))
                 (common (car (completion-try-completion str table pred pt metadata)))
                 (common2 (try-completion str corfu--candidates)))
            (message "DEBUG: common=%s, common2=%s, metadata=%s" common common2 metadata)
            (if (> (length common2) (length common))
                (setq common common2))
            (if (and (stringp common)
                     (not (string= str common)))
                (insert (substring common pt))))))))

@liuyinz
Copy link

liuyinz commented Aug 19, 2022

It really doesn't work for filepath completion after I tested, maybe I will fix it when i'm not busy after a few days.

Quick fix:

  (defun corfu-complete-common-or-next ()
    "Complete common prefix or go to next candidate."
    (interactive)
    (if (= corfu--total 1)
        (progn
          (corfu--goto 1)
          (corfu-insert))
      (let* ((input (car corfu--input))
             (str (if (thing-at-point 'filename) (file-name-nondirectory input) input))
             (pt (length str))
             (common (try-completion str corfu--candidates)))
        (if (and (> pt 0)
                 (stringp common)
                 (not (string= str common)))
            (insert (substring common pt))
          (corfu-next)))))

@minad
Copy link
Owner

minad commented Mar 26, 2024

I've just added corfu-expand in f30b15b. See also https://github.com/minad/corfu?tab=readme-ov-file#expanding-to-the-common-candidate-prefix-with-tab.

corfu/corfu.el

Lines 1296 to 1302 in afc8895

(defun corfu-expand ()
"Expands the common prefix of all candidates.
If the currently selected candidate is previewed, invoke
`corfu-complete' instead. Expansion relies on the completion
styles via `completion-try-completion'. Return non-nil if the
input has been expanded."
(interactive)

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

No branches or pull requests

4 participants