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

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

[Richard Stallman]

[I sent this message a weeks ago 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.

Thanks for your time in considering this bug report.

Joe

======================================================================
In GNU Emacs 22.1.1 (i686-pc-linux-gnu, GTK+ Version 2.8.20)
 of 2007-06-27 on artemis
Windowing system distributor `The X.Org Foundation', version 11.0.70000000
configured using `configure  '--prefix=/home/jbw/local2' '--enable-debug' '--disable-nls' '--with-x-toolkit=gtk' 'CFLAGS=-O0 -g3 -ggdb''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: en_US.UTF-8
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: jbw
  value of $LANG: nil
  locale-coding-system: utf-8
  default-enable-multibyte-characters: t

Major mode: Lisp Interaction

Minor modes in effect:
  TeX-source-specials-mode: t
  auto-fill-function: do-auto-fill
  shell-dirtrack-mode: t
  outline-minor-mode: t
  desktop-save-mode: t
  url-handler-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  unify-8859-on-encoding-mode: t
  utf-translate-cjk-mode: t
  auto-compression-mode: t
  temp-buffer-resize-mode: t
  size-indication-mode: t
  line-number-mode: t
  transient-mark-mode: t
------- End of forwarded message -------

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

[Richard Stallman]

[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.

Thanks for your time in considering this bug report.

Joe

======================================================================
In GNU Emacs 22.1.1 (i686-pc-linux-gnu, GTK+ Version 2.8.20)
 of 2007-06-27 on artemis
Windowing system distributor `The X.Org Foundation', version 11.0.70000000
configured using `configure  '--prefix=/home/jbw/local2' '--enable-debug' '--disable-nls' '--with-x-toolkit=gtk' 'CFLAGS=-O0 -g3 -ggdb''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: en_US.UTF-8
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: jbw
  value of $LANG: nil
  locale-coding-system: utf-8
  default-enable-multibyte-characters: t

Major mode: Lisp Interaction

Minor modes in effect:
  TeX-source-specials-mode: t
  auto-fill-function: do-auto-fill
  shell-dirtrack-mode: t
  outline-minor-mode: t
  desktop-save-mode: t
  url-handler-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  unify-8859-on-encoding-mode: t
  utf-translate-cjk-mode: t
  auto-compression-mode: t
  temp-buffer-resize-mode: t
  size-indication-mode: t
  line-number-mode: t
  transient-mark-mode: t
------- End of forwarded message -------

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

[YAMAMOTO Mitsuharu]

>>>>> 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 */

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

[Chong Yidong]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

> `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.
>
> 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?

I think your reasoning is correct, but instead of

    if (!STRINGP (it->stack[it->sp - 1].string))

could you do

    if (BUFFERP (object))

instead?

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

[YAMAMOTO Mitsuharu]

>>>>> On Fri, 12 Oct 2007 15:59:57 -0400, Chong Yidong <cyd@stupidchicken.com> said:

> I think your reasoning is correct, but instead of

>     if (!STRINGP (it->stack[it->sp - 1].string))

> could you do

>     if (BUFFERP (object))

> instead?

Thanks for the comment.  This is more concise.

>>>>> On Thu, 11 Oct 2007 16:29:41 +0900, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:

> 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.

On second thought, this is not strictly correct.  If there're multiple
display specs, `position' is reused among them.

  3838	      for (; CONSP (prop); prop = XCDR (prop))
  3839		{
  3840		  if (handle_single_display_spec (it, XCAR (prop), object,
  3841						  position, display_replaced_p))
  3842		    display_replaced_p = 1;

The patch below breaks the loop for multiple display specs if a
replacing spec is found in a display property of a string.  This might
be a bit ad hoc, but I think drastic changes should be avoided for
Emacs 22.

				     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	15 Oct 2007 08:10:21 -0000
*************** handle_display_prop (it)
*** 3839,3845 ****
  	{
  	  if (handle_single_display_spec (it, XCAR (prop), object,
  					  position, display_replaced_p))
! 	    display_replaced_p = 1;
  	}
      }
    else if (VECTORP (prop))
--- 3839,3851 ----
  	{
  	  if (handle_single_display_spec (it, XCAR (prop), object,
  					  position, display_replaced_p))
! 	    {
! 	      display_replaced_p = 1;
! 	      /* If some text in a string is replaced, `position' no
! 		 longer points to the position of `object'.  */
! 	      if (STRINGP (object))
! 		break;
! 	    }
  	}
      }
    else if (VECTORP (prop))
*************** handle_display_prop (it)
*** 3848,3854 ****
        for (i = 0; i < ASIZE (prop); ++i)
  	if (handle_single_display_spec (it, AREF (prop, i), object,
  					position, display_replaced_p))
! 	  display_replaced_p = 1;
      }
    else
      {
--- 3854,3866 ----
        for (i = 0; i < ASIZE (prop); ++i)
  	if (handle_single_display_spec (it, AREF (prop, i), object,
  					position, display_replaced_p))
! 	  {
! 	    display_replaced_p = 1;
! 	    /* If some text in a string is replaced, `position' no
! 	       longer points to the position of `object'.  */
! 	    if (STRINGP (object))
! 	      break;
! 	  }
      }
    else
      {
*************** 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
--- 4249,4264 ----
  	  /* 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 (BUFFERP (object))
! 	    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 (BUFFERP (object))
! 	    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 */
  
--- 4272,4279 ----
  	  /* 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 (BUFFERP (object))
! 	    it->current.pos = start_pos;
  	}
  #endif /* HAVE_WINDOW_SYSTEM */

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

[Chong Yidong]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

>> 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.
>
> On second thought, this is not strictly correct.  If there're multiple
> display specs, `position' is reused among them.
>
> The patch below breaks the loop for multiple display specs if a
> replacing spec is found in a display property of a string.  This might
> be a bit ad hoc, but I think drastic changes should be avoided for
> Emacs 22.

Looks good to me; and I agree that it would be best to keep this in
Emacs 23 only.

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

[Richard Stallman]

    > The patch below breaks the loop for multiple display specs if a
    > replacing spec is found in a display property of a string.  This might
    > be a bit ad hoc, but I think drastic changes should be avoided for
    > Emacs 22.

    Looks good to me; and I agree that it would be best to keep this in
    Emacs 23 only.

I will go along with that.  Please install the fix, and update
FOR-RELEASE.

Thank you.

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

[YAMAMOTO Mitsuharu]

>>>>> On Tue, 16 Oct 2007 00:11:11 -0400, Richard Stallman <rms@gnu.org> said:

>> The patch below breaks the loop for multiple display specs if a
>> replacing spec is found in a display property of a string.  This might
>> be a bit ad hoc, but I think drastic changes should be avoided for
>> Emacs 22.

>     Looks good to me; and I agree that it would be best to keep this in
>     Emacs 23 only.

> I will go along with that.  Please install the fix, and update
> FOR-RELEASE.

Done.  Actually this is related to two items in FOR-RELEASE:

** jbw@macs.hw.ac.uk, Sep 19: redisplay goes horribly wrong when a
before-string contains multiple display properties
http://lists.gnu.org/archive/html/emacs-devel/2007-09/msg02442.html

** jbw@macs.hw.ac.uk, Sep 19: part of display property on before-string
 property is not displayed
http://lists.gnu.org/archive/html/emacs-devel/2007-10/msg00138.html

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

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

[Richard Stallman]

    Done.  Actually this is related to two items in FOR-RELEASE:

    ** jbw@macs.hw.ac.uk, Sep 19: redisplay goes horribly wrong when a
    before-string contains multiple display properties
    http://lists.gnu.org/archive/html/emacs-devel/2007-09/msg02442.html

    ** jbw@macs.hw.ac.uk, Sep 19: part of display property on before-string
     property is not displayed
    http://lists.gnu.org/archive/html/emacs-devel/2007-10/msg00138.html

Does this patch fix both?  That is nice.

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

[Richard Stallman]

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.

Thanks for your time in considering this bug report.

Joe

======================================================================
In GNU Emacs 22.1.1 (i686-pc-linux-gnu, GTK+ Version 2.8.20)
 of 2007-06-27 on artemis
Windowing system distributor `The X.Org Foundation', version 11.0.70000000
configured using `configure  '--prefix=/home/jbw/local2' '--enable-debug' '--disable-nls' '--with-x-toolkit=gtk' 'CFLAGS=-O0 -g3 -ggdb''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: en_US.UTF-8
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: jbw
  value of $LANG: nil
  locale-coding-system: utf-8
  default-enable-multibyte-characters: t

Major mode: Lisp Interaction

Minor modes in effect:
  TeX-source-specials-mode: t
  auto-fill-function: do-auto-fill
  shell-dirtrack-mode: t
  outline-minor-mode: t
  desktop-save-mode: t
  url-handler-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  unify-8859-on-encoding-mode: t
  utf-translate-cjk-mode: t
  auto-compression-mode: t
  temp-buffer-resize-mode: t
  size-indication-mode: t
  line-number-mode: t
  transient-mark-mode: t
------- End of forwarded message -------