Home
Reading
Searching
Subscribe
Sponsors
Statistics
Posting
Contact
Spam
Lists
Links
About
Hosting
Filtering
Features Download
Marketing
Archives
FAQ
Blog
 
Gmane
From: sand <sand <at> blarg.net>
Subject: Indentation bug in function INDENT-SEXP (from CVS sources)
Newsgroups: gmane.emacs.bugs
Date: Tuesday 30th January 2007 19:12:32 UTC (over 9 years ago)
This is with a version of GNU Emacs compiled from the CVS mainline sources
of (roughly) August 11th, 2006.  I have applied the multi-tty patch, so I'm
limited in the CVS versions I can use.  I'm confident that the problem has
not been fixed since, as I describe below.

Here's an Emacs Lisp definition with bad indentation:

(defadvice yank (after after-yank-indent-region activate)
  (when (memq major-mode '(c++-mode
                           c-mode
                           emacs-lisp-mode
                       erlang-mode
                       java-mode
                       lisp-mode
                       scheme-mode
                           scheme48-mode))
    (let ((mark-even-if-inactive t))
      (indent-region (region-beginning) (region-end) nil))))

Place mark at the beginning of the "erlang-mode" line and point at the
beginning of the "lisp-mode" line and run the "indent-region" command.  The
erlang, java *and* lisp lines get indented.  The "scheme-mode" line does
not:

(defadvice yank (after after-yank-indent-region activate)
  (when (memq major-mode '(c++-mode
                           c-mode
                           emacs-lisp-mode
                           erlang-mode
                           java-mode
                           lisp-mode
                       scheme-mode
                           scheme48-mode))
    (let ((mark-even-if-inactive t))
      (indent-region (region-beginning) (region-end) nil))))

I would expect the "lisp-mode" line to not move, based on the chosen
region.  One can argue that the seen behavior is the correct behavior, as
point is on the beginning of the "lisp-mode" line.  However, the same three
lines change if you put mark at the beginning of the "erlang-mode" line and
point at the *end* of the "java-mode" line (before the newline).  No part
of the "lisp-mode" line is part of the region, so that is definitely the
wrong behavior.  And in any case, I belive that the "lisp-mode" line should
not have changed in the first case, following the principle of least
surprise.

[WARNING: Description ends, analysis and patch follow!]

The bug is in the underlying INDENT-SEXP function.  It does test against
the ENDPOS argument (marking the end of the region), but this test occurs
at the top of the loop, before we have advanced to the following line, so
we get the off-by-one error we see above.  There have been two revisions to
lisp-mode.el since I compiled from CVS, neither of which deal with
INDENT-SEXP, so the problem should still be in the most recent sources.

I have attached a patch that appears to fix the problem.  With it, both
tests indent only the erlang and java lines.  The change hoists the
FORWARD-LINE call up and earlier in the code, and compares ENDPOS against
point immediately afterwards, setting OUTER-LOOP-DONE to true if we have
reached ENDPOS.  (Whether or not this is the right solution long-term is
another matter; INDENT-SEXP is already rather hairy.)

Derek

--
[email protected]

------------------------------ cut here ------------------------------

--- lisp-mode.el	2007-01-30 10:37:45.000000000 -0800
+++ mod-lisp-mode.el	2007-01-30 10:37:39.000000000 -0800
@@ -1128,8 +1128,11 @@
 		     next-depth 0)))
 	(or outer-loop-done endpos
 	    (setq outer-loop-done (<= next-depth 0)))
+        (forward-line 1)
+        (if (and endpos (<= endpos (point)))
+            (setq outer-loop-done t))
 	(if outer-loop-done
-	    (forward-line 1)
+            nil
 	  (while (> last-depth next-depth)
 	    (setq indent-stack (cdr indent-stack)
 		  last-depth (1- last-depth)))
@@ -1138,7 +1141,6 @@
 		  last-depth (1+ last-depth)))
 	  ;; Now go to the next line and indent it according
 	  ;; to what we learned from parsing the previous one.
-	  (forward-line 1)
 	  (setq bol (point))
 	  (skip-chars-forward " t")
 	  ;; But not if the line is blank, or just a comment
 
CD: 3ms