bug#55764: 29.0.50; sh-mode: Support mksh's alternate case brace syntax

bug#55764: 29.0.50; sh-mode: Support mksh's alternate case brace syntax

[Visuwesh]

[-- Attachment #1: Type: text/plain, Size: 448 bytes --]

mksh and OpenBSD's ksh support an alternative case syntax for historical
reasons [1],

    case $i {
    *pattern) do ;;
    *pattern2) do2 ;;
    }


Currently, sh-mode cannot handle this and the easy way out of writing a
semicolon after $i is out since,

   % case "foo"; { *o) echo 1;; }
   /bin/mksh: syntax error: unexpected ';'

I came with the attached patch after a bit of trial and error but I'm
not sure if it is the right way to do it.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mksh-braces.patch --]
[-- Type: text/x-diff, Size: 810 bytes --]

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 4d2554c087..588d9038e5 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1971,9 +1971,10 @@
                              (sh-var-value 'sh-indent-for-case-label)))
     (`(:before . ,(or "(" "{" "[" "while" "if" "for" "case"))
      (cond
-      ((and (equal token "{") (smie-rule-parent-p "for"))
+      ((and (equal token "{") (or (smie-rule-parent-p "for")
+                                  (smie-rule-parent-p "case")))
        (let ((data (smie-backward-sexp "in")))
-         (when (equal (nth 2 data) "for")
+         (when (member (nth 2 data) '("for" "case"))
            `(column . ,(smie-indent-virtual)))))
       ((not (smie-rule-prev-p "&&" "||" "|"))
        (when (smie-rule-hanging-p)

[-- Attachment #3: Type: text/plain, Size: 356 bytes --]


--

[1] Here's what the mksh manual says,

    For historical reasons, open and close braces may be used instead
    of in and esac, for example: “case $foo { (ba[rz]|blah) date ;; }”

and OpenBSD's ksh manual says,

    For historical reasons, open and close braces may be used instead of
    in and esac e.g. case $foo { *) echo bar; }.

bug#55764: 29.0.50; sh-mode: Support mksh's alternate case brace syntax

[Lars Ingebrigtsen]

Visuwesh <visuweshm@gmail.com> writes:

> mksh and OpenBSD's ksh support an alternative case syntax for historical
> reasons [1],
>
>     case $i {
>     *pattern) do ;;
>     *pattern2) do2 ;;
>     }
>
> Currently, sh-mode cannot handle this and the easy way out of writing a
> semicolon after $i is out since,
>
>    % case "foo"; { *o) echo 1;; }
>    /bin/mksh: syntax error: unexpected ';'
>
> I came with the attached patch after a bit of trial and error but I'm
> not sure if it is the right way to do it.

I think that might make sense, but perhaps Stefan has some comments here
(added to the CCs).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#55764: 29.0.50; sh-mode: Support mksh's alternate case brace syntax

[Visuwesh]

[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]

[வியாழன் ஜூன் 02, 2022] Lars Ingebrigtsen wrote:

> Visuwesh <visuweshm@gmail.com> writes:
>
>> mksh and OpenBSD's ksh support an alternative case syntax for historical
>> reasons [1],
>>
>>     case $i {
>>     *pattern) do ;;
>>     *pattern2) do2 ;;
>>     }
>>
>> Currently, sh-mode cannot handle this and the easy way out of writing a
>> semicolon after $i is out since,
>>
>>    % case "foo"; { *o) echo 1;; }
>>    /bin/mksh: syntax error: unexpected ';'
>>
>> I came with the attached patch after a bit of trial and error but I'm
>> not sure if it is the right way to do it.
>
> I think that might make sense, but perhaps Stefan has some comments here
> (added to the CCs).

To no one's surprise, the patch is not the right approach.  IIUC, when I
use braces, *pattern) thingies are not recognised as "case-)" anymore so
sh-indent-for-case-label does not get applied.  I tried a bit more but I
only got it to respect the first line but still it is definitely not the
right approach,


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mksh-braces.patch --]
[-- Type: text/x-diff, Size: 1511 bytes --]

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 4d2554c087..96ff8e8627 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1971,9 +1971,9 @@ sh-smie-sh-rules
                              (sh-var-value 'sh-indent-for-case-label)))
     (`(:before . ,(or "(" "{" "[" "while" "if" "for" "case"))
      (cond
-      ((and (equal token "{") (smie-rule-parent-p "for"))
+      ((and (equal token "{") (smie-rule-parent-p "for" "case"))
        (let ((data (smie-backward-sexp "in")))
-         (when (equal (nth 2 data) "for")
+         (when (member (nth 2 data) '("for" "case"))
            `(column . ,(smie-indent-virtual)))))
       ((not (smie-rule-prev-p "&&" "||" "|"))
        (when (smie-rule-hanging-p)
@@ -2021,10 +2021,13 @@ sh-smie-sh-rules
     ('(:after . "in") (sh-var-value 'sh-indent-for-case-label))
     ;; sh-indent-for-continuation: Line continuations are handled differently.
     (`(:after . ,(or "(" "{" "["))
-     (if (not (looking-at ".[ \t]*[^\n \t#]"))
-         (sh-var-value 'sh-indent-after-open)
-       (goto-char (1- (match-end 0)))
-       `(column . ,(current-column))))
+     (cond
+      ((smie-rule-parent-p "case")
+       (sh-var-value 'sh-indent-for-case-label))
+      ((not (looking-at ".[ \t]*[^\n \t#]"))
+       (sh-var-value 'sh-indent-after-open))
+      (t (goto-char (1- (match-end 0)))
+         `(column . ,(current-column)))))
     ;; sh-indent-after-function: we don't handle it differently.
     ))
 

bug#55764: 29.0.50; sh-mode: Support mksh's alternate case brace syntax

[Stefan Kangas]

Visuwesh <visuweshm@gmail.com> writes:

> [வியாழன் ஜூன் 02, 2022] Lars Ingebrigtsen wrote:
>
>> Visuwesh <visuweshm@gmail.com> writes:
>>
>>> mksh and OpenBSD's ksh support an alternative case syntax for historical
>>> reasons [1],
>>>
>>>     case $i {
>>>     *pattern) do ;;
>>>     *pattern2) do2 ;;
>>>     }
>>>
>>> Currently, sh-mode cannot handle this and the easy way out of writing a
>>> semicolon after $i is out since,
>>>
>>>    % case "foo"; { *o) echo 1;; }
>>>    /bin/mksh: syntax error: unexpected ';'
>>>
>>> I came with the attached patch after a bit of trial and error but I'm
>>> not sure if it is the right way to do it.
>>
>> I think that might make sense, but perhaps Stefan has some comments here
>> (added to the CCs).
>
> To no one's surprise, the patch is not the right approach.  IIUC, when I
> use braces, *pattern) thingies are not recognised as "case-)" anymore so
> sh-indent-for-case-label does not get applied.  I tried a bit more but I
> only got it to respect the first line but still it is definitely not the
> right approach,

Did you make any further progress here?

bug#55764: 29.0.50; sh-mode: Support mksh's alternate case brace syntax

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 521 bytes --]

> To no one's surprise, the patch is not the right approach.  IIUC, when I
> use braces, *pattern) thingies are not recognised as "case-)" anymore so
> sh-indent-for-case-label does not get applied.  I tried a bit more but I
> only got it to respect the first line but still it is definitely not the
> right approach,

The first part of the puzzle is to fix the syntax-category of the `)` so
it's recognized as a "case-)" rather than a normal ("matched") paren.

The patch below seems to do the trick.


        Stefan



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: openbsd-case.patch --]
[-- Type: text/x-diff, Size: 1788 bytes --]

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index cc521cb0591..b34be83367a 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1053,7 +1053,8 @@ sh-font-lock-paren
                     ;; a normal command rather than the real `in' keyword.
                     ;; I.e. we should look back to try and find the
                     ;; corresponding `case'.
-                    (and (looking-at ";\\(?:;&?\\|[&|]\\)\\|\\_<in")
+                    ;; Also recognize OpenBSD's case X { ... } (bug#55764).
+                    (and (looking-at ";\\(?:;&?\\|[&|]\\)\\|\\_<in\\|.{")
                          ;; ";; esac )" is a case that looks
                          ;; like a case-pattern but it's really just a close
                          ;; paren after a case statement.  I.e. if we skipped
@@ -2053,9 +2054,9 @@ sh-smie-sh-rules
                              (sh-var-value 'sh-indent-for-case-label)))
     (`(:before . ,(or "(" "{" "[" "while" "if" "for" "case"))
      (cond
-      ((and (equal token "{") (smie-rule-parent-p "for"))
+      ((and (equal token "{") (smie-rule-parent-p "for" "case"))
        (let ((data (smie-backward-sexp "in")))
-         (when (equal (nth 2 data) "for")
+         (when (member (nth 2 data) '("for" "case"))
            `(column . ,(smie-indent-virtual)))))
       ((not (smie-rule-prev-p "&&" "||" "|"))
        (when (smie-rule-hanging-p)
diff --git a/test/manual/indent/shell.sh b/test/manual/indent/shell.sh
index 5b3fb0e66fb..42a981d312e 100755
--- a/test/manual/indent/shell.sh
+++ b/test/manual/indent/shell.sh
@@ -189,3 +189,10 @@ bar ()
 
     fi
 }
+
+case $i {                       # Bug#55764
+    *pattern)
+        (cd .; echo hi);
+        do1 ;;
+    *pattern2) do2 ;;
+}

bug#55764: 29.0.50; sh-mode: Support mksh's alternate case brace syntax

[Stefan Kangas]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> To no one's surprise, the patch is not the right approach.  IIUC, when I
>> use braces, *pattern) thingies are not recognised as "case-)" anymore so
>> sh-indent-for-case-label does not get applied.  I tried a bit more but I
>> only got it to respect the first line but still it is definitely not the
>> right approach,
>
> The first part of the puzzle is to fix the syntax-category of the `)` so
> it's recognized as a "case-)" rather than a normal ("matched") paren.
>
> The patch below seems to do the trick.

If you think it looks good, then please install.  Nothing stood out
when I read it over, FWIW.

>
>
>         Stefan
>
>
> diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
> index cc521cb0591..b34be83367a 100644
> --- a/lisp/progmodes/sh-script.el
> +++ b/lisp/progmodes/sh-script.el
> @@ -1053,7 +1053,8 @@ sh-font-lock-paren
>                      ;; a normal command rather than the real `in' keyword.
>                      ;; I.e. we should look back to try and find the
>                      ;; corresponding `case'.
> -                    (and (looking-at ";\\(?:;&?\\|[&|]\\)\\|\\_<in")
> +                    ;; Also recognize OpenBSD's case X { ... } (bug#55764).
> +                    (and (looking-at ";\\(?:;&?\\|[&|]\\)\\|\\_<in\\|.{")
>                           ;; ";; esac )" is a case that looks
>                           ;; like a case-pattern but it's really just a close
>                           ;; paren after a case statement.  I.e. if we skipped
> @@ -2053,9 +2054,9 @@ sh-smie-sh-rules
>                               (sh-var-value 'sh-indent-for-case-label)))
>      (`(:before . ,(or "(" "{" "[" "while" "if" "for" "case"))
>       (cond
> -      ((and (equal token "{") (smie-rule-parent-p "for"))
> +      ((and (equal token "{") (smie-rule-parent-p "for" "case"))
>         (let ((data (smie-backward-sexp "in")))
> -         (when (equal (nth 2 data) "for")
> +         (when (member (nth 2 data) '("for" "case"))
>             `(column . ,(smie-indent-virtual)))))
>        ((not (smie-rule-prev-p "&&" "||" "|"))
>         (when (smie-rule-hanging-p)
> diff --git a/test/manual/indent/shell.sh b/test/manual/indent/shell.sh
> index 5b3fb0e66fb..42a981d312e 100755
> --- a/test/manual/indent/shell.sh
> +++ b/test/manual/indent/shell.sh
> @@ -189,3 +189,10 @@ bar ()
>
>      fi
>  }
> +
> +case $i {                       # Bug#55764
> +    *pattern)
> +        (cd .; echo hi);
> +        do1 ;;
> +    *pattern2) do2 ;;
> +}

bug#55764: 29.0.50; sh-mode: Support mksh's alternate case brace syntax

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> The patch below seems to do the trick.
> If you think it looks good, then please install.  Nothing stood out
> when I read it over, FWIW.

Thanks for the reminder.  Pushed.
Closing,


        Stefan