Dov Feldstern wrote:
Enrico Forestieri wrote:
On Thu, Oct 30, 2008 at 07:51:41AM -0400, rgheck wrote:
http://bugzilla.lyx.org/show_bug.cgi?id=5061
Can someone who knows about InsetMathNest review the patch Vincent
attached to this bug? It looks like it's probably correct, but I
don't know about this stuff.
Me neither, but looking at cs22930 this seems to be an oversight
by Dov. Indeed, I don't think that he meant replacing
FuncRequest(LFUN_FINISHED_FORWARD);
by
cmd = FuncRequest(finish_lfun);
where finish_lfun is either LFUN_FINISHED_RIGHT or LFUN_FINISHED_LEFT.
So, I think that the patch does the right thing.
No, I don't think it was an oversight, Vincent's patch breaks visual
cursor movements in math.
There may still be a bug in my original code, though, I'm looking this
over now with respect to 5061. I'll let you know when I have something...
Dov
Okay, there are actually two separate issues here:
(1) boundary is incorrectly set to true in this case (my bug, correctly
diagnosed, but incorrectly fixed, by Vincent); The attached patch fixes it. If
nobody has objections to it, I'll check it in tomorrow evening.
This is good enough to fix #5061 in the normal case.
(2) However, #5061 still exists when boundary is set to true for a correct
reason; for example, try this recipe:
- create a new file
- insert an equation
- press the right arrow key to jump out of the equation
- press Ctrl-Return
- switch the language to an RTL language (e.g., Hebrew)
Result: The cursor jumps back up to the same line as the newline, and must type
some character to get correctly rendered on the new line.
I think the correct solution is that even when boundary==true, the cursor should
*not* be painted after the previous character, if the previous character is
Ctrl-Enter or Ctrl-Shift-Enter. I can't think of *any* situation where we would
legitimately want to paint the cursor *after* such a character. Opinions?
Dov
BTW, Enrico --- thanks for checking svn blame (or are you using git?) for this!
--- I don't think I would have noticed this issue otherwise...
diff -r dc325cbabe98 src/Cursor.cpp
--- a/src/Cursor.cpp Fri Oct 31 01:26:11 2008 +0000
+++ b/src/Cursor.cpp Fri Oct 31 13:14:48 2008 +0200
@@ -507,10 +507,11 @@
new_cur.pos() = right_pos + 1;
// set the boundary to true in two situations:
if (
- // 1. if new_pos is now lastpos (which means that we're moving
- // right to the end of an LTR chunk which is at the end of an
- // RTL paragraph);
- new_cur.pos() == lastpos()
+ // 1. if new_pos is now lastpos, and we're in an RTL paragraph
+ // (this means that we're moving right to the end of an LTR chunk
+ // which is at the end of an RTL paragraph);
+ (new_cur.pos() == lastpos()
+ && paragraph().isRTL(buffer().params()))
// 2. if the position *after* right_pos is RTL (we want to be
// *after* right_pos, not before right_pos + 1!)
|| paragraph().getFontSettings(bv().buffer().params(),
@@ -598,10 +599,11 @@
new_cur.pos() = left_pos + 1;
// set the boundary to true in two situations:
if (
- // 1. if new_pos is now lastpos (which means that we're moving left
- // to the end of an RTL chunk which is at the end of an LTR
- // paragraph);
- new_cur.pos() == lastpos()
+ // 1. if new_pos is now lastpos and we're in an LTR paragraph
+ // (this means that we're moving left to the end of an RTL chunk
+ // which is at the end of an LTR paragraph);
+ (new_cur.pos() == lastpos()
+ && !paragraph().isRTL(buffer().params()))
// 2. if the position *after* left_pos is not RTL (we want to be
// *after* left_pos, not before left_pos + 1!)
|| !paragraph().getFontSettings(bv().buffer().params(),
