changeset 107611:118ff750e43e

Continue working on handling of properties in bidi iteration. Region display and extension seems to work. Solved a crash in bidirectional display of etc/HELLO. (HELLO display still not 100% OK, e.g. near Kannada.) .gdbinit (pitx): Display some bidi information about the iterator. dispextern.h (BIDI_AT_BASE_LEVEL): Enclose definition in parentheses. xdisp.c (handle_stop_backwards): Save and restore it->current and it->position, instead of expecting the caller to do that. (next_element_from_buffer): When moving across stop_charpos, record it in prev_stop. When IT_CHARPOS backs up, call handle_stop_backwards only if above the base embedding level. This solves the crash while displaying etc/HELLO in bidi mode.

--- a/src/.gdbinit	Sat Jan 02 05:57:50 2010 -0500
+++ b/src/.gdbinit	Sat Jan 02 10:57:35 2010 -0500
@@ -271,6 +271,9 @@
     end
   end
   printf "\n"
+  if ($it->bidi_p)
+    printf "BIDI: base_stop=%d prev_stop=%d level=%d\n", $it->base_level_stop, $it->prev_stop, $it->bidi_it.resolved_level
+  end
   if ($it->region_beg_charpos >= 0)
     printf "reg=%d-%d ", $it->region_beg_charpos, $it->region_end_charpos
   end

--- a/src/ChangeLog.bidi	Sat Jan 02 05:57:50 2010 -0500
+++ b/src/ChangeLog.bidi	Sat Jan 02 10:57:35 2010 -0500
@@ -1,3 +1,18 @@
[phone]  Eli Zaretskii  <[e-mail]>
+
+	* .gdbinit (pitx): Display some bidi information about the
+	iterator.
+
+	* dispextern.h (BIDI_AT_BASE_LEVEL): Enclose definition in
+	parentheses.
+
+	* xdisp.c (handle_stop_backwards): Save and restore it->current
+	and it->position, instead of expecting the caller to do that.
+	(next_element_from_buffer): When moving across stop_charpos,
+	record it in prev_stop.  When IT_CHARPOS backs up, call
+	handle_stop_backwards only if above the base embedding level.
+	This solves the crash while displaying etc/HELLO in bidi mode.
+
 2009-12-26  Eli Zaretskii  <[e-mail]>
 
 	* xdisp.c (handle_stop_backwards): Call compute_stop_pos in the

--- a/src/dispextern.h	Sat Jan 02 05:57:50 2010 -0500
+++ b/src/dispextern.h	Sat Jan 02 10:57:35 2010 -0500
@@ -1812,7 +1812,7 @@
 /* Value is non-zero when the bidi iterator is at base paragraph
    embedding level.  */
 #define BIDI_AT_BASE_LEVEL(BIDI_IT) \
-  (BIDI_IT).resolved_level == (BIDI_IT).level_stack[0].level
+  ((BIDI_IT).resolved_level == (BIDI_IT).level_stack[0].level)
 
 
 /***********************************************************************
@@ -2013,10 +2013,11 @@
   EMACS_INT stop_charpos;
 
   /* Previous stop position, i.e. the last one before the current
-     buffer position.  */
+     iterator position in `current'.  */
   EMACS_INT prev_stop;
 
-  /* Last stop_pos at the current paragraph's embedding level.  */
+  /* Last stop position iterated across whose embedding level is equal
+     to the current paragraph's embedding level.  */
   EMACS_INT base_level_stop;
 
   /* Maximum string or buffer position + 1.  ZV when iterating over

--- a/src/xdisp.c	Sat Jan 02 05:57:50 2010 -0500
+++ b/src/xdisp.c	Sat Jan 02 10:57:35 2010 -0500
@@ -6550,19 +6550,18 @@
 
 /* Scan forward from CHARPOS in the current buffer, until we find a
    stop position > current IT's position.  Then handle the stop
-   position before that.
-
-   This is called when we are reordering bidirectional text.  The
-   caller should save and restore IT and in particular the bidi_p
-   flag, because this function modifies them.  */
+   position before that.  This is called when we bump into a stop
+   position while reordering bidirectional text.  */
 
 static void
 handle_stop_backwards (it, charpos)
      struct it *it;
      EMACS_INT charpos;
 {
+  EMACS_INT where_we_are = IT_CHARPOS (*it);
+  struct display_pos save_current = it->current;
+  struct text_pos save_position = it->position;
   struct text_pos pos1;
-  EMACS_INT where_we_are = IT_CHARPOS (*it);
   EMACS_INT next_stop;
 
   /* Scan in strict logical order.  */
@@ -6584,6 +6583,9 @@
   it->stop_charpos = it->prev_stop;
   handle_stop (it);
   it->stop_charpos = next_stop;
+  it->bidi_p = 1;
+  it->current = save_current;
+  it->position = save_position;
 }
 
 /* Load IT with the next display element from current_buffer.  Value
@@ -6685,38 +6687,39 @@
 	     them all now, in buffer's logical order, until we find
 	     and handle the last stop_charpos that precedes our
 	     current position.  */
-	  struct it save_it = *it;
-
 	  handle_stop_backwards (it, it->stop_charpos);
-	  it->bidi_p = 1;
-	  it->current = save_it.current;
-	  it->position = save_it.position;
 	  return GET_NEXT_DISPLAY_ELEMENT (it);
 	}
       else
 	{
-	  /* If we are at base paragraph embedding level, take note of
-	     the last stop position seen at this level.  */
-	  if (BIDI_AT_BASE_LEVEL (it->bidi_it))
-	    it->base_level_stop = it->stop_charpos;
+	  if (it->bidi_p)
+	    {
+	      /* Take note of the stop position we just moved across,
+		 for when we will move back across it.  */
+	      it->prev_stop = it->stop_charpos;
+	      /* If we are at base paragraph embedding level, take
+		 note of the last stop position seen at this
+		 level.  */
+	      if (BIDI_AT_BASE_LEVEL (it->bidi_it))
+		it->base_level_stop = it->stop_charpos;
+	    }
 	  handle_stop (it);
 	  return GET_NEXT_DISPLAY_ELEMENT (it);
 	}
     }
-  else if (it->bidi_p && IT_CHARPOS (*it) < it->prev_stop)
-    {
-      struct it save_it = *it;
-
+  else if (it->bidi_p
+	   /* We can sometimes back up for reasons that have nothing
+	      to do with bidi reordering.  E.g., compositions.  The
+	      code below is only needed when we are above the base
+	      embedding level, so test for that explicitly.  */
+	   && !BIDI_AT_BASE_LEVEL (it->bidi_it)
+	   && IT_CHARPOS (*it) < it->prev_stop)
+    {
       if (it->base_level_stop <= 0)
 	it->base_level_stop = 1;
       if (IT_CHARPOS (*it) < it->base_level_stop)
 	abort ();
-      if (BIDI_AT_BASE_LEVEL (it->bidi_it))
-	abort ();
       handle_stop_backwards (it, it->base_level_stop);
-      it->bidi_p = 1;
-      it->current = save_it.current;
-      it->position = save_it.position;
       return GET_NEXT_DISPLAY_ELEMENT (it);
     }
   else