bug#22320: Overlays with an 'invisible property break stacking of overlay faces

[Clément Pit--Claudel]

On 01/07/2016 01:46 PM, Eli Zaretskii wrote:
>> Cc: 22320@debbugs.gnu.org
>> From: Clément Pit--Claudel <clement.pitclaudel@live.com>
>> Date: Thu, 7 Jan 2016 12:03:38 -0500
>>
>>>   http://lists.gnu.org/archive/html/emacs-pretest-bug/2005-04/msg00338.html
>>
>> Interesting, thanks for the pointer. I read the discussion, but I see no 
>> rationale there for this choice. Was one ever given?
> 
> I do see a rationale provided here:
> 
>   http://lists.gnu.org/archive/html/emacs-pretest-bug/2005-04/msg00339.html
> 
> The change causes the code to keep using the same face in cases where
> that is not controversial, and fall back on 'default' otherwise.

I understand what the change does, but I don't see that message as justifying 
it; instead, it asserts that the font-lock face is arbitrary; on the other 
hand, what I'm seeing with that code currently is a large section of the buffer 
highlighted in blue, and every time a folded section follows a character that's 
not in my current font (so about half of all folded sections) the ellipsis has 
the wrong background, making it stand out.

>> Why is this better than, for example, keeping the face of the first 
>> invisible character and extending it to the whole ellipsis?
> 
> That would go back to the original problem, whereby each ellipsis
> could have a different face depending on the faces in the invisible
> text.

No, my proposal would be to make all three dots use the face of the first 
hidden character.

> I think it's confusing to have the faces of the invisible text
> show through, don't you agree?

Not really; in a sense they already show through, since changing the faces of 
the hidden text can change the way it's displayed. Applying the face of the 
first hidden character to all three dots of the ellipsis has multiple 
advantages:

* If an incorrectly spelled word is invisible, the dots are underlined.
* If the first hidden character in fact does have the same face (from the user 
perspective) as the preceding one, the ellipsis is never reset to the default 
face (the font fallback issue means that at the moment I can have perfectly 
uniform-looking text, and folding still gives the default face to the ellipsis).

Invisible text is often used for folding sections of a buffer. Folding hides 
information, but we have an opportunity to show some of that information; 
namely, the information carried by the first invisible character. The current 
solution does take the information contained in the first character into 
account, but then makes it stand out or not based on a complex criterion.

In fact, I wonder if this isn't two problems we're looking at: the first one is 
falling back to default, and the second one is that falling back to default 
ignores the surrounding overlays. My ideal solution would be to use the first 
hidden character (or make it configurable), but even without this I feel that 
it would be a progress for the *face* of the ellipses to be reset to default, 
and then for that face to be merged with overlays. In other word one would 
consider that ellipses always have the default face, plus overlays that cover 
the full ellipsis.

> Ellipsis just indicates there's some hidden text, why should it "inherit" the
> face of that hidden text?

I think of ellipses as a compressed representation of that text; barring a 
better way to summarize that text, inheriting the faces of its first character 
seems like a decent approach. If I write red-green-red and color each word with 
the corresponding color, then make "green" invisible, my spontaneous 
expectation is that three green dots will appear (mostly because I don't think 
of invisible as invisible, but rather "compressed"/"folded")

>> Am I mistaken to think that this choice is also inconsistent with the way 
>> faces on composed characters are handled? It seems that composing a sequence 
>> of characters into "..." will keep the face of the first compose character. 
>> That default is useful in many places (such as Arthur's nameless mode, or in 
>> flyspell or flycheck when an error covers a sequence of characters composed 
>> into a single one). Is there a good reason to treat ellipses standing for 
>> invisible spans differently?
> 
> I'm not sure I understand what you describe here.  Can you show me
> some simple Lisp which demonstrates the situation?

Sure (also posted in reply to your other message):

(with-current-buffer (get-buffer-create "emulating invisibility with 
composition works")
  (erase-buffer)
  (fundamental-mode)
  (add-to-invisibility-spec '(outline . t))
  (insert "line1 line2 line3")
  ;; This composition snippet was taken from Arthur's nameless-mode:
  (compose-region 7 12 (cdr (apply #'append (mapcar (lambda (x) (list '(Br . 
Bl) x)) "..."))))
  (let ((ov (make-overlay 7 8)))
    (overlay-put ov 'face '(:underline t)))
  (let ((ov (make-overlay (point-min) (point-max))))
    (overlay-put ov 'face '(:background "red")))
  (pop-to-buffer (current-buffer)))

compared to

(with-current-buffer (get-buffer-create "using invisibility directly doesn't 
work")
  (erase-buffer)
  (fundamental-mode)
  (add-to-invisibility-spec '(outline . t))
  (insert "line1 line2 line3")
  (let ((ov (make-overlay 7 12)))
    (overlay-put ov 'invisible 'outline))
  (let ((ov (make-overlay 7 8)))
    (overlay-put ov 'face '(:underline t)))
  (let ((ov (make-overlay (point-min) (point-max))))
    (overlay-put ov 'face '(:background "red")))
  (pop-to-buffer (current-buffer)))

>> The interactions of the current behaviour with transient mark mode are 
>> especially confusing
> 
> I agree, but I learned long ago that one should use invisible text as
> little as possible, and in particular expect that text properties and
> overlays put on invisible text will have some specific effect.  The
> display engine skips the invisible text entirely, looking only at its
> first character (and sometimes the last); if you think about this, you
> will immediately understand that having properties and overlays on
> invisible text is playing with fire -- basically, you bump into
> undefined behavior.
>
>> Using transient-mark-mode and selecting characters one by one from the end 
>> of the buffer, "i", "h", and "g" are progressively selected, then nothing 
>> happens despite "def" being in fact selected (so pressing C-w at that point 
>> kills more than highlighted), then "def" and "c" get highlighted at the same 
>> time. This means that the highlighted region doesn't actually correspond to 
>> the marked region. Is that also expected?
> 
> The ellipsis gets highlighted as soon as the last visible character
> before the invisible text has the same face as the invisible text.
> That is what you see.  So yes, this is expected.
> 
> Like I said: it's a hard problem, and the solution only caters to the
> simple, no-brainer, use cases.  I'm not surprised that you can come up
> with weird situations, but I cannot see a better, more general
> solution here.

Inheriting the face of the first character and applying it to the whole 
ellipsis would work here.

>>>> (with-current-buffer (get-buffer-create "org-bug")
>>>>   (erase-buffer)
>>>>   (org-mode)
>>>>   (insert "* line1\n** line2\n* line3")
>>>>   (org-shifttab)
>>>>   (pop-to-buffer (current-buffer)))
>>>
>>> You say that this use case is only broken in Emacs 25, but I see this
>>> in all the previous versions as well (as expected, given when this was
>>> coded).  So if you really see this working correctly in Emacs 24.4,
>>> then some other factors on your system were at work here.  Maybe you
>>> didn't test this exact test case there?
>>
>> You're right; this example works fine in 24.3, but it is indeed broken in 
>> 24.5. I tested both with -Q:
> 
> No, it behaves the same in 24.3 and in 24.5 (and in all versions
> before that).  I don't understand how you see something different.
> 
>>    Works:  GNU Emacs 24.3.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.8) 
>> of 2015-06-17 on clem-w50-mint
>>    Broken: GNU Emacs 24.5.2 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.8) 
>> of 2015-06-20 on clem-w50-mint
>>
>> Is this something that changed in org then?
> 
> No, nothing changed.  Which version of Org did you use in each Emacs
> release?  (I used the one bundled with that release.)

I'm using the bundled Org with emacs -Q in both cases.

>>>> (with-current-buffer (get-buffer-create "flyspell-bug")
>>>>   (erase-buffer)
>>>>   (text-mode)
>>>>   (add-to-invisibility-spec '(outline . t))
>>>>   (insert "line1\nline2\nline3")
>>>>   (flyspell-check-region-doublons (point-min) (point-max))
>>>>   (let ((ov (make-overlay 7 12)))
>>>>     (overlay-put ov 'invisible 'outline))
>>>>   (pop-to-buffer (current-buffer)))
>>>
>>> What exactly is the problem here?  I don't see anything that looks
>>> problematic.  In particular, there are no duplicate misspelling in
>>> this buffer, so flyspell-check-region-doublons should do nothing.
>>> What am I missing?
>>
>> Indeed, I use aspell instead of ispell. It seems to ignore numbers, and thus 
>> flags the first two instances of "line" as duplicates.
> 
> And what problems do you see then?

aspell marks line2 as a doublon, places an underline on it, and thus the 
invisible text doesn't inherit the selection face when I mark the whole buffer.

signature.asc
Description: OpenPGP digital signature