Dov Feldstern wrote:
Dov Feldstern wrote:
Abdelrazak Younes wrote:
One problem with the current patch --- it breaks the solutions we've
been working on for cursor movement --- exiting insets from the
beginning seems to be broken again --- either with or without my fix.
I'm sure it can be worked out, but it'll require a little more work
(however, I won't be able to get to this anymore until at least
tomorrow night).
The problem is with the current patch, and has nothing to do with the
bidi patches. Also in all-English paragraphs, there's a problem trying
to leave math insets from the left. It's probably some small edge-case
in here...
Fixed. Everything seems to be working fine now. I would still appreciate
it if someone could review the whole patch carefully, because this does
affect general code (not just RTL).
See my notes with the original post of the patch at
http://permalink.gmane.org/gmane.editors.lyx.devel/83401. The
differences since then are (a) I fixed the problem of leaving insets
from their beginning; (b) I went ahead and removed the conditions which
start with "if (false &&...)".
I would be very happy if this could go in *before* the beta-3 --- just
to give it a lot more testing. Either way, Abdel is definitely right:
RTL should *not* be turned on by default without this patch --- it can
crash non-bidi documents as well.
Thanks!
Dov
Index: src/Text2.cpp
===================================================================
--- src/Text2.cpp (revision 18264)
+++ src/Text2.cpp (working copy)
@@ -717,8 +717,12 @@
pos_type pos = cur.pos();
Paragraph & par = cur.paragraph();
- if (cur.boundary() && pos > 0)
+ if (cur.boundary() && pos > 0) {
--pos;
+ // We just moved to the previous row ---
+ // we're going to be needing it's bidi tables!
+ bidi.computeTables(par, cur.buffer(), cur.textRow());
+ }
if (pos > 0) {
if (pos == cur.lastpos())
@@ -904,9 +908,18 @@
return false;
if (cur.pos() == cur.lastpos())
return false;
- Inset * inset = cur.nextInset();
+ Inset * inset = front ? cur.nextInset() : cur.prevInset();
if (!isHighlyEditableInset(inset))
return false;
+ /*
+ * Apparently, when entering an inset we are expected to be positioned
+ * *before* it in the containing paragraph, regardless of the direction
+ * from which we are entering. Otherwise, cursor placement goes awry,
+ * and when we exit from the beginning, we'll be placed *after* the
+ * inset.
+ */
+ if (!front)
+ --cur.pos();
inset->edit(cur, front);
return true;
}
@@ -917,27 +930,28 @@
// Tell BufferView to test for FitCursor in any case!
cur.updateFlags(Update::FitCursor);
- if (!cur.boundary() && cur.pos() > 0 &&
- cur.textRow().pos() == cur.pos() &&
- !cur.paragraph().isLineSeparator(cur.pos()-1) &&
- !cur.paragraph().isNewline(cur.pos()-1)) {
- return setCursor(cur, cur.pit(), cur.pos(), true, true);
- }
- if (cur.pos() != 0) {
- bool updateNeeded = setCursor(cur, cur.pit(), cur.pos() - 1,
true, false);
+ if (cur.pos() > 0) {
+ if (cur.boundary())
+ return setCursor(cur, cur.pit(), cur.pos(), true,
false);
+
+ bool updateNeeded = false;
+ // If checkAndActivateInset returns true, that means that
+ // the cursor was placed inside it, so we're done
if (!checkAndActivateInset(cur, false)) {
- /** FIXME: What's this cause purpose???
- bool boundary = cur.boundary();
- if (false && !boundary &&
- bidi.isBoundary(cur.buffer(), cur.paragraph(),
cur.pos() + 1))
- updateNeeded |=
- setCursor(cur, cur.pit(), cur.pos() +
1, true, true);
- */
+ if (!cur.boundary() &&
+ cur.textRow().pos() == cur.pos() /*&&
+ !cur.paragraph().isLineSeparator(cur.pos()-1) &&
+ !cur.paragraph().isNewline(cur.pos() - 1)*/) {
+ updateNeeded |= setCursor(cur, cur.pit(),
cur.pos(),
+
true, true);
+ }
+ updateNeeded |= setCursor(cur, cur.pit(),cur.pos() - 1,
+ true,
false);
}
return updateNeeded;
}
- if (cur.pit() != 0) {
+ if (cur.pit() > 0) {
// Steps into the paragraph above
return setCursor(cur, cur.pit() - 1, getPar(cur.pit() -
1).size());
}
@@ -956,6 +970,8 @@
true, false);
bool updateNeeded = false;
+ // If checkAndActivateInset returns true, that means that
+ // the cursor was placed inside it, so we're done
if (!checkAndActivateInset(cur, true)) {
if (cur.textRow().endpos() == cur.pos() + 1 &&
cur.textRow().endpos() != cur.lastpos() &&
@@ -964,9 +980,6 @@
cur.boundary(true);
}
updateNeeded |= setCursor(cur, cur.pit(), cur.pos() +
1, true, cur.boundary());
- if (false && bidi.isBoundary(cur.buffer(),
cur.paragraph(),
- cur.pos()))
- updateNeeded |= setCursor(cur, cur.pit(),
cur.pos(), true, true);
}
return updateNeeded;
}
