* bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options
@ 2024-11-06 21:19 john muhl
2024-11-06 21:22 ` john muhl
0 siblings, 1 reply; 11+ messages in thread
From: john muhl @ 2024-11-06 21:19 UTC (permalink / raw)
To: 74235
Tags: patch
For some silly reason the lua-ts-{inferior,luacheck}-program
options ended up with an option for “None”. Obviously setting your
Lua interpreter or linter program to nil is not going to work.
Any chance it could make it into emacs-30?
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread* bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options 2024-11-06 21:19 bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options john muhl @ 2024-11-06 21:22 ` john muhl 2024-11-07 5:58 ` Eli Zaretskii 0 siblings, 1 reply; 11+ messages in thread From: john muhl @ 2024-11-06 21:22 UTC (permalink / raw) To: 74235 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: 0001-Remove-nil-from-some-lua-ts-mode-options.patch --] [-- Type: text/x-patch, Size: 1163 bytes --] From 9875334458c9b6820eeb1f5762c994e52cc9cdba Mon Sep 17 00:00:00 2001 From: john muhl <jm@pub.pink> Date: Wed, 6 Nov 2024 14:56:24 -0600 Subject: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options * 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) --- lisp/progmodes/lua-ts-mode.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lisp/progmodes/lua-ts-mode.el b/lisp/progmodes/lua-ts-mode.el index 20bc1f3e158..1ab0b29608f 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 'string :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 'string :version "30.1") (defcustom lua-ts-inferior-options '("-i") -- 2.47.0 [-- Attachment #2: Type: text/plain, Size: 324 bytes --] john muhl <jm@pub.pink> writes: > Tags: patch > > For some silly reason the lua-ts-{inferior,luacheck}-program > options ended up with an option for “None”. Obviously setting your > Lua interpreter or linter program to nil is not going to work. > > Any chance it could make it into emacs-30? > > Thanks. ^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options 2024-11-06 21:22 ` john muhl @ 2024-11-07 5:58 ` Eli Zaretskii 2024-11-07 15:04 ` john muhl 2024-11-08 14:57 ` john muhl 0 siblings, 2 replies; 11+ messages in thread From: Eli Zaretskii @ 2024-11-07 5:58 UTC (permalink / raw) To: john muhl; +Cc: 74235 > 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? > (defcustom lua-ts-luacheck-program "luacheck" > "Location of the Luacheck program." > - :type '(choice (const :tag "None" nil) string) > + :type 'string > :version "30.1") I think if we require this to be the name of a program, 'file' is a better value for :type, because it allows completion if the user types an absolute file name. > (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 'string > :version "30.1") > > (defcustom lua-ts-inferior-options '("-i") > -- > 2.47.0 > > john muhl <jm@pub.pink> writes: > > > Tags: patch > > > > For some silly reason the lua-ts-{inferior,luacheck}-program > > options ended up with an option for “None”. Obviously setting your > > Lua interpreter or linter program to nil is not going to work. > > > > Any chance it could make it into emacs-30? lua-ts-mode is new in Emacs 30, so we can install these changes on the release branch, as a stopgap. But I'd like to see a cleaner solution installed later, even if we decide to install that on master due to its complexity. Thanks. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options 2024-11-07 5:58 ` Eli Zaretskii @ 2024-11-07 15:04 ` john muhl 2024-11-08 14:57 ` john muhl 1 sibling, 0 replies; 11+ messages in thread From: john muhl @ 2024-11-07 15:04 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 74235 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? Good point. I’ll work on that very soon. >> (defcustom lua-ts-luacheck-program "luacheck" >> "Location of the Luacheck program." >> - :type '(choice (const :tag "None" nil) string) >> + :type 'string >> :version "30.1") > > I think if we require this to be the name of a program, 'file' is a > better value for :type, because it allows completion if the user types > an absolute file name. Makes sense. >> (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 'string >> :version "30.1") >> >> (defcustom lua-ts-inferior-options '("-i") >> -- >> 2.47.0 >> >> john muhl <jm@pub.pink> writes: >> >> > Tags: patch >> > >> > For some silly reason the lua-ts-{inferior,luacheck}-program >> > options ended up with an option for “None”. Obviously setting your >> > Lua interpreter or linter program to nil is not going to work. >> > >> > Any chance it could make it into emacs-30? > > lua-ts-mode is new in Emacs 30, so we can install these changes on the > release branch, as a stopgap. But I'd like to see a cleaner solution > installed later, even if we decide to install that on master due to > its complexity. > > Thanks. Then let’s put this one on hold and I’ll work on the improvements you suggest. Hopefully it doesn’t require significant changes and we can skip the stopgap and get it all into 30. Thanks for the help. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options 2024-11-07 5:58 ` Eli Zaretskii 2024-11-07 15:04 ` john muhl @ 2024-11-08 14:57 ` john muhl 2024-11-08 15:24 ` Eli Zaretskii 1 sibling, 1 reply; 11+ messages in thread From: john muhl @ 2024-11-08 14:57 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 74235 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. Thanks. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options 2024-11-08 14:57 ` john muhl @ 2024-11-08 15:24 ` Eli Zaretskii 2024-11-09 20:54 ` john muhl 0 siblings, 1 reply; 11+ messages in thread From: Eli Zaretskii @ 2024-11-08 15:24 UTC (permalink / raw) To: john muhl; +Cc: 74235 > 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? ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options 2024-11-08 15:24 ` Eli Zaretskii @ 2024-11-09 20:54 ` john muhl 2024-11-10 5:43 ` Eli Zaretskii 2024-11-10 5:45 ` Philip Kaludercic 0 siblings, 2 replies; 11+ messages in thread From: john muhl @ 2024-11-09 20:54 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 74235 [-- Attachment #1: Type: text/plain, Size: 1353 bytes --] 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. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Fix-some-lua-ts-mode-options-Bug-74235.patch --] [-- Type: text/x-patch, Size: 6048 bytes --] 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") + (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)))) + (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." -- 2.47.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options 2024-11-09 20:54 ` john muhl @ 2024-11-10 5:43 ` Eli Zaretskii 2024-11-10 14:48 ` john muhl 2024-11-10 5:45 ` Philip Kaludercic 1 sibling, 1 reply; 11+ messages in thread From: Eli Zaretskii @ 2024-11-10 5:43 UTC (permalink / raw) To: john muhl; +Cc: 74235 > From: john muhl <jm@pub.pink> > Cc: 74235@debbugs.gnu.org > Date: Sat, 09 Nov 2024 14:54:47 -0600 > > + (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") The text of user-error should say something about being unable to find the program, not just about the variable being nil, since you use executable-find. Otherwise, LGTM, thanks. ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options 2024-11-10 5:43 ` Eli Zaretskii @ 2024-11-10 14:48 ` john muhl 2024-11-14 8:26 ` Eli Zaretskii 0 siblings, 1 reply; 11+ messages in thread From: john muhl @ 2024-11-10 14:48 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 74235 [-- Attachment #1: Type: text/plain, Size: 737 bytes --] Eli Zaretskii <eliz@gnu.org> writes: >> From: john muhl <jm@pub.pink> >> Cc: 74235@debbugs.gnu.org >> Date: Sat, 09 Nov 2024 14:54:47 -0600 >> >> + (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") > > The text of user-error should say something about being unable to find > the program, not just about the variable being nil, since you use > executable-find. > > Otherwise, LGTM, thanks. I guess the executable-find check is redundant anyway since comint/process will tell you “Searching for program: No such file or directory, luaz” if you get it wrong. Thanks again. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Fix-some-lua-ts-mode-options-Bug-74235.patch --] [-- Type: text/x-patch, Size: 5969 bytes --] From b170e406f834806842761726de47ca3afeb05cfe 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 set. --- lisp/progmodes/lua-ts-mode.el | 84 ++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 41 deletions(-) diff --git a/lisp/progmodes/lua-ts-mode.el b/lisp/progmodes/lua-ts-mode.el index 4ea453c9b65..a7763377177 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,49 @@ 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 (not lua-ts-inferior-program) + (user-error "You must set `lua-ts-inferior-program' to use this command") + (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)))) + (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." -- 2.47.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options 2024-11-10 14:48 ` john muhl @ 2024-11-14 8:26 ` Eli Zaretskii 0 siblings, 0 replies; 11+ messages in thread From: Eli Zaretskii @ 2024-11-14 8:26 UTC (permalink / raw) To: john muhl; +Cc: 74235-done > From: john muhl <jm@pub.pink> > Cc: 74235@debbugs.gnu.org > Date: Sun, 10 Nov 2024 08:48:39 -0600 > > Eli Zaretskii <eliz@gnu.org> writes: > > >> From: john muhl <jm@pub.pink> > >> Cc: 74235@debbugs.gnu.org > >> Date: Sat, 09 Nov 2024 14:54:47 -0600 > >> > >> + (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") > > > > The text of user-error should say something about being unable to find > > the program, not just about the variable being nil, since you use > > executable-find. > > > > Otherwise, LGTM, thanks. > > I guess the executable-find check is redundant anyway since > comint/process will tell you “Searching for program: No such file > or directory, luaz” if you get it wrong. Thanks, installed on the emacs-30 branch, and closing the bug. Btw, I noticed that you are marking with ";" the log entries of changes which should definitely appear in the produced ChangeLog. Is this on purpose, or did you misunderstand the meaning of the semi-colon in the first line of the commit log messages? ^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options 2024-11-09 20:54 ` john muhl 2024-11-10 5:43 ` Eli Zaretskii @ 2024-11-10 5:45 ` Philip Kaludercic 1 sibling, 0 replies; 11+ messages in thread From: Philip Kaludercic @ 2024-11-10 5:45 UTC (permalink / raw) To: john muhl; +Cc: Eli Zaretskii, 74235 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-11-14 8:26 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-11-06 21:19 bug#74235: [PATCH] ; Remove 'nil' from some 'lua-ts-mode' options john muhl 2024-11-06 21:22 ` john muhl 2024-11-07 5:58 ` Eli Zaretskii 2024-11-07 15:04 ` john muhl 2024-11-08 14:57 ` john muhl 2024-11-08 15:24 ` Eli Zaretskii 2024-11-09 20:54 ` john muhl 2024-11-10 5:43 ` Eli Zaretskii 2024-11-10 14:48 ` john muhl 2024-11-14 8:26 ` Eli Zaretskii 2024-11-10 5:45 ` Philip Kaludercic
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).