org-move-subtree-down whitespace dance

org-move-subtree-down whitespace dance

[Marco Wahl]

Hello everyone,

After having stumbled over

    https://emacs.stackexchange.com/questions/32144/org-mode-org-move-subtree-down-function-strange-behavior

I also think the behavior of org-move-subtree-down is strange.

What about pulling out the dance with the whitespace from
org-move-subtree-down and just move the subtree as it is?


Best regards
            Marco

Re: org-move-subtree-down whitespace dance

[Nicolas Goaziou]

Hello,

Marco Wahl <marcowahlsoft@gmail.com> writes:

> After having stumbled over
>
>     https://emacs.stackexchange.com/questions/32144/org-mode-org-move-subtree-down-function-strange-behavior
>
> I also think the behavior of org-move-subtree-down is strange.
>
> What about pulling out the dance with the whitespace from
> org-move-subtree-down and just move the subtree as it is?

There's at least a minor issue. I you like to separate your heading with
N spaces, you must keep N blank lines at the end of the buffer at all
time. Granted, the current implementation requires the same thing before
the first headline, but those are easier to see and maintain.

Intuitively, I would say the current behaviour is better, but feel free
to provide a patch so we can test the other way.

Regards,

-- 
Nicolas Goaziou

Re: org-move-subtree-down whitespace dance

[Marco Wahl]

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]

Hi,

Thanks for your time!

>> After having stumbled over
>>
>>     https://emacs.stackexchange.com/questions/32144/org-mode-org-move-subtree-down-function-strange-behavior
>>
>> I also think the behavior of org-move-subtree-down is strange.
>>
>> What about pulling out the dance with the whitespace from
>> org-move-subtree-down and just move the subtree as it is?
>
> There's at least a minor issue. I you like to separate your heading with
> N spaces, you must keep N blank lines at the end of the buffer at all
> time. Granted, the current implementation requires the same thing before
> the first headline, but those are easier to see and maintain.

Agreed.  This is a point.

Possibly one could write some kind of formatter to maintain a certain
scheme to separate subtrees with N blank lines.

> Intuitively, I would say the current behaviour is better, but feel free
> to provide a patch so we can test the other way.

I still propose to let org-move-subtree-down simply just move the
subtree as it is.  Concretely see the patch.


Thanks for giving it a chance and best regards
                                              Marco


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-Remove-special-whitespace-treatment-when-moving-.patch --]
[-- Type: text/x-patch, Size: 3270 bytes --]

From f5f9f9748a8e9b57282bdc60bc14efd0e8e8bea1 Mon Sep 17 00:00:00 2001
From: Marco Wahl <marcowahlsoft@gmail.com>
Date: Wed, 11 Apr 2018 14:55:00 +0200
Subject: [PATCH] org: Remove special whitespace treatment when moving subtree

* lisp/org.el (org-move-subtree-down): Remove special whitespace treatment.
* testing/lisp/test-org.el (test-org/drag-element-backward):
  Accommodate test.
---
 lisp/org.el              | 29 ++++-------------------------
 testing/lisp/test-org.el |  2 +-
 2 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 075c8f32f..d16d73cd4 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -8128,32 +8128,22 @@ case."
 	 (ins-point (make-marker))
 	 (cnt (abs arg))
 	 (col (current-column))
-	 beg beg0 end txt folded ne-beg ne-end ne-ins ins-end)
+	 beg end txt folded)
      ;; Select the tree
      (org-back-to-heading)
-     (setq beg0 (point))
-     (save-excursion
-       (setq ne-beg (org-back-over-empty-lines))
-       (setq beg (point)))
+     (setq beg (point))
      (save-match-data
        (save-excursion (outline-end-of-heading)
 		       (setq folded (org-invisible-p)))
        (progn (org-end-of-subtree nil t)
 	      (unless (eobp) (backward-char))))
      (outline-next-heading)
-     (setq ne-end (org-back-over-empty-lines))
      (setq end (point))
-     (goto-char beg0)
-     (when (and (> arg 0) (org-first-sibling-p) (< ne-end ne-beg))
-       ;; include less whitespace
-       (save-excursion
-	 (goto-char beg)
-	 (forward-line (- ne-beg ne-end))
-	 (setq beg (point))))
+     (goto-char beg)
      ;; Find insertion point, with error handling
      (while (> cnt 0)
        (unless (and (funcall movfunc) (looking-at org-outline-regexp))
-	 (goto-char beg0)
+	 (goto-char beg)
 	 (user-error "Cannot move past superior level or buffer limit"))
        (setq cnt (1- cnt)))
      (when (> arg 0)
@@ -8162,7 +8152,6 @@ case."
        (save-excursion
 	 (org-back-over-empty-lines)
 	 (or (bolp) (newline))))
-     (setq ne-ins (org-back-over-empty-lines))
      (move-marker ins-point (point))
      (setq txt (buffer-substring beg end))
      (org-save-markers-in-region beg end)
@@ -8176,18 +8165,8 @@ case."
        (org-reinstall-markers-in-region bbb)
        (move-marker ins-point bbb))
      (or (bolp) (insert "\n"))
-     (setq ins-end (point))
      (goto-char ins-point)
      (org-skip-whitespace)
-     (when (and (< arg 0)
-		(org-first-sibling-p)
-		(> ne-ins ne-beg))
-       ;; Move whitespace back to beginning
-       (save-excursion
-	 (goto-char ins-end)
-	 (let ((kill-whole-line t))
-	   (kill-line (- ne-ins ne-beg)) (point)))
-       (insert (make-string (- ne-ins ne-beg) ?\n)))
      (move-marker ins-point nil)
      (if folded
 	 (outline-hide-subtree)
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index fe2111392..2438508bf 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -3965,7 +3965,7 @@ Text.
   ;; Pathological case: handle call with point in blank lines right
   ;; after a headline.
   (should
-   (equal "* H2\n* H1\nText\n\n"
+   (equal "* H2\n\n* H1\nText\n"
 	  (org-test-with-temp-text "* H1\nText\n* H2\n\n<point>"
 	    (org-drag-element-backward)
 	    (buffer-string)))))
-- 
2.17.0

Re: org-move-subtree-down whitespace dance

[Bastien]

Hi Marco,

Marco Wahl <marcowahlsoft@gmail.com> writes:

> I still propose to let org-move-subtree-down simply just move the
> subtree as it is.  Concretely see the patch.

I think this new behavior is more predictible, I applied your patch,
thanks a lot!

-- 
 Bastien