Subject: Re: bug#66159: 30.0.50; lua-ts-mode semantic indentation problems

From: john muhl <[email-masked]>
Date: Sun, 24 Sep 2023 13:20:23 -0500

Eli Zaretskii <[email-masked]> writes:

>> > Another thing that bothers me is that I prefer Gassanenko-style packing
>> > of `end' keywords so that they vertically align with the scope of the
>> > opened block, as it saves so much vertical space and is easier for me to
>> > read, but lua-ts-mode moves it to the latest innermost indentation
>> > level, as opposed to the outermost depending on the count of ends in the
>> > line itself:
>> 
>> I don't see any reason not to support that style but I'm not sure how to
>> do it. A patch would be welcome but I'll try to figure it out sometime.
>
> Maybe introduce indentation styles into lua-ts-mode, like CC Mode and
> c-ts-mode have?

I’ll have a look at what the c-ts-mode styles do and see what might be
applicable. In this case the changes can be accommodated by default.

Andrey Listopadov <[email-masked]> writes:

> Thanks! The patch seems to work as I would expect for most cases.
>
>> I don't see any reason not to support that style but I'm not sure how to
>> do it. A patch would be welcome but I'll try to figure it out sometime.
>
> A far as I understand it, in the `lua-mode' the overall line indentation
> is computed via subtracting indentation for every `end' in that line,
> e.g. `end end end' subtracts `lua-indent-level three times from current
> indent level.

Thanks for the explanation. The attached patch should make end packing
work now.

>> Sure. It's a new mode so nothing is really set in stone. Let me know if
>> you have other suggestions.
>
> I also noticed that `lua-ts-inferior-lua` for some reason starts itself
> in a new frame instead of just a new window. I haven't seen this
> behavior with other inferior-*-modes, and the `lua-start-process'
> function from `lua-mode' doesn't spawn a new frame either.
>
> Looking at the code, it seems that it was a deliberate choice, as I see
> the use of `display-buffer-pop-up-frame' in the `display-buffer' ACTION
> arg.

It did use a window originally but I was asked to change it:

>> +    (pop-to-buffer buffer)))
>
> I believe that `display-buffer-pop-up-frame' is the preferred alternative
> to `pop-to-buffer' these days.

Maybe I misunderstood and it was only about not using ‘pop-to-buffer’
and should have been ‘display-buffer-pop-up-window’. CC’d Philip for
clarification.

If you customize where you want it with ‘display-buffer-alist’ that
should be respected.

>From c32ce5aef2c8692cb93e6d91a7dc08af4b50c210 Mon Sep 17 00:00:00 2001
From: john muhl <[email-masked]>
Date: Sat, 23 Sep 2023 10:49:11 -0500
Subject: [PATCH] Improve indentation in lus-ts-mode (Bug#66159)

- Align "then" with if/elseif/else/end when it appears at the
  beginning of a line.

- Align a single-line of "end"s with the beginning of the
  outermost block being closed.

- Anchor indent of the first child in arguments, parameters and
  tables to the parent and the other children to the first.

* lisp/progmodes/lua-ts-mode.el (lua-ts--first-child)
(lua-ts--end-indent-offset)
(lua-ts--simple-indent-rules): Improve indentation.
* test/lisp/progmodes/lua-ts-mode-resources/indent.erts: Add
tests.
---
 lisp/progmodes/lua-ts-mode.el                 |  26 ++-
 .../lua-ts-mode-resources/indent.erts         | 179 ++++++++++++++++++
 2 files changed, 201 insertions(+), 4 deletions(-)

diff --git a/lisp/progmodes/lua-ts-mode.el b/lisp/progmodes/lua-ts-mode.el
index 030a3585158..fcab8ec610b 100644
--- a/lisp/progmodes/lua-ts-mode.el
+++ b/lisp/progmodes/lua-ts-mode.el
@@ -233,6 +233,17 @@ lua-ts--font-lock-settings
    '((ERROR) @font-lock-warning-face))
   "Tree-sitter font-lock settings for `lua-ts-mode'.")
 
+(defun lua-ts--first-child (node _p _b &rest _)
+  "Matches if NODE is the first among its siblings."
+  (= (treesit-node-index node) 1))
+
+(defun lua-ts--end-indent-offset (_n _p _b &rest _)
+  "Calculate indent offset based on `end' count."
+  (let* ((beg (line-beginning-position))
+         (end (line-end-position))
+         (count (count-matches "end" beg end)))
+    (- (* (1- count) lua-ts-indent-offset))))
+
 (defvar lua-ts--simple-indent-rules
   `((lua
      ((parent-is "chunk") column-0 0)
@@ -240,9 +251,10 @@ lua-ts--simple-indent-rules
      ((parent-is "block") parent-bol 0)
      ((node-is "}") parent-bol 0)
      ((node-is ")") parent-bol 0)
+     ((node-is "then") standalone-parent 0)
      ((node-is "else_statement") parent-bol 0)
      ((node-is "elseif_statement") parent-bol 0)
-     ((node-is "end") parent-bol 0)
+     ((node-is "end") parent-bol lua-ts--end-indent-offset)
      ((node-is "until") parent-bol 0)
      ((parent-is "for_statement") parent-bol lua-ts-indent-offset)
      ((parent-is "function_declaration") parent-bol lua-ts-indent-offset)
@@ -251,9 +263,15 @@ lua-ts--simple-indent-rules
      ((parent-is "else_statement") parent-bol lua-ts-indent-offset)
      ((parent-is "repeat_statement") parent-bol lua-ts-indent-offset)
      ((parent-is "while_statement") parent-bol lua-ts-indent-offset)
-     ((parent-is "table_constructor") parent-bol lua-ts-indent-offset)
-     ((parent-is "arguments") parent-bol lua-ts-indent-offset)
-     ((parent-is "parameters") parent-bol lua-ts-indent-offset)
+     ((and (parent-is "table_constructor") lua-ts--first-child)
+      parent-bol lua-ts-indent-offset)
+     ((parent-is "table_constructor") (nth-sibling 1) 0)
+     ((and (parent-is "arguments") lua-ts--first-child)
+      parent-bol lua-ts-indent-offset)
+     ((parent-is "arguments") (nth-sibling 1) 0)
+     ((and (parent-is "parameters") lua-ts--first-child)
+      parent-bol lua-ts-indent-offset)
+     ((parent-is "parameters") (nth-sibling 1) 0)
      ((parent-is "ERROR") no-indent 0))))
 
 (defvar lua-ts--syntax-table
diff --git a/test/lisp/progmodes/lua-ts-mode-resources/indent.erts b/test/lisp/progmodes/lua-ts-mode-resources/indent.erts
index 040225c8580..b0cfe11ef6f 100644
--- a/test/lisp/progmodes/lua-ts-mode-resources/indent.erts
+++ b/test/lisp/progmodes/lua-ts-mode-resources/indent.erts
@@ -32,6 +32,22 @@ f({
 ;(function()
 return false
 )()
+
+function foo (e)
+    if e == nil
+      then return 1000
+    else return e
+    end
+end
+
+function foo (e)
+    if e == nil
+    then
+        return 1000
+    else
+        return e
+    end
+end
 =-=
 print(
   0,
@@ -57,6 +73,22 @@ f({
 ;(function()
   return false
 )()
+
+function foo (e)
+  if e == nil
+  then return 1000
+  else return e
+  end
+end
+
+function foo (e)
+  if e == nil
+  then
+    return 1000
+  else
+    return e
+  end
+end
 =-=-=
 
 Name: Argument Indent
@@ -77,6 +109,13 @@ cost = 2,
 length = 8,
 	parallelism = 4,
 })
+
+fn(1,
+2,
+     3)
+
+fn(1, 2,
+3)
 =-=
 function h(
   string,
@@ -93,6 +132,13 @@ local p = h(
     length = 8,
     parallelism = 4,
   })
+
+fn(1,
+   2,
+   3)
+
+fn(1, 2,
+   3)
 =-=-=
 
 Name: Continuation Indent
@@ -150,3 +196,136 @@ repeat
   z = z * 2
 until z > 12
 =-=-=
+
+Name: Parameter Indent
+
+=-=
+fn(a,
+b) end
+
+fn(a, b,
+c) end
+
+fn(
+a,
+b
+) end
+=-=
+fn(a,
+   b) end
+
+fn(a, b,
+   c) end
+
+fn(
+  a,
+  b
+) end
+=-=-=
+
+Code:
+  (lambda ()
+    (setq indent-tabs-mode nil)
+    (setq lua-ts-indent-offset 4)
+    (lua-ts-mode)
+    (indent-region (point-min) (point-max)))
+
+Name: Table Indent
+
+=-=
+local Recipe = {
+    Floor={up={Floor=true,Wall=true},
+        down={Floor=true,Wall=true},
+        left={Floor=true,Wall=true},
+        right={Floor=true,Wall=true}},
+    Wall={up={Floor=true,Wall=true},
+        down={Floor=true,Wall=true},
+        left={Floor=true,Wall=true},
+        right={Floor=true,Wall=true}},
+    Corridor={up={Corridoor=true},
+        down={Corridoor=true},
+        left={Corridoor=true},
+        right={Corridoor=true}}
+}
+
+local Other = {
+a = 1,
+ b = 2,
+  c = 3,
+}
+=-=
+local Recipe = {
+    Floor={up={Floor=true,Wall=true},
+           down={Floor=true,Wall=true},
+           left={Floor=true,Wall=true},
+           right={Floor=true,Wall=true}},
+    Wall={up={Floor=true,Wall=true},
+          down={Floor=true,Wall=true},
+          left={Floor=true,Wall=true},
+          right={Floor=true,Wall=true}},
+    Corridor={up={Corridoor=true},
+              down={Corridoor=true},
+              left={Corridoor=true},
+              right={Corridoor=true}}
+}
+
+local Other = {
+    a = 1,
+    b = 2,
+    c = 3,
+}
+=-=-=
+
+Name: Single Line End
+
+=-=
+function lowest_entropy_cell(world)
+    local lowest,res=math.huge,nil
+    for y=1,world.height do
+        for x=1,world.width do
+            local cell=world:get(x,y)
+            if cell.is_set then
+                local e=cell_enthropy(cell)
+                trace(e)
+                if e <= lowest then
+                    lowest,res=e,{x,y}
+                end end end end
+    return res or {math.random(w.w),math.random(w.h)}
+end
+
+for y=1,world.height do
+    for x=1,world.width do
+        local cell=world:get(x,y)
+        if cell.is_set then
+            local e=cell_enthropy(cell)
+            trace(e)
+            if e <= lowest then
+                lowest,res=e,{x,y}
+            end
+            end end end
+=-=
+function lowest_entropy_cell(world)
+    local lowest,res=math.huge,nil
+    for y=1,world.height do
+        for x=1,world.width do
+            local cell=world:get(x,y)
+            if cell.is_set then
+                local e=cell_enthropy(cell)
+                trace(e)
+                if e <= lowest then
+                    lowest,res=e,{x,y}
+    end end end end
+    return res or {math.random(w.w),math.random(w.h)}
+end
+
+for y=1,world.height do
+    for x=1,world.width do
+        local cell=world:get(x,y)
+        if cell.is_set then
+            local e=cell_enthropy(cell)
+            trace(e)
+            if e <= lowest then
+                lowest,res=e,{x,y}
+            end
+end end end
+=-=-=
-- 
2.41.0