Re: Add completion to compilation-read-command

[Spyros Roum <spyros.roum@posteo.net>]

[-- Attachment #1.1: Type: text/plain, Size: 2368 bytes --]

Philip Kaludercic wrote:
>> 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.
You are right, I have initiated the process.
>> 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.

I did check compile-tests.el and found no references to 
`compilation-read-command`,
that's why I said I think it's not tested currently.
I'll not add any tests, I wouldn't even know how to test something 
interactive like that.

>> 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.

I'm not sure the level of detail I should go in describing each function 
I touch, so I wrote (what I think is) a lot. Let me know if it's not 
necessary or if it's fine as is. I did see some examples from the 
wiki/other commits and I think it's fine.

>> +  "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.

I changed it to active voice. I'll also say I'm not a native speaker, so 
if you think
it's still weird/not good enough, please let me know.

I've attached a new draft based on your feedback. Additionally, I've 
renamed the new function to something that describes that completion 
happens based on history, according to Juri's input.

One additional question: The NEWS file is for version 31.1. Should the 
`:version` tag also be for 31.1 instead of 31?

[-- Attachment #1.2: Type: text/html, Size: 4095 bytes --]

[-- Attachment #2: 0001-Add-option-for-making-compilation-read-command-use-c.patch --]
[-- Type: text/x-patch, Size: 3321 bytes --]

From 7e140c1ab5bfc7440753ab3aff2c3ce7eb38414e 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

* etc/NEWS: Add to NEWS
* lisp/progmodes/compile.el (compilation-read-command):
Call function based on `compilation-read-command-function`.

(compilation-prompt-read-shell-command): The existing
functionality from compilation-read-command extracted to
a function.

(compilation-prompt-read-with-history-completion): A
function that uses completing-read to read the command.

(compilation-read-command-function): The new option that
controlls which function is used.
---
 etc/NEWS                  |  8 ++++++++
 lisp/progmodes/compile.el | 28 +++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index ca107bb4938..cfb137c2399 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -147,6 +147,14 @@ will still be on that candidate after "*Completions*" is updated with a
 new list of completions.  The candidate is automatically deselected when
 the "*Completions*" buffer is hidden.
 
+---
+*** New user option 'compilation-read-command-function'.
+This option controls what function is used to read user input for
+'compilation-read-command'.
+It defaults to 'compilation-prompt-read-shell-command', which preserves
+existing behavior.  When set to 'compilation-prompt-read-with-history-completion',
+'completing-read' is used allowing autocomplete based on past runs of 'compile'.
+
 ** Windows
 
 +++
diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 6784a12fd63..d01e8c69017 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -1797,12 +1797,38 @@ 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-with-history-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
+  "`compilation-read-command' uses this function to get user's input.
+Defaults to `compilation-prompt-read-shell-command',
+but 'compilation-prompt-read-with-history-completion' can be used instead for
+a completing version based on past runs."
+  :version "31.0"
+  :type 'function
+  :options
+  (list
+    #'compilation-prompt-read-shell-command
+    #'compilation-prompt-read-with-history-completion))
+
+(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)
-- 
2.47.1