bug#19102: 24.4; outline-move-subtree-up/down error at last and second-last subtree

[Stefan Monnier <monnier@IRO.UMontreal.CA>]

> +	(maybe-forward-char (lambda ()
> +			      (and (eobp) (not (bolp)) (newline))
> +			      (and (eolp) (not (bolp)) (forward-char 1))))

If we add a newline, we know don't want to do a `forward-char'.  So:

   			      (if (and (eobp) (not (bolp))) (newline)
   			        (and (eolp) (not (bolp)) (forward-char 1)))))

Also, I don't understand the (not (bolp)) test in the second line: the
code it replaces only tested (= (char-after) ?\n).  So I think it'd be
better to use:

   			      (if (and (eobp) (not (bolp))) (newline)
   			        (if (eq (char-after) ?\n) (forward-char 1)))))

And to be closer to the original code, I'd swap the two tests.  And I'd
use `insert' rather than `newline' since I don't want to run abbrev
expansions and things like that.

   			      (if (eq (char-after) ?\n) (forward-char 1)
   			        (if (and (eobp) (not (bolp))) (insert "\n")))))

> +	(empty-last-line (save-excursion
> +			   (goto-char (point-max))
> +			   (and (bolp) (eolp))))

At point-max, we know that (eolp) is non-nil, so you can just write

   	(empty-last-line (save-excursion
   			   (goto-char (point-max))
   			   (bolp))))

> -		 (error "Cannot move past superior level")))
> +		 (with-demoted-errors "%s"
> +		   (message "Cannot move past superior level"))))

This is wrong.  Did you mean maybe to replace `error' with
`user-error' instead, maybe?

> +    ;; If we added a newline to move forward, delete it.
> +    (save-excursion
> +      (goto-char (point-max))
> +      (when (and (bolp) (eolp) (not empty-last-line))
> +	(delete-char -1)))))

Let's not bother.  There's no harm in adding a missing final newline
when we modify such a line-oriented file.


        Stefan