Re: Display problems with 'before-string in overlay

[Lennart Borgman (gmail)]

Chong Yidong wrote:
> "Lennart Borgman (gmail)" <address@hidden> writes:
>
>> Here is a version that I believe works. It just does local changes to
>> cursor_row_p. I seems to me that is sufficient. I have not seen any
>> problems with the display of 'display property parts, only with cursor
>> positioning.
>
> Thanks for the patch.

Thanks for fixing it up. Sorry for not doing it myself, but I was in a 
hurry.


> My main concern with such an approach is that this will be slow for
> long multi-line strings filling most of the window.  In such a case,
> we will basically scan over all glyphs every redisplay cycle.  On the
> other hand, maybe this situation need not bother us right now.

You mean multi-line 'display strings? Then there is something I 
misunderstand. I mean

1) Scanning starts from the end and stops as soon as STRINGP is true.
2) It is the glyph row that is scanned. Is that not just a line?


> There are various problems with this patch:
>
>   1. Due to an off-by-one error, you start scanning from beyond the
>      end of the glyph row, which can cause a segfault.

Now you see why I normally do not touch C code ;-)


>   2. You have a mixup between Lisp_Object and int in the return value
>      of Fget_char_property.

Ah, yes, I thought it was a pointer.


>   3. The call to get_char property is unnecessary, since
>      string_buffer_position checks only the display property.

Thanks, I forgot that.


> I still believe it's adviseable to hold off redisplay changes till
> Emacs 22.2, but if RMS insists, I guess we might as well check it in
> instead of continuing this thread.

I would be glad if we included this. I have already got complaints about 
this bug from some testers of my code.

I think it would be good if this code were tested with all libraries 
using 'display prop as soon as possible.

My worst doubt about this code is that I am not sure whether the 
necessary information actually always is available when cursor_row_p is 
called, but it looks to me like it must be that.