[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Indentation bug in function INDENT-SEXP (from CVS sources)
From: |
sand |
Subject: |
Indentation bug in function INDENT-SEXP (from CVS sources) |
Date: |
Tue, 30 Jan 2007 11:12:32 -0800 |
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
--
sand@blarg.net
------------------------------ 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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Indentation bug in function INDENT-SEXP (from CVS sources),
sand <=