bug#70381: [PATCH] New command 'completion-preview-complete'

[Eshel Yaron]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: Ergus <spacibba@aol.com>
>> Date: Sun, 14 Apr 2024 13:21:54 +0200
>> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>>
>> Following a recent discussion on emacs-devel[0], this patch adds a new
>> command for Completion Preview mode that completes the symbol at point up
>> to the longest common prefix of all completion candidates.  This patch
>> also adds a visual indication for the longest common prefix when showing
>> it as part of the completion preview, so the user can tell how far the new
>> 'completion-preview-complete' will complete before invoking this command.
>> For example, if the symbol at point is "foo", and the completion
>> candidates are "foobar" and "foobaz", then the preview shows "bar" with
>> the common prefix "ba" highlighted in face 'completion-preview-common'.
>
> Thanks.
>
>> * lisp/completion-preview.el (completion-preview--try-table):
>> Return longest common prefix and list of suffixes instead of
>> list of full candidates.  Add illustrative comment.
>> (completion-preview--capf-wrapper, completion-preview--update)
>> (completion-preview--show, completion-preview-insert)
>> (completion-preview-next-candidate): Adjust.
>> (completion-preview-common): New face.
>> (completion-preview-exact): Distinguish from 'c-p-common'.
>> (completion-preview-complete): New command.   ^^^^^^^^^^
>> (completion-preview-active-mode-map): Bind it.
>> (completion-preview-mode): Mention it in docstring.
>> (completion-preview-commands): Add 'c-p-complete'.
>                                       ^^^^^^^^^^^^
> Please don't abbreviate symbols in the log entries.  Those are
> frequently used to search for changes of functions/variables, and such
> abbreviations defeat those searches.

All right, fixed in the updated patch below.

> If you are annoyed by the need to type long strings, I usually find
> M-/ instrumental in reducing that annoyance considerably.
>
>> +(defface completion-preview-common
>>    '((((supports :underline t))
>>       :underline t :inherit completion-preview)
>>      (((supports :weight bold))
>>       :weight bold :inherit completion-preview)
>>      (t :background "gray"))
>> -  "Face for exact completion preview overlay."
>> +  "Face for completions longest common prefix in the completion preview."
>                ^^^^^^^^^^^
> This word is redundant here.  I'd replace it with "the".

Done.

>> +(defvar-local completion-preview--inhibit-update-p nil
>> +  "Whether to inhibit updateing the completion preview following this 
>> command.")
>                          ^^^^^^^^^
> "updating"

Fixed.

>> +      (set-text-properties 0 (length suffix)
>> +                           `(face ,(if (cdr suffixes)
>> +                                       'completion-preview
>> +                                     'completion-preview-exact))
>> +                           suffix)
>> +      (set-text-properties 0 (length common)
>> +                           `(face ,(if (cdr suffixes)
>> +                                       'completion-preview-common
>> +                                     'completion-preview-exact))
>> +                           common)
>
> Is the use of back-ticks really necessary here?
>
>> +      (set-text-properties 0 (length suffix)
>> +                           `(face ,(if (cdr sufs)
>> +                                       'completion-preview
>> +                                     'completion-preview-exact))
>> +                           suffix)
>
> Likewise here (and in few other places).

Not necessary, no.  So I've changed these to regular (list ...) forms.
I kept a couple of other backticks that do make the code clearer IMO.


Thanks for taking a look, here's the updated patch:

v2-0001-New-command-completion-preview-complete.patch
Description: Text Data