Subject: Re: bug#79309: lua-mode: minor problems and suggestions

From: john muhl <[email-masked]>
Date: Tue, 26 Aug 2025 22:34:24 -0500

Thanks for the review. Attached patch fixes these and a few other
little things.

Mattias Engdegård <[email-masked]> writes:

> Some observations about the recently added lua-mode.el:
>
>  394   "A regexp that matches Lua builtin functions & variables.
>  395 
>  396 This is a compilation of 5.1, 5.2 and 5.3 builtins taken from the
>  397 index of respective Lua reference manuals.")
>
> Clearly meant as a doc string. Bracket error?

Fixed.

> 1266            (or
>
> This `or` has only a single operand. Simple oversight or indication of something worse?

Looks like there used to be a second part but it got refactored up
into the first part and the ‘or’ just hung around.

> 1270                (save-excursion
> 1271                  (and (goto-char prev-line)
> 1272                       ;; Check last token of previous nonblank line
> 1273                       (lua-last-token-continues-p)))))
>
> `goto-char` never returns nil, but its presence as a condition
> here suggest that the author may have thought otherwise.

Removed the ‘and’ here.

> (Line 996 is similar.)

Wrapped it and the recursive call to lua-find-matching-token-word
on the next line in progn to make it clear the goto-char is for
side-effect and not return value.

> 2099         for type = (if (string-match-p "\\`(E" msg) :error :warning)
>
> A regexp is overkill here; `string-prefix-p` is simpler.

The plan is to merge the common parts between the lua modes. The
lua-ts-mode flymake backend uses string-prefix-p there so I left
this for now and it’ll get replaced with the lua-ts flymake code.

> 1821       (while (re-search-forward "[\"'\\\t\\\n]" nil t)
>
> This regexp is a bit muddled; too many backslashes here.
> You could even rewrite the whole function using `replace-regexp-in-string`, maybe
>
>   (concat "'"
>           (replace-regexp-in-string
>            (rx (or ?\" ?' ?\t ?\n ?\\))
>            (lambda (s)
>              (cdr (assq (aref s 0) '((?\" . "\\\"")
>                                      (?\\ . "\\\\")
>                                      (?\n . "\\n")
>                                      (?\t . "\\t")
>                                      (?'  . "\\'")))))
>            str t t)
>           "'"))
>
> without any need for a temporary buffer.

That works.

>From ae1adfe0936e4b53a978bde6fa851110eac8965d Mon Sep 17 00:00:00 2001
From: john muhl <[email-masked]>
Date: Tue, 26 Aug 2025 15:12:26 -0500
Subject: [PATCH] ; Various code cleanup in 'lua-mode' (Bug#79309)

* lisp/progmodes/lua-mode.el (lua-indent-level): Contemporary Lua
style guides no longer recommend 3 as default. Change type to
'natnum'.
(lua-always-show, lua-get-block-token-info)
(lua--backward-up-list-noerror, lua-make-indentation-info-pair)
(lua-accumulate-indentation-info)
(lua-calculate-indentation-block-modifier): Quote function names.
(lua-toggle-electric-state): Re-format to avoid confusion.
(lua-is-continuing-statement-p-1): Fix typo.
(lua--builtins): Move docstring to correct location.
(lua-is-continuing-statement-p-1): Remove unnecessary 'or'.
(lua-make-lua-string): Remove excessive backslashes and
unnecessary use of a temporary buffer.
(lua-find-matching-token-word): Make it clear that 'goto-char' is
only used only for its side-effect.
---
 lisp/progmodes/lua-mode.el | 110 ++++++++++++++++++-------------------
 1 file changed, 54 insertions(+), 56 deletions(-)

diff --git a/lisp/progmodes/lua-mode.el b/lisp/progmodes/lua-mode.el
index f8d9ed98f1a..2e051d6d552 100644
--- a/lisp/progmodes/lua-mode.el
+++ b/lisp/progmodes/lua-mode.el
@@ -150,10 +150,10 @@ lua
   :prefix "lua-"
   :group 'languages)
 
-(defcustom lua-indent-level 3
+(defcustom lua-indent-level 4
   "Amount by which Lua subexpressions are indented."
-  :type 'integer
-  :safe #'integerp
+  :type 'natnum
+  :safe #'natnump
   :version "31.1")
 
 (defcustom lua-comment-start "-- "
@@ -183,7 +183,7 @@ lua-default-command-switches
   :version "31.1")
 
 (defcustom lua-always-show t
-  "Non-nil means display lua-process-buffer after sending a command."
+  "Non-nil means display `lua-process-buffer' after sending a command."
   :type 'boolean
   :group 'lua)
 
@@ -204,7 +204,6 @@ lua-documentation-url
   :type 'string
   :group 'lua)
 
-
 (defvar lua-process nil
   "The active Lua process.")
 
@@ -390,11 +389,11 @@ lua-region-end
                       (concat (module-name-re x) (module-members-re x)))
                     modules
                     "\\|")
-         "\\)"))))
-  "A regexp that matches Lua builtin functions & variables.
+         "\\)")))
+    "A regexp that matches Lua builtin functions & variables.
 
-This is a compilation of 5.1, 5.2 and 5.3 builtins taken from the
-index of respective Lua reference manuals.")
+This is a compilation of 5.1-5.4 builtins taken from the index of
+respective Lua reference manuals."))
 
 (defvar lua-font-lock-keywords
   `(;; Highlight the hash-bang line "#!/foo/bar/lua" as comment
@@ -917,7 +916,7 @@ lua-indentation-modifier-regexp
                      (any ")]}"))))))
 
 (defun lua-get-block-token-info (token)
-  "Return the block token info entry for TOKEN from lua-block-token-alist."
+  "Return the block token info entry for TOKEN from `lua-block-token-alist'."
   (assoc token lua-block-token-alist))
 
 (defun lua-get-token-match-re (token-info direction)
@@ -993,9 +992,10 @@ lua-find-matching-token-word
                                (eq match-type 'middle-or-open)
                                (eq found-type 'middle-or-open)
                                (eq match-type found-type))
-                           (goto-char found-pos)
-                           (lua-find-matching-token-word
-                            found-token search-direction))
+                           (progn
+                             (goto-char found-pos)
+                             (lua-find-matching-token-word
+                              found-token search-direction)))
                 (when maybe-found-pos
                   (goto-char maybe-found-pos)
                   (throw 'found maybe-found-pos)))
@@ -1202,7 +1202,7 @@ lua-first-token-continues-p
            (match-beginning 2))))))
 
 (defun lua--backward-up-list-noerror ()
-  "Safe version of lua-backward-up-list that does not signal an error."
+  "Safe version of `lua-backward-up-list' that does not signal an error."
   (condition-case nil
       (lua-backward-up-list)
     (scan-error nil)))
@@ -1249,40 +1249,39 @@ lua--continuation-breaking-line-p
                             lua-funcheader)))))
 
 (defun lua-is-continuing-statement-p-1 ()
-  "Return non-nil if current lined continues a statement.
+  "Return non-nil if current line continues a statement.
 
 More specifically, return the point in the line that is continued.
 The criteria for a continuing statement are:
 
-* the last token of the previous line is a continuing op,
-  OR the first token of the current line is a continuing op
+* The last token of the previous line is a continuing op,
+  OR the first token of the current line is a continuing op.
 
-* the expression is not enclosed by a parentheses/braces/brackets"
+* The expression is not enclosed by a parentheses/braces/brackets."
   (let (prev-line continuation-pos parent-block-opener)
     (save-excursion (setq prev-line (lua-forward-line-skip-blanks 'back)))
     (and prev-line
          (not (lua--continuation-breaking-line-p))
          (save-excursion
-           (or
-            ;; Binary operator or keyword that implies continuation.
-            (and (setq continuation-pos
-                       (or (lua-first-token-continues-p)
-                           (save-excursion
-                             (and (goto-char prev-line)
-                                  ;; Check last token of previous nonblank line
-                                  (lua-last-token-continues-p)))))
-                 (not
-                  ;; Operators/keywords does not create continuation
-                  ;; inside some blocks:
-                  (and (setq parent-block-opener
-                             (car-safe (lua--backward-up-list-noerror)))
-                       (or
-                        ;; Inside parens/brackets
-                        (member parent-block-opener '("(" "["))
-                        ;; Inside braces if it is a comma
-                        (and (eq (char-after continuation-pos) ?,)
-                             (equal parent-block-opener "{")))))
-                 continuation-pos))))))
+           ;; Binary operator or keyword that implies continuation.
+           (and (setq continuation-pos
+                      (or (lua-first-token-continues-p)
+                          (save-excursion
+                            (goto-char prev-line)
+                            ;; Check last token of previous nonblank line
+                            (lua-last-token-continues-p))))
+                (not
+                 ;; Operators/keywords does not create continuation
+                 ;; inside some blocks:
+                 (and (setq parent-block-opener
+                            (car-safe (lua--backward-up-list-noerror)))
+                      (or
+                       ;; Inside parens/brackets
+                       (member parent-block-opener '("(" "["))
+                       ;; Inside braces if it is a comma
+                       (and (eq (char-after continuation-pos) ?,)
+                            (equal parent-block-opener "{")))))
+                continuation-pos)))))
 
 (defun lua-is-continuing-statement-p (&optional parse-start)
   "Return non-nil if PARSE-START should be indented as continuation line.
@@ -1328,7 +1327,7 @@ lua-is-continuing-statement-p
 (defun lua-make-indentation-info-pair (found-token found-pos)
   "Create a pair from FOUND-TOKEN and FOUND-POS for indentation calculation.
 
-This is a helper function to lua-calculate-indentation-info.
+This is a helper function to `lua-calculate-indentation-info'.
 Don't use standalone."
   (cond
    ;; Functions are a bit tricky to indent right.  They can appear in a
@@ -1525,7 +1524,7 @@ lua-calculate-indentation-info
     indentation-info))
 
 (defun lua-accumulate-indentation-info (reversed-indentation-info)
-  "Accumulate indent information from lua-calculate-indentation-info.
+  "Accumulate indent information from `lua-calculate-indentation-info'.
 Returns either the relative indentation shift, or the absolute column to
 indent to.
 
@@ -1553,8 +1552,8 @@ lua-accumulate-indentation-info
 
 (defun lua-calculate-indentation-block-modifier (&optional parse-end)
   "Return amount by which this line modifies the indentation.
-Beginnings of blocks add lua-indent-level once each, and endings of
-blocks subtract lua-indent-level once each.  This function is used to
+Beginnings of blocks add `lua-indent-level' once each, and endings of
+blocks subtract `lua-indent-level' once each.  This function is used to
 determine how the indentation of the following line relates to this one.
 
 The argument PARSE-END is a buffer position that bounds the calculation."
@@ -1814,19 +1813,17 @@ lua-process-init-code
 
 (defun lua-make-lua-string (str)
   "Convert STR to Lua literal."
-  (save-match-data
-    (with-temp-buffer
-      (insert str)
-      (goto-char (point-min))
-      (while (re-search-forward "[\"'\\\t\\\n]" nil t)
-        (cond
-         ((string= (match-string 0) "\n")
-          (replace-match "\\\\n"))
-         ((string= (match-string 0) "\t")
-          (replace-match "\\\\t"))
-         (t
-          (replace-match "\\\\\\&" t))))
-      (concat "'" (buffer-string) "'"))))
+  (concat "'"
+          (replace-regexp-in-string
+           (rx (or ?\" ?' ?\t ?\n ?\\))
+           (lambda (s)
+             (cdr (assq (aref s 0) '((?\" . "\\\"")
+                                     (?\\ . "\\\\")
+                                     (?\n . "\\n")
+                                     (?\t . "\\t")
+                                     (?'  . "\\'")))))
+           str t t)
+          "'"))
 
 ;;;###autoload
 (defalias 'run-lua #'lua-start-process)
@@ -2031,7 +2028,8 @@ lua-toggle-electric-state
   (interactive "P")
   (let ((num_arg (prefix-numeric-value arg)))
     (setq lua-electric-flag (cond ((or (null arg)
-                                       (zerop num_arg)) (not lua-electric-flag))
+                                       (zerop num_arg))
+                                   (not lua-electric-flag))
                                   ((< num_arg 0) nil)
                                   ((> num_arg 0) t))))
   (message "%S" lua-electric-flag))
-- 
2.50.1