Please, could someone respond regarding this patch from Elazar?
It fixes issue (3) of http://bugzilla.lyx.org/show_bug.cgi?id=3551,
which is a regression.
It works.
The concept makes sense.
The implementation -- well, we don't love it, but we haven't gotten any
better suggestions. (The question is: how to identify the difference
between being "at an inset" vs. "inside it".)
It's safe --- at the very worst, it will only affect bidi users.
So please, can we have it okayed and committed?
I'm attaching the patch again, against latest svn.
Thanks!
Dov Feldstern wrote:
Dov Feldstern wrote:
Jean-Marc Lasgouttes wrote:
"Dov" == Dov Feldstern
<[EMAIL PROTECTED]> writes:
Dov> So, if we move Martin's test out of cursorX, we're going to have
Dov> to add it in 4 or 5 new places.
Very good argument. Thanks for checking.
Dov> Finally, (a) Martin's test has been in for a while (since r10513,
Dov> 10/2005), I don't know why it's only now that we're questioning
Dov> whether this really belongs here or not;
Because it is only now that we have RtL-knowledgeable people looking
at this code. Martin at the time did the best he could to fix problems
reported by users (I think). Butthe RtL code in LyX has been rotting
for a long time.
JMarc
So where does this leave us with regard to the proposed patch...?
So, can this patch go in? Again, to recap: the concept is IMO correct;
it works; we don't love the way that we do it, but haven't gotten any
better suggestions...
Index: src/Text.cpp
===================================================================
--- src/Text.cpp (revision 18579)
+++ src/Text.cpp (working copy)
@@ -1696,8 +1696,25 @@
// edge (bidi!) -- MV
if (sl.pos() < par.size()) {
font = getFont(buffer, par, sl.pos());
+ // The cursor position inside an inset is expected to be
+ // relative to the inset's left edge. In RTL paragraphs,
+ // however, we currently have the position relative to the
+ // inset's *right* edge. We must explicitly correct the x
+ // position, by removing the width of the inset itself.
+ //
+ // Naturally, this correction should only take place *inside*
+ // the inset. To test whether we are actually inside the inset
+ // (and not only "at" the inset; i.e., at the position at
+ // which the inset is located in the surrounding paragraph,
+ // but not yet inside the inset), we compare the cursor slice
+ // stored at the cursor, to the cursor slice provided as a
+ // parameter. If they are equal, that means we're not in the
+ // inset which is at character sl.pos() in the pargraph, but
+ // we're right before it. Otherwise, it means that the inset
+ // was pushed onto the cursor's stack, and we must be inside.
if (!boundary && font.isVisibleRightToLeft()
- && par.isInset(sl.pos()))
+ && par.isInset(sl.pos())
+ && (&bv.cursor().top())!=&sl)
x -= par.getInset(sl.pos())->width();
}
return int(x);
