Package: emacs;
Reported by: john muhl <jm <at> pub.pink>
Date: Wed, 6 Nov 2024 21:21:01 UTC
Severity: normal
Tags: patch
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
Message #29 received at 74235 <at> debbugs.gnu.org (full text, mbox):
From: Philip Kaludercic <philipk <at> posteo.net> To: john muhl <jm <at> pub.pink> Cc: Eli Zaretskii <eliz <at> gnu.org>, 74235 <at> debbugs.gnu.org Subject: Re: bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options Date: Sun, 10 Nov 2024 05:45:14 +0000
john muhl <jm <at> pub.pink> writes:
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: john muhl <jm <at> pub.pink>
>>> Cc: 74235 <at> debbugs.gnu.org
>>> Date: Fri, 08 Nov 2024 08:57:37 -0600
>>>
>>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>>
>>> >> From: john muhl <jm <at> 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 <at> 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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.