I like it! The only thing missing is the EPM which works with the patch. I added this in set_font_spaces_magic.patch. Moreover I made your patch a bit simpler. I hope it's ok like that.

Order to apply:
rtl_spaces.patch
set_font_spaces_magic.patch
space_lang_change.patch

Stefan


Attachment: rtl_spaces.patch
Description: Binary data

Attachment: series
Description: Binary data

Attachment: set_font_space_lang_magic.patch
Description: Binary data

Attachment: space_lang_change_magic.patch
Description: Binary data



Stefan Schimanski wrote:
Guy in the other thread prefers some magic here that the first space is moved to the other side (so in fact the LyX 1.3 behavior. What do you think? We could probably implement such a magic in the insertChar handler.

Right, and so does Ran. Attached is a patch which tries to do this, along the lines of what I suggested in http://permalink.gmane.org/ gmane.editors.lyx.general/39211 .

The patch is to be applied over the first four of Stefan's patches (http://permalink.gmane.org/gmane.editors.lyx.devel/87019) and instead of rtl_spaces.patch. (I now see Stefan has already committed those, so if you just remove the patches you already applied, and then svn update, this patch can be applied immediately).

Actually, we should still probably use some kind of EPM mechanism to get rid of consecutive visual spaces; but that's a second layer, I think this patch should be applied ASAP if it's okay with everyone, even without the EPM. (Stefan --- the latest patch you sent for the EPM still doesn't seem to solve the problem we were seeing (abc[FED ] hij); it'll arise less frequently, though, with my suggested patch applied.)

Dov
Index: lyx-devel/src/Bidi.cpp
===================================================================
--- lyx-devel.orig/src/Bidi.cpp 2007-06-07 22:47:12.000000000 +0300
+++ lyx-devel/src/Bidi.cpp      2007-06-08 02:55:40.000000000 +0300
@@ -95,7 +95,14 @@
        pos_type const body_pos = par.beginOfBody();

        for (pos_type lpos = start_; lpos <= end_; ++lpos) {
-               bool is_space = par.isLineSeparator(lpos);
+               bool is_space = false;
+ // We do not handle spaces around an RTL segment in a special way anymore. + // Neither do we do so when generating the LaTeX, so setting is_space + // to false makes the view in the GUI consistent with the output of LaTeX
+               // later. The old setting was:
+               //bool is_space = par.isLineSeparator(lpos);
+ // FIXME: once we're sure that this is what we really want, we should just
+               // get rid of this variable...
                pos_type const pos =
                        (is_space && lpos + 1 <= end_ &&
                         !par.isLineSeparator(lpos + 1) &&
Index: lyx-devel/src/Text.cpp
===================================================================
--- lyx-devel.orig/src/Text.cpp 2007-06-07 22:43:02.000000000 +0300
+++ lyx-devel/src/Text.cpp      2007-06-08 02:58:47.000000000 +0300
@@ -713,7 +713,50 @@
                }
        }

-       // First check, if there will be two blanks together or a blank at
+ // In Bidi text, we want spaces to be treated in a special way: spaces
+       // which are between words in different languages should get the
+       // paragraph's language; otherwise, spaces should keep the language
+ // they were originally typed in. This is only in effect while typing; + // after the text is already typed in, the user can always go back and + // explicitly set the language of a space as desired. But 99.9% of the
+       // time, what we're doing here is what the user actually meant.
+       //
+ // The following cases are the ones in which the language of the space + // should be changed to match that of the containing paragraph. In the
+       // depictions, lowercase is LTR, uppercase is RTL, underscore (_)
+ // represents a space, pipe (|) represents the cursor position (so the + // character before it is the one just typed in). The different cases
+       // are depicted logically (not visually), from left to right:
+       //
+       // 1. A_a|
+       // 2. a_A|
+       //
+ // Theoretically, there are other situations that we should, perhaps, deal + // with (e.g.: a|_A, A|_a). In practice, though, there really isn't any
+       // point (to understand why, just try to create this situation...).
+
+       if ((cur.pos() >= 2) && (par.isLineSeparator(cur.pos() - 1))) {
+               Font pre_space_font  = getFont(buffer, par, cur.pos() - 2);
+               Font post_space_font = real_current_font; // not 
getFont(cur_pos)!
+                                                                               
                  // current character hasn't
+                                                                               
                  // been placed in the
+                                                                               
                  // buffer yet!
+               if (pre_space_font.isVisibleRightToLeft() !=
+                       post_space_font.isVisibleRightToLeft()) {
+                       // Set the space's language to match the language of the
+                       // adjacent character whose direction is the paragraph's
+                       // direction
+                       Language const * lang =
+                               (pre_space_font.isVisibleRightToLeft() ==
+                                par.isRightToLeftPar(buffer.params())) ?
+                               pre_space_font.language() : 
post_space_font.language();
+
+                       par.setFont(cur.pos() - 1,
+                                       getFont(buffer, par, cur.pos() - 
1).setLanguage(lang));
+               }
+       }
+       
+       // Next check, if there will be two blanks together or a blank at
        // the beginning of a paragraph.
        // I decided to handle blanks like normal characters, the main
        // difference are the special checks when calculating the row.fill

Attachment: PGP.sig
Description: Signierter Teil der Nachricht

Reply via email to