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

[Stephen Berman <stephen.berman@gmx.net>]

On Thu, 20 Nov 2014 17:22:50 +1000 Paul Rankin <paul@tilk.co> wrote:

> I needed to rewrite this for functionality with my own package, so I
> rewrote it very arrogantly and this is what I came up with...
>
> (defun fountain-outline-shift-down (&optional arg)
>   (interactive "p")
>   (outline-back-to-heading)
>   (let* ((move-func
>           (if (< 0 arg)
>               'outline-get-next-sibling
>             'outline-get-last-sibling))
>          (end-point-func
>           '(lambda ()
>              (outline-end-of-subtree)
>              (if (and (eolp)
>                       (not (bolp))
>                       (eobp))
>                  (open-line 1))
>              (unless (eobp)
>                (forward-char 1))
>              (point)))
>          (beg (point))
>          (end
>           (save-excursion
>             (funcall end-point-func)))
>          (folded
>           (save-excursion
>             (outline-end-of-heading)
>             (outline-invisible-p)))
>          (insert-point (make-marker))
>          (i (abs arg)))
>     (goto-char beg)
>     (while (< 0 i)
>       (or (funcall move-func)
>           (progn (goto-char beg)
>                  (message "Cannot move past superior level")))
>       (setq i (1- i)))
>     (if (< 0 arg)                       ; what does this bit do??
>         (funcall end-point-func))
>     (move-marker insert-point (point))
>     (insert (delete-and-extract-region beg end))
>     (goto-char insert-point)
>     (if folded
>         (hide-subtree))
>     (set-marker insert-point nil)))

This basically does the same thing as the patch I proposed -- adding a
newline if necessary to ensure forward movement -- except that if there
was no empty line after the last subtree, your code leaves the added
newline dangling.  If you add that bit then there is effectively no
difference between your version and mine.  However, yours is slightly
shorter and slightly cleaner, since it avoids a couple of setq's of
let-bound variables, so maybe it's the better fix (after adding the line
deletion bit; also, it's not necessary and AFAIK stylistically
discouraged to quote a lambda form; and finally, I'm not sure if
open-line is more or less appropriate here than newline -- maybe both
are too heavyweight and (insert "\n") or even (terpri) would suffice?

> The only thing I'm not sure about is the line marked above. This is not
> intended for Emacs, just wanna see if I'm on the right track :)

The line you marked tests whether we're moving the subtree down
(positive arg) and if so ensures we find the insertion point after it.
Or are you asking about something else?

Steve Berman