bug#74410: 31.0.50; completion-at-point doesn't work in custom buffers

bug#74410: 31.0.50; completion-at-point doesn't work in custom buffers

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Package: Emacs
Version: 31.0.50


For example:

    src/emacs -Q --eval '(customize-face `fixed-pitch)'
    ... move point to the font family field ...
    ... delete the "space" from "Monospace" then do:
    M-x completion-at-point RET

This will presumably do nothing at all.
`widget-complete` completes it back to "Monospace" (at least on
`master` where I installed a patch for that).

The problem is that cus-edit doesn't setup
`completion-at-point-functions`.  Instead it sets up a special keymap to
remap M-TAB to `widget-complete` instead of relying on the
global binding.

It works OK for a default config, but it doesn't interact well with
setups that use different keybindings or different completion UIs based
on `completion-at-point-functions`.  E.g. `corfu-mode` partly works but
not fully (e.g. `corfu-auto` doesn't have any effect).

The patch below leaves the key remapping for now, but adds an
appropriate function to `completion-at-point-functions` so that
other UIs such as `completion-at-point` can do their job properly.

Comments/objection?


        Stefan


diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index e0668d4ba86..02e0b541f05 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -5503,7 +5503,8 @@ Custom-mode
   (make-local-variable 'custom-options)
   (make-local-variable 'custom-local-buffer)
   (custom--initialize-widget-variables)
-  (add-hook 'widget-edit-functions 'custom-state-buffer-message nil t))
+  (add-hook 'completion-at-point-functions #'widget-completions-at-point nil t)
+  (add-hook 'widget-edit-functions #'custom-state-buffer-message nil t))
 
 (defun custom--revert-buffer (_ignore-auto _noconfirm)
   (unless custom--invocation-options
diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 58a36820ac7..1538720adef 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -1399,7 +1399,7 @@ widget-complete
   "Complete content of editable field from point.
 When not inside a field, signal an error."
   (interactive)
-  (let ((data (widget-completions-at-point)))
+  (let ((data (widget-completions-at-point 'error)))
     (cond
      ((functionp data) (funcall data))
      ((consp data)
@@ -1414,11 +1414,11 @@ widget-complete
 ;; so it's not really obsolete (yet).
 ;; (make-obsolete 'widget-complete 'completion-at-point "24.1")
 
-(defun widget-completions-at-point ()
+(defun widget-completions-at-point (&optional error)
   (let ((field (widget-field-find (point))))
     (if field
         (widget-apply field :completions-function)
-      (error "Not in an editable field"))))
+      (if error (error "Not in an editable field")))))
 
 ;;; Setting up the buffer.
 

bug#74410: 31.0.50; completion-at-point doesn't work in custom buffers

[Juri Linkov]

> For example:
>
>     src/emacs -Q --eval '(customize-face `fixed-pitch)'
>     ... move point to the font family field ...
>     ... delete the "space" from "Monospace" then do:
>     M-x completion-at-point RET
>
> This will presumably do nothing at all.
> `widget-complete` completes it back to "Monospace" (at least on
> `master` where I installed a patch for that).
>
> The problem is that cus-edit doesn't setup
> `completion-at-point-functions`.  Instead it sets up a special keymap to
> remap M-TAB to `widget-complete` instead of relying on the
> global binding.
>
> It works OK for a default config, but it doesn't interact well with
> setups that use different keybindings or different completion UIs based
> on `completion-at-point-functions`.  E.g. `corfu-mode` partly works but
> not fully (e.g. `corfu-auto` doesn't have any effect).
>
> The patch below leaves the key remapping for now, but adds an
> appropriate function to `completion-at-point-functions` so that
> other UIs such as `completion-at-point` can do their job properly.
>
> Comments/objection?

Now finally it's possible to select a completion using arrow keys
like for in-buffer completions, thanks.

Currently this works only when typing 'M-x completion-at-point RET',
not by 'M-C-i' that is bound to 'widget-complete'.  Maybe
'M-C-i' should be rebound to 'completion-at-point' in widgets?

bug#74410: 31.0.50; completion-at-point doesn't work in custom buffers

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Currently this works only when typing 'M-x completion-at-point RET',
> not by 'M-C-i' that is bound to 'widget-complete'.  Maybe
> 'M-C-i' should be rebound to 'completion-at-point' in widgets?

Maybe it would make even more sense to just remove the `widget-complete`
binding since M-TAB should already be bound appropriately, but the
problem is that we don't know that the code which inserts the widget
into a buffer has properly setup the `completion-at-point-functions`
hook for `completion-at-point` to work.

Maybe we should try and detect when that is not the case and emit
a warning if so (with the hope to be able to obsolete `widget-complete`
in some future version).


        Stefan

bug#74410: 31.0.50; completion-at-point doesn't work in custom buffers

[Juri Linkov]

>> Currently this works only when typing 'M-x completion-at-point RET',
>> not by 'M-C-i' that is bound to 'widget-complete'.  Maybe
>> 'M-C-i' should be rebound to 'completion-at-point' in widgets?
>
> Maybe it would make even more sense to just remove the `widget-complete`
> binding since M-TAB should already be bound appropriately, but the
> problem is that we don't know that the code which inserts the widget
> into a buffer has properly setup the `completion-at-point-functions`
> hook for `completion-at-point` to work.
>
> Maybe we should try and detect when that is not the case and emit
> a warning if so (with the hope to be able to obsolete `widget-complete`
> in some future version).

Or to make the binding conditional with something like this:

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index ba99847f488..0701442447c 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -1079,7 +1079,10 @@ widget-global-map
 (defvar widget-field-keymap
   (let ((map (copy-keymap widget-keymap)))
     (define-key map "\C-k" #'widget-kill-line)
-    (define-key map "\M-\t" #'widget-complete)
+    (define-key map "\M-\t" `(menu-item "" 'widget-complete
+                              :filter ,(lambda (cmd)
+                                         (unless completion-at-point-functions
+                                           cmd))))
     (define-key map "\C-m" #'widget-field-activate)
     ;; Since the widget code uses a `field' property to identify fields,
     ;; ordinary beginning-of-line does the right thing.

bug#74410: 31.0.50; completion-at-point doesn't work in custom buffers

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Or to make the binding conditional with something like this:
>
> diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
> index ba99847f488..0701442447c 100644
> --- a/lisp/wid-edit.el
> +++ b/lisp/wid-edit.el
> @@ -1079,7 +1079,10 @@ widget-global-map
>  (defvar widget-field-keymap
>    (let ((map (copy-keymap widget-keymap)))
>      (define-key map "\C-k" #'widget-kill-line)
> -    (define-key map "\M-\t" #'widget-complete)
> +    (define-key map "\M-\t" `(menu-item "" 'widget-complete
> +                              :filter ,(lambda (cmd)
> +                                         (unless completion-at-point-functions
> +                                           cmd))))
>      (define-key map "\C-m" #'widget-field-activate)
>      ;; Since the widget code uses a `field' property to identify fields,
>      ;; ordinary beginning-of-line does the right thing.

Cute 🙂
I really like the fact that Emacs can do this, but we shouldn't abuse
it, because it makes it more difficult for users to figure out
what's going on.


        Stefan

bug#74410: 31.0.50; completion-at-point doesn't work in custom buffers

[Juri Linkov]

>> Or to make the binding conditional with something like this:
>>
>> @@ -1079,7 +1079,10 @@ widget-global-map
>>  (defvar widget-field-keymap
>>    (let ((map (copy-keymap widget-keymap)))
>>      (define-key map "\C-k" #'widget-kill-line)
>> -    (define-key map "\M-\t" #'widget-complete)
>> +    (define-key map "\M-\t" `(menu-item "" 'widget-complete
>> +                              :filter ,(lambda (cmd)
>> +                                         (unless completion-at-point-functions
>> +                                           cmd))))
>>      (define-key map "\C-m" #'widget-field-activate)
>>      ;; Since the widget code uses a `field' property to identify fields,
>>      ;; ordinary beginning-of-line does the right thing.
>
> Cute 🙂
> I really like the fact that Emacs can do this, but we shouldn't abuse
> it, because it makes it more difficult for users to figure out
> what's going on.

Indeed, this was just an example.  But it would be nice to do the same
one way or another.  Maybe 'widget-complete' itself should check for
'completion-at-point-functions', or something like this.