From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kaushal Modi Subject: Re: Backward incompatible outline-invisible-p change in emacs master for Org Date: Wed, 14 Jun 2017 17:18:41 +0000 Message-ID: References: <87a85i2nji.fsf@nicolasgoaziou.fr> <87mv9ay02y.fsf@bzg.fr> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="f403045ea28604ca770551eebf46" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:48344) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dLBwX-0005BQ-2u for emacs-orgmode@gnu.org; Wed, 14 Jun 2017 13:18:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dLBwV-0008Rh-R3 for emacs-orgmode@gnu.org; Wed, 14 Jun 2017 13:18:57 -0400 In-Reply-To: <87mv9ay02y.fsf@bzg.fr> List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: Bastien Guerry Cc: Paul Rankin , emacs-org list , Nicolas Goaziou --f403045ea28604ca770551eebf46 Content-Type: text/plain; charset="UTF-8" On Wed, Jun 14, 2017 at 1:02 PM Bastien Guerry wrote: > The question is: why this patch in the first place? Paul authored it > and I committed it, so I should know--but I don't. Maybe Paul know or > you know better? > I later found the reason for that commit here: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24073 Here is the emacs -Q recipe that Paul posted on that debbugs report: 1. emacs -Q 2. insert ";;; heading" 3. M-: (outline-on-heading-p) => t 4. C-a 5. M-: (put-text-property (point) (1+ (point)) 'invisible 'foo) 6. M-; (outline-on-heading-p) => nil Expected results: (outline-on-heading-p) => t Actual results: (outline-on-heading-p) => nil This shows the relation between outline-on-heading-p and outline-invisible-p: (defun outline-on-heading-p (&optional invisible-ok) "Return t if point is on a (visible) heading line. If INVISIBLE-OK is non-nil, an invisible heading line is ok too." (save-excursion (beginning-of-line) (and (bolp) (or invisible-ok (not (outline-invisible-p))) (looking-at outline-regexp)))) Basically the expectation is that a outline heading be not marked as "invisible" by any 'foo invisible property. Outline headings should be marked invisible by only 'outline invisible property. Also as the function is prefixed with "outline-", that kind of makes sense. On the other hand, in org, we need a function that returns non-nil for *any* invisible property. So that commit breaks org's expectation. This looks fine, Thanks. > but I'd rather revert the faulty Emacs commit if > it is not necessary. > > Until Paul enlighten us, I'll have a deeper look. > After reviewing debbugs 24073, the commit looks correct for emacs master and outline package. WDYT? -- Kaushal Modi --f403045ea28604ca770551eebf46 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Wed, Jun 14, 2017 at 1:02 PM Bastien Guerry <bzg@gnu.org> wrote:
The question is: why t= his patch in the first place?=C2=A0 Paul authored it
and I committed it, so I should know--but I don't.=C2=A0 Maybe Paul kno= w or
you know better?

I later found the reason fo= r that commit here:=C2=A0https://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D24073

Here is= the emacs -Q recipe that Paul posted on that debbugs report:

1. emacs -Q
2. insert ";;; hea= ding"
3. M-: (outline-on-heading-p)
=C2=A0 =C2=A0 =3D> t
4. C-a
5. M-: (put-text-property= (point) (1+ (point)) 'invisible 'foo)
6. M-; (outline-on-heading-p)
=C2=A0 = =C2=A0 =3D> nil

Expected results:

(outline-on-heading-p)
=C2=A0 =C2=A0 =3D> t

Actual results:

=
(outline-on-heading-p)
=C2=A0 =C2=A0 =3D> nil

This shows = the relation between outline-on-heading-p and outline-invisible-p:

(defun outline-on-heading-p (&optional invisible-= ok)
=C2=A0 "Return t if point is on a (visible) heading line= .
If INVISIBLE-OK is non-nil, an invisible heading line is ok too= ."
=C2=A0 (save-excursion
=C2=A0 =C2=A0 (beginning= -of-line)
=C2=A0 =C2=A0 (and (bolp) (or invisible-ok (not (outlin= e-invisible-p)))
(looking-at outline-regexp))))

=
Basically the expectation is that a outline heading be not marked as &= quot;invisible" by any 'foo invisible property. Outline headings s= hould be marked invisible by only 'outline invisible property.

Also as the function is prefixed with "outline-&q= uot;, that kind of makes sense.

On the other hand,= in org, we need a function that returns non-nil for *any* invisible proper= ty. So that commit breaks org's expectation.

This looks fine,

= Thanks.
=C2=A0
but I'd ra= ther revert the faulty Emacs commit if
it is not necessary.

Until Paul enlighten us, I'll have a deeper look.
=
After reviewing debbugs 24073, the commit looks correct for = emacs master and outline package. WDYT?=C2=A0
--

Kaushal Modi

--f403045ea28604ca770551eebf46--