Stefan Schimanski wrote:
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.
I incorporated some of the changes you proposed. But the main change you
made I'd rather not use: I don't want to set the entire font of the
space; only it's language (think underbar, size, etc. --- some of these
things may matter even though it's only a space).
Attached is the patch after the corrections you suggested, except for
that one.
Regarding the other stuff --- now the EPM is removing *too* much. Try
typing: 'abc' <space> F12 DEF <space> F12 -- at this point, the trailing
space already disappeared; which is perhaps correct from the EPM's point
of view, but not from the users... As soon as the user would type
something in after the space, then the space_magic would take care of
that space anyway, so the EPM shouldn't be doing it. There may not be
any way around this --- it may be that to fix what we were complaining
about with the EPM, you have to do it this way, I don't know. But the
current behavior is not, IMO, what the user wants.
I suggest keeping the attached patch at the *bottom* of the stack now
--- I think we all more or less agree with it, and as soon as we get the
OKs, it can be committed (I'd still like to hear from Ran and Guy and
any other Bidi users about it, though). So the attached is to be applied
to a clean svn tree.
On top of those, we can continue experimenting with the EPM --- though
again, I'm not sure anymore that it's really necessary, and I also agree
somewhat with the issues that Andre' raised...
Dov
Index: src/Text.cpp
===================================================================
--- src/Text.cpp (revision 18709)
+++ src/Text.cpp (working copy)
@@ -5,6 +5,7 @@
*
* \author Asger Alstrup
* \author Lars Gullik Bjønnes
+ * \author Dov Feldstern
* \author Jean-Marc Lasgouttes
* \author John Levon
* \author André Pönitz
@@ -713,7 +714,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 const & pre_space_font = getFont(buffer, par, cur.pos() -
2);
+ Font const & post_space_font = real_current_font; // not
+ // getFont(cur_pos)! current character isn't in the
buffer yet!
+ bool pre_space_rtl = pre_space_font.isVisibleRightToLeft();
+ bool post_space_rtl = post_space_font.isVisibleRightToLeft();
+
+ if (pre_space_rtl != post_space_rtl) {
+ // Set the space's language to match the language of
the
+ // adjacent character whose direction is the paragraph's
+ // direction; don't touch other properties of the font
+ Language const * lang =
+ (pre_space_rtl ==
par.isRightToLeftPar(buffer.params())) ?
+ pre_space_font.language() :
post_space_font.language();
+
+ Font space_font = getFont(buffer, par, cur.pos() - 1);
+ space_font.setLanguage(lang);
+ par.setFont(cur.pos() - 1, space_font);
+ }
+ }
+
+ // 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
Index: src/Bidi.cpp
===================================================================
--- src/Bidi.cpp (revision 18709)
+++ src/Bidi.cpp (working copy)
@@ -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) &&
