Invisibility bug: `invisible' vs `display'

Invisibility bug: `invisible' vs `display'

[Daniel Brockman]

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

Try this:


[-- Attachment #2: Type: application/x-emacs-lisp, Size: 988 bytes --]

[-- Attachment #3: Type: text/plain, Size: 1078 bytes --]


The test inserts the following text:

   A: *
   B: ?
   C: *?
   D: ?*
   E: *?*
   F: ?*?
   G: ??

The `*' and `?' characters have `invisibile' properties
with a value of `t', so I expect to see none of them.

The `?' characters all have `display' properties with a
value of "!" (one of them has "$" instead, for clarity).
This, I believe, _should_ be irrelevant.

In other words, I expect to see the following:

   A:
   B:
   C:
   D:
   E:
   F:
   G:

The actual result I get is this:

   A:
   B: ?
   C:
   D: ?
   E:
   F: ?
   G: ?!

Those exclamation marks should not be there.  What they all
have in common is that none of them are preceded by
invisible characters _without_ `display' properties.

That is, a character with both `invisible' and `display'
properties appears to display correctly (i.e., not at all)
if and only if it is preceded by an invisible character
without a `display' property.

I get the same (AFAICS) erroneous results if I put an image
or some space instead of a string on the `display' property.

-- 
Daniel Brockman <daniel@brockman.se>

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: Invisibility bug: `invisible' vs `display'

[Daniel Brockman]

Daniel Brockman <daniel@brockman.se> writes:

> The actual result I get is this:
>
>    A:
>    B: ?
>    C:
>    D: ?
>    E:
>    F: ?
>    G: ?!

Sorry.  I got tricked by the display property.

The actual result I get is this:

   A:
   B: !
   C:
   D: !
   E:
   F: !
   G: !!

-- 
Daniel Brockman <daniel@brockman.se>

Re: Invisibility bug: `invisible' vs `display'

[Chong Yidong]

Daniel Brockman <daniel@brockman.se> writes:

> The `*' and `?' characters have `invisibile' properties
> with a value of `t', so I expect to see none of them.
>
> The `?' characters all have `display' properties with a
> value of "!" (one of them has "$" instead, for clarity).
> This, I believe, _should_ be irrelevant.

The `display' text property overrides `invisible'.  This has been true
since the `display' property was introduced in Emacs 21.  Whether it
should do this is debatable---I can think of arguments either
way---but I think we should not change this before the Emacs 22
release.

Re: Invisibility bug: `invisible' vs `display'

[Daniel Brockman]

Chong Yidong <cyd@stupidchicken.com> writes:

> Daniel Brockman <daniel@brockman.se> writes:
>
>> The `*' and `?' characters have `invisibile' properties
>> with a value of `t', so I expect to see none of them.
>>
>> The `?' characters all have `display' properties with a
>> value of "!" (one of them has "$" instead, for clarity).
>> This, I believe, _should_ be irrelevant.
>
> The `display' text property overrides `invisible'.

If that were true, then all the `?' characters should
display as exclamation marks, but they don't, so the
`display' text property does not override `invisible'.

This still looks like a bug to me.

Let me state again what I see:  The `display' property
overrides `invisible' if and only if the previous character
is visible.  How is this reasonable or useful in any way?

-- 
Daniel Brockman <daniel@brockman.se>

Re: Invisibility bug: `invisible' vs `display'

[David Kastrup]

Daniel Brockman <daniel@brockman.se> writes:

> Chong Yidong <cyd@stupidchicken.com> writes:
>
>> Daniel Brockman <daniel@brockman.se> writes:
>>
>>> The `*' and `?' characters have `invisibile' properties
>>> with a value of `t', so I expect to see none of them.
>>>
>>> The `?' characters all have `display' properties with a
>>> value of "!" (one of them has "$" instead, for clarity).
>>> This, I believe, _should_ be irrelevant.
>>
>> The `display' text property overrides `invisible'.
>
> If that were true, then all the `?' characters should
> display as exclamation marks, but they don't, so the
> `display' text property does not override `invisible'.
>
> This still looks like a bug to me.
>
> Let me state again what I see:  The `display' property
> overrides `invisible' if and only if the previous character
> is visible.  How is this reasonable or useful in any way?

When folding stretches of code, it does not make sense to have images
in between cluster all together and stick out in spite of the
invisibility of the region.

I am not saying that the current behavior is perfect, but it would
seem you would want to replace it with something that can cause quite
a bit of trouble, and I don't think we have the time to shake out all
repercussions before the release.

-- 
David Kastrup

Re: Invisibility bug: `invisible' vs `display'

[Daniel Brockman]

David Kastrup <dak@gnu.org> writes:

> Daniel Brockman <daniel@brockman.se> writes:
>
>> Let me state again what I see:  The `display' property
>> overrides `invisible' if and only if the previous character
>> is visible.  How is this reasonable or useful in any way?
>
> When folding stretches of code, it does not make sense to have images
> in between cluster all together and stick out in spite of the
> invisibility of the region.

I completely agree.

> I am not saying that the current behavior is perfect, but it would
> seem you would want to replace it with something that can cause quite
> a bit of trouble, and I don't think we have the time to shake out all
> repercussions before the release.

On the contrary, I want to replace the current behavior with
something that makes `invisible' _always_ override `display'.

The current behavior is for `invisible' to override `display'
in _almost_ every case.  In fact, all except one edge case.
I simply want to eliminate this anomaly, which is causing me
quite a bit of trouble.

Here is my use case:  I have some text that I want to make
invisible, so I set its `invisible' property to `t'.
The result:  If the first character happens to be an image
(or a pixel space), that and only that remains visible.

-- 
Daniel Brockman <daniel@brockman.se>

Re: Invisibility bug: `invisible' vs `display'

[Daniel Brockman]

Daniel Brockman <daniel@brockman.se> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> Daniel Brockman <daniel@brockman.se> writes:
>>
>>> Let me state again what I see:  The `display' property
>>> overrides `invisible' if and only if the previous character
>>> is visible.  How is this reasonable or useful in any way?
>>
>> When folding stretches of code, it does not make sense to
>> have images in between cluster all together and stick out
>> in spite of the invisibility of the region.
>
> I completely agree.

To clarify, this is exactly what I am trying to fix.

>> I am not saying that the current behavior is perfect, but it would
>> seem you would want to replace it with something that can cause quite
>> a bit of trouble, and I don't think we have the time to shake out all
>> repercussions before the release.
>
> On the contrary, I want to replace the current behavior with
> something that makes `invisible' _always_ override `display'.
>
> The current behavior is for `invisible' to override `display'
> in _almost_ every case.  In fact, all except one edge case.
> I simply want to eliminate this anomaly, which is causing me
> quite a bit of trouble.

This patch fixes the problem for me.

*** old/emacs/src/xdisp.c	2006-12-30 00:29:45.000000000 +0100
--- new/emacs/src/xdisp.c	2007-02-22 03:29:37.000000000 +0100
***************
*** 762,769 ****
    /* Handle `face' before `display' because some sub-properties of
       `display' need to know the face.  */
    {&Qface,		FACE_PROP_IDX,		handle_face_prop},
-   {&Qdisplay,		DISPLAY_PROP_IDX,	handle_display_prop},
    {&Qinvisible,		INVISIBLE_PROP_IDX,	handle_invisible_prop},
    {&Qcomposition,	COMPOSITION_PROP_IDX,	handle_composition_prop},
    {NULL,		0,			NULL}
  };
--- 762,769 ----
    /* Handle `face' before `display' because some sub-properties of
       `display' need to know the face.  */
    {&Qface,		FACE_PROP_IDX,		handle_face_prop},
    {&Qinvisible,		INVISIBLE_PROP_IDX,	handle_invisible_prop},
+   {&Qdisplay,		DISPLAY_PROP_IDX,	handle_display_prop},
    {&Qcomposition,	COMPOSITION_PROP_IDX,	handle_composition_prop},
    {NULL,		0,			NULL}
  };

I don't know enough about the display code to be able to say
whether this is a safe change, but it seems very logical to
me to handle the `invisible' property first, since invisible
text should just be skipped.

In fact, why not move it all the way to the top of that list?

-- 
Daniel Brockman <daniel@brockman.se>

Re: Invisibility bug: `invisible' vs `display'

[Kim F. Storm]

Daniel Brockman <daniel@brockman.se> writes:

> When folding stretches of code, it does not make sense to
> have images in between cluster all together and stick out
> in spite of the invisibility of the region.

That is clearly a bug.

> This patch fixes the problem for me.

Thank you.

Mixing invisible and display properties -- with the desire to actually
get the effects of the display property looks very obscure to me, and
cannot image that any code is actually relying on such functionality.

I think the change is safe, so I installed it.



> *** old/emacs/src/xdisp.c	2006-12-30 00:29:45.000000000 +0100
> --- new/emacs/src/xdisp.c	2007-02-22 03:29:37.000000000 +0100
> ***************
> *** 762,769 ****
>     /* Handle `face' before `display' because some sub-properties of
>        `display' need to know the face.  */
>     {&Qface,		FACE_PROP_IDX,		handle_face_prop},
> -   {&Qdisplay,		DISPLAY_PROP_IDX,	handle_display_prop},
>     {&Qinvisible,		INVISIBLE_PROP_IDX,	handle_invisible_prop},
>     {&Qcomposition,	COMPOSITION_PROP_IDX,	handle_composition_prop},
>     {NULL,		0,			NULL}
>   };
> --- 762,769 ----
>     /* Handle `face' before `display' because some sub-properties of
>        `display' need to know the face.  */
>     {&Qface,		FACE_PROP_IDX,		handle_face_prop},
>     {&Qinvisible,		INVISIBLE_PROP_IDX,	handle_invisible_prop},
> +   {&Qdisplay,		DISPLAY_PROP_IDX,	handle_display_prop},
>     {&Qcomposition,	COMPOSITION_PROP_IDX,	handle_composition_prop},
>     {NULL,		0,			NULL}
>   };
>
> I don't know enough about the display code to be able to say
> whether this is a safe change, but it seems very logical to
> me to handle the `invisible' property first, since invisible
> text should just be skipped.
>
> In fact, why not move it all the way to the top of that list?

It seems like the natural thing to do, but I'm not confident that
it is ok to skip handling the `fontified' property.

But it seems superfluous to process face properties on invisible text,
so it looks safe to move Qinvisible before the Qface entry.
Can anyone see any problems with that??

--
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Invisibility bug: `invisible' vs `display'

[David Kastrup]

storm@cua.dk (Kim F. Storm) writes:

> Daniel Brockman <daniel@brockman.se> writes:
>
>> When folding stretches of code, it does not make sense to
>> have images in between cluster all together and stick out
>> in spite of the invisibility of the region.
>
> That is clearly a bug.
>
>> This patch fixes the problem for me.
>
> Thank you.
>
> Mixing invisible and display properties -- with the desire to actually
> get the effects of the display property looks very obscure to me, and
> cannot image that any code is actually relying on such functionality.
>
> I think the change is safe, so I installed it.

Sigh.  If you use preview-latex on material where _some_ of it is made
invisible using TeX-fold-mode, the desired outcome will be to get the
display, and get it once only.

I can't vouch for the patch being either a step in the right or wrong
direction.

But I'd clearly like to see fewer "I think it should work and can't
imagine anybody actually using it, anyway" patches at this point in
the game.

If we don't have the time or resources of assessing the ramifications
of a patch, there is really little incentive to not rather try them
out in the course of version 22.2.

-- 
David Kastrup

Re: Invisibility bug: `invisible' vs `display'

[Daniel Brockman]

David Kastrup <dak@gnu.org> writes:

> storm@cua.dk (Kim F. Storm) writes:
>
>> Mixing invisible and display properties -- with the desire to actually
>> get the effects of the display property looks very obscure to me, and
>> cannot image that any code is actually relying on such functionality.
>>
>> I think the change is safe, so I installed it.
>
> Sigh.  If you use preview-latex on material where _some_ of it is made
> invisible using TeX-fold-mode, the desired outcome will be to get the
> display, and get it once only.

The bug did not let you override the `invisible' property
using the `display' property whenever you wanted; it _only_
let you do that at the start of invisible text --- clearly
counter-intuitive, counter-useful, illogical, and erroneous.

In other words, the bug prevents you from using `invisible'
to turn text invisible (because that will fail if the first
character happens to be an image or space or something),
while not providing any generally useful feature in return.

The reason I'm laboring this point is that you do not seem to
appreciate the obscurity of the now-eliminated special case.

Are you saying that preview-latex relies on being able to
make visible the _first_ character in a stretch of invisible
text by putting a `display' property on it?

> I can't vouch for the patch being either a step in the
> right or wrong direction.

Ignoring for a moment the fact that the old behavior was
very strange (it was almost exactly like the new behavior,
except for an obscure special case), are there actually any
arguments for having `display' override `invisible'?

If you want some text to show, why not just set `invisible'
to nil on that text?

> But I'd clearly like to see fewer "I think it should work and can't
> imagine anybody actually using it, anyway" patches at this point in
> the game.

One can never be sure that no code is relying on obscure bugs,
but I _can_ imagine that quite some code is actually relying
on the manual being right in that ``any non-`nil' `invisible'
property makes a character invisible [...] if you don't alter
the default value of `buffer-invisibility-spec'.''[1]

-- 
Daniel Brockman <daniel@brockman.se>

[1]  (info "(elisp)Invisible Text")

Re: Invisibility bug: `invisible' vs `display'

[David Kastrup]

Daniel Brockman <daniel@brockman.se> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> storm@cua.dk (Kim F. Storm) writes:
>>
>>> Mixing invisible and display properties -- with the desire to actually
>>> get the effects of the display property looks very obscure to me, and
>>> cannot image that any code is actually relying on such functionality.
>>>
>>> I think the change is safe, so I installed it.
>>
>> Sigh.  If you use preview-latex on material where _some_ of it is made
>> invisible using TeX-fold-mode, the desired outcome will be to get the
>> display, and get it once only.
>
> The bug did not let you override the `invisible' property
> using the `display' property whenever you wanted;

How do you know what I want?

> it _only_ let you do that at the start of invisible text --- clearly
> counter-intuitive, counter-useful, illogical, and erroneous.

A display embedded in an invisible area should obviously not be
visible.  At the immediate edge, it is not clear what should take
preference.  If we have a display overlay with identical start and end
points both of which advance-on-insert, then the overlay _clearly_
marks the position _between_ the text before and behind it.  If the
text _behind_ it is invisible, this should obviously not affect the
overlay.  Even more so if the displayed overlay is
non-advance-on-insert.

Very clearly your proposed behavior is counter-intuitive,
counter-useful, illogical, and erroneous.

But since the patch does not involve any check related to "start of
invisible", it would, anyway, seem to do much more than the purported
fix.

So we not only get new, erroroneous behavior, but also we get an
undetermined and untested amount of such.

> The reason I'm laboring this point is that you do not seem to
> appreciate the obscurity of the now-eliminated special case.

And you do not seem to appreciate the non-obscurity of a number of not
so special cases that are now affected.

>> I can't vouch for the patch being either a step in the right or
>> wrong direction.
>
> Ignoring for a moment the fact that the old behavior was very
> strange (it was almost exactly like the new behavior, except for an
> obscure special case), are there actually any arguments for having
> `display' override `invisible'?
>
> If you want some text to show, why not just set `invisible'
> to nil on that text?

Display properties are not necessarily a part of text.

>> But I'd clearly like to see fewer "I think it should work and can't
>> imagine anybody actually using it, anyway" patches at this point in
>> the game.
>
> One can never be sure that no code is relying on obscure bugs, but I
> _can_ imagine that quite some code is actually relying on the manual
> being right in that ``any non-`nil' `invisible' property makes a
> character invisible [...] if you don't alter the default value of
> `buffer-invisibility-spec'.''[1]

You yourself said that the bug was obscure and showed only in obscure
cases.  I did not see anybody claim the same for the purported fix,
and there have been no tests.

-- 
David Kastrup

Re: Invisibility bug: `invisible' vs `display'

[Daniel Brockman]

David Kastrup <dak@gnu.org> writes:

> Daniel Brockman <daniel@brockman.se> writes:
>
>> The bug did not let you override the `invisible' property
>> using the `display' property whenever you wanted;
>
> How do you know what I want?

Let me rephrase.  The bug did not let you override the `invisible'
property using the `display' property in arbitrary situations.

>> it _only_ let you do that at the start of invisible text --- clearly
>> counter-intuitive, counter-useful, illogical, and erroneous.
>
> A display embedded in an invisible area should obviously not
> be visible.

I'm glad we agree on that.

> At the immediate edge, it is not clear what should take preference.
> If we have a display overlay with identical start and end points
> both of which advance-on-insert, then the overlay _clearly_ marks
> the position _between_ the text before and behind it.  If the text
> _behind_ it is invisible, this should obviously not affect the
> overlay.  Even more so if the displayed overlay is
> non-advance-on-insert.

How can an overlay with identical start and end points
display anything?

>> If you want some text to show, why not just set `invisible'
>> to nil on that text?
>
> Display properties are not necessarily a part of text.

Well, set it on an overlay if you want.  What's the difference?

-- 
Daniel Brockman <daniel@brockman.se>

Re: Invisibility bug: `invisible' vs `display'

[Kim F. Storm]

David Kastrup <dak@gnu.org> writes:

> I can't vouch for the patch being either a step in the right or wrong
> direction.

At this time, it is obviously a step in the wrong direction.

So I have reverted the change for now.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Invisibility bug: `invisible' vs `display'

[Johan Bockgård]

Daniel Brockman <daniel@brockman.se> writes:

> How can an overlay with identical start and end points display
> anything?

Through the `before-string' and `after-string' properties.

-- 
Johan Bockgård

Re: Invisibility bug: `invisible' vs `display'

[David Kastrup]

Daniel Brockman <daniel@brockman.se> writes:

> How can an overlay with identical start and end points
> display anything?

Using the display property.  Are you sure that you understand the
difference between overlays and text properties?  Text properties are
bound to characters, overlays to buffer positions.

-- 
David Kastrup

Re: Invisibility bug: `invisible' vs `display'

[Daniel Brockman]

David Kastrup <dak@gnu.org> writes:

> Daniel Brockman <daniel@brockman.se> writes:
>
>> How can an overlay with identical start and end points
>> display anything?
>
> Using the display property.  Are you sure that you understand the
> difference between overlays and text properties?  Text properties are
> bound to characters, overlays to buffer positions.

Well, I'm obviously confused.  I thought overlay properties
applied to the characters under the overlay.

-- 
Daniel Brockman <daniel@brockman.se>

Re: Invisibility bug: `invisible' vs `display'

[David Kastrup]

Daniel Brockman <daniel@brockman.se> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> Daniel Brockman <daniel@brockman.se> writes:
>>
>>> How can an overlay with identical start and end points
>>> display anything?
>>
>> Using the display property.  Are you sure that you understand the
>> difference between overlays and text properties?  Text properties are
>> bound to characters, overlays to buffer positions.
>
> Well, I'm obviously confused.  I thought overlay properties
> applied to the characters under the overlay.

They apply to the region between its buffer positions.  Which is
usually pretty similar to "characters under the overlay".  But
zero-length overlays exist and work, too, and when copying text from a
buffer, overlay properties are not copied as part of the text.

You can even have overlays that are restricted to a single window
displaying the buffer and will not have an effect on other windows
showing the same buffer.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Invisibility bug: `invisible' vs `display'

[Daniel Brockman]

David Kastrup <dak@gnu.org> writes:

> Daniel Brockman <daniel@brockman.se> writes:
>
>> David Kastrup <dak@gnu.org> writes:
>>
>>> Daniel Brockman <daniel@brockman.se> writes:
>>>
>>>> How can an overlay with identical start and end points
>>>> display anything?
>>>
>>> Using the display property.  Are you sure that you understand the
>>> difference between overlays and text properties?  Text properties are
>>> bound to characters, overlays to buffer positions.
>>
>> Well, I'm obviously confused.  I thought overlay properties
>> applied to the characters under the overlay.
>
> They apply to the region between its buffer positions.
> Which is usually pretty similar to "characters under the
> overlay".  But zero-length overlays exist and work, too,
> and when copying text from a buffer, overlay properties
> are not copied as part of the text.

I can't get zero-length overlays to display anything using
the `display' property.  Here's what I'm doing:

   (overlay-put (make-overlay (point) (point)) 'display "foo")

> You can even have overlays that are restricted to a single
> window displaying the buffer and will not have an effect
> on other windows showing the same buffer.

That's cool.  I didn't know that.

-- 
Daniel Brockman <daniel@brockman.se>

Re: Invisibility bug: `invisible' vs `display'

[David Kastrup]

Daniel Brockman <daniel@brockman.se> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> They apply to the region between its buffer positions.  Which is
>> usually pretty similar to "characters under the overlay".  But
>> zero-length overlays exist and work, too, and when copying text
>> from a buffer, overlay properties are not copied as part of the
>> text.
>
> I can't get zero-length overlays to display anything using
> the `display' property.  Here's what I'm doing:
>
>    (overlay-put (make-overlay (point) (point)) 'display "foo")

Hm.  Weird.  It works using before-string and after-string, though.
Looking at preview-latex, it actually moves stuff from 'display to
'before-string in the zero-length case.

This can probably can well be considered a bug, but it is a bug that
has been around for eternities, so we are not going to win much
applause trying to fix it before the release.

Afterwards, however, should be a good time to tackle this
inconsistency along with the others.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Invisibility bug: `invisible' vs `display'

[Daniel Brockman]

David Kastrup <dak@gnu.org> writes:

> Daniel Brockman <daniel@brockman.se> writes:
>
>> David Kastrup <dak@gnu.org> writes:
>>
>>> They apply to the region between its buffer positions.  Which is
>>> usually pretty similar to "characters under the overlay".  But
>>> zero-length overlays exist and work, too, and when copying text
>>> from a buffer, overlay properties are not copied as part of the
>>> text.
>>
>> I can't get zero-length overlays to display anything using
>> the `display' property.  Here's what I'm doing:
>>
>>    (overlay-put (make-overlay (point) (point)) 'display "foo")
>
> Hm.  Weird.  It works using before-string and after-string, though.
> Looking at preview-latex, it actually moves stuff from 'display to
> 'before-string in the zero-length case.

Okay.  However --- and I guess for the record, --- I can't
see that the display patch affects this in any way.

   (progn
     (insert "foo")
     (overlay-put (make-overlay (point) (point)) 'after-string "bar")
     (insert (propertize "baz" 'invisible t))

The above displays the same with and without the patch.
Likewise for `before-string'.

> This can probably can well be considered a bug, but it is a bug that
> has been around for eternities, so we are not going to win much
> applause trying to fix it before the release.
>
> Afterwards, however, should be a good time to tackle this
> inconsistency along with the others.

I agree that it could be considered a bug.

-- 
Daniel Brockman <daniel@brockman.se>