]> git.eshelyaron.com Git - emacs.git/commitdiff
Retrospective commit from 2009-08-12.
authorEli Zaretskii <eliz@gnu.org>
Thu, 31 Dec 2009 21:09:28 +0000 (16:09 -0500)
committerEli Zaretskii <eliz@gnu.org>
Thu, 31 Dec 2009 21:09:28 +0000 (16:09 -0500)
An (unsuccessful) attempt to solve the issue with row->start and row->end.

 xdisp.c (set_iterator_to_next, reseat, reseat_1)
 (reseat_at_next_visible_line_start): Accept additional argument
 force_logical_p; all callers changed.  If force_logical_p is
 non-zero, force iteration in buffer's logical order even in bidi
 buffers.
 dispnew.c (direct_output_for_insert): Call set_iterator_to_next
 with additional argument zero.
 dispextern.h (set_iterator_to_next): Now accepts an additional
 argument.

src/ChangeLog.bidi
src/dispextern.h
src/dispnew.c
src/xdisp.c

index c36ab231fa828b108b88e8cef974f1dcd85d9620..515f74b39eae36fafc5c40d2a0741b03849897d4 100644 (file)
@@ -1,3 +1,29 @@
+2009-09-12  Eli Zaretskii  <eliz@gnu.org>
+
+       * dispnew.c (direct_output_for_insert): Give up if we are
+       reordering bidirectional text.
+
+       * dispextern.h (IT_STACK_SIZE): Enlarge to 5.
+
+       * xdisp.c (display_line): Set row->end and it->start for the next
+       row to the next character in logical order.  If we are reordering
+       bidi text, push and pop the iterator before and after momentarily
+       iterating in logical order.
+
+2009-09-11  Eli Zaretskii  <eliz@gnu.org>
+
+       * xdisp.c (set_iterator_to_next, reseat, reseat_1)
+       (reseat_at_next_visible_line_start): Accept additional argument
+       force_logical_p; all callers changed.  If force_logical_p is
+       non-zero, force iteration in buffer's logical order even in bidi
+       buffers.
+
+       * dispnew.c (direct_output_for_insert): Call set_iterator_to_next
+       with additional argument zero.
+
+       * dispextern.h (set_iterator_to_next): Now accepts an additional
+       argument.
+
 2009-08-29  Eli Zaretskii  <eliz@gnu.org>
 
        * xdisp.c (set_cursor_from_row): Don't assume glyph->charpos
index 3cc1c83c7e3f0aa811e252cd45fc49974d079de1..36533a3f4c5ff46f99a6b9191bda018503c3de18 100644 (file)
@@ -1927,7 +1927,7 @@ enum it_method {
   NUM_IT_METHODS
 };
 
-#define IT_STACK_SIZE 4
+#define IT_STACK_SIZE 5
 
 /* Iterator for composition (both for static and automatic).  */
 struct composition_it
@@ -2825,7 +2825,7 @@ void init_iterator P_ ((struct it *, struct window *, int,
 void init_iterator_to_row_start P_ ((struct it *, struct window *,
                                     struct glyph_row *));
 int get_next_display_element P_ ((struct it *));
-void set_iterator_to_next P_ ((struct it *, int));
+void set_iterator_to_next P_ ((struct it *, int, int));
 void start_display P_ ((struct it *, struct window *, struct text_pos));
 void move_it_to P_ ((struct it *, int, int, int, int, int));
 void move_it_vertically P_ ((struct it *, int));
index d8cab59dbe9edb8387e8e8799754c0c5285473db..d74462d31b87fff9d6ec7a8f0d66ca4292664711 100644 (file)
@@ -1760,6 +1760,8 @@ check_matrix_invariants (w)
 
       /* Check that end position of `row' is equal to start position
         of next row.  */
+      /* WARNING: This assumption is blatantly incorrect when we are
+        reordering bdirectional text for display!!  */
       if (next->enabled_p && MATRIX_ROW_DISPLAYS_TEXT_P (next))
        {
          xassert (MATRIX_ROW_END_CHARPOS (row)
@@ -3500,6 +3502,8 @@ direct_output_for_insert (g)
       || !display_completed
       /* Give up if buffer appears in two places.  */
       || buffer_shared > 1
+      /* Give up if we need to reorder bidirectional text.  */
+      || !NILP (current_buffer->bidi_display_reordering)
       /* Give up if currently displaying a message instead of the
         minibuffer contents.  */
       || (EQ (selected_window, minibuf_window)
@@ -3608,7 +3612,7 @@ direct_output_for_insert (g)
 
       delta += 1;
       delta_bytes += it.len;
-      set_iterator_to_next (&it, 1);
+      set_iterator_to_next (&it, 1, 0);
     }
 
   /* Give up if we hit the right edge of the window.  We would have
@@ -3626,7 +3630,7 @@ direct_output_for_insert (g)
     {
       if (it2.c == '\t')
        return 0;
-      set_iterator_to_next (&it2, 1);
+      set_iterator_to_next (&it2, 1, 0);
     }
 
   /* Number of new glyphs produced.  */
index 54ac640da64c26cac7456ed602543cc63c2d65ec..947a743ae70888a398d97320784e11d1897952f2 100644 (file)
@@ -963,11 +963,11 @@ 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));
+static void reseat P_ ((struct it *, struct text_pos, int, int));
+static void reseat_1 P_ ((struct it *, struct text_pos, int, int));
 static void back_to_previous_visible_line_start P_ ((struct it *));
 void reseat_at_previous_visible_line_start P_ ((struct it *));
-static void reseat_at_next_visible_line_start P_ ((struct it *, int));
+static void reseat_at_next_visible_line_start P_ ((struct it *, int, int));
 static int next_element_from_ellipsis P_ ((struct it *));
 static int next_element_from_display_vector P_ ((struct it *));
 static int next_element_from_string P_ ((struct it *));
@@ -2823,7 +2823,7 @@ init_iterator (it, w, charpos, bytepos, row, base_face_id)
       it->start = it->current;
 
       /* Compute faces etc.  */
-      reseat (it, it->current.pos, 1);
+      reseat (it, it->current.pos, 1, 0);
     }
 
   CHECK_IT (it);
@@ -2883,7 +2883,7 @@ start_display (it, w, pos)
              if (it->current.dpvec_index >= 0
                  || it->current.overlay_string_index >= 0)
                {
-                 set_iterator_to_next (it, 1);
+                 set_iterator_to_next (it, 1, 0);
                  move_it_in_display_line_to (it, -1, -1, 0);
                }
 
@@ -5242,7 +5242,7 @@ forward_to_next_line_start (it, skipped_p)
       && it->c == '\n'
       && CHARPOS (it->position) == IT_CHARPOS (*it))
     {
-      set_iterator_to_next (it, 0);
+      set_iterator_to_next (it, 0, 0);
       it->c = 0;
       return 1;
     }
@@ -5263,7 +5263,7 @@ forward_to_next_line_start (it, skipped_p)
       if (!get_next_display_element (it))
        return 0;
       newline_found_p = it->what == IT_CHARACTER && it->c == '\n';
-      set_iterator_to_next (it, 0);
+      set_iterator_to_next (it, 0, 0);
     }
 
   /* If we didn't find a newline near enough, see if we can use a
@@ -5296,7 +5296,7 @@ forward_to_next_line_start (it, skipped_p)
                 && !newline_found_p)
            {
              newline_found_p = ITERATOR_AT_END_OF_LINE_P (it);
-             set_iterator_to_next (it, 0);
+             set_iterator_to_next (it, 0, 0);
            }
        }
     }
@@ -5397,22 +5397,24 @@ reseat_at_previous_visible_line_start (it)
      struct it *it;
 {
   back_to_previous_visible_line_start (it);
-  reseat (it, it->current.pos, 1);
+  reseat (it, it->current.pos, 1, 0);
   CHECK_IT (it);
 }
 
 
 /* Reseat iterator IT on the next visible line start in the current
    buffer.  ON_NEWLINE_P non-zero means position IT on the newline
-   preceding the line start.  Skip over invisible text that is so
-   because of selective display.  Compute faces, overlays etc at the
-   new position.  Note that this function does not skip over text that
-   is invisible because of text properties.  */
+   preceding the line start.  FORCE_LOGICAL_P non-zero means force
+   iteration in logical order even if we are reordering bidirectional
+   text.  Skip over invisible text that is so because of selective
+   display.  Compute faces, overlays etc at the new position.  Note
+   that this function does not skip over text that is invisible
+   because of text properties.  */
 
 static void
-reseat_at_next_visible_line_start (it, on_newline_p)
+reseat_at_next_visible_line_start (it, on_newline_p, force_logical_p)
      struct it *it;
-     int on_newline_p;
+     int on_newline_p, force_logical_p;
 {
   int newline_found_p, skipped_p = 0;
 
@@ -5443,13 +5445,16 @@ reseat_at_next_visible_line_start (it, on_newline_p)
        }
       else if (IT_CHARPOS (*it) > BEGV)
        {
+         if (on_newline_p
+             && !(force_logical_p || !it->bidi_p))
+           abort ();
          --IT_CHARPOS (*it);
          --IT_BYTEPOS (*it);
-         reseat (it, it->current.pos, 0);
+         reseat (it, it->current.pos, 0, 1);
        }
     }
   else if (skipped_p)
-    reseat (it, it->current.pos, 0);
+    reseat (it, it->current.pos, 0, force_logical_p);
 
   CHECK_IT (it);
 }
@@ -5463,17 +5468,19 @@ reseat_at_next_visible_line_start (it, on_newline_p)
 /* Change IT's current position to POS in current_buffer.  If FORCE_P
    is non-zero, always check for text properties at the new position.
    Otherwise, text properties are only looked up if POS >=
-   IT->check_charpos of a property.  */
+   IT->check_charpos of a property.  If FORCE_LOGICAL_P is non-zero,
+   force iteration in logical order even when reordering bidirectional
+   text.  */
 
 static void
-reseat (it, pos, force_p)
+reseat (it, pos, force_p, force_logical_p)
      struct it *it;
      struct text_pos pos;
-     int force_p;
+     int force_p, force_logical_p;
 {
   int original_pos = IT_CHARPOS (*it);
 
-  reseat_1 (it, pos, 0);
+  reseat_1 (it, pos, 0, force_logical_p);
 
   /* Determine where to check text properties.  Avoid doing it
      where possible because text property lookup is very expensive.  */
@@ -5487,13 +5494,15 @@ reseat (it, pos, force_p)
 
 
 /* Change IT's buffer position to POS.  SET_STOP_P non-zero means set
-   IT->stop_pos to POS, also.  */
+   IT->stop_pos to POS, also.  FORCE_LOGICAL_P non-zero means force
+   iteration in logical order even when reordering bidirectional
+   text.  */
 
 static void
-reseat_1 (it, pos, set_stop_p)
+reseat_1 (it, pos, set_stop_p, force_logical_p)
      struct it *it;
      struct text_pos pos;
-     int set_stop_p;
+     int set_stop_p, force_logical_p;
 {
   /* Don't call this function when scanning a C string.  */
   xassert (it->s == NULL);
@@ -5518,7 +5527,7 @@ reseat_1 (it, pos, set_stop_p)
   it->string_from_display_prop_p = 0;
   it->face_before_selective_p = 0;
 
-  if (it->bidi_p)
+  if (it->bidi_p && !force_logical_p)
     {
       /* FIXME: L2R below is just for easyness of testing, as we
         currently support only left-to-right paragraphs.  The value
@@ -5729,7 +5738,7 @@ get_next_display_element (it)
                }
              else
                {
-                 set_iterator_to_next (it, 0);
+                 set_iterator_to_next (it, 0, 0);
                }
              goto get_next;
            }
@@ -6051,6 +6060,9 @@ get_next_display_element (it)
    RESEAT_P non-zero means if called on a newline in buffer text,
    skip to the next visible line start.
 
+   FORCE_LOGICAL_P non-zero means force iteration in logical order
+   even when reordering bidirectional text.
+
    Functions get_next_display_element and set_iterator_to_next are
    separate because I find this arrangement easier to handle than a
    get_next_display_element function that also increments IT's
@@ -6062,9 +6074,9 @@ get_next_display_element (it)
    decrement position function which would not be easy to write.  */
 
 void
-set_iterator_to_next (it, reseat_p)
+set_iterator_to_next (it, reseat_p, force_logical_p)
      struct it *it;
-     int reseat_p;
+     int reseat_p, force_logical_p;
 {
   /* Reset flags indicating start and end of a sequence of characters
      with box.  Reset them at the start of this function because
@@ -6078,7 +6090,7 @@ set_iterator_to_next (it, reseat_p)
         current_buffer.  Advance in the buffer, and maybe skip over
         invisible lines that are so because of selective display.  */
       if (ITERATOR_AT_END_OF_LINE_P (it) && reseat_p)
-       reseat_at_next_visible_line_start (it, 0);
+       reseat_at_next_visible_line_start (it, 0, force_logical_p);
       else if (it->cmp_it.id >= 0)
        {
          IT_CHARPOS (*it) += it->cmp_it.nchars;
@@ -6097,7 +6109,7 @@ set_iterator_to_next (it, reseat_p)
        {
          xassert (it->len != 0);
 
-         if (! it->bidi_p)
+         if (!(it->bidi_p && !force_logical_p))
            {
              IT_BYTEPOS (*it) += it->len;
              IT_CHARPOS (*it) += 1;
@@ -6148,14 +6160,14 @@ set_iterator_to_next (it, reseat_p)
 
          /* Skip over characters which were displayed via IT->dpvec.  */
          if (it->dpvec_char_len < 0)
-           reseat_at_next_visible_line_start (it, 1);
+           reseat_at_next_visible_line_start (it, 1, 1);
          else if (it->dpvec_char_len > 0)
            {
              if (it->method == GET_FROM_STRING
                  && it->n_overlay_strings > 0)
                it->ignore_overlay_strings_at_pos_p = 1;
              it->len = it->dpvec_char_len;
-             set_iterator_to_next (it, reseat_p);
+             set_iterator_to_next (it, reseat_p, 0);
            }
 
          /* Maybe recheck faces after display vector */
@@ -6461,7 +6473,7 @@ next_element_from_ellipsis (it)
       it->saved_face_id = it->face_id;
       it->method = GET_FROM_BUFFER;
       it->object = it->w->buffer;
-      reseat_at_next_visible_line_start (it, 1);
+      reseat_at_next_visible_line_start (it, 1, 1);
       it->face_before_selective_p = 1;
     }
 
@@ -6849,7 +6861,7 @@ move_it_in_display_line_to (struct it *it,
 
       if (it->area != TEXT_AREA)
        {
-         set_iterator_to_next (it, 1);
+         set_iterator_to_next (it, 1, 0);
          continue;
        }
 
@@ -6957,7 +6969,7 @@ move_it_in_display_line_to (struct it *it,
                                }
                            }
 
-                         set_iterator_to_next (it, 1);
+                         set_iterator_to_next (it, 1, 0);
                          /* On graphical terminals, newlines may
                             "overflow" into the fringe if
                             overflow-newline-into-fringe is non-nil.
@@ -7053,7 +7065,7 @@ move_it_in_display_line_to (struct it *it,
 
       /* The current display element has been consumed.  Advance
         to the next.  */
-      set_iterator_to_next (it, 1);
+      set_iterator_to_next (it, 1, 0);
 
       /* Stop if lines are truncated and IT's current x-position is
         past the right edge of the window now.  */
@@ -7298,13 +7310,13 @@ move_it_to (it, to_charpos, to_x, to_y, to_vpos, op)
          goto out;
 
        case MOVE_NEWLINE_OR_CR:
-         set_iterator_to_next (it, 1);
+         set_iterator_to_next (it, 1, 0);
          it->continuation_lines_width = 0;
          break;
 
        case MOVE_LINE_TRUNCATED:
          it->continuation_lines_width = 0;
-         reseat_at_next_visible_line_start (it, 0);
+         reseat_at_next_visible_line_start (it, 0, 0);
          if ((op & MOVE_TO_POS) != 0
              && IT_CHARPOS (*it) > to_charpos)
            {
@@ -7330,7 +7342,7 @@ move_it_to (it, to_charpos, to_x, to_y, to_vpos, op)
                {
                  line_start_x = it->current_x + it->pixel_width
                    - it->last_visible_x;
-                 set_iterator_to_next (it, 0);
+                 set_iterator_to_next (it, 0, 0);
                }
            }
          else
@@ -7416,7 +7428,7 @@ move_it_vertically_backward (it, dy)
      reseat to skip forward over invisible text, set up the iterator
      to deliver from overlay strings at the new position etc.  So,
      use reseat_1 here.  */
-  reseat_1 (it, it->current.pos, 1);
+  reseat_1 (it, it->current.pos, 1, 0);
 
   /* We are now surely at a line start.  */
   it->current_x = it->hpos = 0;
@@ -7546,7 +7558,7 @@ move_it_past_eol (it)
 
   rc = move_it_in_display_line_to (it, Z, 0, MOVE_TO_POS);
   if (rc == MOVE_NEWLINE_OR_CR)
-    set_iterator_to_next (it, 0);
+    set_iterator_to_next (it, 0, 0);
 }
 
 
@@ -7575,7 +7587,7 @@ move_it_by_lines (it, dvpos, need_y_p)
 
       pos = *vmotion (IT_CHARPOS (*it), dvpos, it->w);
       SET_TEXT_POS (textpos, pos.bufpos, pos.bytepos);
-      reseat (it, textpos, 1);
+      reseat (it, textpos, 1, 0);
       it->vpos += pos.vpos;
       it->current_y += pos.vpos;
     }
@@ -7611,7 +7623,7 @@ move_it_by_lines (it, dvpos, need_y_p)
       start_charpos = IT_CHARPOS (*it);
       for (i = -dvpos; i > 0 && IT_CHARPOS (*it) > BEGV; --i)
        back_to_previous_visible_line_start (it);
-      reseat (it, it->current.pos, 1);
+      reseat (it, it->current.pos, 1, 0);
 
       /* Move further back if we end up in a string or an image.  */
       while (!IT_POS_VALID_AFTER_MOVE_P (it))
@@ -7625,7 +7637,7 @@ move_it_by_lines (it, dvpos, need_y_p)
          /* If start of line is still in string or image,
             move further back.  */
          back_to_previous_visible_line_start (it);
-         reseat (it, it->current.pos, 1);
+         reseat (it, it->current.pos, 1, 0);
          dvpos--;
        }
 
@@ -10226,7 +10238,7 @@ display_tool_bar_line (it, height)
       if (ITERATOR_AT_END_OF_LINE_P (it))
        break;
 
-      set_iterator_to_next (it, 1);
+      set_iterator_to_next (it, 1, 0);
     }
 
  out:;
@@ -16548,6 +16560,7 @@ display_line (it)
   int wrap_row_used = -1, wrap_row_ascent, wrap_row_height;
   int wrap_row_phys_ascent, wrap_row_phys_height;
   int wrap_row_extra_line_spacing;
+  struct display_pos row_end;
 
   /* We always start displaying at hpos zero even if hscrolled.  */
   xassert (it->hpos == 0 && it->current_x == 0);
@@ -16636,6 +16649,7 @@ display_line (it)
 
          it->continuation_lines_width = 0;
          row->ends_at_zv_p = 1;
+         row_end = it->current;
          break;
        }
 
@@ -16685,7 +16699,7 @@ display_line (it)
                                  it->max_phys_ascent + it->max_phys_descent);
          row->extra_line_spacing = max (row->extra_line_spacing,
                                         it->max_extra_line_spacing);
-         set_iterator_to_next (it, 1);
+         set_iterator_to_next (it, 1, 0);
          continue;
        }
 
@@ -16764,7 +16778,7 @@ display_line (it)
                                  || IT_DISPLAYING_WHITESPACE (it)))
                            goto back_to_wrap;
 
-                         set_iterator_to_next (it, 1);
+                         set_iterator_to_next (it, 1, 0);
                          if (IT_OVERFLOW_NEWLINE_INTO_FRINGE (it))
                            {
                              if (!get_next_display_element (it))
@@ -16870,6 +16884,7 @@ display_line (it)
                      it->max_phys_descent = phys_descent;
                    }
 
+                 row_end = it->current;
                  break;
                }
              else if (new_x > it->first_visible_x)
@@ -16903,7 +16918,10 @@ display_line (it)
 
          /* End of this display line if row is continued.  */
          if (row->continued_p || row->ends_at_zv_p)
-           break;
+           {
+             row_end = it->current;
+             break;
+           }
        }
 
     at_end_of_line:
@@ -16929,14 +16947,26 @@ display_line (it)
            row->glyphs[TEXT_AREA]->charpos = CHARPOS (it->position);
 
          /* Consume the line end.  This skips over invisible lines.  */
-         set_iterator_to_next (it, 1);
+         if (it->bidi_p)
+           {
+             /* When we are reordering bidi text, we still need the
+                next character in logical order, to set row->end
+                correctly below.  */
+             push_it (it);
+             set_iterator_to_next (it, 1, 1);
+             row_end = it->current;
+             pop_it (it);
+           }
+         set_iterator_to_next (it, 1, 0);
          it->continuation_lines_width = 0;
+         if (!it->bidi_p)
+           row_end = it->current;
          break;
        }
 
       /* Proceed with next display element.  Note that this skips
         over lines invisible because of selective display.  */
-      set_iterator_to_next (it, 1);
+      set_iterator_to_next (it, 1, 0);
 
       /* If we truncate lines, we are done when the last displayed
         glyphs reach past the right margin of the window.  */
@@ -16968,6 +16998,7 @@ display_line (it)
                  it->continuation_lines_width = 0;
                  row->ends_at_zv_p = 1;
                  row->exact_window_width_line_p = 1;
+                 row_end = it->current;
                  break;
                }
              if (ITERATOR_AT_END_OF_LINE_P (it))
@@ -16979,10 +17010,11 @@ display_line (it)
 
          row->truncated_on_right_p = 1;
          it->continuation_lines_width = 0;
-         reseat_at_next_visible_line_start (it, 0);
+         reseat_at_next_visible_line_start (it, 0, 0);
          row->ends_at_zv_p = FETCH_BYTE (IT_BYTEPOS (*it) - 1) != '\n';
          it->hpos = hpos_before;
          it->current_x = x_before;
+         row_end = it->current;
          break;
        }
     }
@@ -17043,7 +17075,7 @@ display_line (it)
   compute_line_metrics (it);
 
   /* Remember the position at which this line ends.  */
-  row->end = it->current;
+  row->end = row_end;
 
   /* Record whether this row ends inside an ellipsis.  */
   row->ends_in_ellipsis_p
@@ -17080,7 +17112,7 @@ display_line (it)
   it->current_y += row->height;
   ++it->vpos;
   ++it->glyph_row;
-  it->start = it->current;
+  it->start = row_end;
   return row->displays_text_p;
 }
 
@@ -19024,7 +19056,7 @@ display_string (string, lisp_string, face_string, face_string_pos,
          break;
        }
 
-      set_iterator_to_next (it, 1);
+      set_iterator_to_next (it, 1, 0);
 
       /* Stop if truncating at the right edge.  */
       if (it->line_wrap == TRUNCATE