all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Ihor Radchenko <yantar92@gmail.com>
To: D <d.williams@posteo.net>
Cc: emacs-orgmode@gnu.org
Subject: Re: [PATCH] Re: org-forward-heading-same-level and the invisible-ok argument
Date: Thu, 27 Aug 2020 19:49:52 +0800	[thread overview]
Message-ID: <87tuwo2tr3.fsf@localhost> (raw)
In-Reply-To: <61342cad-ed4c-59ef-d2fe-685de58df5de@posteo.net>

> 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 do not think that setting visibility the leading stars is a correct
approach to control the movement commands. After second though about the
issue you raised in the first email, I think that it would make more
sense for org-forward-heading-same-level to check if any part of the
heading line is visible to decide if we need to skip it (instead of
current approach checking only the point at the beginning of the
headline). Any mode aiming to make org-forward-heading-same-level skip a
heading will then just need to make the whole heading invisible.
Skipping partially visible headlines would be a violation of the
docstring.

Best,
Ihor


D <d.williams@posteo.net> writes:

>> 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.
> 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))
> 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))


  reply	other threads:[~2020-08-27 11:51 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-25 16:46 org-forward-heading-same-level and the invisible-ok argument D
2020-08-26  1:30 ` Ihor Radchenko
2020-08-26 21:33   ` [PATCH] " D
2020-08-27 11:49     ` Ihor Radchenko [this message]
2020-08-28 12:27       ` [PATCH] " D
2020-08-28 13:43         ` Ihor Radchenko
2020-08-28 17:49           ` D
2020-08-29  5:10             ` Ihor Radchenko
2020-08-30 22:07               ` [PATCH] " D
2020-09-06  6:35                 ` Bastien
2020-09-06 11:09                   ` D
2020-09-07  5:06                     ` Bastien
2020-09-07  6:25                       ` Ihor Radchenko
2020-09-07 18:31                         ` D
2020-09-08  9:28                           ` Bastien
2020-09-08 20:00                             ` D
2020-09-09  8:09                               ` Bastien
2020-09-09  4:15                             ` Ihor Radchenko
2020-09-09  8:08                               ` Bastien

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87tuwo2tr3.fsf@localhost \
    --to=yantar92@gmail.com \
    --cc=d.williams@posteo.net \
    --cc=emacs-orgmode@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.