[PATCH] Re: org-forward-heading-same-level and the invisible-ok argument

[D <d.williams@posteo.net>]

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


> I guess it is simply because nobody though that the leading stars can be
> hidden via fontification.
> 
> I think the whole issue can be fixed by changing the call to
> org-invisible-p inside org-forward-heading-same-level. org-invisible-p
> has an optional third argument to ignore text hidden via fontification.
> You can try to make a patch for org-forward-heading-same-level and
> similar commands adding that third argument.

I looked into the git repository and noticed that org-invisible-p
already, thanks to a patch from Nicolas Goaziou (b1822760f4).
What I am wondering is whether it would make more sense as an argument
for org-forward-heading-same-level and similar commands, or as a
(potentially buffer-local) org variable to tweak the behavior, given
it's most likely going to affect these functions as interactive commands
(see alternative_org.el.diff).  This version has the advantage of
allowing minor modes to easily mess with Org's behavior buffer-locally.

I considered how adding a third argument to
org-forward-heading-same-level, but realized that adding another
invisibility argument would kinda leak a little complexity, as we'd wind
up with TWO invisibility-related arguments (which only differ in subtle
ways) and their interactions.  So instead I'd recommend to instead allow
a distinct, non-nil option for invisible-ok, for example `t', `nil' and
`except-folding' (see org.el.diff).  This version would be more
transparent, as it would not change the behavior of ALL calls in the
buffer, but comes at the cost of minor modes needing to overshadow
bindings to accomplish the same.

I whipped up a quick diff for both versions, and will gladly make a
patch once we settled on one, but I wanted to discuss which is
preferable before making an uninformed decision on my own.

[-- Attachment #2: org.el.diff --]
[-- Type: text/x-patch, Size: 1423 bytes --]

diff --git a/lisp/org.el b/lisp/org.el
index 71dbc611e..b8e6d47c2 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -20482,7 +20482,12 @@ entry."
   "Move forward to the ARG'th subheading at same level as this one.
 Stop at the first and last subheadings of a superior heading.
 Normally this only looks at visible headings, but when INVISIBLE-OK is
-non-nil it will also look at invisible ones."
+non-nil it will also look at invisible ones.
+
+If INVISIBLE-OK is set to the symbol `except-folding', continue
+ignoring all parts that are invisible due to folding of a
+headline, a block or a drawer, i.e., not because of
+fontification."
   (interactive "p")
   (let ((backward? (and arg (< arg 0))))
     (if (org-before-first-heading-p)
@@ -20495,12 +20500,14 @@ non-nil it will also look at invisible ones."
 	    (result (point)))
 	(while (and (> count 0)
 		    (funcall f org-outline-regexp-bol nil 'move))
-	  (let ((l (- (match-end 0) (match-beginning 0) 1)))
+	  (let ((l (- (match-end 0) (match-beginning 0) 1))
+		(folding-only (eq 'except-folding invisible-ok)))
 	    (cond ((< l level) (setq count 0))
 		  ((and (= l level)
-			(or invisible-ok
+			(or (and invisible-ok (not folding-only))
 			    (not (org-invisible-p
-				  (line-beginning-position)))))
+				  (line-beginning-position)
+				  folding-only))))
 		   (cl-decf count)
 		   (when (= l level) (setq result (point)))))))
 	(goto-char result))

[-- Attachment #3: alternative_org.el.diff --]
[-- Type: text/x-patch, Size: 1424 bytes --]

diff --git a/lisp/org.el b/lisp/org.el
index 71dbc611e..8e0040814 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -20478,6 +20478,18 @@ entry."
 		((looking-at-p re) (forward-line))
 		(t (throw 'exit t))))))))
 
+
+(defcustom org-navigate-invisible-headings nil
+  "If non-nil, navigate through (unfolded) invisible headings normally.
+Commands such as `org-forward-heading-same-level' and
+`org-forward-heading-same-level' will consider all invisible
+headings except for those that are invisible due to folding of a
+headline, a block or a drawer."
+  :group 'org
+  :type '(choice
+	  (const :tag "Don't navigate invisible headings" nil)
+	  (const :tag "Navigate invisible (unfolded) headings" t)))
+
 (defun org-forward-heading-same-level (arg &optional invisible-ok)
   "Move forward to the ARG'th subheading at same level as this one.
 Stop at the first and last subheadings of a superior heading.
@@ -20498,9 +20510,11 @@ non-nil it will also look at invisible ones."
 	  (let ((l (- (match-end 0) (match-beginning 0) 1)))
 	    (cond ((< l level) (setq count 0))
 		  ((and (= l level)
-			(or invisible-ok
+			(or (and invisible-ok
+				 (not org-navigate-invisible-headings))
 			    (not (org-invisible-p
-				  (line-beginning-position)))))
+				  (line-beginning-position)
+				  org-navigate-invisible-headings))))
 		   (cl-decf count)
 		   (when (= l level) (setq result (point)))))))
 	(goto-char result))