Re: [jbw@macs.hw.ac.uk: part of display property on before-string property is not displayed]

[YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>]

>>>>> On Wed, 03 Oct 2007 22:02:17 -0400, Richard Stallman <rms@gnu.org> said:

> [I sent this message twice but did not get a response.]

> Would someone please fix this and ack?

> ------- Start of forwarded message -------
> X-Spam-Status: No, score=0.0 required=5.0 tests=UNPARSEABLE_RELAY 
> 	autolearn=failed version=3.1.0
> To: bug-gnu-emacs@gnu.org
> From: Joe Wells <jbw@macs.hw.ac.uk>
> Date: Tue, 18 Sep 2007 23:04:11 +0100
> MIME-Version: 1.0
> Content-Type: text/plain; charset=utf-8
> Subject: part of display property on before-string property is not displayed

> When a string is the display property of a portion of a before-string
> property of an overlay, part of the display property can disappear.
> In this case the first character of the display property does not get
> shown.

> Reproduce by evaluating this code:

>   (let ((buf (get-buffer-create "foo")))
>     (with-current-buffer buf
>       (display-buffer buf)
>       (erase-buffer)
>       (dolist (o (overlays-in (point-min) (point-max)))
>         (delete-overlay o))
>       (insert "ABC")
>       (let ((o (make-overlay 2 3))
>             (s (copy-sequence "DEF")))
>         (put-text-property 1 2 'display "123" s)
>         (overlay-put o 'display "Y")
>         (overlay-put o 'before-string s))))

> You will see the ?foo? window pop up and in the window you will see
> ?AD23FYC?.  I expected that instead ?AD123FYC? would be shown.

> This seems like a bug.

A suspicious part is at line 4240 in xdisp.c (line numbers are those
in the EMACS_22_BASE branch):

  3906	handle_single_display_spec (it, spec, object, position,
  3907				    display_replaced_before_p)

  4222	      if (STRINGP (value))
  4223		{
  4224		  if (SCHARS (value) == 0)
  4225		    {
  4226		      pop_it (it);
  4227		      return -1;  /* Replaced by "", i.e. nothing.  */
  4228		    }
  4229		  it->string = value;
  4230		  it->multibyte_p = STRING_MULTIBYTE (it->string);
  4231		  it->current.overlay_string_index = -1;
  4232		  IT_STRING_CHARPOS (*it) = IT_STRING_BYTEPOS (*it) = 0;
  4233		  it->end_charpos = it->string_nchars = SCHARS (it->string);
  4234		  it->method = GET_FROM_STRING;
  4235		  it->stop_charpos = 0;
  4236		  it->string_from_display_prop_p = 1;
  4237		  /* Say that we haven't consumed the characters with
  4238		     `display' property yet.  The call to pop_it in
  4239		     set_iterator_to_next will clean this up.  */
  4240		  *position = start_pos;
  4241		}

where `position' points to it->current.string_pos when
handle_single_display_spec above is called while processing a
`display' property in a overlay string, and thus line 4240 overrides
the effect of line 4232.

  3784	handle_display_prop (it)

  3792	  if (STRINGP (it->string))
  3793	    {
  3794	      object = it->string;
  3795	      position = &it->current.string_pos;
  3796	    }
  3797	  else
  3798	    {
  3799	      XSETWINDOW (object, it->w);
  3800	      position = &it->current.pos;
  3801	    }

  3855	      int ret = handle_single_display_spec (it, prop, object, position, 0);

I guess the reason to pretend as if the `display' property were not
processed yet at line 4240 is to process overlay strings at the start
position of the `display' property later.  If this guess is correct,
then this adjustment would be necessary only when processing a
`display' property within a buffer text.

Could someone more familiar with redisplay check if the following
patch DTRT?

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Index: src/xdisp.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xdisp.c,v
retrieving revision 1.1149.2.11
diff -c -p -r1.1149.2.11 xdisp.c
*** src/xdisp.c	9 Oct 2007 01:28:33 -0000	1.1149.2.11
--- src/xdisp.c	11 Oct 2007 07:04:31 -0000
*************** handle_single_display_spec (it, spec, ob
*** 4237,4249 ****
  	  /* Say that we haven't consumed the characters with
  	     `display' property yet.  The call to pop_it in
  	     set_iterator_to_next will clean this up.  */
! 	  *position = start_pos;
  	}
        else if (CONSP (value) && EQ (XCAR (value), Qspace))
  	{
  	  it->method = GET_FROM_STRETCH;
  	  it->object = value;
! 	  *position = it->position = start_pos;
  	}
  #ifdef HAVE_WINDOW_SYSTEM
        else
--- 4237,4252 ----
  	  /* Say that we haven't consumed the characters with
  	     `display' property yet.  The call to pop_it in
  	     set_iterator_to_next will clean this up.  */
! 	  if (!STRINGP (it->stack[it->sp - 1].string))
! 	    it->current.pos = start_pos;
  	}
        else if (CONSP (value) && EQ (XCAR (value), Qspace))
  	{
  	  it->method = GET_FROM_STRETCH;
  	  it->object = value;
! 	  it->position = start_pos;
! 	  if (!STRINGP (it->stack[it->sp - 1].string))
! 	    it->current.pos = start_pos;
  	}
  #ifdef HAVE_WINDOW_SYSTEM
        else
*************** handle_single_display_spec (it, spec, ob
*** 4257,4263 ****
  	  /* Say that we haven't consumed the characters with
  	     `display' property yet.  The call to pop_it in
  	     set_iterator_to_next will clean this up.  */
! 	  *position = start_pos;
  	}
  #endif /* HAVE_WINDOW_SYSTEM */
  
--- 4260,4267 ----
  	  /* Say that we haven't consumed the characters with
  	     `display' property yet.  The call to pop_it in
  	     set_iterator_to_next will clean this up.  */
! 	  if (!STRINGP (it->stack[it->sp - 1].string))
! 	    it->current.pos = start_pos;
  	}
  #endif /* HAVE_WINDOW_SYSTEM */