Re: address@hidden: Overlay string not displayed on text with `display' property]

[Kim F. Storm]

YAMAMOTO Mitsuharu <address@hidden> writes:

> There seems to be some problems with this patch.  Besides an apparent
> typo (compute_stop_pos), some assertions are violated with the test
> case you mentioned:

> And crashed if the last one was changed to (put-text-property 2 3
> 'display "DISP"), though the case that overlay strings are not at
> boundaries of display property was not what I intended.

Thanks for testing.

Please try the new patch below.

The additional changes to the iterator stack are needed to support
overlay strings around an image (didn't work with my previous patch).

Actually, with these changes, the use of the iterator stack is much
cleaner than before, as there are no longer any implicit assumptions
about what the state was before a push, causing specific iterator
settings (like it->method) to be explicitly set after a pop.

[I removed the lengthy comment from RMS in reseat_1, as the change
 is obviously the right thing to do -- also proved by the +3 years
 that have elapsed since it was added].


*** dispextern.h        28 May 2006 21:09:42 +0200      1.218
--- dispextern.h        13 Jun 2006 12:42:07 +0200      
***************
*** 1819,1824 ****
--- 1819,1826 ----
    NUM_IT_METHODS
  };
  
+ #define IT_STACK_SIZE 4
+ 
  struct it
  {
    /* The window in which we iterate over current_buffer (or a string).  */
***************
*** 1930,1939 ****
--- 1932,1943 ----
      int stop_charpos;
      int face_id;
      Lisp_Object string;
+     int image_id;
      struct display_pos pos;
      int end_charpos;
      int string_nchars;
      enum glyph_row_area area;
+     enum it_method method;
      unsigned multibyte_p : 1;
      unsigned string_from_display_prop_p : 1;
      unsigned display_ellipsis_p : 1;
***************
*** 1942,1948 ****
      short voffset;
      Lisp_Object font_height;
    }
!   stack[2];
  
    /* Stack pointer.  */
    int sp;
--- 1946,1952 ----
      short voffset;
      Lisp_Object font_height;
    }
!   stack[IT_STACK_SIZE];
  
    /* Stack pointer.  */
    int sp;


*** xdisp.c     03 Jun 2006 23:54:30 +0200      1.1102
--- xdisp.c     13 Jun 2006 13:27:33 +0200      
***************
*** 916,921 ****
--- 916,922 ----
  static void compute_line_metrics P_ ((struct it *));
  static void run_redisplay_end_trigger_hook P_ ((struct it *));
  static int get_overlay_strings P_ ((struct it *, int));
+ static int get_overlay_strings_1 P_ ((struct it *, int, int));
  static void next_overlay_string P_ ((struct it *));
  static void reseat P_ ((struct it *, struct text_pos, int));
  static void reseat_1 P_ ((struct it *, struct text_pos, int));
***************
*** 2909,2916 ****
         also ``processed'' overlay strings at ZV.  */
        while (it->sp)
        pop_it (it);
!       it->current.overlay_string_index = -1;
!       it->method = GET_FROM_BUFFER;
        if (CHARPOS (pos->pos) == ZV)
        it->overlay_strings_at_end_processed_p = 1;
      }
--- 2910,2917 ----
         also ``processed'' overlay strings at ZV.  */
        while (it->sp)
        pop_it (it);
!       xassert (it->current.overlay_string_index == -1);
!       xassert (it->method == GET_FROM_BUFFER);
        if (CHARPOS (pos->pos) == ZV)
        it->overlay_strings_at_end_processed_p = 1;
      }
***************
*** 3021,3027 ****
          if (handled == HANDLED_RECOMPUTE_PROPS)
            break;
          else if (handled == HANDLED_RETURN)
!           return;
          else if (handled == HANDLED_OVERLAY_STRING_CONSUMED)
            handle_overlay_change_p = 0;
        }
--- 3022,3039 ----
          if (handled == HANDLED_RECOMPUTE_PROPS)
            break;
          else if (handled == HANDLED_RETURN)
!           {
!             /* We still want to show before and after strings from
!                overlays even if the actual buffer text is replaced.  */
!             if (!handle_overlay_change_p || it->sp > 1)
!               return;
!             if (!get_overlay_strings_1 (it, 0, 0))
!               return;
!             it->string_from_display_prop_p = 0;
!             handle_overlay_change_p = 0;
!             handled = HANDLED_RECOMPUTE_PROPS;
!             break;
!           }
          else if (handled == HANDLED_OVERLAY_STRING_CONSUMED)
            handle_overlay_change_p = 0;
        }
***************
*** 4543,4555 ****
        int display_ellipsis_p = it->stack[it->sp - 1].display_ellipsis_p;
  
        pop_it (it);
!       xassert (it->stop_charpos >= BEGV
!              && it->stop_charpos <= it->end_charpos);
!       it->string = Qnil;
        it->current.overlay_string_index = -1;
-       SET_TEXT_POS (it->current.string_pos, -1, -1);
        it->n_overlay_strings = 0;
-       it->method = GET_FROM_BUFFER;
  
        /* If we're at the end of the buffer, record that we have
         processed the overlay strings there already, so that
--- 4555,4571 ----
        int display_ellipsis_p = it->stack[it->sp - 1].display_ellipsis_p;
  
        pop_it (it);
!       xassert (it->sp > 0
!              || (it->stop_charpos >= BEGV
!                  && it->stop_charpos <= it->end_charpos));
!       if (it->sp == 0)
!       {
!         xassert (NILP (it->string));
!         // SET_TEXT_POS (it->current.string_pos, -1, -1);
!         xassert (it->method == GET_FROM_BUFFER);
!       }
        it->current.overlay_string_index = -1;
        it->n_overlay_strings = 0;
  
        /* If we're at the end of the buffer, record that we have
         processed the overlay strings there already, so that
***************
*** 4805,4811 ****
     least one overlay string was found.  */
  
  static int
! get_overlay_strings (it, charpos)
       struct it *it;
       int charpos;
  {
--- 4821,4827 ----
     least one overlay string was found.  */
  
  static int
! get_overlay_strings_1 (it, charpos, compute_stop_p)
       struct it *it;
       int charpos;
  {
***************
*** 4827,4838 ****
        /* Make sure we know settings in current_buffer, so that we can
         restore meaningful values when we're done with the overlay
         strings.  */
!       compute_stop_pos (it);
        xassert (it->face_id >= 0);
  
        /* Save IT's settings.  They are restored after all overlay
         strings have been processed.  */
!       xassert (it->sp == 0);
        push_it (it);
  
        /* Set up IT to deliver display elements from the first overlay
--- 4843,4855 ----
        /* Make sure we know settings in current_buffer, so that we can
         restore meaningful values when we're done with the overlay
         strings.  */
!       if (compute_stop_p)
!       compute_stop_pos (it);
        xassert (it->face_id >= 0);
  
        /* Save IT's settings.  They are restored after all overlay
         strings have been processed.  */
!       xassert (!compute_stop_p || it->sp == 0);
        push_it (it);
  
        /* Set up IT to deliver display elements from the first overlay
***************
*** 4844,4856 ****
        it->end_charpos = SCHARS (it->string);
        it->multibyte_p = STRING_MULTIBYTE (it->string);
        it->method = GET_FROM_STRING;
      }
!   else
!     {
!       it->string = Qnil;
!       it->current.overlay_string_index = -1;
!       it->method = GET_FROM_BUFFER;
!     }
  
    CHECK_IT (it);
  
--- 4861,4882 ----
        it->end_charpos = SCHARS (it->string);
        it->multibyte_p = STRING_MULTIBYTE (it->string);
        it->method = GET_FROM_STRING;
+       return 1;
      }
! 
!   it->current.overlay_string_index = -1;
!   return 0;
! }
! 
! static int
! get_overlay_strings (it, charpos)
!      struct it *it;
!      int charpos;
! {
!   it->string = Qnil;
!   it->method = GET_FROM_BUFFER;
! 
!   (void) get_overlay_strings_1 (it, charpos, 1);
  
    CHECK_IT (it);
  
***************
*** 4875,4887 ****
  {
    struct iterator_stack_entry *p;
  
!   xassert (it->sp < 2);
    p = it->stack + it->sp;
  
    p->stop_charpos = it->stop_charpos;
    xassert (it->face_id >= 0);
    p->face_id = it->face_id;
!   p->string = it->string;
    p->pos = it->current;
    p->end_charpos = it->end_charpos;
    p->string_nchars = it->string_nchars;
--- 4901,4918 ----
  {
    struct iterator_stack_entry *p;
  
!   xassert (it->sp < IT_STACK_SIZE);
    p = it->stack + it->sp;
  
    p->stop_charpos = it->stop_charpos;
    xassert (it->face_id >= 0);
    p->face_id = it->face_id;
!   p->image_id = it->image_id;
!   p->method = it->method;
!   if (it->method == GET_FROM_IMAGE)
!     p->string = it->object;
!   else
!     p->string = it->string;
    p->pos = it->current;
    p->end_charpos = it->end_charpos;
    p->string_nchars = it->string_nchars;
***************
*** 4914,4921 ****
    p = it->stack + it->sp;
    it->stop_charpos = p->stop_charpos;
    it->face_id = p->face_id;
!   it->string = p->string;
    it->current = p->pos;
    it->end_charpos = p->end_charpos;
    it->string_nchars = p->string_nchars;
    it->area = p->area;
--- 4945,4962 ----
    p = it->stack + it->sp;
    it->stop_charpos = p->stop_charpos;
    it->face_id = p->face_id;
!   it->method = p->method;
!   it->image_id = p->image_id;
    it->current = p->pos;
+   if (it->method == GET_FROM_IMAGE)
+     {
+       it->object = it->string;
+       it->string = Qnil;
+     }
+   else
+     it->string = p->string;
+   if (NILP (it->string))
+     SET_TEXT_POS (it->current.string_pos, -1, -1);
    it->end_charpos = p->end_charpos;
    it->string_nchars = p->string_nchars;
    it->area = p->area;
***************
*** 5233,5247 ****
    IT_STRING_BYTEPOS (*it) = -1;
    it->string = Qnil;
    it->method = GET_FROM_BUFFER;
-   /* RMS: I added this to fix a bug in move_it_vertically_backward
-      where it->area continued to relate to the starting point
-      for the backward motion.  Bug report from
-      Nick Roberts <address@hidden> on 19 May 2003.
-      However, I am not sure whether reseat still does the right thing
-      in general after this change.  */
    it->area = TEXT_AREA;
    it->multibyte_p = !NILP (current_buffer->enable_multibyte_characters);
    it->sp = 0;
    it->face_before_selective_p = 0;
  
    if (set_stop_p)
--- 5274,5283 ----
    IT_STRING_BYTEPOS (*it) = -1;
    it->string = Qnil;
    it->method = GET_FROM_BUFFER;
    it->area = TEXT_AREA;
    it->multibyte_p = !NILP (current_buffer->enable_multibyte_characters);
    it->sp = 0;
+   it->string_from_display_prop_p = 0;
    it->face_before_selective_p = 0;
  
    if (set_stop_p)
***************
*** 5809,5817 ****
              && it->sp > 0)
            {
              pop_it (it);
!             if (STRINGP (it->string))
                goto consider_string_end;
-             it->method = GET_FROM_BUFFER;
            }
        }
        break;
--- 5845,5852 ----
              && it->sp > 0)
            {
              pop_it (it);
!             if (it->method == GET_FROM_STRING)
                goto consider_string_end;
            }
        }
        break;
***************
*** 5823,5835 ****
           if the `display' property takes up the whole string.  */
        xassert (it->sp > 0);
        pop_it (it);
!       it->image_id = 0;
!       if (STRINGP (it->string))
!       {
!         it->method = GET_FROM_STRING;
!         goto consider_string_end;
!       }
!       it->method = GET_FROM_BUFFER;
        break;
  
      default:
--- 5858,5865 ----
           if the `display' property takes up the whole string.  */
        xassert (it->sp > 0);
        pop_it (it);
!       if (it->method == GET_FROM_STRING)
!       goto consider_string_end;
        break;
  
      default:

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