Re: [O] A Microsoftesque detail in org

[Rasmus]

Hi,

Thanks for the comments.

Nicolas Goaziou <address@hidden> writes:


> "RET breaks headline text" may be more accurate.

OK.

>> +  (let* ((context (if org-return-follows-link (org-element-context)
>> +                (org-element-at-point)))
>> +     (type (org-element-type context)))
>> +    (cond
>> +     ;; At a headline
>> +     ((and (eq type 'headline) (not (bolp)))
>
> You are removing an optimization here. 
>
> Checking if point is on a headline/inlinetask doesn't require to use
> `org-element-at-point'/`org-element-context'. It is faster to simply
> check for `org-outline-regexp' (or derived) at bol.
>
> This optimization is less important than it used to be, now that
> properties drawers are at a fixed location. Nevertheless, it might be
> worth keeping it in mind.

I did it 'cause it's easier, I think, to read a single cond than first an
if and then a cond.  The latter also seems easier to fix in the future.

>> +      (org-show-entry)
>> +      (let ((string ""))
>> +    (unless (and (save-excursion
>> +                   (beginning-of-line)
>> +                   (looking-at org-complex-heading-regexp))
>> +                 (or (and (match-beginning 3)
>> +                          (< (point)
>> +                             (save-excursion
>> +                               (goto-char (match-beginning 4))
>> +                               (skip-chars-backward " \t")
>> +                               (point))))
>> +                     (and (match-beginning 5)
>> +                          (>= (point) (match-beginning 5)))))
>> +        ;; Point is on headline keywords, tags or cookies.  Do not break
>> +        ;; them: add a newline after the headline instead.
>> +      (setq string (delete-and-extract-region
>> +                    (point) (or (match-beginning 5)
>> +                                (line-end-position))))
>
> The `setq' is not necessary here. Bind it within `let' instead.

You are right.

>> +      (when (match-beginning 5)
>> +        (insert (make-string (length string) ?\ ))))
>
>   ?\  -> ?\s
>   
> If you add this feature, please augment `test-org/return' from
> "test-org.el" accordingly.

Done and and attached.

Anybody against pushing this?

—Rasmus

-- 
There are known knowns; there are things we know that we know

>From a7e6e6b9c96c13be9ebdc0a5c4c2f5033cefbfc7 Mon Sep 17 00:00:00 2001
From: Rasmus <address@hidden>
Date: Fri, 15 May 2015 13:08:11 +0200
Subject: [PATCH 1/2] org.el: RET breaks headline text

* org.el (org-return): RET breaks headline text.
* test-org.el (test-org/return): Test org-return on headline text.
* ORG-NEWS: Add entry on new org-return behavior.
---
 etc/ORG-NEWS             |  4 ++++
 lisp/org.el              | 32 ++++++++++++++++++++++----------
 testing/lisp/test-org.el | 28 ++++++++++++++++++++++++++--
 3 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 3803060..a3e1ae2 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -93,6 +93,10 @@ functions.  The Org version of these functions skips over 
inline tasks
 *** ~org-element-context~ no longer return objects in keywords
 ~org-element-context~ used to return objects on some keywords, i.e.,
 =TITLE=, =DATE= and =AUTHOR=.  It now returns only the keyword.
+*** Behavior of ~org-return~ changed
+If point is before or after the true headline text, insert newline
+without changing the text.  If point is on headline text preserve
+tags.
 ** Removed functions
 *** Removed function ~org-translate-time~
 Use ~org-timestamp-translate~ instead.
diff --git a/lisp/org.el b/lisp/org.el
index 4b44a94..93183f9 100755
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -21185,16 +21185,28 @@ will not happen if point is in a table or on a 
\"dead\"
 object (e.g., within a comment).  In these case, you need to use
 `org-open-at-point' directly."
   (interactive)
-  (if (and (save-excursion
-            (beginning-of-line)
-            (looking-at org-todo-line-regexp))
-          (match-beginning 3)
-          (>= (point) (match-beginning 3)))
-      ;; Point is on headline tags.  Do not break them: add a newline
-      ;; after the headline instead.
-      (progn (org-show-entry)
-            (end-of-line)
-            (if indent (newline-and-indent) (newline)))
+  (if (and (not (bolp))
+          (save-excursion (beginning-of-line)
+                          (looking-at org-complex-heading-regexp)))
+      ;; At a headline.
+      (let ((string (if (or (< (point)
+                              (save-excursion
+                                (goto-char (match-beginning 4))
+                                (skip-chars-backward " \t")
+                                (point)))
+                           (and (match-beginning 5)
+                                (>= (point) (match-beginning 5))))
+                       ;; Point is on headline keywords, tags or cookies.  Do 
not break
+                       ;; them: add a newline after the headline instead.
+                       ""
+                     (delete-and-extract-region (point) (or (match-beginning 5)
+                                                            
(line-end-position))))))
+       (when (match-beginning 5)
+         (insert (make-string (length string) ?\s)))
+       (end-of-line)
+       (org-show-entry)
+       (if indent (newline-and-indent) (newline))
+       (save-excursion (insert (org-trim string))))
     (let* ((context (if org-return-follows-link (org-element-context)
                      (org-element-at-point)))
           (type (org-element-type context)))
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 7d09288..ecfece5 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -878,12 +878,36 @@
          (org-test-with-temp-text "- A\n<point>- B"
            (org-return t)
            (buffer-string))))
-  ;; Special case: on tags part of a headline, add a newline below it
-  ;; instead of breaking it.
+  ;; On tags part of a headline, add a newline below it instead of
+  ;; breaking it.
   (should
    (equal "* H :tag:\n"
          (org-test-with-temp-text "* H :<point>tag:"
            (org-return)
+           (buffer-string))))
+  ;; Before headline text, add a newline below it instead of breaking
+  ;; it.
+  (should
+   (equal "* TODO H :tag:\n"
+         (org-test-with-temp-text "* <point>TODO H :tag:"
+                                  (org-return)
+                                  (buffer-string))))
+  (should
+   (equal "* TODO [#B] H :tag:\n"
+         (org-test-with-temp-text "* TODO<point> [#B] H :tag:"
+                                  (org-return)
+                                  (buffer-string))))
+  ;; At headline text, break headline text but preserve tags.
+  (should
+   (equal "* TODO [#B] foo    :tag:\nbar"
+         (org-test-with-temp-text "* TODO [#B] foo<point>bar :tag:"
+                                  (org-return)
+                                  (buffer-string))))
+  ;; At bol of headline insert newline.
+  (should
+   (equal "\n* h"
+         (org-test-with-temp-text "<point>* h"
+           (org-return)
            (buffer-string)))))
 
 (ert-deftest test-org/meta-return ()
-- 
2.4.0