From: Stephen Berman <stephen.berman@gmx.net>
To: Eli Zaretskii <eliz@gnu.org>
Cc: paul@tilk.co, 19102@debbugs.gnu.org
Subject: bug#19102: 24.4; outline-move-subtree-up/down error at last and second-last subtree
Date: Fri, 21 Nov 2014 18:31:19 +0100 [thread overview]
Message-ID: <87fvdcmpeg.fsf@rosalinde.fritz.box> (raw)
In-Reply-To: <8361e84yxb.fsf@gnu.org> (Eli Zaretskii's message of "Fri, 21 Nov 2014 12:42:56 +0200")
[-- Attachment #1: Type: text/plain, Size: 2690 bytes --]
On Fri, 21 Nov 2014 12:42:56 +0200 Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: paul@tilk.co, 19102@debbugs.gnu.org
>> Date: Fri, 21 Nov 2014 11:32:20 +0100
>>
>> > Does it make sense to fix outline-move-subtree-up/down so that they
>> > also work when there's no empty line after the last subtree? That's
>>
>> It didn't occur to me before, but your question prompted me to check and
>> I see outline-mode derives from text-mode, which sets
>> require-final-newline to mode-require-final-newline, so I guess the
>> assumption is indeed that files in outline-mode should end in a newline.
>
> Does doing that solve the problem without the need to delete the added
> line?
Yes. As long as there is a final newline, the only change the current
code needs is to replace `=' with `eq'. So given that
require-final-newline is true for files in outline-mode, the only case
that requires adding a final newline is a non-file outline-mode buffer
that lacks a final newline.
>> > one thing that looks inelegant in your patch, and might also cause
>> > some (minor) trouble: what if the user types C-g before this function
>> > finishes?
>>
>> We could avoid the trouble by wrapping the newline call in
>> unwind-protect, couldn't we?
>
> Yes, but it's better to avoid that in the first place.
>
>> But can C-g really take effect here?
>> There is no place in the function where execution halts to wait for user
>> feedback.
>
> C-g sets a flag that is checked by evaluation.
I don't understand how this could result in C-g taking effect before the
function finishes; could you elaborate?
>> I guess those are idle speculations, it does seem that just keeping a
>> final newline added by the function is the simplest (and perhaps best)
>> fix, so I guess we should commit some version of that fix and see if
>> anyone complains.
>
> Go for it, and thanks.
I think that, once the non-file buffer case is taken into account (and
not doing means moving a subtree can corrupt the outline by putting two
headers on the same line), the cleanest fix is basically the one Paul
Rankin proposed in his last post. I've attached it as a diff against
emacs-24, where I assume the fix should be committed (I added a comment
and tweaked the function Paul posted to avoid irrelevant changes to the
current code, and also restricted the error handling by making it a
user-error and having it signal only when the user attempts to move over
a higher outline level, avoiding an inappropriate message at bob or
eob). Does this patch qualify as a tiny change, or does Paul have a
copyright assignment on file (I don't have access to the file)?
Steve Berman
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: bug#19102 patch --]
[-- Type: text/x-patch, Size: 2289 bytes --]
diff --git a/lisp/outline.el b/lisp/outline.el
index c7cad31..3243e15 100644
--- a/lisp/outline.el
+++ b/lisp/outline.el
@@ -645,31 +645,38 @@ the match data is set appropriately."
(defun outline-move-subtree-down (&optional arg)
"Move the current subtree down past ARG headlines of the same level."
(interactive "p")
- (let ((movfunc (if (> arg 0) 'outline-get-next-sibling
- 'outline-get-last-sibling))
- (ins-point (make-marker))
- (cnt (abs arg))
- beg end folded)
- ;; Select the tree
- (outline-back-to-heading)
- (setq beg (point))
- (save-match-data
- (save-excursion (outline-end-of-heading)
- (setq folded (outline-invisible-p)))
- (outline-end-of-subtree))
- (if (= (char-after) ?\n) (forward-char 1))
- (setq end (point))
- ;; Find insertion point, with error handling
+ (outline-back-to-heading)
+ (let* ((movfunc (if (> arg 0) 'outline-get-next-sibling
+ 'outline-get-last-sibling))
+ ;; Find the end of the subtree to be moved as well as the point to
+ ;; move it to, adding a newline if necessary, to ensure these points
+ ;; are at bol on the line below the subtree.
+ (end-point-func (lambda ()
+ (outline-end-of-subtree)
+ (if (and (eobp) (eolp) (not (bolp)))
+ (insert-char ?\n))
+ (unless (eobp)
+ (forward-char 1))
+ (point)))
+ (beg (point))
+ (folded (save-match-data
+ (outline-end-of-heading)
+ (outline-invisible-p)))
+ (end (save-match-data
+ (funcall end-point-func)))
+ (ins-point (make-marker))
+ (cnt (abs arg)))
+ ;; Find insertion point, with error handling.
(goto-char beg)
(while (> cnt 0)
(or (funcall movfunc)
- (progn (goto-char beg)
- (error "Cannot move past superior level")))
+ (unless (or (bobp) (eobp))
+ (goto-char beg)
+ (user-error "Cannot shift past higher level")))
(setq cnt (1- cnt)))
(if (> arg 0)
- ;; Moving forward - still need to move over subtree
- (progn (outline-end-of-subtree)
- (if (= (char-after) ?\n) (forward-char 1))))
+ ;; Moving forward - still need to move over subtree.
+ (funcall end-point-func))
(move-marker ins-point (point))
(insert (delete-and-extract-region beg end))
(goto-char ins-point)
next prev parent reply other threads:[~2014-11-21 17:31 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-19 8:29 bug#19102: 24.4; outline-move-subtree-up/down error at last and second-last subtree Paul Rankin
2014-11-19 13:17 ` Stephen Berman
2014-11-19 15:54 ` Eli Zaretskii
2014-11-19 17:09 ` Stephen Berman
2014-11-19 18:56 ` Eli Zaretskii
2014-11-19 20:14 ` Stephen Berman
2014-11-19 20:32 ` Eli Zaretskii
2014-11-19 22:07 ` Stephen Berman
2014-11-20 6:46 ` Paul Rankin
2014-11-20 10:08 ` Stephen Berman
2014-11-20 13:26 ` Paul Rankin
2014-11-20 16:21 ` Eli Zaretskii
2014-11-21 10:32 ` Stephen Berman
2014-11-21 10:42 ` Eli Zaretskii
2014-11-21 17:31 ` Stephen Berman [this message]
2014-11-21 19:56 ` Eli Zaretskii
2014-11-21 20:04 ` Stephen Berman
2014-11-22 3:49 ` Paul Rankin
2014-11-22 16:32 ` Stefan Monnier
2014-11-22 16:45 ` Eli Zaretskii
2014-11-22 22:20 ` Stephen Berman
2014-11-24 4:07 ` Stefan Monnier
2014-11-25 21:58 ` Stephen Berman
2014-11-26 2:34 ` Paul Rankin
2014-11-26 13:38 ` Stephen Berman
2014-11-20 7:22 ` Paul Rankin
2014-11-20 10:09 ` Stephen Berman
2014-11-20 13:43 ` Paul Rankin
2014-11-21 10:33 ` Stephen Berman
2014-11-26 2:43 ` Stefan Monnier
2014-11-26 13:38 ` Stephen Berman
2014-11-26 15:54 ` Stefan Monnier
2014-11-26 19:04 ` Stephen Berman
2014-11-26 22:11 ` Stefan Monnier
2014-11-26 22:25 ` Stephen Berman
2014-11-27 2:18 ` Stefan Monnier
2014-11-27 10:12 ` Stephen Berman
2014-11-27 17:15 ` Stefan Monnier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87fvdcmpeg.fsf@rosalinde.fritz.box \
--to=stephen.berman@gmx.net \
--cc=19102@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=paul@tilk.co \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.