Patch: bugfix for outline-mode

Patch: bugfix for outline-mode

[Ben North]

I noticed a couple of bugs in outline-mode, which surface when the last
line of the file is a heading line, and there is no final newline in the
buffer.  Both bugs show up when trying to use `outline-promote' or
`outline-demote'.  Suppose a buffer in outline-mode contains

------------------------------------------------------------------------
* H1

** H2

*** H3<end-of-buffer>
------------------------------------------------------------------------

i.e., there is no final newline.

Put point before "H3" and press C-c C-< (`outline-promote').  Point
moves to the start of the line, but H3 is not promoted.  (With a final
newline, the line is correctly changed to "** H3".)

Still with no final newline, put point before "H2" and press C-c C-<.
Emacs goes into a loop, and needs C-g to break out.  (Again, with a
final newline, behaviour is correct.)

I think the behaviour of `outline-next-visible-heading' causes the
problem.  Normally, when there are no more headings below point,
`outline-next-visible-heading' moves point to the start of the last line
of the buffer.  But when the last line of the buffer has no newline, and
is a heading line, this does not move point, causing
`outline-get-next-sibling' to loop forever.  It seems reasonable to me
for point to be left at the very end of the buffer by
`outline-next-visible-heading' in the case that there is no next visible
heading (the whitespace changes make this patch look bigger than it is):


--- ORIG--outline.el    2006-12-15 15:37:59.538393000 +0000
+++ NEW--outline.el     2006-12-15 15:38:53.575300000 +0000
@@ -652,19 +652,22 @@
   (if (< arg 0)
       (beginning-of-line)
     (end-of-line))
-  (while (and (not (bobp)) (< arg 0))
-    (while (and (not (bobp))
-               (re-search-backward (concat "^\\(?:" outline-regexp "\\)")
-                                   nil 'move)
-               (outline-invisible-p)))
-    (setq arg (1+ arg)))
-  (while (and (not (eobp)) (> arg 0))
-    (while (and (not (eobp))
-               (re-search-forward (concat "^\\(?:" outline-regexp "\\)")
-                                  nil 'move)
-               (outline-invisible-p (match-beginning 0))))
-    (setq arg (1- arg)))
-  (beginning-of-line))
+  (let ((found-heading-p))
+    (while (and (not (bobp)) (< arg 0))
+      (while (and (not (bobp))
+                  (setq found-heading-p
+                        (re-search-backward (concat "^\\(?:" outline-regexp
"\\)")
+                                            nil 'move))
+                  (outline-invisible-p)))
+      (setq arg (1+ arg)))
+    (while (and (not (eobp)) (> arg 0))
+      (while (and (not (eobp))
+                  (setq found-heading-p
+                        (re-search-forward (concat "^\\(?:" outline-regexp
"\\)")
+                                           nil 'move))
+                  (outline-invisible-p (match-beginning 0))))
+      (setq arg (1- arg)))
+    (if found-heading-p (beginning-of-line))))

 (defun outline-previous-visible-heading (arg)
   "Move to the previous heading line.


This patch also makes `outline-next-visible-heading' correctly return
nil in this case.  This fixes the bug I initially noticed.  I don't
think any other code relies on the previous behaviour.  Short changelog
description would be

        * outline.el (outline-next-visible-heading): Fix behaviour when
        buffer does not have final newline.


In fact, the behaviour of `outline-get-next-sibling' is incorrect with
the code as it is when the last line of the buffer has no newline, but
it doesn't cause a loop for outline-promote in this case.

Also, the documentation for `outline-promote' and `outline-demote'
describes the sense of the prefix argument backwards.  I think the
actual behaviour (promote/demote children by default) is desirable, and
the documentation should be changed as follows:


--- ORIG--outline.el    2006-12-15 15:37:59.538393000 +0000
+++ NEW1--outline.el    2006-12-15 15:45:40.406884000 +0000
@@ -473,7 +473,7 @@

 (defun outline-promote (&optional children)
   "Promote headings higher up the tree.
-If prefix argument CHILDREN is given, promote also all the children.
+Unless prefix argument CHILDREN is given, promote also all the children.
 If the region is active in `transient-mark-mode', promote all headings
 in the region."
   (interactive
@@ -509,7 +509,7 @@

 (defun outline-demote (&optional children)
   "Demote headings lower down the tree.
-If prefix argument CHILDREN is given, demote also all the children.
+Unless prefix argument CHILDREN is given, demote also all the children.
 If the region is active in `transient-mark-mode', demote all headings
 in the region."
   (interactive


(Separately diff'd against same ORIG, but should apply in either order
with small fuzz.)  Short changelog description would be

        * outline.el (outline-promote): Fix docstring.
        (outline-demote): Likewise.

Re: Patch: bugfix for outline-mode

[Ben North]

Earlier I wrote:
 > I noticed a couple of bugs in outline-mode, which surface when the last
 > line of the file is a heading line, and there is no final newline in the
 > buffer.

Apologies; the patch got mangled.  Second try, also making a tiny
stylistic improvement (no double parentheses in (let) form):

--- ORIG--outline.el    2006-12-15 15:37:59.538393000 +0000
+++ NEW--outline.el     2006-12-15 15:38:53.575300000 +0000
@@ -652,19 +652,22 @@
   (if (< arg 0)
       (beginning-of-line)
     (end-of-line))
-  (while (and (not (bobp)) (< arg 0))
-    (while (and (not (bobp))
-               (re-search-backward (concat "^\\(?:" outline-regexp "\\)")
-                                   nil 'move)
-               (outline-invisible-p)))
-    (setq arg (1+ arg)))
-  (while (and (not (eobp)) (> arg 0))
-    (while (and (not (eobp))
-               (re-search-forward (concat "^\\(?:" outline-regexp "\\)")
-                                  nil 'move)
-               (outline-invisible-p (match-beginning 0))))
-    (setq arg (1- arg)))
-  (beginning-of-line))
+  (let (found-heading-p)
+    (while (and (not (bobp)) (< arg 0))
+      (while (and (not (bobp))
+                  (setq found-heading-p
+                        (re-search-backward (concat "^\\(?:" 
outline-regexp "\\)")
+                                            nil 'move))
+                  (outline-invisible-p)))
+      (setq arg (1+ arg)))
+    (while (and (not (eobp)) (> arg 0))
+      (while (and (not (eobp))
+                  (setq found-heading-p
+                        (re-search-forward (concat "^\\(?:" 
outline-regexp "\\)")
+                                           nil 'move))
+                  (outline-invisible-p (match-beginning 0))))
+      (setq arg (1- arg)))
+    (if found-heading-p (beginning-of-line))))
 
 (defun outline-previous-visible-heading (arg)
   "Move to the previous heading line.


Doc fix patch made it OK.