On Wed, Jun 14, 2017 at 1:02 PM Bastien Guerry <bzg@gnu.org> 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