unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#60833: [PATCH] sh-script.el: Add support for Zsh's case branches ; |.
@ 2023-01-15 13:28 Philippe Altherr
  2023-01-21  7:33 ` Eli Zaretskii
       [not found] ` <handler.60833.B.16737920195794.ack@debbugs.gnu.org>
  0 siblings, 2 replies; 6+ messages in thread
From: Philippe Altherr @ 2023-01-15 13:28 UTC (permalink / raw)
  To: 60833


[-- Attachment #1.1: Type: text/plain, Size: 1886 bytes --]

In shell scripts, case branches traditionally end with ;;. Bash
additionally supports case branches ending with ;& and ;;&. Zsh similarly
supports case branches ending with ;& and ;|. Currently sh-script.el
supports case branches ending with ;;, ;&, and ;;&, but not with ;|. The
attached patch adds support for case branches ending with ;|.

I have tested the patch by defining all the modified functions
(sh-smie-sh-rules, sh-font-lock-paren) and constants (sh-smie-sh-grammar,
sh-smie-rc-grammar, sh-smie--sh-operators, sh-smie--sh-operators-re,
sh-smie--sh-operators-back-re) in my .emacs (in a (with-eval-after-load
'sh-script ...) statement).

Here is an example indented without the patch:

case $input in
    *a* ) echo A;;
    *b* ) echo B;&
    *c* ) echo C;;&
    *d* ) echo D;|
*e* ) echo E;;
esac

and with the (simulated) patch:

case $input in
    *a* ) echo A;;
    *b* ) echo B;&
    *c* ) echo C;;&
    *d* ) echo D;|
    *e* ) echo E;;
esac

The first change in the patch replaces an (eq (char-before) ?|)
<https://github.com/emacs-mirror/emacs/blob/77ca6aa56e3425c87861cab8abce52bee3697cf4/lisp/progmodes/sh-script.el#L1045>
with
(and (eq (char-before) ?|) (not (eq (char-before (1- (point))) ?\;))). It
is needed to avoid confusing ;| tokens with plain | tokens. I wonder
however whether there would be a cleaner way of expressing the same.

The second change replaces a (looking-at ";[;&]\\|\\_<in")
<https://github.com/emacs-mirror/emacs/blob/77ca6aa56e3425c87861cab8abce52bee3697cf4/lisp/progmodes/sh-script.el#L1056>
with (looking-at ";\\(?:;&?\\|[&|]\\)\\|\\_<in"). The original expression
is looking for ;; and ;& tokens but not for ;;& tokens, which looks like an
oversight to me. That's why I have changed it to look for ;;, ;&, ;| but
also ;;&.

The other changes simply add support for ;| where there was previously
support for ;;, ;&, and ;;&.

Philippe

[-- Attachment #1.2: Type: text/html, Size: 2368 bytes --]

[-- Attachment #2: 0001-Add-support-for-Zsh-s-case-branches.patch --]
[-- Type: application/x-patch, Size: 3427 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#60833: [PATCH] sh-script.el: Add support for Zsh's case branches ; |.
  2023-01-15 13:28 bug#60833: [PATCH] sh-script.el: Add support for Zsh's case branches ; | Philippe Altherr
@ 2023-01-21  7:33 ` Eli Zaretskii
  2023-01-23  4:13   ` Philippe Altherr
       [not found] ` <handler.60833.B.16737920195794.ack@debbugs.gnu.org>
  1 sibling, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2023-01-21  7:33 UTC (permalink / raw)
  To: Philippe Altherr; +Cc: 60833

> From: Philippe Altherr <philippe.altherr@gmail.com>
> Date: Sun, 15 Jan 2023 14:28:08 +0100
> 
> In shell scripts, case branches traditionally end with ;;. Bash additionally supports case branches ending with
> ;& and ;;&. Zsh similarly supports case branches ending with ;& and ;|. Currently sh-script.el supports case
> branches ending with ;;, ;&, and ;;&, but not with ;|. The attached patch adds support for case branches
> ending with ;|.
> 
> I have tested the patch by defining all the modified functions (sh-smie-sh-rules, sh-font-lock-paren) and
> constants (sh-smie-sh-grammar, sh-smie-rc-grammar, sh-smie--sh-operators, sh-smie--sh-operators-re,
> sh-smie--sh-operators-back-re) in my .emacs (in a (with-eval-after-load 'sh-script ...) statement).
> 
> Here is an example indented without the patch:
> 
> case $input in
>     *a* ) echo A;;
>     *b* ) echo B;&
>     *c* ) echo C;;&
>     *d* ) echo D;|
> *e* ) echo E;;
> esac
> 
> and with the (simulated) patch:
> 
> case $input in
>     *a* ) echo A;;
>     *b* ) echo B;&
>     *c* ) echo C;;&
>     *d* ) echo D;|
>     *e* ) echo E;;
> esac

Thanks.  First, would it be possible to add tests for these
situations?

And second, your contributions (this and the other one) are larger
than we can accept without your assigning the copyright to the FSF.
Would you like to start your legal paperwork at this time, so that we
could accept the changes after it is completed?  If so, I will send
you the form to fill.

Thanks again for your interest in Emacs.





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#60833: [PATCH] sh-script.el: Add support for Zsh's case branches ; |.
  2023-01-21  7:33 ` Eli Zaretskii
@ 2023-01-23  4:13   ` Philippe Altherr
  2023-01-23 15:16     ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Philippe Altherr @ 2023-01-23  4:13 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60833


[-- Attachment #1.1: Type: text/plain, Size: 519 bytes --]

>
> Thanks.  First, would it be possible to add tests for these
> situations?
>

I added a test case in the attached updated patch. I wasn't able to test it
though. Please make sure it works as expected.


> And second, your contributions (this and the other one) are larger
> than we can accept without your assigning the copyright to the FSF.
> Would you like to start your legal paperwork at this time, so that we
> could accept the changes after it is completed?  If so, I will send
> you the form to fill.
>

Sure

[-- Attachment #1.2: Type: text/html, Size: 934 bytes --]

[-- Attachment #2: 0001-Add-support-for-Zsh-s-case-branches.patch --]
[-- Type: application/octet-stream, Size: 3654 bytes --]

From 7d39639f0c1cdae468750b012a773b6b549753ac Mon Sep 17 00:00:00 2001
From: Philippe Altherr <philippe.altherr@gmail.com>
Date: Sun, 15 Jan 2023 13:37:00 +0100
Subject: [PATCH] Add support for Zsh's case branches ;|.

---
 lisp/progmodes/sh-script.el | 18 +++++++++++-------
 test/manual/indent/shell.sh |  1 +
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 17c22ff475..2982bb3f34 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1042,7 +1042,9 @@ sh-font-lock-paren
                        ;; Maybe we've bumped into an escaped newline.
                        (sh-is-quoted-p (point)))
                 (backward-char 1))
-              (when (eq (char-before) ?|)
+              (when (and
+                     (eq (char-before) ?|)
+                     (not (eq (char-before (1- (point))) ?\;)))
                 (backward-char 1) t)))
         (and (> (point) (1+ (point-min)))
              (progn (backward-char 2)
@@ -1053,7 +1055,7 @@ 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")
+                    (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
@@ -1784,8 +1786,9 @@ sh-smie-sh-grammar
       (pattern (rpattern) ("case-(" rpattern))
       (branches (branches ";;" branches)
                 (branches ";&" branches) (branches ";;&" branches) ;bash.
+                (branches ";|" branches) ;zsh.
                 (pattern "case-)" cmd)))
-    '((assoc ";;" ";&" ";;&"))
+    '((assoc ";;" ";&" ";;&" ";|"))
     '((assoc ";" "&") (assoc "&&" "||") (assoc "|" "|&")))))
 
 (defconst sh-smie--sh-operators
@@ -2055,11 +2058,11 @@ sh-smie-sh-rules
 	 `(column . ,(smie-indent-virtual))))))
     ;; FIXME: Maybe this handling of ;; should be made into
     ;; a smie-rule-terminator function that takes the substitute ";" as arg.
-    (`(:before . ,(or ";;" ";&" ";;&"))
-     (if (and (smie-rule-bolp) (looking-at ";;?&?[ \t]*\\(#\\|$\\)"))
+    (`(:before . ,(or ";;" ";&" ";;&" ";|"))
+     (if (and (smie-rule-bolp) (looking-at ";\\(?:;&?\\|[&|]\\)?[ \t]*\\(#\\|$\\)"))
          (cons 'column (smie-indent-keyword ";"))
        (smie-rule-separator kind)))
-    (`(:after . ,(or ";;" ";&" ";;&"))
+    (`(:after . ,(or ";;" ";&" ";;&" ";|"))
      (with-demoted-errors "SMIE rule error: %S"
        (smie-backward-sexp token)
        (cons 'column
@@ -2148,8 +2151,9 @@ sh-smie-rc-grammar
       (pattern (pattern "|" pattern))
       (branches (branches ";;" branches)
                 (branches ";&" branches) (branches ";;&" branches) ;bash.
+                (branches ";|" branches) ;zsh.
                 (pattern "case-)" cmd)))
-    '((assoc ";;" ";&" ";;&"))
+    '((assoc ";;" ";&" ";;&" ";|"))
     '((assoc "case") (assoc ";" "&") (assoc "&&" "||") (assoc "|" "|&")))))
 
 (defun sh-smie--rc-after-special-arg-p ()
diff --git a/test/manual/indent/shell.sh b/test/manual/indent/shell.sh
index bd4a74f705..5b3fb0e66f 100755
--- a/test/manual/indent/shell.sh
+++ b/test/manual/indent/shell.sh
@@ -140,6 +140,7 @@     bar ()
         5) hello ;;
         4) hello ;&
         4) hello ;;&
+        4) hello ;|
         5) hello ;;
         5) hello ;;
     esac
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#60833: [PATCH] sh-script.el: Add support for Zsh's case branches ; |.
  2023-01-23  4:13   ` Philippe Altherr
@ 2023-01-23 15:16     ` Eli Zaretskii
  0 siblings, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2023-01-23 15:16 UTC (permalink / raw)
  To: Philippe Altherr; +Cc: 60833

> From: Philippe Altherr <philippe.altherr@gmail.com>
> Date: Mon, 23 Jan 2023 05:13:36 +0100
> Cc: 60833@debbugs.gnu.org
> 
>  And second, your contributions (this and the other one) are larger
>  than we can accept without your assigning the copyright to the FSF.
>  Would you like to start your legal paperwork at this time, so that we
>  could accept the changes after it is completed?  If so, I will send
>  you the form to fill.
> 
> Sure

Form sent off-list.

(I will review the patch soon.)





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#60833: Acknowledgement ([PATCH] sh-script.el: Add support for Zsh's case branches ; |.)
       [not found] ` <handler.60833.B.16737920195794.ack@debbugs.gnu.org>
@ 2023-02-17 15:20   ` Philippe Altherr
  2023-03-02 10:50     ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Philippe Altherr @ 2023-02-17 15:20 UTC (permalink / raw)
  To: 60833


[-- Attachment #1.1: Type: text/plain, Size: 905 bytes --]

Here is an updated patch against the current master branch.

On Sun, Jan 15, 2023 at 3:14 PM GNU bug Tracking System <
help-debbugs@gnu.org> wrote:

> Thank you for filing a new bug report with debbugs.gnu.org.
>
> This is an automatically generated reply to let you know your message
> has been received.
>
> Your message is being forwarded to the package maintainers and other
> interested parties for their attention; they will reply in due course.
>
> Your message has been sent to the package maintainer(s):
>  bug-gnu-emacs@gnu.org
>
> If you wish to submit further information on this problem, please
> send it to 60833@debbugs.gnu.org.
>
> Please do not send mail to help-debbugs@gnu.org unless you wish
> to report a problem with the Bug-tracking system.
>
> --
> 60833: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=60833
> GNU Bug Tracking System
> Contact help-debbugs@gnu.org with problems
>

[-- Attachment #1.2: Type: text/html, Size: 1657 bytes --]

[-- Attachment #2: 0001-Add-support-for-Zsh-s-case-branches.patch --]
[-- Type: application/octet-stream, Size: 3658 bytes --]

From f5c500be11a2a77f2a9e45f68a6010924d86e945 Mon Sep 17 00:00:00 2001
From: Philippe Altherr <philippe.altherr@gmail.com>
Date: Sun, 15 Jan 2023 13:37:00 +0100
Subject: [PATCH] Add support for Zsh's case branches ;|.

---
 lisp/progmodes/sh-script.el | 18 +++++++++++-------
 test/manual/indent/shell.sh |  1 +
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 17c22ff4751..2982bb3f345 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1042,7 +1042,9 @@ sh-font-lock-paren
                        ;; Maybe we've bumped into an escaped newline.
                        (sh-is-quoted-p (point)))
                 (backward-char 1))
-              (when (eq (char-before) ?|)
+              (when (and
+                     (eq (char-before) ?|)
+                     (not (eq (char-before (1- (point))) ?\;)))
                 (backward-char 1) t)))
         (and (> (point) (1+ (point-min)))
              (progn (backward-char 2)
@@ -1053,7 +1055,7 @@ 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")
+                    (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
@@ -1784,8 +1786,9 @@ sh-smie-sh-grammar
       (pattern (rpattern) ("case-(" rpattern))
       (branches (branches ";;" branches)
                 (branches ";&" branches) (branches ";;&" branches) ;bash.
+                (branches ";|" branches) ;zsh.
                 (pattern "case-)" cmd)))
-    '((assoc ";;" ";&" ";;&"))
+    '((assoc ";;" ";&" ";;&" ";|"))
     '((assoc ";" "&") (assoc "&&" "||") (assoc "|" "|&")))))
 
 (defconst sh-smie--sh-operators
@@ -2055,11 +2058,11 @@ sh-smie-sh-rules
 	 `(column . ,(smie-indent-virtual))))))
     ;; FIXME: Maybe this handling of ;; should be made into
     ;; a smie-rule-terminator function that takes the substitute ";" as arg.
-    (`(:before . ,(or ";;" ";&" ";;&"))
-     (if (and (smie-rule-bolp) (looking-at ";;?&?[ \t]*\\(#\\|$\\)"))
+    (`(:before . ,(or ";;" ";&" ";;&" ";|"))
+     (if (and (smie-rule-bolp) (looking-at ";\\(?:;&?\\|[&|]\\)?[ \t]*\\(#\\|$\\)"))
          (cons 'column (smie-indent-keyword ";"))
        (smie-rule-separator kind)))
-    (`(:after . ,(or ";;" ";&" ";;&"))
+    (`(:after . ,(or ";;" ";&" ";;&" ";|"))
      (with-demoted-errors "SMIE rule error: %S"
        (smie-backward-sexp token)
        (cons 'column
@@ -2148,8 +2151,9 @@ sh-smie-rc-grammar
       (pattern (pattern "|" pattern))
       (branches (branches ";;" branches)
                 (branches ";&" branches) (branches ";;&" branches) ;bash.
+                (branches ";|" branches) ;zsh.
                 (pattern "case-)" cmd)))
-    '((assoc ";;" ";&" ";;&"))
+    '((assoc ";;" ";&" ";;&" ";|"))
     '((assoc "case") (assoc ";" "&") (assoc "&&" "||") (assoc "|" "|&")))))
 
 (defun sh-smie--rc-after-special-arg-p ()
diff --git a/test/manual/indent/shell.sh b/test/manual/indent/shell.sh
index bd4a74f7054..5b3fb0e66fb 100755
--- a/test/manual/indent/shell.sh
+++ b/test/manual/indent/shell.sh
@@ -140,6 +140,7 @@     bar ()
         5) hello ;;
         4) hello ;&
         4) hello ;;&
+        4) hello ;|
         5) hello ;;
         5) hello ;;
     esac
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#60833: Acknowledgement ([PATCH] sh-script.el: Add support for Zsh's case branches ; |.)
  2023-02-17 15:20   ` bug#60833: Acknowledgement ([PATCH] sh-script.el: Add support for Zsh's case branches ; |.) Philippe Altherr
@ 2023-03-02 10:50     ` Eli Zaretskii
  0 siblings, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2023-03-02 10:50 UTC (permalink / raw)
  To: Philippe Altherr; +Cc: 60833-done

> From: Philippe Altherr <philippe.altherr@gmail.com>
> Date: Fri, 17 Feb 2023 16:20:59 +0100
> 
> Here is an updated patch against the current master branch.

Thanks, installed on master, and closing the bug.





^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-03-02 10:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-15 13:28 bug#60833: [PATCH] sh-script.el: Add support for Zsh's case branches ; | Philippe Altherr
2023-01-21  7:33 ` Eli Zaretskii
2023-01-23  4:13   ` Philippe Altherr
2023-01-23 15:16     ` Eli Zaretskii
     [not found] ` <handler.60833.B.16737920195794.ack@debbugs.gnu.org>
2023-02-17 15:20   ` bug#60833: Acknowledgement ([PATCH] sh-script.el: Add support for Zsh's case branches ; |.) Philippe Altherr
2023-03-02 10:50     ` Eli Zaretskii

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).