Re: Display problems with 'before-string in overlay

[Chong Yidong]

Richard Stallman <address@hidden> writes:

>     At this stage of the release cycle, a grave bug is one that makes Emacs
>     crash, or causes really bad redisplay behaviour.
>
> I think redisplay bugs are serious bugs.

In this case, I know what the fix is (see attached patch).  The
trouble is that it's a rather big change, and likely to cause
problems, whereas the problem it corrects is not commonly encountered
(cursor positioning in multi-line overlay before/after-strings).

So I'd like to delay this till after Emacs 22.1.  WDYT?

*** emacs/src/keyboard.c.~1.899.~       2007-04-04 13:05:31.000000000 -0400
--- emacs/src/keyboard.c        2007-04-13 14:04:15.000000000 -0400
***************
*** 2010,2021 ****
          && !NILP (val = get_char_property_and_overlay
                              (make_number (PT), Qdisplay, Qnil, &overlay))
          && display_prop_intangible_p (val)
!         && (!OVERLAYP (overlay)
!             ? get_property_and_range (PT, Qdisplay, &val, &beg, &end, Qnil)
!             : (beg = OVERLAY_POSITION (OVERLAY_START (overlay)),
!                end = OVERLAY_POSITION (OVERLAY_END (overlay))))
!         && (beg < PT /* && end > PT   <- It's always the case.  */
!             || (beg <= PT && STRINGP (val) && SCHARS (val) == 0)))
        {
          xassert (end > PT);
          SET_PT (PT < last_pt
--- 2010,2022 ----
          && !NILP (val = get_char_property_and_overlay
                              (make_number (PT), Qdisplay, Qnil, &overlay))
          && display_prop_intangible_p (val)
!         && (OVERLAYP (overlay)
!             ? (beg = OVERLAY_POSITION (OVERLAY_START (overlay)),
!                end = OVERLAY_POSITION (OVERLAY_END (overlay)),
!                beg <= PT)
!             : (get_property_and_range (PT, Qdisplay, &val, &beg, &end, Qnil),
!                (beg < PT
!                 || (beg <= PT && STRINGP (val) && SCHARS (val) == 0)))))
        {
          xassert (end > PT);
          SET_PT (PT < last_pt
*** emacs/src/dispextern.h.~1.225.~     2007-01-21 13:34:43.000000000 -0500
--- emacs/src/dispextern.h      2007-04-13 13:42:58.000000000 -0400
***************
*** 2636,2642 ****
  struct glyph_row *row_containing_pos P_ ((struct window *, int,
                                          struct glyph_row *,
                                          struct glyph_row *, int));
! int string_buffer_position P_ ((struct window *, Lisp_Object, int));
  int line_bottom_y P_ ((struct it *));
  int display_prop_intangible_p P_ ((Lisp_Object));
  void resize_echo_area_exactly P_ ((void));
--- 2636,2642 ----
  struct glyph_row *row_containing_pos P_ ((struct window *, int,
                                          struct glyph_row *,
                                          struct glyph_row *, int));
! int string_buffer_position P_ ((struct window *, Lisp_Object, int, 
Lisp_Object *));
  int line_bottom_y P_ ((struct it *));
  int display_prop_intangible_p P_ ((Lisp_Object));
  void resize_echo_area_exactly P_ ((void));
*** emacs/src/xdisp.c.~1.1146.~ 2007-04-12 16:23:44.000000000 -0400
--- emacs/src/xdisp.c   2007-04-13 14:09:22.000000000 -0400
***************
*** 4425,4434 ****
     called asynchronously from note_mouse_highlight.  */
  
  int
! string_buffer_position (w, string, around_charpos)
       struct window *w;
       Lisp_Object string;
       int around_charpos;
  {
    Lisp_Object limit, prop, pos;
    const int MAX_DISTANCE = 1000;
--- 4425,4435 ----
     called asynchronously from note_mouse_highlight.  */
  
  int
! string_buffer_position (w, string, around_charpos, overlay)
       struct window *w;
       Lisp_Object string;
       int around_charpos;
+      Lisp_Object *overlay;
  {
    Lisp_Object limit, prop, pos;
    const int MAX_DISTANCE = 1000;
***************
*** 4438,4444 ****
    limit = make_number (min (XINT (pos) + MAX_DISTANCE, ZV));
    while (!found && !EQ (pos, limit))
      {
!       prop = Fget_char_property (pos, Qdisplay, Qnil);
        if (!NILP (prop) && display_prop_string_p (prop, string))
        found = 1;
        else
--- 4439,4445 ----
    limit = make_number (min (XINT (pos) + MAX_DISTANCE, ZV));
    while (!found && !EQ (pos, limit))
      {
!       prop = get_char_property_and_overlay (pos, Qdisplay, Qnil, overlay);
        if (!NILP (prop) && display_prop_string_p (prop, string))
        found = 1;
        else
***************
*** 4451,4457 ****
        limit = make_number (max (XINT (pos) - MAX_DISTANCE, BEGV));
        while (!found && !EQ (pos, limit))
        {
!         prop = Fget_char_property (pos, Qdisplay, Qnil);
          if (!NILP (prop) && display_prop_string_p (prop, string))
            found = 1;
          else
--- 4452,4458 ----
        limit = make_number (max (XINT (pos) - MAX_DISTANCE, BEGV));
        while (!found && !EQ (pos, limit))
        {
!         prop = get_char_property_and_overlay (pos, Qdisplay, Qnil, overlay);
          if (!NILP (prop) && display_prop_string_p (prop, string))
            found = 1;
          else
***************
*** 11884,11890 ****
                                                  Qcursor, (glyph)->object),
                      !NILP (cprop))
                  && (pos = string_buffer_position (w, glyph->object,
!                                                   string_before_pos),
                      (pos == 0   /* From overlay */
                       || pos == pt_old)))
                {
--- 11885,11891 ----
                                                  Qcursor, (glyph)->object),
                      !NILP (cprop))
                  && (pos = string_buffer_position (w, glyph->object,
!                                                   string_before_pos, 0),
                      (pos == 0   /* From overlay */
                       || pos == pt_old)))
                {
***************
*** 11933,11952 ****
        Lisp_Object string;
        struct glyph *stop = glyph;
        int pos;
  
        limit = make_number (pt_old + 1);
        glyph = string_start;
        x = string_start_x;
        string = glyph->object;
!       pos = string_buffer_position (w, string, string_before_pos);
!       /* If STRING is from overlay, LAST_POS == 0.  We skip such glyphs
!        because we always put cursor after overlay strings.  */
!       while (pos == 0 && glyph < stop)
        {
          string = glyph->object;
          SKIP_GLYPHS (glyph, stop, x, EQ (glyph->object, string));
          if (glyph < stop)
!           pos = string_buffer_position (w, glyph->object, string_before_pos);
        }
  
        while (glyph < stop)
--- 11934,11955 ----
        Lisp_Object string;
        struct glyph *stop = glyph;
        int pos;
+       Lisp_Object overlay = Qnil;
  
        limit = make_number (pt_old + 1);
        glyph = string_start;
        x = string_start_x;
        string = glyph->object;
!       pos = string_buffer_position (w, string, string_before_pos, &overlay);
!       /* If STRING is from overlay, skip its glyphs because we always
!        put cursor after overlay strings.  */
!       while ((pos == 0 || !NILP (overlay)) && glyph < stop)
        {
          string = glyph->object;
          SKIP_GLYPHS (glyph, stop, x, EQ (glyph->object, string));
          if (glyph < stop)
!           pos = string_buffer_position (w, glyph->object,
!                                         string_before_pos, overlay);
        }
  
        while (glyph < stop)
***************
*** 11960,11966 ****
          SKIP_GLYPHS (glyph, stop, x, EQ (glyph->object, string));
          /* Skip glyphs from an overlay.  */
          while (glyph < stop
!                && ! string_buffer_position (w, glyph->object, pos))
            {
              string = glyph->object;
              SKIP_GLYPHS (glyph, stop, x, EQ (glyph->object, string));
--- 11963,11969 ----
          SKIP_GLYPHS (glyph, stop, x, EQ (glyph->object, string));
          /* Skip glyphs from an overlay.  */
          while (glyph < stop
!                && ! string_buffer_position (w, glyph->object, pos, 0))
            {
              string = glyph->object;
              SKIP_GLYPHS (glyph, stop, x, EQ (glyph->object, string));
***************
*** 15854,15865 ****
    if (PT == MATRIX_ROW_END_CHARPOS (row))
      {
        /* If the row ends with a newline from a string, we don't want
!        the cursor there, but we still want it at the start of the
!        string if the string starts in this row.
         If the row is continued it doesn't end in a newline.  */
        if (CHARPOS (row->end.string_pos) >= 0)
!       cursor_row_p = (row->continued_p
!                       || PT >= MATRIX_ROW_START_CHARPOS (row));
        else if (MATRIX_ROW_ENDS_IN_MIDDLE_OF_CHAR_P (row))
        {
          /* If the row ends in middle of a real character,
--- 15857,15866 ----
    if (PT == MATRIX_ROW_END_CHARPOS (row))
      {
        /* If the row ends with a newline from a string, we don't want
!        the cursor there.
         If the row is continued it doesn't end in a newline.  */
        if (CHARPOS (row->end.string_pos) >= 0)
!       cursor_row_p = row->continued_p;
        else if (MATRIX_ROW_ENDS_IN_MIDDLE_OF_CHAR_P (row))
        {
          /* If the row ends in middle of a real character,
***************
*** 23021,23027 ****
              struct glyph_row *r = MATRIX_ROW (w->current_matrix, vpos);
              int start = MATRIX_ROW_START_CHARPOS (r);
  
!             pos = string_buffer_position (w, object, start);
              if (pos > 0)
                mouse_face = get_char_property_and_overlay (make_number (pos),
                                                            Qmouse_face,
--- 23022,23028 ----
              struct glyph_row *r = MATRIX_ROW (w->current_matrix, vpos);
              int start = MATRIX_ROW_START_CHARPOS (r);
  
!             pos = string_buffer_position (w, object, start, 0);
              if (pos > 0)
                mouse_face = get_char_property_and_overlay (make_number (pos),
                                                            Qmouse_face,
***************
*** 23105,23111 ****
                    struct glyph_row *r
                      = MATRIX_ROW (w->current_matrix, vpos);
                    int start = MATRIX_ROW_START_CHARPOS (r);
!                   int pos = string_buffer_position (w, object, start);
                    if (pos > 0)
                      {
                        help = Fget_char_property (make_number (pos),
--- 23106,23112 ----
                    struct glyph_row *r
                      = MATRIX_ROW (w->current_matrix, vpos);
                    int start = MATRIX_ROW_START_CHARPOS (r);
!                   int pos = string_buffer_position (w, object, start, 0);
                    if (pos > 0)
                      {
                        help = Fget_char_property (make_number (pos),
***************
*** 23160,23166 ****
                      struct glyph_row *r
                        = MATRIX_ROW (w->current_matrix, vpos);
                      int start = MATRIX_ROW_START_CHARPOS (r);
!                     int pos = string_buffer_position (w, object, start);
                      if (pos > 0)
                        pointer = Fget_char_property (make_number (pos),
                                                      Qpointer, w->buffer);
--- 23161,23167 ----
                      struct glyph_row *r
                        = MATRIX_ROW (w->current_matrix, vpos);
                      int start = MATRIX_ROW_START_CHARPOS (r);
!                     int pos = string_buffer_position (w, object, start, 0);
                      if (pos > 0)
                        pointer = Fget_char_property (make_number (pos),
                                                      Qpointer, w->buffer);