bug#17842: sh-script's indentation gets confused by \;

bug#17842: sh-script's indentation gets confused by \;

[Stefan Monnier]

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

Splitting into another bug.


[-- Attachment #2: Type: message/rfc822, Size: 4132 bytes --]

From: Reiner Steib <reiner.steib@gmx.de>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: 17818@debbugs.gnu.org
Subject: Re: bug#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore
Date: Mon, 23 Jun 2014 18:04:50 +0200
Message-ID: <20140623180450.pgyd1ksywokgcs8s@imap.uni-ulm.de>

On Fri, 20 Jun 2014, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>> if true; do
>>   echo "My sh-basic-offset offset should be 2."
>> fi
>
> The above "do" should be "then", right?

Sure, sorry. (I corrected it in my test-script, but not in the mail).

> I installed the patch below which fixes some of the problem (the first
> hunk fixes an incorrect guess and the second fixes the code so the guess
> is actually activated).
>
> Please confirm that the result is OK for your use case.

Works much better, thanks.

However, I tested[1] the new code with the following script and found
some incorrect indents (see diff below, hope the whitespace doesn't
get changed).

Bye, Reiner.

[1] M-x sh-learn-buffer-indent RET, M-x mark-whole-buffer RET, M-x
     indent-region RET, M-x diff-buffer-with-file RET

#!/bin/bash

if true; then
   echo "My sh-basic-offset offset should be 2."
fi

for a in 1 2; do
   echo "My sh-basic-offset offset should be 2."
done

filter_1 ()
{
   tr -d '"' |
     awk -F\; '{ if ($7 == 0 || $7 == 1) { print $7 } ; \
       print $5 "," $1 }' |
     grep -v "^,"
}

filter_3 ()
{
   tr -d '"`' | tr '	' ' ' | \
     awk -F\; -f filter.awk | \
     grep -v "^," | sort -t, -k2,2
}

tail -q -n+2 assets-ws.csv | awk -F \; -f merge.awk \
   <( cat file1.csv file2.csv ) - | \
   filter_2 | $conv | \
   sed -f a.sed | sort -t\; -k3,3 -k1,1 > w.csv

# end



--- shell-script-test.sh
+++ #<buffer shell-script-test.sh>
@@ -12,21 +12,21 @@
  filter_1 ()
  {
    tr -d '"' |
-    awk -F\; '{ if ($7 == 0 || $7 == 1) { print $7 } ; \
+awk -F\; '{ if ($7 == 0 || $7 == 1) { print $7 } ; \
        print $5 "," $1 }' |
-    grep -v "^,"
+grep -v "^,"
  }

  filter_3 ()
  {
    tr -d '"`' | tr '	' ' ' | \
-    awk -F\; -f filter.awk | \
-    grep -v "^," | sort -t, -k2,2
+  awk -F\; -f filter.awk | \
+grep -v "^," | sort -t, -k2,2
  }

  tail -q -n+2 assets-ws.csv | awk -F \; -f merge.awk \
-  <( cat file1.csv file2.csv ) - | \
-  filter_2 | $conv | \
-  sed -f a.sed | sort -t\; -k3,3 -k1,1 > w.csv
+					  <( cat file1.csv file2.csv ) - | \
+					  filter_2 | $conv | \
+					  sed -f a.sed | sort -t\; -k3,3 -k1,1 > w.csv

  # end

Diff finished.  Mon Jun 23 17:56:00 2014

bug#17842: sh-script's indentation gets confused by \;

[Stefan Monnier]

> However, I tested[1] the new code with the following script and found
> some incorrect indents (see diff below, hope the whitespace doesn't
> get changed).

These are due to the new indentation code.

> --- shell-script-test.sh
> +++ #<buffer shell-script-test.sh>
> @@ -12,21 +12,21 @@
>   filter_1 ()
>   {
>     tr -d '"' |
> -    awk -F\; '{ if ($7 == 0 || $7 == 1) { print $7 } ; \
> +awk -F\; '{ if ($7 == 0 || $7 == 1) { print $7 } ; \
>         print $5 "," $1 }' |
> -    grep -v "^,"
> +grep -v "^,"
>   }

>   filter_3 ()
>   {
>     tr -d '"`' | tr '	' ' ' | \
> -    awk -F\; -f filter.awk | \
> -    grep -v "^," | sort -t, -k2,2
> +  awk -F\; -f filter.awk | \
> +grep -v "^," | sort -t, -k2,2
>   }

>   tail -q -n+2 assets-ws.csv | awk -F \; -f merge.awk \
> -  <( cat file1.csv file2.csv ) - | \
> -  filter_2 | $conv | \
> -  sed -f a.sed | sort -t\; -k3,3 -k1,1 > w.csv
> +					  <( cat file1.csv file2.csv ) - | \
> +					  filter_2 | $conv | \
> +					  sed -f a.sed | sort -t\; -k3,3 -k1,1 > w.csv

I installed the patch below which fixes the problem with \;
and also with the indentation after "a | b |".

The "<(cat...)" line is still not indented as you want it because you
expect a different logic from the one followed by the current
indentation code (the current code indents multi-line elements connected
by \ as multi line constructs rather than "one long wrapped line", so
the <(...) gets aligned with the other arguments to the "sort" command).


        Stefan


=== modified file 'lisp/progmodes/sh-script.el'
--- lisp/progmodes/sh-script.el	2014-06-20 14:23:30 +0000
+++ lisp/progmodes/sh-script.el	2014-06-24 19:55:01 +0000
@@ -466,6 +466,9 @@
 	?~ "_"
 	?, "_"
 	?= "."
+	?\; "."
+	?| "."
+	?& "."
 	?< "."
 	?> ".")
   "The syntax table to use for Shell-Script mode.
@@ -1837,6 +1840,40 @@
    ((equal tok "in") (sh-smie--sh-keyword-in-p))
    (t (sh-smie--keyword-p))))
 
+(defun sh-smie--default-forward-token ()
+  (forward-comment (point-max))
+  (buffer-substring-no-properties
+   (point)
+   (progn (if (zerop (skip-syntax-forward "."))
+              (while (progn (skip-syntax-forward "w_'")
+                            (looking-at "\\\\"))
+                (forward-char 2)))
+          (point))))
+
+(defun sh-smie--default-backward-token ()
+  (forward-comment (- (point)))
+  (let ((pos (point))
+        (n (skip-syntax-backward ".")))
+    (if (or (zerop n)
+            (and (eq n -1)
+                 (let ((p (point)))
+                   (if (eq -1 (% (skip-syntax-backward "\\") 2))
+                       t
+                     (goto-char p)
+                     nil))))
+        (while
+            (progn (skip-syntax-backward "w_'")
+                   (or (not (zerop (skip-syntax-backward "\\")))
+                       (when (eq ?\\ (char-before (1- (point))))
+                         (let ((p (point)))
+                           (forward-char -1)
+                           (if (eq -1 (% (skip-syntax-backward "\\") 2))
+                               t
+                             (goto-char p)
+                             nil))))))
+      (goto-char (- (point) (% (skip-syntax-backward "\\") 2))))
+    (buffer-substring-no-properties (point) pos)))
+
 (defun sh-smie-sh-forward-token ()
   (if (and (looking-at "[ \t]*\\(?:#\\|\\(\\s|\\)\\|$\\)")
            (save-excursion
@@ -1865,7 +1902,7 @@
         tok))
      (t
       (let* ((pos (point))
-             (tok (smie-default-forward-token)))
+             (tok (sh-smie--default-forward-token)))
         (cond
          ((equal tok ")") "case-)")
          ((equal tok "(") "case-(")
@@ -1909,7 +1946,7 @@
       (goto-char (match-beginning 1))
       (match-string-no-properties 1))
      (t
-      (let ((tok (smie-default-backward-token)))
+      (let ((tok (sh-smie--default-backward-token)))
         (cond
          ((equal tok ")") "case-)")
          ((equal tok "(") "case-(")
@@ -1939,18 +1976,18 @@
     (`(:after . "case-)") (- (sh-var-value 'sh-indent-for-case-alt)
                              (sh-var-value 'sh-indent-for-case-label)))
     ((and `(:before . ,_)
-          (guard (when sh-indent-after-continuation
-                   (save-excursion
+          ;; After a line-continuation, make sure the rest is indented.
+          (guard sh-indent-after-continuation)
+          (guard (save-excursion
                      (ignore-errors
                        (skip-chars-backward " \t")
-                       (sh-smie--looking-back-at-continuation-p))))))
-     ;; After a line-continuation, make sure the rest is indented.
-     (let* ((sh-indent-after-continuation nil)
-            (indent (smie-indent-calculate))
-            (initial (sh-smie--continuation-start-indent)))
-       (when (and (numberp indent) (numberp initial)
-                  (<= indent initial))
-         `(column . ,(+ initial sh-indentation)))))
+                     (sh-smie--looking-back-at-continuation-p))))
+          (let initial (sh-smie--continuation-start-indent))
+          (guard (let* ((sh-indent-after-continuation nil)
+                        (indent (smie-indent-calculate)))
+                   (and (numberp indent) (numberp initial)
+                        (<= indent initial)))))
+     `(column . ,(+ initial sh-indentation)))
     (`(:before . ,(or `"(" `"{" `"["))
      (when (smie-rule-hanging-p)
        (if (not (smie-rule-prev-p "&&" "||" "|"))
@@ -1974,7 +2011,12 @@
                             (smie-rule-bolp))))
                  (current-column)
                (smie-indent-calculate)))))
-    (`(:after . ,(or `"|" `"&&" `"||")) (if (smie-rule-parent-p token) nil 4))
+    (`(:before . ,(or `"|" `"&&" `"||"))
+     (unless (smie-rule-parent-p token)
+       (smie-backward-sexp token)
+       `(column . ,(+ (funcall smie-rules-function :elem 'basic)
+                      (smie-indent-virtual)))))
+
     ;; Attempt at backward compatibility with the old config variables.
     (`(:before . "fi") (sh-var-value 'sh-indent-for-fi))
     (`(:before . "done") (sh-var-value 'sh-indent-for-done))
@@ -2095,7 +2137,7 @@
      ;;    tok))
      (t
       (let* ((pos (point))
-             (tok (smie-default-forward-token)))
+             (tok (sh-smie--default-forward-token)))
         (cond
          ;; ((equal tok ")") "case-)")
          ((and tok (string-match "\\`[a-z]" tok)
@@ -2136,7 +2178,7 @@
      ;;  (goto-char (match-beginning 1))
      ;;  (match-string-no-properties 1))
      (t
-      (let ((tok (smie-default-backward-token)))
+      (let ((tok (sh-smie--default-backward-token)))
         (cond
          ;; ((equal tok ")") "case-)")
          ((and tok (string-match "\\`[a-z]" tok)