bug#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore

bug#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore

[Reiner Steib]

Hi,

in Emacs 24.3.91, `sh-learn-buffer-indent' doesn't seem to learn the
current indent rules of the buffer anymore.  It worked correctly in
23.3 and better in 24.1, see below.

(I use `sh-learn-buffer-indent' in `sh-set-shell-hook'.)

Consider this shell script:

###
$ cat shell-script-test.sh
#!/bin/sh

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

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

* In Emacs 23.3 (==> *expected behavior*):

- emacs -title emacs-23.3 shell-script-test.sh
- M-x sh-learn-buffer-indent RET
- C-x b *indent* RET

  Switching to buffer *indent* shows:

  Indentation values for buffer shell-script-test.sh.
  0 indentation variables have different values.

  Comments follow default indentation.

  Initial value of sh-basic-offset: 4
  Suggested sh-basic-offset:  2

  Learned variable settings:
    sh-indent-after-if 2
    sh-indent-for-fi 0
    sh-indent-after-loop-construct 2
    sh-indent-for-done 0
    sh-indent-comment t

- Put point after first "then", hit RET and TAB (or C-j)
- the new line is indented by 2 spaces (==> *expected behavior*)

- Put point after first "echo" line, hit RET and TAB (or C-j)
- the new line is indented by 2 spaces (==> *expected behavior*)

* Emacs 24.1:

- emacs -title emacs-24.1 -Q shell-script-test.sh
- M-x sh-learn-buffer-indent RET

Result:
  Message: Buffer is read-only: #<buffer *indent*>

  Buffer *indent* exists but it is empty.

Something is wrong, but indentation works as expected.

- Put point after first "then", hit RET and TAB (or C-j)
- the new line is indented by 2 spaces (==> *expected behavior*)

- Put point after first "echo" line, hit RET and TAB (or C-j)
- the new line is indented by 2 spaces (==> *expected behavior*)

* Emacs 24.3.91:

- emacs -title emacs-24.1 -Q shell-script-test.sh
- M-x sh-learn-buffer-indent RET

Message: Local rules set

- Put point after first "then", hit RET (or C-j and TAB) [1]
- the new line is indented by 4 spaces (==> *wrong behavior*)

- Put point after first "echo" line, hitRET (or C-j and TAB)
- the new line is indented by 4 spaces (==> *wrong behavior*)
  and the "echo" line is indented by 4 spaces (==> *wrong behavior*)

In GNU Emacs 24.3.91.1 (i686-pc-mingw32)
 of 2014-05-12 on LEG570
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --enable-checking 'CFLAGS=-O0 -g3' CPPFLAGS=-DGLYPH_DEBUG=1'

Important settings:
  value of $LANG: C.ISO-8859-1
  locale-coding-system: cp1252


Bye, Reiner.


[1] NEWS says:

*** `electric-indent-mode' is now enabled by default.
Typing RET reindents the current line and indents the new line.
`C-j' inserts a newline but does not indent.  In some programming modes,
additional characters are electric (eg `{').
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

bug#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore

[Stefan Monnier]

> in Emacs 24.3.91, `sh-learn-buffer-indent' doesn't seem to learn the
> current indent rules of the buffer anymore.  It worked correctly in
> 23.3 and better in 24.1, see below.

24.4 uses SMIE, so the indentation rules are quite different and they
don't obey all the sh-indent-* variables.  Also the "learning" code uses
SMIE's generic learning code which is brand new and hasn't seen much
testing yet.

So, there might be bugs at several levels:
1- setting sh-use-smie to nil should revert to the 24.3 behavior.
   This is just a stop-gap setting that will disappear in some future
   release (depending on how many bug reports we get about the SMIE
   code ;-).
2- If you set the sh-indent-* vars learned by the non-SMIE code, the
   SMIE code should indent as desired.
3- the generic SMIE learning code should behave about as well as the old
   sh-learn-buffer-indent (i.e. "poorly" in my opinion ;-).


        Stefan

bug#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore

[Stefan Monnier]

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

The above "do" should be "then", right?

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.



        Stefan


=== modified file 'lisp/emacs-lisp/smie.el'
--- lisp/emacs-lisp/smie.el	2014-06-20 01:05:40 +0000
+++ lisp/emacs-lisp/smie.el	2014-06-20 21:07:06 +0000
@@ -2138,7 +2138,7 @@
                   nil
                 (push (cons (+ offset (nth 2 sig)) sig) rules)
                 ;; Adjust the rest of the data.
-                (pcase-dolist ((and cotrace `(,count ,toffset ,trace))
+                (pcase-dolist ((and cotrace `(,count ,toffset . ,trace))
                                cotraces)
                   (setf (nth 1 cotrace) (- toffset offset))
                   (dolist (sig trace)
@@ -2167,15 +2167,14 @@
     (cond
      ((null config) (message "Nothing to change"))
      ((null smie-config--buffer-local)
-      (message "Local rules set")
-      (setq smie-config--buffer-local config))
+      (smie-config-local config)
+      (message "Local rules set"))
      ((y-or-n-p "Replace existing local config? ")
       (message "Local rules replaced")
-      (setq smie-config--buffer-local config))
+      (smie-config-local config))
      ((y-or-n-p "Merge with existing local config? ")
       (message "Local rules adjusted")
-      (setq smie-config--buffer-local
-            (append config smie-config--buffer-local)))
+      (smie-config-local (append config smie-config--buffer-local)))
      (t
       (message "Rules guessed: %S" config)))))
 

bug#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore

[Reiner Steib]

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#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore

[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 aren't due to the guessing part, but the indentation code, so I'll
deal with them in a separate report.


        Stefan