Elazar Leibovich wrote:
The proposed patch affects RTL documents only, and fixes partially the
bug that made the cursor "jump" to the inset's end, when it was just
before the inset.
Good job tracking this down, Elazar!
I tried the patch out, and it does work for the outermost insets. Do you
understand *why* it works (and as part of that, why it doesn't work for
deeper insets)? I haven't looked at the patch or the surrounding code
very closely at all yet (and I may not have time to until at least the
weekend), but the fact that the fix is only partial seems to me to hint
that perhaps there's a better way to fix this, which would cover all
cases. You know --- curing the symptom instead of the root cause type
thing... I may be totally wrong, though... But it would help if we could
explain in words how the cursor position is being determined, and why
it's going wrong in these cases.
Just a few notes which I don't know if you're aware of, and which may
(or may not) help you in looking into this further:
*) I'm pretty sure that this problem did not exist in 1.3.X (I don't
know about 1.4.X) --- but you may want to look at this code there,
perhaps you can see what has changed since then...
*) Note that what you're trying to fix is problem (3) in bug 3551. See
the comments there, and especially look at bug 1965 and its fix, which I
suspect may be related to our problem. I'm not sure, though. If you've
already gone through cursorX(), I think you'll be able to judge that
better than me.
*) Is this at all related to the boundary (I think it probably is)?
*) how (if at all) does all this related to the way the cursor behaves
before plain LTR text embedded in RTL?
Also, a few important general comments:
*) Please svn update! r18129 was over 300 revisions ago! Since then
many things may have changed --- both in the source (meaning that it
becomes less and less likely that the patches will apply cleanly --- and
in fact, yours doesn't), and in the behavior (meaning you may be fixing
things which are already fixed, or which behave a little differently
than what you're trying to fix). Given the rate of commits in LyX, you
really should svn update quite regularly; and certainly before
submitting a patch. Also, you want to get the benefit of your patches
which have already made it into the trunk!
*) This is related: please make sure that when you submit a patch,
only differences which are relevant to the given patch are included.
The patch which you sent includes changes to Paragraph.cpp which were
meant to fix something else, and which we've already ended up fixing
differently; changes to Cursor.h and Cursor.cpp which may be changes
which should be made, but are not related to this specific issue --- so
if you think they should be fixed, please send them to the mailing list,
but separately from other patches.
Note that this is not *only* to avoid confusion (though that's
important, too) --- it's also to make sure that the patch's behavior is
not affected by pieces of code which are not related and are not part of
what everyone else is seeing...
*) A note regarding the last two points: when you svn update, and also
when you're playing around with different patches, you have to do it
carefully. You don't want an update to interfere with your fix, and you
want to easily separate the different patches from each other. One way
is to manually keep track of changes before updating, or before starting
to work on a different patch: svn diff into a file for saving (to make
sure you have a copy of your changes should anything go wrong with an
update, for example). Another tool, which mostly automates this process
(and which I've only discovered in the past few weeks, but which I
already don't know how one can program without), is called quilt. The
best place I've found for getting started with it is here:
http://www.coffeebreaks.org/blogs/wp-content/archives/talks/2005/quilt/quiltintro-s5.html
Keep up the good work!
Dov
------------------------------------------------------------------------
Index: Cursor.h
===================================================================
--- Cursor.h (revision 18129)
+++ Cursor.h (working copy)
@@ -143,7 +143,7 @@
/// get some interesting description of top position
void info(odocstream & os) const;
/// are we in math mode (2), text mode (1) or unsure (0)?
- int currentMode();
+ int currentMode() const;
/// reset cursor bottom to the beginning of the given inset
// (sort of 'chroot' environment...)
void reset(Inset &);
Index: Text.cpp
===================================================================
--- Text.cpp (revision 18129)
+++ Text.cpp (working copy)
@@ -1658,10 +1658,21 @@
// Make sure inside an inset we always count from the left
// edge (bidi!) -- MV
+ // lyxerr << "depth " << bv.cursor().depth() << endl;
if (sl.pos() < par.size()) {
font = getFont(buffer, par, sl.pos());
+ //This fixes the cursor position in insets in RTL
+ //Paragraphs. I added an assertion to make sure we
+ //are IN some kind of inset. (so far it was applied even
+ //if the cursor was right before a paragraph, since the
+ //position of the cursor in the paragraph is the same
+ //regardless of whether or not the cursor is in the inset or
not.
+ //FIXME: we need to be sure somehow that we're not in front of
an
+ //inset, this solution don't work in nested insets (mathed in a
+ //footnote.
if (!boundary && font.isVisibleRightToLeft()
- && par.isInset(sl.pos()))
+ && par.isInset(sl.pos())
+ && bv.cursor().depth()>1)
x -= par.getInset(sl.pos())->width();
}
return int(x);
Index: Paragraph.cpp
===================================================================
--- Paragraph.cpp (revision 18129)
+++ Paragraph.cpp (working copy)
@@ -1415,10 +1415,14 @@
Paragraph::getUChar(BufferParams const & bparams, pos_type pos) const
{
value_type c = getChar(pos);
- if (!lyxrc.rtl_support)
+ if (true || !lyxrc.rtl_support)
return c;
- value_type uc = c;
+ /* This was previously used to get the paranthesis correctly in
+ * RtL paragraphs. Now it seems to have no other effect than breaking
+ * paste in math insets when in RtL mode.
+ * To revert the change delete the "true ||" above.
+ * value_type uc = c;
switch (c) {
case '(':
uc = ')';
@@ -1448,7 +1452,7 @@
if (uc != c && getFontSettings(bparams, pos).isRightToLeft())
return uc;
else
- return c;
+ return c;*/
}
Index: Cursor.cpp
===================================================================
--- Cursor.cpp (revision 18129)
+++ Cursor.cpp (working copy)
@@ -382,7 +382,7 @@
}
-int Cursor::currentMode()
+int Cursor::currentMode() const
{
BOOST_ASSERT(!empty());
for (int i = depth() - 1; i >= 0; --i) {
