bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options

[Philip Kaludercic <philipk@posteo.net>]

john muhl <jm@pub.pink> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: john muhl <jm@pub.pink>
>>> Cc: 74235@debbugs.gnu.org
>>> Date: Fri, 08 Nov 2024 08:57:37 -0600
>>> 
>>> Eli Zaretskii <eliz@gnu.org> writes:
>>> 
>>> >> From: john muhl <jm@pub.pink>
>>> >> Date: Wed, 06 Nov 2024 15:22:34 -0600
>>> >> 
>>> >> * lisp/progmodes/lua-ts-mode.el (lua-ts-luacheck-program):
>>> >> (lua-ts-inferior-program): Remove 'nil' as it is never a valid
>>> >> choice.  (Bug#74235)
>>> >
>>> > IMO, the use of these options in the code is not clean: if these
>>> > programs are not installed, the commands which reference these options
>>> > will signal errors whose diagnostic might not clearly indicate the
>>> > root cause.  I would like to see a cleaner handling of such
>>> > contingencies by this mode.  Does this make sense, and if so, would
>>> > you like to suggest changes to that effect?
>>> 
>>> The only idea I’ve come up with so far is showing a warning that
>>> mentions the incorrectly set option. Does that sound like what you
>>> have in mind? If not I’d appreciate some more explicit guidance.
>>
>> How about signaling user-error from the commands that invoke those
>> programs, when they are missing?
>
> Agree that’s better than a warning. It seems flymake already
> handles the case where it can’t find the requested program so I
> left that part as is.
>
> From 3e49aa1086f2498ccbad396cf8c799c57919847e Mon Sep 17 00:00:00 2001
> From: john muhl <jm@pub.pink>
> Date: Sat, 9 Nov 2024 11:01:45 -0600
> Subject: [PATCH] ; Fix some 'lua-ts-mode' options (Bug#74235)
>
> * lisp/progmodes/lua-ts-mode.el (lua-ts-luacheck-program):
> (lua-ts-inferior-program): Switch to 'file' type and remove 'nil'
> as a choice.
> (lua-ts-inferior-lua): Ensure 'lua-ts-inferior-program' is a
> valid executable.
> ---
>  lisp/progmodes/lua-ts-mode.el | 85 ++++++++++++++++++-----------------
>  1 file changed, 44 insertions(+), 41 deletions(-)
>
> diff --git a/lisp/progmodes/lua-ts-mode.el b/lisp/progmodes/lua-ts-mode.el
> index 4ea453c9b65..2323461ed4c 100644
> --- a/lisp/progmodes/lua-ts-mode.el
> +++ b/lisp/progmodes/lua-ts-mode.el
> @@ -72,7 +72,7 @@ lua-ts-indent-offset
>  
>  (defcustom lua-ts-luacheck-program "luacheck"
>    "Location of the Luacheck program."
> -  :type '(choice (const :tag "None" nil) string)
> +  :type 'file
>    :version "30.1")
>  
>  (defcustom lua-ts-inferior-buffer "*Lua*"
> @@ -83,7 +83,7 @@ lua-ts-inferior-buffer
>  
>  (defcustom lua-ts-inferior-program "lua"
>    "Program to run in the inferior Lua process."
> -  :type '(choice (const :tag "None" nil) string)
> +  :type 'file
>    :version "30.1")
>  
>  (defcustom lua-ts-inferior-options '("-i")
> @@ -632,47 +632,50 @@ lua-ts-flymake-luacheck
>  (defun lua-ts-inferior-lua ()
>    "Run a Lua interpreter in an inferior process."
>    (interactive)
> -  (unless (comint-check-proc lua-ts-inferior-buffer)
> -    (apply #'make-comint-in-buffer
> -           (string-replace "*" "" lua-ts-inferior-buffer)
> -           lua-ts-inferior-buffer
> -           lua-ts-inferior-program
> -           lua-ts-inferior-startfile
> -           lua-ts-inferior-options)
> -    (when lua-ts-inferior-history
> +  (if (or (not lua-ts-inferior-program)
> +          (not (executable-find lua-ts-inferior-program)))
> +      (user-error "You must set `lua-ts-inferior-program' to use this command")

Isn't this error message misleading as `lua-ts-inferior-program' can be
set (as it is by default), but the executable couldn't be located (which
isn't a mistake by the user, but just due to Lua not being installed)?

> +    (unless (comint-check-proc lua-ts-inferior-buffer)
> +      (apply #'make-comint-in-buffer
> +             (string-replace "*" "" lua-ts-inferior-buffer)
> +             lua-ts-inferior-buffer
> +             lua-ts-inferior-program
> +             lua-ts-inferior-startfile
> +             lua-ts-inferior-options)
> +      (when lua-ts-inferior-history
>          (set-process-sentinel (get-buffer-process lua-ts-inferior-buffer)
>                                'lua-ts-inferior--write-history))
> -    (with-current-buffer lua-ts-inferior-buffer
> -      (setq-local comint-input-ignoredups t
> -                  comint-input-ring-file-name lua-ts-inferior-history
> -                  comint-prompt-read-only t
> -                  comint-prompt-regexp (rx-to-string `(: bol
> -                                                         ,lua-ts-inferior-prompt
> -                                                         (1+ space))))
> -      (comint-read-input-ring t)
> -      (add-hook 'comint-preoutput-filter-functions
> -                (lambda (string)
> -                  (if (equal string (concat lua-ts-inferior-prompt-continue " "))
> -                      string
> -                    (concat
> -                     ;; Filter out the extra prompt characters that
> -                     ;; accumulate in the output when sending regions
> -                     ;; to the inferior process.
> -                     (replace-regexp-in-string (rx-to-string
> -                                                `(: bol
> -                                                    (* ,lua-ts-inferior-prompt
> -                                                       (? ,lua-ts-inferior-prompt)
> -                                                       (1+ space))
> -                                                    (group (* nonl))))
> -                                               "\\1" string)
> -                     ;; Re-add the prompt for the next line.
> -                     lua-ts-inferior-prompt " ")))
> -                nil t)))
> -  (select-window (display-buffer lua-ts-inferior-buffer
> -                                 '((display-buffer-reuse-window
> -                                    display-buffer-pop-up-window)
> -                                   (reusable-frames . t))))
> -  (get-buffer-process (current-buffer)))
> +      (with-current-buffer lua-ts-inferior-buffer
> +        (setq-local comint-input-ignoredups t
> +                    comint-input-ring-file-name lua-ts-inferior-history
> +                    comint-prompt-read-only t
> +                    comint-prompt-regexp (rx-to-string `(: bol
> +                                                           ,lua-ts-inferior-prompt
> +                                                           (1+ space))))

This should be equivalent to

  (rx bol (literal lua-ts-inferior-prompt) (1+ space))

that macroexpands to

  (concat "^" (regexp-quote inferior-lisp-program) "[[:space:]]+")

without the need to load rx at run-time.

> +        (comint-read-input-ring t)
> +        (add-hook 'comint-preoutput-filter-functions
> +                  (lambda (string)
> +                    (if (equal string (concat lua-ts-inferior-prompt-continue " "))
> +                        string
> +                      (concat
> +                       ;; Filter out the extra prompt characters that
> +                       ;; accumulate in the output when sending regions
> +                       ;; to the inferior process.
> +                       (replace-regexp-in-string
> +                        (rx-to-string `(: bol
> +                                          (* ,lua-ts-inferior-prompt
> +                                             (? ,lua-ts-inferior-prompt)
> +                                             (1+ space))
> +                                          (group (* nonl))))
> +                        "\\1" string)
> +                       ;; Re-add the prompt for the next line.
> +                       lua-ts-inferior-prompt " ")))
> +                  nil t)))
> +    (select-window (display-buffer lua-ts-inferior-buffer
> +                                   '((display-buffer-reuse-window
> +                                      display-buffer-pop-up-window)
> +                                     (reusable-frames . t))))
> +    (get-buffer-process (current-buffer))))
>  
>  (defun lua-ts-send-buffer ()
>    "Send current buffer to the inferior Lua process."

-- 
	Philip Kaludercic on siskin