Re: Add completion to compilation-read-command

[Philip Kaludercic <philipk@posteo.net>]

Spyros Roum <spyros.roum@posteo.net> writes:

> Philip Kaludercic wrote:
>>> To make sure I understand correctly, when using `completing-read` to
>>> read user input, that is narrowing,
>>> while the `C-x <up>` and *Completions* buffer is completion.
>> No, `completing-read' just invokes an interface, that frontends can
>> implement a UI for.  The default UI provides a completing/expanding
>> interface, while Vertico, Helm, Ivy, etc. do selecting/narrowing.  When
>> using `completing-read' you cannot really assume one or the other, so in
>> effect one has to find a middle ground.  It is best you try your code in
>> emacs -Q without any changes and see how it behaves.  Things like SPC
>> doing something else than you would expect is just one pitfall, others
>> I can recall are providing text that is difficult to input (I wrote a
>> package a few years back called "insert-kaomoji" that used
>> `completing-read' to prompt the user eastern-style emoticons; it is easy
>> to use with a selecting framework, but more inconvenient if the user is
>> first made to complete a string that is difficult to write, as most of
>> the characters are not easy to type).
>
> I tried the attached patch with `emacs -q` and now I understand how
> this would be very annoying without something like vertico installed
> haha. Nevertheless, it seems to work!

Of course it will work, after all it is just text (input) with some
convenient bindings for easy completion.  Even if SPC is bound to
something else, you can always use C-q SPC.

> I'm attaching a patch based on the conversation so far to get feedback on.
>
> I believe I should also write something to the NEWS file.

Right, you can always check the CONTRIBUTE file in the emacs.git root.

> Also, I'm wondering if this counts as non-trivial enough that I would
> need to sign the FSF copyright.

It looks like it to me, and either way it is not bad to do that so that
you have it behind you for future contributions either to Emacs or ELPA.

> Regarding testing, as far as I can tell, there are currently no tests
> for `compilation-read-command`.
> Should I add anything? If yes, I'll probably need someone to point to
> existing tests for similar things that I can copy from.

You could take a look at test/lisp/progmodes/compile-tests.el, but I
don't think this breaks the patch, especially when dealing with
something as interactive as what you are proposing.

>
> Am I forgetting anything else?
> From e1068206662913978d541f924205a0615f8d2d95 Mon Sep 17 00:00:00 2001
> From: Spyros Roum <spyros.roum@posteo.net>
> Date: Wed, 25 Dec 2024 17:32:31 +0200
> Subject: [PATCH] Add option for making compilation-read-command use
>  completing-read

A ChageLog style commit message would be expected here as well.  You can
inject the structure in vc-mode using C-c C-w.

>
> ---
>  lisp/progmodes/compile.el | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
> index 6784a12fd63..a318937e96d 100644
> --- a/lisp/progmodes/compile.el
> +++ b/lisp/progmodes/compile.el
> @@ -1797,12 +1797,34 @@ compilation-mode-font-lock-keywords
>     '((compilation--ensure-parse))
>     compilation-mode-font-lock-keywords))
>  
> -(defun compilation-read-command (command)
> +(defun compilation-prompt-read-shell-command (command)
>    (read-shell-command "Compile command: " command
>                        (if (equal (car compile-history) command)
>                            '(compile-history . 1)
>                          'compile-history)))
>  
> +(defun compilation-prompt-read-command-with-completion (command)
> +  (completing-read "Compile command: " compile-history
> +                   nil nil command
> +                   (if (equal (car compile-history) command)
> +                       '(compile-history . 1)
> +                     'compile-history)))
> +
> +(defcustom compilation-read-command-function
> +  'compilation-prompt-read-shell-command

Please sharp-quote (#').

> +  "Function used by `compilation-read-command' to get user's input.
> +Defaults to `compilation-prompt-read-shell-command',
> +but `compilation-prompt-read-command-with-completion' can be used instead for

If possible, avoid the passive phrase here.

> +a completing version based on past runs."
> +  :version "31.0"
> +  :type 'function
> +  :options '(compilation-prompt-read-command-with-completion))

It would also be good to add the default value here.  I prefer using
(list ...) and sharp-quoting all the function names, so that the
byte-compiler verifies that these are all known.

> +
> +(defun compilation-read-command (command)
> +  "Prompt user for command to run.
> +`compilation-read-command-function' controls the way input is read from the minibuffer."
> +  (funcall compilation-read-command-function command))
> +
>  \f
>  ;;;###autoload
>  (defun compile (command &optional comint)