unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore
@ 2014-06-20 14:54 Reiner Steib
  2014-06-20 15:48 ` Stefan Monnier
  2014-06-20 21:10 ` Stefan Monnier
  0 siblings, 2 replies; 5+ messages in thread
From: Reiner Steib @ 2014-06-20 14:54 UTC (permalink / raw)
  To: 17818

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/





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

* bug#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore
  2014-06-20 14:54 bug#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore Reiner Steib
@ 2014-06-20 15:48 ` Stefan Monnier
  2014-06-20 21:10 ` Stefan Monnier
  1 sibling, 0 replies; 5+ messages in thread
From: Stefan Monnier @ 2014-06-20 15:48 UTC (permalink / raw)
  To: Reiner Steib; +Cc: 17818

> 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





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

* bug#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore
  2014-06-20 14:54 bug#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore Reiner Steib
  2014-06-20 15:48 ` Stefan Monnier
@ 2014-06-20 21:10 ` Stefan Monnier
  2014-06-23 16:04   ` Reiner Steib
  1 sibling, 1 reply; 5+ messages in thread
From: Stefan Monnier @ 2014-06-20 21:10 UTC (permalink / raw)
  To: Reiner Steib; +Cc: 17818

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






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

* bug#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore
  2014-06-20 21:10 ` Stefan Monnier
@ 2014-06-23 16:04   ` Reiner Steib
  2014-06-24 13:49     ` Stefan Monnier
  0 siblings, 1 reply; 5+ messages in thread
From: Reiner Steib @ 2014-06-23 16:04 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 17818

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







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

* bug#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore
  2014-06-23 16:04   ` Reiner Steib
@ 2014-06-24 13:49     ` Stefan Monnier
  0 siblings, 0 replies; 5+ messages in thread
From: Stefan Monnier @ 2014-06-24 13:49 UTC (permalink / raw)
  To: Reiner Steib; +Cc: 17818-done

> 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





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

end of thread, other threads:[~2014-06-24 13:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-20 14:54 bug#17818: 24.3.91; sh-learn-buffer-indent doesn't learn current indent anymore Reiner Steib
2014-06-20 15:48 ` Stefan Monnier
2014-06-20 21:10 ` Stefan Monnier
2014-06-23 16:04   ` Reiner Steib
2014-06-24 13:49     ` Stefan Monnier

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