Mercurial > emacs

changeset 107587:0ff1b8888f6b

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Retrospective commit from 2009-08-12. 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.
author Eli Zaretskii <eliz@gnu.org>
date Thu, 31 Dec 2009 16:09:28 -0500
parents cbca7f94b057
children 1104f4d707b1
files src/ChangeLog.bidi src/dispextern.h src/dispnew.c src/xdisp.c
diffstat 4 files changed, 128 insertions(+), 66 deletions(-) [+]
line wrap: on
line diff
--- a/src/ChangeLog.bidi	Thu Dec 31 15:56:07 2009 -0500
+++ b/src/ChangeLog.bidi	Thu Dec 31 16:09:28 2009 -0500
@@ -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
--- a/src/dispextern.h	Thu Dec 31 15:56:07 2009 -0500
+++ b/src/dispextern.h	Thu Dec 31 16:09:28 2009 -0500
@@ -1927,7 +1927,7 @@
   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_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));
--- a/src/dispnew.c	Thu Dec 31 15:56:07 2009 -0500
+++ b/src/dispnew.c	Thu Dec 31 16:09:28 2009 -0500
@@ -1760,6 +1760,8 @@
 
       /* 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 @@
       || !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 @@
 
       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 @@
     {
       if (it2.c == '\t')
 	return 0;
-      set_iterator_to_next (&it2, 1);
+      set_iterator_to_next (&it2, 1, 0);
     }
 
   /* Number of new glyphs produced.  */
--- a/src/xdisp.c	Thu Dec 31 15:56:07 2009 -0500
+++ b/src/xdisp.c	Thu Dec 31 16:09:28 2009 -0500
@@ -963,11 +963,11 @@
 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 @@
       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 @@
 	      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 @@
       && 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 @@
       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 @@
 		 && !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 @@
      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.  */
-
-static void
-reseat_at_next_visible_line_start (it, on_newline_p)
-     struct it *it;
-     int on_newline_p;
+   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, force_logical_p)
+     struct it *it;
+     int on_newline_p, force_logical_p;
 {
   int newline_found_p, skipped_p = 0;
 
@@ -5443,13 +5445,16 @@
 	}
       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 @@
 /* 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.  */
-
-static void
-reseat (it, pos, force_p)
+   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, 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 @@
 
 
 /* Change IT's buffer position to POS.  SET_STOP_P non-zero means set
-   IT->stop_pos to POS, also.  */
-
-static void
-reseat_1 (it, pos, set_stop_p)
+   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, 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 @@
   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 @@
 		}
 	      else
 		{
-		  set_iterator_to_next (it, 0);
+		  set_iterator_to_next (it, 0, 0);
 		}
 	      goto get_next;
 	    }
@@ -6051,6 +6060,9 @@
    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 @@
    decrement position function which would not be easy to write.  */
 
 void
-set_iterator_to_next (it, reseat_p)
-     struct it *it;
-     int reseat_p;
+set_iterator_to_next (it, reseat_p, force_logical_p)
+     struct it *it;
+     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 @@
 	 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 @@
 	{
 	  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 @@
 
 	  /* 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 @@
       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 @@
 
       if (it->area != TEXT_AREA)
 	{
-	  set_iterator_to_next (it, 1);
+	  set_iterator_to_next (it, 1, 0);
 	  continue;
 	}
 
@@ -6957,7 +6969,7 @@
 				}
 			    }
 
-			  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 @@
 
       /* 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 @@
 	  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 @@
 		{
 		  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 @@
      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 @@
 
   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 @@
 
       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 @@
       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 @@
 	  /* 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 @@
       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 @@
   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 @@
 
 	  it->continuation_lines_width = 0;
 	  row->ends_at_zv_p = 1;
+	  row_end = it->current;
 	  break;
 	}
 
@@ -16685,7 +16699,7 @@
 				  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 @@
 				  || 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 @@
 		      it->max_phys_descent = phys_descent;
 		    }
 
+		  row_end = it->current;
 		  break;
 		}
 	      else if (new_x > it->first_visible_x)
@@ -16903,7 +16918,10 @@
 
 	  /* 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 @@
 	    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 @@
 		  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 @@
 
 	  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 @@
   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 @@
   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 @@
 	  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