From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stephen Berman Newsgroups: gmane.emacs.bugs Subject: bug#19102: 24.4; outline-move-subtree-up/down error at last and second-last subtree Date: Wed, 26 Nov 2014 20:04:23 +0100 Message-ID: <87y4qx6ax4.fsf@rosalinde.fritz.box> References: <87lhn7s522.fsf@rosalinde.fritz.box> <83lhn789tq.fsf@gnu.org> <87h9xvruan.fsf@rosalinde.fritz.box> <83tx1v6mur.fsf@gnu.org> <87a93nrlql.fsf@rosalinde.fritz.box> <83lhn76iex.fsf@gnu.org> <8761easv3s.fsf@rosalinde.fritz.box> <8761e26q12.fsf@rosalinde.fritz.box> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: ger.gmane.org 1417028729 4156 80.91.229.3 (26 Nov 2014 19:05:29 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Wed, 26 Nov 2014 19:05:29 +0000 (UTC) Cc: paul@tilk.co, 19102@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Nov 26 20:05:19 2014 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Xthtv-0003fS-25 for geb-bug-gnu-emacs@m.gmane.org; Wed, 26 Nov 2014 20:05:19 +0100 Original-Received: from localhost ([::1]:35625 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xthtu-0002sb-Ec for geb-bug-gnu-emacs@m.gmane.org; Wed, 26 Nov 2014 14:05:18 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:39459) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xthtl-0002qU-01 for bug-gnu-emacs@gnu.org; Wed, 26 Nov 2014 14:05:14 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xthte-0005rw-RZ for bug-gnu-emacs@gnu.org; Wed, 26 Nov 2014 14:05:08 -0500 Original-Received: from debbugs.gnu.org ([140.186.70.43]:49347) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xthte-0005rr-Oi for bug-gnu-emacs@gnu.org; Wed, 26 Nov 2014 14:05:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1Xthte-0006Ib-72 for bug-gnu-emacs@gnu.org; Wed, 26 Nov 2014 14:05:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Stephen Berman Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 26 Nov 2014 19:05:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 19102 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 19102-submit@debbugs.gnu.org id=B19102.141702867324165 (code B ref 19102); Wed, 26 Nov 2014 19:05:02 +0000 Original-Received: (at 19102) by debbugs.gnu.org; 26 Nov 2014 19:04:33 +0000 Original-Received: from localhost ([127.0.0.1]:46560 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XthtA-0006Hh-EQ for submit@debbugs.gnu.org; Wed, 26 Nov 2014 14:04:32 -0500 Original-Received: from mout.gmx.net ([212.227.15.19]:52981) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xtht7-0006HX-AZ for 19102@debbugs.gnu.org; Wed, 26 Nov 2014 14:04:30 -0500 Original-Received: from rosalinde.fritz.box ([89.245.127.251]) by mail.gmx.com (mrgmx002) with ESMTPSA (Nemesis) id 0MNq4h-1XrGEt07Kd-007TL7; Wed, 26 Nov 2014 20:04:24 +0100 In-Reply-To: (Stefan Monnier's message of "Wed, 26 Nov 2014 10:54:19 -0500") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (gnu/linux) X-Provags-ID: V03:K0:3rJb+/CXQw0ldOsgVlxOXDHBOz70sJyKFl/SfO/Sl/NzRlon7pE HLyOW7FZYCyYTuYQYVc6teu9FEdWQV/vyXhWiINEDICnkyFxEKQQu7OYsYFPqRAa3aKodBT uxiI2RoztJZBkX8ZOwi5T5asd9mZLyn2+VKV+lC/BQoGAy1hrZss7FF4fyZ7Lf/ktNjqkir SW38KNuFwJUyhHgDC9D3A== X-UI-Out-Filterresults: notjunk:1; X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:96638 Archived-At: --=-=-= Content-Type: text/plain On Wed, 26 Nov 2014 10:54:19 -0500 Stefan Monnier wrote: >> Oops, you commented on the last version of my patch, which, however, I >> had already considered superseded by the last version of the command >> subsequently posted by Paul Rankin, which in effect already addressed >> your concerns. Rather than repost the diff for the latter, I'm >> appending the two corrected versions of outline-move-subtree-down to >> facilitate comparison and deciding which to use. > > Please just send the diff that (you think) should be applied. > Or two diffs to apply in sequence if you want to distinguish your > changes from Paul's. Ok, I've done the latter, appended below. The first change just fixes the bug, incorporating your simplification of my patch for when to move forward and when to add a newline. The second change is Paul's refactoring of the code to avoid setq's of let-bound variables, which I think make the code cleaner and more elegant (the version he posted also fixed the bug, but I prefer your simplification). >> The only thing I added to both is a condition for triggering the >> user-error, since the message doesn't seem appropriate when you try to >> move a subtree down at eob or up at bob (if you don't think it's worth >> avoiding the message there, I'll remove the condition). > > I don't see why we should avoid the error in those cases: AFAICT, we > can't do what the user asked, so we should signal an error. Ok, I removed that part. Steve Berman --=-=-= Content-Type: text/x-patch Content-Disposition: inline Content-Description: Fix for bug#19102 diff --git a/lisp/outline.el b/lisp/outline.el index c7cad31..61ee7ff 100644 --- a/lisp/outline.el +++ b/lisp/outline.el @@ -649,27 +649,32 @@ the match data is set appropriately." 'outline-get-last-sibling)) (ins-point (make-marker)) (cnt (abs arg)) + ;; Make sure we can move forward to find the end of the + ;; subtree and the insertion point. + (maybe-forward-char (lambda () + (if (eq (char-after) ?\n) (forward-char 1) + (if (and (eobp) (not (bolp))) (insert "\n"))))) beg end folded) - ;; Select the tree + ;; 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)) + (funcall maybe-forward-char) (setq end (point)) - ;; Find insertion point, with error handling + ;; 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"))) + (user-error "Cannot move past superior level"))) (setq cnt (1- cnt))) (if (> arg 0) - ;; Moving forward - still need to move over subtree + ;; Moving forward - still need to move over subtree. (progn (outline-end-of-subtree) - (if (= (char-after) ?\n) (forward-char 1)))) + (funcall maybe-forward-char))) (move-marker ins-point (point)) (insert (delete-and-extract-region beg end)) (goto-char ins-point) --=-=-= Content-Type: text/x-patch Content-Disposition: inline Content-Description: Refactoring patch diff --git a/lisp/outline.el b/lisp/outline.el index 61ee7ff..bb56341 100644 --- a/lisp/outline.el +++ b/lisp/outline.el @@ -645,25 +645,25 @@ 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)) - ;; Make sure we can move forward to find the end of the - ;; subtree and the insertion point. - (maybe-forward-char (lambda () - (if (eq (char-after) ?\n) (forward-char 1) - (if (and (eobp) (not (bolp))) (insert "\n"))))) - 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)) - (funcall maybe-forward-char) - (setq end (point)) + (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 (eq (char-after) ?\n) (forward-char 1) + (if (and (eobp) (not (bolp))) (insert "\n"))) + (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) @@ -673,8 +673,7 @@ the match data is set appropriately." (setq cnt (1- cnt))) (if (> arg 0) ;; Moving forward - still need to move over subtree. - (progn (outline-end-of-subtree) - (funcall maybe-forward-char))) + (funcall end-point-func)) (move-marker ins-point (point)) (insert (delete-and-extract-region beg end)) (goto-char ins-point) --=-=-=--