emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [RFC] Rewrite org-(forward|backward)-paragraph
@ 2020-06-03 22:21 Nicolas Goaziou
  2020-06-08 14:42 ` Kévin Le Gouguec
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Goaziou @ 2020-06-03 22:21 UTC (permalink / raw)
  To: Org Mode List

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

Hello,

Here is a proposal for new `org-forward-paragraph' and
`org-backward-paragraph' functions, currently bound to <C-DOWN> and
<C-UP>. Note that functions bound to M-{ and M-} are /not/
paragraph-related functions, but we might want to reconsider it at some
point.

In any case, the purpose of this rewrite is to mimic more closely
expected behaviour from `forward-paragraph' and `backward-paragraph'
functions, as found, e.g., in Text mode. Unlike Text mode, navigation in
Org mode is usually not linear, but both should feel the same, for
example, when the document is indeed linear.

These functions try to stop at interesting places, and skip mundane ones
(e.g., tables are treated as a single block) so navigation is still
fast.

I also added support for repeat argument.

WDYT? Also, what should be done with M-{ and M-}?

Regards,

-- 
Nicolas Goaziou

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Rewrite org-forward|backward-paragraph --]
[-- Type: text/x-diff, Size: 29525 bytes --]

From 4ba83f948f9d8ab26261382b66450d531798f73e Mon Sep 17 00:00:00 2001
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Date: Thu, 30 Apr 2020 16:00:18 +0200
Subject: [PATCH] Rewrite `org-forward-paragraph' and `org-backward-paragraph'

* lisp/org.el (org-forward-paragraph):
(org-backward-paragraph): Rewrite functions.  Add repeat argument.
Mimic more closely regular `forward|backward-paragraph' functions.
(org--forward-paragraph-once):
(org--backward-paragraph-once):
(org--paragraph-at-point): New functions.
* testing/lisp/test-org.el (test-org/forward-paragraph):
(test-org/backward-paragraph): Update tests.  Add some.

Signed-off-by: Nicolas Goaziou <mail@nicolasgoaziou.fr>
---
 lisp/org.el              | 440 ++++++++++++++++++++++++++-------------
 testing/lisp/test-org.el | 255 ++++++++++++++++-------
 2 files changed, 471 insertions(+), 224 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index b869e12e1..59187fe52 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -20500,156 +20500,300 @@ With ARG, repeats or can move forward if negative."
   (interactive "p")
   (org-next-visible-heading (- arg)))
 
-(defun org-forward-paragraph ()
-  "Move forward to beginning of next paragraph or equivalent.
-
-The function moves point to the beginning of the next visible
-structural element, which can be a paragraph, a table, a list
-item, etc.  It also provides some special moves for convenience:
-
-  - On an affiliated keyword, jump to the beginning of the
-    relative element.
-  - On an item or a footnote definition, move to the second
-    element inside, if any.
-  - On a table or a property drawer, jump after it.
-  - On a verse or source block, stop after blank lines."
+(defun org-forward-paragraph (&optional arg)
+  "Move forward by a paragraph, or equivalent, unit.
+
+With argument ARG, do it ARG times;
+a negative argument ARG = -N means move backward N paragraphs.
+
+The function moves point between two structural
+elements (paragraphs, tables, lists, etc.).
+
+It also provides the following special moves for convenience:
+
+  - on a table or a property drawer, move to its beginning;
+  - on comment, example, export, source and verse blocks, stop
+    at blank lines;
+  - skip consecutive clocks, diary S-exps, and keywords."
+  (interactive "^p")
+  (unless arg (setq arg 1))
+  (if (< arg 0) (org-backward-paragraph (- arg))
+    (while (and (> arg 0) (not (eobp)))
+      (org--forward-paragraph-once)
+      (cl-decf arg))
+    ;; Return moves left.
+    arg))
+
+(defun org-backward-paragraph (&optional arg)
+  "Move backward by a paragraph, or equivalent, unit.
+
+With argument ARG, do it ARG times;
+a negative argument ARG = -N means move forward N paragraphs.
+
+The function moves point between two structural
+elements (paragraphs, tables, lists, etc.).
+
+It also provides the following special moves for convenience:
+
+  - on a table or a property drawer, move to its beginning;
+  - on comment, example, export, source and verse blocks, stop
+    at blank lines;
+  - skip consecutive clocks, diary S-exps, and keywords."
+  (interactive "^p")
+  (unless arg (setq arg 1))
+  (if (< arg 0) (org-forward-paragraph (- arg))
+    (while (and (> arg 0) (not (bobp)))
+      (org--backward-paragraph-once)
+      (cl-decf arg))
+    ;; Return moves left.
+    arg))
+
+(defun org--paragraph-at-point ()
+  "Return paragraph, or equivalent, element at point.
+
+Paragraph element at point is the element at point, with the
+following special cases:
+
+- treat table rows (resp. node properties) as the table
+  \(resp. property drawer) containing them.
+
+- treat plain lists with an item every line as a whole.
+
+- treat consecutive keywords, clocks, and diary-sexps as a single
+  block.
+
+Function may return a real element, or a pseudo-element with type
+`pseudo-paragraph'."
+  (let* ((e (org-element-at-point))
+	 (type (org-element-type e))
+	 ;; If we need to fake a new pseudo-element, triplet is
+	 ;;
+	 ;;   (BEG END PARENT)
+	 ;;
+	 ;; where BEG and END are element boundaries, and PARENT the
+	 ;; element containing it, or nil.
+	 (triplet
+	  (cond
+	   ((memq type '(table property-drawer))
+	    (list (org-element-property :begin e)
+		  (org-element-property :end e)
+		  (org-element-property :parent e)))
+	   ((memq type '(node-property table-row))
+	    (let ((e (org-element-property :parent e)))
+	      (list (org-element-property :begin e)
+		    (org-element-property :end e)
+		    (org-element-property :parent e))))
+	   ((memq type '(clock diary-sexp keyword))
+	    (let* ((regexp (pcase type
+			     (`clock org-clock-line-re)
+			     (`diary-sexp "%%(")
+			     (_ org-keyword-regexp)))
+		   (end (if (< 0 (org-element-property :post-blank e))
+			    (org-element-property :end e)
+			  (org-with-wide-buffer
+			   (forward-line)
+			   (while (looking-at regexp) (forward-line))
+			   (skip-chars-forward " \t\n")
+			   (line-beginning-position))))
+		   (begin (org-with-point-at (org-element-property :begin e)
+			    (while (and (not (bobp)) (looking-at regexp))
+			      (forward-line -1))
+			    ;; We may have gotten one line too far.
+			    (if (looking-at regexp)
+				(point)
+			      (line-beginning-position 2)))))
+	      (list begin end (org-element-property :parent e))))
+	   ;; Find the full plain list containing point, the check it
+	   ;; contains exactly one line per item.
+	   ((let ((l (org-element-lineage e '(plain-list) t)))
+	      (while (memq (org-element-type (org-element-property :parent l))
+			   '(item plain-list))
+		(setq l (org-element-property :parent l)))
+	      (and l
+		   (org-with-point-at (org-element-property :post-affiliated l)
+		     (forward-line (length (org-element-property :structure l)))
+		     (= (point) (org-element-property :contents-end l)))
+		   ;; Return value.
+		   (list (org-element-property :begin l)
+			 (org-element-property :end l)
+			 (org-element-property :parent l)))))
+	   (t nil))))			;no triplet: return element
+    (pcase triplet
+      (`(,b ,e ,p)
+       (org-element-create
+	'pseudo-paragraph
+	(list :begin b :end e :parent p :post-blank 0 :post-affiliated b)))
+      (_ e))))
+
+(defun org--forward-paragraph-once ()
+  "Move forward to end of paragraph or equivalent, once.
+See `org-forward-paragraph'."
   (interactive)
-  (unless (eobp)
-    (let* ((deactivate-mark nil)
-	   (element (org-element-at-point))
-	   (type (org-element-type element))
-	   (post-affiliated (org-element-property :post-affiliated element))
-	   (contents-begin (org-element-property :contents-begin element))
-	   (contents-end (org-element-property :contents-end element))
-	   (end (let ((end (org-element-property :end element)) (parent element))
-		  (while (and (setq parent (org-element-property :parent parent))
-			      (= (org-element-property :contents-end parent) end))
-		    (setq end (org-element-property :end parent)))
-		  end)))
-      (cond ((not element)
-	     (skip-chars-forward " \r\t\n")
-	     (or (eobp) (beginning-of-line)))
-	    ;; On affiliated keywords, move to element's beginning.
-	    ((< (point) post-affiliated)
-	     (goto-char post-affiliated))
-	    ;; At a table row, move to the end of the table.  Similarly,
-	    ;; at a node property, move to the end of the property
-	    ;; drawer.
-	    ((memq type '(node-property table-row))
-	     (goto-char (org-element-property
-			 :end (org-element-property :parent element))))
-	    ((memq type '(property-drawer table)) (goto-char end))
-	    ;; Consider blank lines as separators in verse and source
-	    ;; blocks to ease editing.
-	    ((memq type '(src-block verse-block))
-	     (when (eq type 'src-block)
-	       (setq contents-end
-		     (save-excursion (goto-char end)
-				     (skip-chars-backward " \r\t\n")
-				     (line-beginning-position))))
-	     (beginning-of-line)
-	     (when (looking-at "[ \t]*$") (skip-chars-forward " \r\t\n"))
-	     (if (not (re-search-forward "^[ \t]*$" contents-end t))
-		 (goto-char end)
-	       (skip-chars-forward " \r\t\n")
-	       (if (= (point) contents-end) (goto-char end)
-		 (beginning-of-line))))
-	    ;; With no contents, just skip element.
-	    ((not contents-begin) (goto-char end))
-	    ;; If contents are invisible, skip the element altogether.
-	    ((org-invisible-p (line-end-position))
-	     (cl-case type
-	       (headline
-		(org-with-limited-levels (outline-next-visible-heading 1)))
-	       ;; At a plain list, make sure we move to the next item
-	       ;; instead of skipping the whole list.
-	       (plain-list (forward-char)
-			   (org-forward-paragraph))
-	       (otherwise (goto-char end))))
-	    ((>= (point) contents-end) (goto-char end))
-	    ((>= (point) contents-begin)
-	     ;; This can only happen on paragraphs and plain lists.
-	     (cl-case type
-	       (paragraph (goto-char end))
-	       ;; At a plain list, try to move to second element in
-	       ;; first item, if possible.
-	       (plain-list (end-of-line)
-			   (org-forward-paragraph))))
-	    ;; When contents start on the middle of a line (e.g. in
-	    ;; items and footnote definitions), try to reach first
-	    ;; element starting after current line.
-	    ((> (line-end-position) contents-begin)
-	     (end-of-line)
-	     (org-forward-paragraph))
-	    (t (goto-char contents-begin))))))
-
-(defun org-backward-paragraph ()
-  "Move backward to start of previous paragraph or equivalent.
-
-The function moves point to the beginning of the current
-structural element, which can be a paragraph, a table, a list
-item, etc., or to the beginning of the previous visible one if
-point is already there.  It also provides some special moves for
-convenience:
-
-  - On an affiliated keyword, jump to the first one.
-  - On a table or a property drawer, move to its beginning.
-  - On comment, example, export, src and verse blocks, stop
-    before blank lines."
+  (save-restriction
+    (widen)
+    (skip-chars-forward " \t\n")
+    (cond
+     ((eobp) nil)
+     ;; When inside a folded part, move out of it.
+     ((pcase (get-char-property-and-overlay (point) 'invisible)
+	(`(,(or `outline `org-hide-block `org-hide-drawer) . ,o)
+	 (goto-char (overlay-end o))
+	 (forward-line)
+	 t)
+	(_ nil)))
+     (t
+      (let* ((element (org--paragraph-at-point))
+	     (type (org-element-type element))
+	     (contents-begin (org-element-property :contents-begin element))
+	     (end (org-element-property :end element))
+	     (post-affiliated (org-element-property :post-affiliated element)))
+	(cond
+	 ((eq type 'plain-list)
+	  (forward-char)
+	  (org--forward-paragraph-once))
+	 ;; If the element is folded, skip it altogether.
+	 ((pcase (org-with-point-at post-affiliated
+		   (get-char-property-and-overlay (line-end-position)
+						  'invisible))
+	    (`(,(or `outline `org-hide-block `org-hide-drawer) . ,o)
+	     (goto-char (overlay-end o))
+	     (forward-line)
+	     t)
+	    (_ nil)))
+	 ;; At a greater element, move inside.
+	 ((and contents-begin
+	       (> contents-begin (point))
+	       (not (eq type 'paragraph)))
+	  (goto-char contents-begin)
+	  ;; Items and footnote definitions contents may not start at
+	  ;; the beginning of the line.  In this case, skip until the
+	  ;; next paragraph.
+	  (cond
+	   ((not (bolp)) (org--forward-paragraph-once))
+	   ((org-previous-line-empty-p) (forward-line -1))
+	   (t nil)))
+	 ;; Move between empty lines in some blocks.
+	 ((memq type '(comment-block example-block export-block src-block
+				     verse-block))
+	  (let ((contents-start
+		 (org-with-point-at post-affiliated
+		   (line-beginning-position 2))))
+	    (if (< (point) contents-start)
+		(goto-char contents-start)
+	      (let ((contents-end
+		     (org-with-point-at end
+		       (skip-chars-backward " \t\n")
+		       (line-beginning-position))))
+		(cond
+		 ((>= (point) contents-end)
+		  (goto-char end)
+		  (skip-chars-backward " \t\n")
+		  (forward-line))
+		 ((re-search-forward "^[ \t]*\n" contents-end :move)
+		  (forward-line -1))
+		 (t nil))))))
+	 (t
+	  ;; Move to element's end.
+	  (goto-char end)
+	  (skip-chars-backward " \t\n")
+	  (forward-line))))))))
+
+(defun org--backward-paragraph-once ()
+  "Move backward to start of paragraph or equivalent, once.
+See `org-backward-paragraph'."
   (interactive)
-  (unless (bobp)
-    (let* ((deactivate-mark nil)
-	   (element (org-element-at-point))
-	   (type (org-element-type element))
-	   (contents-end (org-element-property :contents-end element))
-	   (post-affiliated (org-element-property :post-affiliated element))
-	   (begin (org-element-property :begin element))
-	   (special?			;blocks handled specially
-	    (memq type '(comment-block example-block export-block src-block
-				       verse-block)))
-	   (contents-begin
-	    (if special?
-		;; These types have no proper contents.  Fake line
-		;; below the block opening line as contents beginning.
-		(save-excursion (goto-char begin) (line-beginning-position 2))
-	      (org-element-property :contents-begin element))))
-      (cond
-       ((not element) (goto-char (point-min)))
-       ((= (point) begin)
-	(backward-char)
-	(org-backward-paragraph))
-       ((<= (point) post-affiliated) (goto-char begin))
-       ;; Special behavior: on a table or a property drawer, move to
-       ;; its beginning.
-       ((memq type '(node-property table-row))
-	(goto-char (org-element-property
-		    :post-affiliated (org-element-property :parent element))))
-       (special?
-	(if (<= (point) contents-begin) (goto-char post-affiliated)
-	  ;; Inside a verse block, see blank lines as paragraph
-	  ;; separators.
-	  (let ((origin (point)))
-	    (skip-chars-backward " \r\t\n" contents-begin)
-	    (when (re-search-backward "^[ \t]*$" contents-begin 'move)
-	      (skip-chars-forward " \r\t\n" origin)
-	      (if (= (point) origin) (goto-char contents-begin)
-		(beginning-of-line))))))
-       ((eq type 'paragraph) (goto-char contents-begin)
-	;; When at first paragraph in an item or a footnote definition,
-	;; move directly to beginning of line.
-	(let ((parent-contents
-	       (org-element-property
-		:contents-begin (org-element-property :parent element))))
-	  (when (and parent-contents (= parent-contents contents-begin))
-	    (beginning-of-line))))
-       ;; At the end of a greater element, move to the beginning of
-       ;; the last element within.
-       ((and contents-end (>= (point) contents-end))
-	(goto-char (1- contents-end))
-	(org-backward-paragraph))
-       (t (goto-char (or post-affiliated begin))))
-      ;; Ensure we never leave point invisible.
-      (when (org-invisible-p (point)) (beginning-of-visual-line)))))
+  (save-restriction
+    (widen)
+    (cond
+     ((bobp) nil)
+     ;; Blank lines at the beginning of the buffer.
+     ((and (org-match-line "^[ \t]*$")
+	   (save-excursion (skip-chars-backward " \t\n") (bobp)))
+      (goto-char (point-min)))
+     ;; When inside a folded part, move out of it.
+     ((pcase (get-char-property-and-overlay (1- (point)) 'invisible)
+	(`(,(or `outline `org-hide-block `org-hide-drawer) . ,o)
+	 (goto-char (1- (overlay-start o)))
+	 (org--backward-paragraph-once)
+	 t)
+	(_ nil)))
+     (t
+      (let* ((element (org--paragraph-at-point))
+	     (type (org-element-type element))
+	     (begin (org-element-property :begin element))
+	     (post-affiliated (org-element-property :post-affiliated element))
+	     (contents-end (org-element-property :contents-end element))
+	     (end (org-element-property :end element))
+	     (parent (org-element-property :parent element))
+	     (reach
+	      ;; Move to the visible empty line above position P, or
+	      ;; to position P.  Return t.
+	      (lambda (p)
+		(goto-char p)
+		(when (and (org-previous-line-empty-p)
+			   (let ((end (line-end-position 0)))
+			     (or (= end (point-min))
+				 (not (org-invisible-p (1- end))))))
+		  (forward-line -1))
+		t)))
+	(cond
+	 ;; Already at the beginning of an element.
+	 ((= begin (point))
+	  (cond
+	   ;; There is a blank line above.  Move there.
+	   ((and (org-previous-line-empty-p)
+		 (not (org-invisible-p (1- (line-end-position 0)))))
+	    (forward-line -1))
+	   ;; At the beginning of the first element within a greater
+	   ;; element.  Move to the beginning of the greater element.
+	   ((and parent (= begin (org-element-property :contents-begin parent)))
+	    (funcall reach (org-element-property :begin parent)))
+	   ;; Since we have to move anyway, find the beginning
+	   ;; position of the element above.
+	   (t
+	    (forward-char -1)
+	    (org--backward-paragraph-once))))
+	 ;; Skip paragraphs at the very beginning of footnote
+	 ;; definitions or items.
+	 ((and (eq type 'paragraph)
+	       (org-with-point-at begin (not (bolp))))
+	  (funcall reach (progn (goto-char begin) (line-beginning-position))))
+	 ;; If the element is folded, skip it altogether.
+	 ((org-with-point-at post-affiliated
+	    (org-invisible-p (line-end-position) t))
+	  (funcall reach begin))
+	 ;; At the end of a greater element, move inside.
+	 ((and contents-end
+	       (<= contents-end (point))
+	       (not (eq type 'paragraph)))
+	  (cond
+	   ((memq type '(footnote-definition plain-list))
+	    (skip-chars-backward " \t\n")
+	    (org--backward-paragraph-once))
+	   ((= contents-end (point))
+	    (forward-char -1)
+	    (org--backward-paragraph-once))
+	   (t
+	    (goto-char contents-end))))
+	 ;; Move between empty lines in some blocks.
+	 ((and (memq type '(comment-block example-block export-block src-block
+					  verse-block))
+	       (let ((contents-start
+		      (org-with-point-at post-affiliated
+			(line-beginning-position 2))))
+		 (when (> (point) contents-start)
+		   (let ((contents-end
+			  (org-with-point-at end
+			    (skip-chars-backward " \t\n")
+			    (line-beginning-position))))
+		     (if (> (point) contents-end)
+			 (progn (goto-char contents-end) t)
+		       (skip-chars-backward " \t\n" begin)
+		       (re-search-backward "^[ \t]*\n" contents-start :move)
+		       t))))))
+	 ;; Move to element's start.
+	 (t
+	  (funcall reach begin))))))))
 
 (defun org-forward-element ()
   "Move forward by one element.
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index f58a87438..54eea3154 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -3651,45 +3651,46 @@ SCHEDULED: <2017-05-06 Sat>
      t))
   ;; Standard test.
   (should
-   (org-test-with-temp-text "P1\n\nP2\n\nP3"
-     (org-forward-paragraph)
-     (looking-at "P2")))
-  ;; Ignore depth.
+   (= 2
+      (org-test-with-temp-text "P1\n\nP2"
+	(org-forward-paragraph)
+	(org-current-line))))
   (should
-   (org-test-with-temp-text "#+BEGIN_CENTER\nP1\n#+END_CENTER\nP2"
-     (org-forward-paragraph)
-     (looking-at "P1")))
+   (= 2
+      (org-test-with-temp-text "P1\n\nP2\n\nP3"
+	(org-forward-paragraph)
+	(org-current-line))))
+  ;; Enter greater elements.
+  (should
+   (= 2
+      (org-test-with-temp-text "#+begin_center\nP1\n#+end_center\nP2"
+	(org-forward-paragraph)
+	(org-current-line))))
   ;; Do not enter elements with invisible contents.
   (should
-   (org-test-with-temp-text "#+BEGIN_CENTER\nP1\n\nP2\n#+END_CENTER\nP3"
-     (org-hide-block-toggle)
-     (org-forward-paragraph)
-     (looking-at "P3")))
-  ;; On an affiliated keyword, jump to the beginning of the element.
+   (= 4
+      (org-test-with-temp-text "* H1\n  P1\n\n* H2"
+	(org-cycle)
+	(org-forward-paragraph)
+	(org-current-line))))
   (should
-   (org-test-with-temp-text "#+name: para\n#+caption: caption\nPara"
-     (org-forward-paragraph)
-     (looking-at "Para")))
-  ;; On an item or a footnote definition, move to the second element
+   (= 6
+      (org-test-with-temp-text "#+begin_center\nP1\n\nP2\n#+end_center\nP3"
+	(org-hide-block-toggle)
+	(org-forward-paragraph)
+	(org-current-line))))
+  ;; On an item or a footnote definition, move past the first element
   ;; inside, if any.
   (should
-   (org-test-with-temp-text "- Item1\n\n  Paragraph\n- Item2"
-     (org-forward-paragraph)
-     (looking-at "  Paragraph")))
-  (should
-   (org-test-with-temp-text "[fn:1] Def1\n\nParagraph\n\n[fn:2] Def2"
-     (org-forward-paragraph)
-     (looking-at "Paragraph")))
-  ;; On an item, or a footnote definition, when the first line is
-  ;; empty, move to the first item.
-  (should
-   (org-test-with-temp-text "- \n\n  Paragraph\n- Item2"
-     (org-forward-paragraph)
-     (looking-at "  Paragraph")))
+   (= 2
+      (org-test-with-temp-text "- Item1\n\n  Paragraph\n- Item2"
+	(org-forward-paragraph)
+	(org-current-line))))
   (should
-   (org-test-with-temp-text "[fn:1]\n\nParagraph\n\n[fn:2] Def2"
-     (org-forward-paragraph)
-     (looking-at "Paragraph")))
+   (= 2
+      (org-test-with-temp-text "[fn:1] Def1\n\nParagraph\n\n[fn:2] Def2"
+	(org-forward-paragraph)
+	(org-current-line))))
   ;; On a table (resp. a property drawer) do not move through table
   ;; rows (resp. node properties).
   (should
@@ -3701,15 +3702,59 @@ SCHEDULED: <2017-05-06 Sat>
        "* H\n<point>:PROPERTIES:\n:prop: value\n:END:\nParagraph"
      (org-forward-paragraph)
      (looking-at "Paragraph")))
-  ;; On a verse or source block, stop after blank lines.
+  ;; Skip consecutive keywords, clocks and diary S-exps.
   (should
-   (org-test-with-temp-text "#+BEGIN_VERSE\nL1\n\nL2\n#+END_VERSE"
+   (org-test-with-temp-text "#+key: val\n  #+key2: val\n#+key3: val\n"
      (org-forward-paragraph)
-     (looking-at "L2")))
+     (eobp)))
   (should
-   (org-test-with-temp-text "#+BEGIN_SRC\nL1\n\nL2\n#+END_SRC"
+   (org-test-with-temp-text "CLOCK: val\n  CLOCK: val\nCLOCK: val\n"
      (org-forward-paragraph)
-     (looking-at "L2"))))
+     (eobp)))
+  (should
+   (org-test-with-temp-text "%%(foo)\n%%(bar)\n%%(baz)\n"
+     (org-forward-paragraph)
+     (eobp)))
+  (should-not
+   (org-test-with-temp-text "#+key: val\n  #+key2: val\n\n#+key3: val\n"
+     (org-forward-paragraph)
+     (eobp)))
+  (should-not
+   (org-test-with-temp-text "#+key: val\nCLOCK: ...\n"
+     (org-forward-paragraph)
+     (eobp)))
+  ;; In a plain list with one item every line, skip the whole list,
+  ;; even with point in the middle of the list.
+  (should
+   (org-test-with-temp-text "- A\n  - B\n- C\n"
+     (org-forward-paragraph)
+     (eobp)))
+  (should
+   (org-test-with-temp-text "- A\n  - <point>B\n- C\n"
+     (org-forward-paragraph)
+     (eobp)))
+  ;; On a comment, verse or source block, stop at "contents"
+  ;; boundaries and blank lines.
+  (should
+   (= 2
+      (org-test-with-temp-text "#+begin_src emacs-lisp\nL1\n\nL2\n#+end_src"
+	(org-forward-paragraph)
+	(org-current-line))))
+  (should
+   (= 3
+      (org-test-with-temp-text "#+begin_verse\n<point>L1\n\nL2\n#+end_verse"
+	(org-forward-paragraph)
+	(org-current-line))))
+  (should
+   (= 5
+      (org-test-with-temp-text "#+begin_comment\nL1\n\n<point>L2\n#+end_comment"
+	(org-forward-paragraph)
+	(org-current-line))))
+  ;; Being on an affiliated keyword shouldn't make any difference.
+  (should
+   (org-test-with-temp-text "#+name: para\n#+caption: caption\nPara"
+     (org-forward-paragraph)
+     (eobp))))
 
 (ert-deftest test-org/backward-paragraph ()
   "Test `org-backward-paragraph' specifications."
@@ -3718,44 +3763,65 @@ SCHEDULED: <2017-05-06 Sat>
    (org-test-with-temp-text "Paragraph"
      (org-backward-paragraph)
      t))
-  ;; Regular test.
+  ;; At blank lines at the very beginning of a buffer, move to
+  ;; point-min.
   (should
-   (org-test-with-temp-text "P1\n\nP2\n\nP3<point>"
+   (org-test-with-temp-text "\n\n<point>\n\nParagraph"
      (org-backward-paragraph)
-     (looking-at "P3")))
+     (bobp)))
+  ;; Regular test.
   (should
-   (org-test-with-temp-text "P1\n\nP2\n\n<point>P3"
-     (org-backward-paragraph)
-     (looking-at-p "P2")))
-  ;; Ignore depth.
+   (= 2
+      (org-test-with-temp-text "P1\n\nP2<point>"
+	(org-backward-paragraph)
+	(org-current-line))))
   (should
-   (org-test-with-temp-text "P1\n\n#+BEGIN_CENTER\nP2\n#+END_CENTER\n<point>P3"
-     (org-backward-paragraph)
-     (looking-at-p "P2")))
-  ;; Ignore invisible elements.
+   (= 4
+      (org-test-with-temp-text "P1\n\nP2\n\nP3<point>"
+	(org-backward-paragraph)
+	(org-current-line))))
+  ;; Try to move on the line above current element.
+  (should
+   (= 2
+      (org-test-with-temp-text "\n\n<point>Paragraph"
+	(org-backward-paragraph)
+	(org-current-line))))
+  ;; Do not leave point in an invisible area.
   (should
-   (org-test-with-temp-text "* H1\n  P1\n* H2"
+   (org-test-with-temp-text "* H1\n  P1\n\n* H2"
      (org-cycle)
      (goto-char (point-max))
      (beginning-of-line)
      (org-backward-paragraph)
      (bobp)))
-  ;; On an affiliated keyword, jump to the first one.
   (should
-   (org-test-with-temp-text
-       "P1\n#+name: n\n#+caption: c1\n#+caption: <point>c2\nP2"
+   (org-test-with-temp-text "#+begin_center\nP1\n\nP2\n#+end_center\n"
+     (org-hide-block-toggle)
+     (goto-char (point-max))
      (org-backward-paragraph)
-     (looking-at-p "#\\+name")))
-  ;; On the second element in an item or a footnote definition, jump
-  ;; to item or the definition.
+     (bobp)))
+  ;; On the first element in an item or a footnote definition, jump
+  ;; before the footnote or the item.
   (should
-   (org-test-with-temp-text "- line1\n\n<point>  line2"
+   (org-test-with-temp-text "- line1<point>"
      (org-backward-paragraph)
-     (looking-at-p "- line1")))
+     (bobp)))
   (should
-   (org-test-with-temp-text "[fn:1] line1\n\n<point>  line2"
+   (org-test-with-temp-text "[fn:1] line1n<point>"
      (org-backward-paragraph)
-     (looking-at-p "\\[fn:1\\] line1")))
+     (bobp)))
+  ;; On the second element in an item or a footnote definition, jump
+  ;; to item or the definition.
+  (should
+   (= 2
+      (org-test-with-temp-text "- line1\n\n<point>  line2"
+	(org-backward-paragraph)
+	(org-current-line))))
+  (should
+   (= 2
+      (org-test-with-temp-text "[fn:1] line1\n\n<point>  line2"
+	(org-backward-paragraph)
+	(org-current-line))))
   ;; On a table (resp. a property drawer), ignore table rows
   ;; (resp. node properties).
   (should
@@ -3763,38 +3829,75 @@ SCHEDULED: <2017-05-06 Sat>
      (org-backward-paragraph)
      (bobp)))
   (should
-   (org-test-with-temp-text "* H\n:PROPERTIES:\n:prop: value\n:END:\n<point>P1"
+   (= 2
+      (org-test-with-temp-text
+	  "* H\n:PROPERTIES:\n:prop: value\n:END:\n<point>P1"
+	(org-backward-paragraph)
+	(org-current-line))))
+  ;; In a plain list with one item every line, skip the whole list,
+  ;; even with point in the middle of the list.
+  (should
+   (org-test-with-temp-text "- A\n  - B\n- C\n<point>"
      (org-backward-paragraph)
-     (looking-at-p ":PROPERTIES:")))
-  ;; On a comment, example, src and verse blocks, stop before blank
-  ;; lines.
+     (bobp)))
   (should
-   (org-test-with-temp-text "#+BEGIN_VERSE\nL1\n\nL2\n\n<point>L3\n#+END_VERSE"
+   (org-test-with-temp-text "- A\n  - B\n- <point>C\n"
      (org-backward-paragraph)
-     (looking-at-p "L2")))
+     (bobp)))
+  ;; Skip consecutive keywords, clocks and diary S-exps.
   (should
-   (org-test-with-temp-text "#+BEGIN_SRC\nL1\n\nL2\n\n<point>L3#+END_SRC"
+   (org-test-with-temp-text "#+key: val\n  #+key2: val\n#+key3: val\n<point>"
      (org-backward-paragraph)
-     (looking-at-p "L2")))
-  ;; In comment, example, export, src and verse blocks, stop below
-  ;; opening line when called from within the block.
+     (bobp)))
   (should
-   (org-test-with-temp-text "#+BEGIN_VERSE\nL1\nL2<point>\n#+END_VERSE"
+   (org-test-with-temp-text "CLOCK: val\n  CLOCK: val\nCLOCK: val\n<point>"
      (org-backward-paragraph)
-     (looking-at-p "L1")))
+     (bobp)))
   (should
-   (org-test-with-temp-text "#+BEGIN_EXAMPLE\nL1\nL2<point>\n#+END_EXAMPLE"
+   (org-test-with-temp-text "%%(foo)\n%%(bar)\n%%(baz)\n<point>"
+     (org-backward-paragraph)
+     (bobp)))
+  (should-not
+   (org-test-with-temp-text "#+key: val\n  #+key2: val\n\n#+key3: val\n<point>"
+     (org-backward-paragraph)
+     (bobp)))
+  (should-not
+   (org-test-with-temp-text "#+key: val\nCLOCK: ...\n<point>"
      (org-backward-paragraph)
-     (looking-at-p "L1")))
+     (bobp)))
+  ;; On a comment, example, source and verse blocks, stop at blank
+  ;; lines.
+  (should
+   (= 1
+      (org-test-with-temp-text
+	  "#+begin_comment\n<point>L1\n\nL2\n\nL3\n#+end_comment"
+	(org-backward-paragraph)
+	(org-current-line))))
+  (should
+   (= 2
+      (org-test-with-temp-text
+	  "#+begin_verse\nL1\n\n<point>L2\n\nL3\n#+end_verse"
+	(org-backward-paragraph)
+	(org-current-line))))
+  (should
+   (= 3
+      (org-test-with-temp-text
+	  "#+begin_src emacs-lisp\nL1\n\nL2\n\n<point>L3\n#+end_src"
+	(org-backward-paragraph)
+	(org-current-line))))
   ;; When called from the opening line itself, however, move to
   ;; beginning of block.
   (should
-   (org-test-with-temp-text "#+BEGIN_<point>EXAMPLE\nL1\n#+END_EXAMPLE"
+   (org-test-with-temp-text "#+begin_<point>example\nL1\n#+end_example"
+     (org-backward-paragraph)
+     (bobp)))
+  ;; On an empty heading, move above it.
+  (should
+   (org-test-with-temp-text "\n* <point>"
      (org-backward-paragraph)
      (bobp)))
-  ;; Pathological case: on an empty heading, move to its beginning.
   (should
-   (org-test-with-temp-text "* <point>H"
+   (org-test-with-temp-text "\n* \n<point>"
      (org-backward-paragraph)
      (bobp))))
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC] Rewrite org-(forward|backward)-paragraph
  2020-06-03 22:21 [RFC] Rewrite org-(forward|backward)-paragraph Nicolas Goaziou
@ 2020-06-08 14:42 ` Kévin Le Gouguec
  2020-06-08 17:24   ` Nicolas Goaziou
  0 siblings, 1 reply; 4+ messages in thread
From: Kévin Le Gouguec @ 2020-06-08 14:42 UTC (permalink / raw)
  To: Org Mode List

Hi Nicolas!

I don't know how useful my feedback will be, since I'm not a heavy user
of paragraph-based movement[1], but here goes!

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> In any case, the purpose of this rewrite is to mimic more closely
> expected behaviour from `forward-paragraph' and `backward-paragraph'
> functions, as found, e.g., in Text mode. Unlike Text mode, navigation in
> Org mode is usually not linear, but both should feel the same, for
> example, when the document is indeed linear.

I've danced around ORG-NEWS to assess the changes; what I observed does
feel closer to text-mode (point moves to the blank lines between
paragraphs instead of to the paragraph starts), the other changes I
could spot do not strike me as deal-breaking:

- point now jumps over tight lists[2] instead of stopping at each item,

- point stops a few more times within code blocks, acting like
  #+begin_src and #+end_src are paragraphs of their own, instead of
  jumping over the whole block; also, forward and backward movements are
  now symmetric 🙌

Are there other situations where you think your changes could be
controversial?

> WDYT? Also, what should be done with M-{ and M-}?

FWIW, I think that reducing the distance between Org mode and The Rest
of Emacs™ is a commendable goal, so I would vote for binding paragraph
functions to M-{ and M-}, and moving element functions to C-<UP> and
C-<DOWN>.  I realize that this might be too big a change for the sake of
conformity though.

(And again: I don't use these functions very often, so my vote probably
shouldn't carry too much weight.)


Thank you for working on this!


[1] Curly brackets are cumbersome with AZERTY, so I never took the habit
    of moving by paragraphs outside org-mode.  Likewise with Org's
    <C-ARROW> bindings: my fingers are too lazy to reach for the arrow
    keys for something as often-used as movement.

[2] I.e. lists without newlines between items.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] Rewrite org-(forward|backward)-paragraph
  2020-06-08 14:42 ` Kévin Le Gouguec
@ 2020-06-08 17:24   ` Nicolas Goaziou
  2020-06-13 15:25     ` Nicolas Goaziou
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Goaziou @ 2020-06-08 17:24 UTC (permalink / raw)
  To: Kévin Le Gouguec; +Cc: Org Mode List

Hello,

Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:

> I don't know how useful my feedback will be, since I'm not a heavy user
> of paragraph-based movement[1], but here goes!

Thank you!

> I've danced around ORG-NEWS to assess the changes; what I observed does
> feel closer to text-mode (point moves to the blank lines between
> paragraphs instead of to the paragraph starts), the other changes I
> could spot do not strike me as deal-breaking:
>
> - point now jumps over tight lists[2] instead of stopping at each
> item,

The idea is to avoid some trivial moves where C-n would be sufficient,
e.g., in tables, properties drawers. Also Text mode skip those, since it
doesn't understand such structures.

> - point stops a few more times within code blocks, acting like
>   #+begin_src and #+end_src are paragraphs of their own, instead of
>   jumping over the whole block; also, forward and backward movements are
>   now symmetric 🙌
>
> Are there other situations where you think your changes could be
> controversial?

I don't think it's much controversial, but stop points are necessarily
opinionated. I hope they make sense.

Also, testing could unveil some bugs.

>> WDYT?e Also, what should be done with M-{ and M-}?
>
> FWIW, I think that reducing the distance between Org mode and The Rest
> of Emacs™ is a commendable goal, so I would vote for binding paragraph
> functions to M-{ and M-}, and moving element functions to C-<UP> and
> C-<DOWN>.  I realize that this might be too big a change for the sake of
> conformity though.

Honestly, I don't know if Sexp-based navigation is useful at all. Does
anyone use such navigation ?

> (And again: I don't use these functions very often, so my vote probably
> shouldn't carry too much weight.)

I don't either. I didn't notice there was a difference until recently.

Regards,

-- 
Nicolas Goaziou


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] Rewrite org-(forward|backward)-paragraph
  2020-06-08 17:24   ` Nicolas Goaziou
@ 2020-06-13 15:25     ` Nicolas Goaziou
  0 siblings, 0 replies; 4+ messages in thread
From: Nicolas Goaziou @ 2020-06-13 15:25 UTC (permalink / raw)
  To: Kévin Le Gouguec; +Cc: Org Mode List

Since there was no negative feedback, I pushed to master.

Thanks.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-06-13 15:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03 22:21 [RFC] Rewrite org-(forward|backward)-paragraph Nicolas Goaziou
2020-06-08 14:42 ` Kévin Le Gouguec
2020-06-08 17:24   ` Nicolas Goaziou
2020-06-13 15:25     ` Nicolas Goaziou

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).