Re: [yet another patch] Cursor movement fine-tuning
On Thu, May 31, 2007 at 09:50:03AM +0200, Stefan Schimanski wrote: > > Am 31.05.2007 um 09:43 schrieb Jean-Marc Lasgouttes: > > >>"Stefan" == Stefan Schimanski <[EMAIL PROTECTED]> writes: > > > >Stefan> This is fine, mostly. I don't like 7. There should be a > >Stefan> position behind the c, because if you type with the cursor in > >Stefan> front of the $$1$ $ the characters appear behind c. In fact > >Stefan> the position in front of $$1 $$ shouldn't exist because typing > >Stefan> there makes no sense. The same is true for the position behind > >Stefan> $$1$$ (see 8). > > > >A whole part of this boundary code has been implemented to catter with > >this particular case. > > What do you mean? Those positions are supposed to exist or not? They do not exist in the buffer structure, but they exist on screen. That's the very reason for the existance of the boundary flag. Andre'
Re: [yet another patch] Cursor movement fine-tuning
On Thu, May 31, 2007 at 09:25:15AM +0200, Stefan Schimanski wrote: > So, removing the whole boundary business, we get this behavious: > > 1) abc| \ndef =right=> abc \n|def > 2) ab|c\ndef =right=> abc\n|def =right=> abc\nd|ef > 3) abc \nd|ef =left=> abc \n|def =left=> abc| \ndef > 4) abc\nd|ef =left=> abc\ndef =left=> ab|c\ndef > 5) abc|\ndef =right=> abc\n|def > 6) abcd|ef =left=> abc|def =left=> abc|def > 7) ab|c\n$$1$$\ndef =right=> abc\n|$$1$$\ndef =right=> abc\n$$|1$$\ndef > 8) abc\n$$|1$$\ndef =right=> abc\n$$1|$$\ndef =right=> abc\n$$1$$|\ndef > 9) abc\n$$1$$|\ndef =right=> abc\n$$1$$\n|def > > This is fine, mostly. I don't like 7. There should be a position > behind the c, because if you type with the cursor in front of the $$1$ > $ the characters appear behind c. In fact the position in front of $$1 > $$ shouldn't exist because typing there makes no sense. The same is > true for the position behind $$1$$ (see 8). > > All these cases apply to cursor left/right. If you use the mouse or > cursor up/down, what should happen? If you click near the end of a line, the cursor should be positioned there. If you click near the beginning, it goes there. Going up/down from the beginning of a line should keep the cursor neear the beginning of the line above/below. Same for the end. Andre'
Re: [yet another patch] Cursor movement fine-tuning
Stefan Schimanski schrieb: Am 31.05.2007 um 10:56 schrieb Michael Gerz: Abdelrazak Younes schrieb: Isn't this related to change-tracking? Change tracking adds meta information to a virtual (i.e. non-existing) end-of-par character at the end of each paragraph. It does not care for cursor stuff. (I haven't follow the thread but I hope that you did not kill any CT-related code. (The cursor movement code should be completely CT-free)) How does this meta information look like? Is it a character? Where is it exactly? After endpos() ? Must it be skipped somehwere? The Paragraph class has an internal CT structure that is updated whenever we insert or delete a character. For end-of-par, the structure also covers position "par.size()" which does not exist. Example in readParagraph(...): // Final change goes to paragraph break: par.setChange(par.size(), change); HTH, Michael
Re: [yet another patch] Cursor movement fine-tuning
Am 31.05.2007 um 10:56 schrieb Michael Gerz: Abdelrazak Younes schrieb: Isn't this related to change-tracking? Change tracking adds meta information to a virtual (i.e. non- existing) end-of-par character at the end of each paragraph. It does not care for cursor stuff. (I haven't follow the thread but I hope that you did not kill any CT-related code. (The cursor movement code should be completely CT- free)) How does this meta information look like? Is it a character? Where is it exactly? After endpos() ? Must it be skipped somehwere? Stefan PGP.sig Description: Signierter Teil der Nachricht
Re: [yet another patch] Cursor movement fine-tuning
Michael Gerz wrote: Abdelrazak Younes schrieb: Isn't this related to change-tracking? Change tracking adds meta information to a virtual (i.e. non-existing) end-of-par character at the end of each paragraph. Ah yes I remembered something about a virtual end-of-par. It does not care for cursor stuff. OK, thanks for the explanation. Abdel.
Re: [yet another patch] Cursor movement fine-tuning
Abdelrazak Younes schrieb: Isn't this related to change-tracking? Change tracking adds meta information to a virtual (i.e. non-existing) end-of-par character at the end of each paragraph. It does not care for cursor stuff. (I haven't follow the thread but I hope that you did not kill any CT-related code. (The cursor movement code should be completely CT-free)) Michael
Re: [yet another patch] Cursor movement fine-tuning
> "Abdelrazak" == Abdelrazak Younes <[EMAIL PROTECTED]> writes: >> Good question. In 1.4.x, the two positions exist. I am not sure >> why the position in front of the display inset is deemed useful. Abdelrazak> Isn't this related to change-tracking? I think it is something else, but what? JMarc
Re: [yet another patch] Cursor movement fine-tuning
Jean-Marc Lasgouttes wrote: "Stefan" == Stefan Schimanski <[EMAIL PROTECTED]> writes: Stefan> Am 31.05.2007 um 09:43 schrieb Jean-Marc Lasgouttes: "Stefan" == Stefan Schimanski <[EMAIL PROTECTED]> writes: Stefan> This is fine, mostly. I don't like 7. There should be a Stefan> position behind the c, because if you type with the cursor in Stefan> front of the $$1$ $ the characters appear behind c. In fact Stefan> the position in front of $$1 $$ shouldn't exist because typing Stefan> there makes no sense. The same is true for the position behind Stefan> $$1$$ (see 8). A whole part of this boundary code has been implemented to catter with this particular case. Stefan> What do you mean? Those positions are supposed to exist or Stefan> not? Good question. In 1.4.x, the two positions exist. I am not sure why the position in front of the display inset is deemed useful. Isn't this related to change-tracking? Abdel.
Re: [yet another patch] Cursor movement fine-tuning
Am 31.05.2007 um 10:13 schrieb Jean-Marc Lasgouttes: "Stefan" == Stefan Schimanski <[EMAIL PROTECTED]> writes: Stefan> The only case I can imagine is while selecting an inset like Stefan> display math. It might seem more intuitive if you can select Stefan> just the line of a display math. But the visual effect will remain the same anyway. Nearly. The margin is overdrawn with the selection color as well if the in-front-of-display-math position does not exist. Stefan PGP.sig Description: Signierter Teil der Nachricht
Re: [yet another patch] Cursor movement fine-tuning
> "Stefan" == Stefan Schimanski <[EMAIL PROTECTED]> writes: Stefan> The only case I can imagine is while selecting an inset like Stefan> display math. It might seem more intuitive if you can select Stefan> just the line of a display math. But the visual effect will remain the same anyway. I have the feeling that there was a reason, but I cannot remember which one. JMarc
Re: [yet another patch] Cursor movement fine-tuning
Am 31.05.2007 um 09:57 schrieb Jean-Marc Lasgouttes: "Stefan" == Stefan Schimanski <[EMAIL PROTECTED]> writes: Stefan> Am 31.05.2007 um 09:43 schrieb Jean-Marc Lasgouttes: "Stefan" == Stefan Schimanski <[EMAIL PROTECTED]> writes: Stefan> This is fine, mostly. I don't like 7. There should be a Stefan> position behind the c, because if you type with the cursor in Stefan> front of the $$1$ $ the characters appear behind c. In fact Stefan> the position in front of $$1 $$ shouldn't exist because typing Stefan> there makes no sense. The same is true for the position behind Stefan> $$1$$ (see 8). A whole part of this boundary code has been implemented to catter with this particular case. Stefan> What do you mean? Those positions are supposed to exist or Stefan> not? Good question. In 1.4.x, the two positions exist. I am not sure why the position in front of the display inset is deemed useful. The only case I can imagine is while selecting an inset like display math. It might seem more intuitive if you can select just the line of a display math. Stefan PGP.sig Description: Signierter Teil der Nachricht
Re: [yet another patch] Cursor movement fine-tuning
> "Stefan" == Stefan Schimanski <[EMAIL PROTECTED]> writes: Stefan> Am 31.05.2007 um 09:43 schrieb Jean-Marc Lasgouttes: >>> "Stefan" == Stefan Schimanski <[EMAIL PROTECTED]> writes: >> Stefan> This is fine, mostly. I don't like 7. There should be a Stefan> position behind the c, because if you type with the cursor in Stefan> front of the $$1$ $ the characters appear behind c. In fact Stefan> the position in front of $$1 $$ shouldn't exist because typing Stefan> there makes no sense. The same is true for the position behind Stefan> $$1$$ (see 8). >> A whole part of this boundary code has been implemented to catter >> with this particular case. Stefan> What do you mean? Those positions are supposed to exist or Stefan> not? Good question. In 1.4.x, the two positions exist. I am not sure why the position in front of the display inset is deemed useful. JMarc
Re: [yet another patch] Cursor movement fine-tuning
Am 31.05.2007 um 09:43 schrieb Jean-Marc Lasgouttes: "Stefan" == Stefan Schimanski <[EMAIL PROTECTED]> writes: Stefan> This is fine, mostly. I don't like 7. There should be a Stefan> position behind the c, because if you type with the cursor in Stefan> front of the $$1$ $ the characters appear behind c. In fact Stefan> the position in front of $$1 $$ shouldn't exist because typing Stefan> there makes no sense. The same is true for the position behind Stefan> $$1$$ (see 8). A whole part of this boundary code has been implemented to catter with this particular case. What do you mean? Those positions are supposed to exist or not? Stefan PGP.sig Description: Signierter Teil der Nachricht
Re: [yet another patch] Cursor movement fine-tuning
> "Stefan" == Stefan Schimanski <[EMAIL PROTECTED]> writes: Stefan> This is fine, mostly. I don't like 7. There should be a Stefan> position behind the c, because if you type with the cursor in Stefan> front of the $$1$ $ the characters appear behind c. In fact Stefan> the position in front of $$1 $$ shouldn't exist because typing Stefan> there makes no sense. The same is true for the position behind Stefan> $$1$$ (see 8). A whole part of this boundary code has been implemented to catter with this particular case. JMarc
Re: [yet another patch] Cursor movement fine-tuning
So, removing the whole boundary business, we get this behavious: 1) abc| \ndef =right=> abc \n|def 2) ab|c\ndef =right=> abc\n|def =right=> abc\nd|ef 3) abc \nd|ef =left=> abc \n|def =left=> abc| \ndef 4) abc\nd|ef =left=> abc\ndef =left=> ab|c\ndef 5) abc|\ndef =right=> abc\n|def 6) abcd|ef =left=> abc|def =left=> abc|def 7) ab|c\n$$1$$\ndef =right=> abc\n|$$1$$\ndef =right=> abc\n$$|1$$\ndef 8) abc\n$$|1$$\ndef =right=> abc\n$$1|$$\ndef =right=> abc\n$$1$$|\ndef 9) abc\n$$1$$|\ndef =right=> abc\n$$1$$\n|def This is fine, mostly. I don't like 7. There should be a position behind the c, because if you type with the cursor in front of the $$1$ $ the characters appear behind c. In fact the position in front of $$1 $$ shouldn't exist because typing there makes no sense. The same is true for the position behind $$1$$ (see 8). All these cases apply to cursor left/right. If you use the mouse or cursor up/down, what should happen? Stefan PGP.sig Description: Signierter Teil der Nachricht
Re: [yet another patch] Cursor movement fine-tuning
On Thu, May 31, 2007 at 02:29:38AM +0300, Dov Feldstern wrote: > At least cursorLeft and cursorRight are much simpler now... I have no idea whether the patch is sound, but I certainly like the structure... Andre'
[yet another patch] Cursor movement fine-tuning
[This should be applied after the patch in http://permalink.gmane.org/gmane.editors.lyx.devel/86074, which fixes bug 3754.] Okay, you guys (Stefan and Andre') are correct, as always ;) . We really don't need the boundary almost anywhere. The comment on boundary_ in DocIterator.h is (almost) right: the only place we're using it at the moment is for Bidi. It should still be corrected in terms of (i,i+1)->(i-1,i), and also I think an overview of how it's used in cursor drawing is important for understanding how it's to be used, so that should stay there (if you want, tomorrow night I'll send in a suggestion for patching the comment). At least cursorLeft and cursorRight are much simpler now... I wonder if this'll speed them up a little, too... One thing that's still bugging is why Left and Right aren't symmetrical. For some reason we still need the condition for Bidi in right, but not in left. I suspect maybe it has to do with the choice of boundary being between (i-1,i), which is in a sense a symmetry break? Anyhow, please test this against all the cursor-related fixes from the past few days, but I think this'll work, and it's much simpler (and it gets rid of the debug variables, too...) Good night! Dov Index: lyx-devel/src/Text2.cpp === --- lyx-devel.orig/src/Text2.cpp2007-05-31 02:14:58.0 +0300 +++ lyx-devel/src/Text2.cpp 2007-05-31 02:17:39.0 +0300 @@ -988,26 +988,6 @@ // not at paragraph start? if (cur.pos() > 0) { - // if on right side of boundary (i.e. not at paragraph end, but line end) - // -> skip it, i.e. set boundary to true, i.e. go only logically left - // there are some exceptions to ignore this: lineseps, newlines, spaces -#if 0 - // some effectless debug code to see the values in the debugger - bool bound = cur.boundary(); - int rowpos = cur.textRow().pos(); - int pos = cur.pos(); - bool sep = cur.paragraph().isSeparator(cur.pos() - 1); - bool newline = cur.paragraph().isNewline(cur.pos() - 1); - bool linesep = cur.paragraph().isLineSeparator(cur.pos() - 1); -#endif - if (!cur.boundary() && - cur.textRow().pos() == cur.pos() && - !cur.paragraph().isLineSeparator(cur.pos() - 1) && - !cur.paragraph().isNewline(cur.pos() - 1) && - !cur.paragraph().isSeparator(cur.pos() - 1)) { - return setCursor(cur, cur.pit(), cur.pos(), true, true); - } - // go left and try to enter inset if (checkAndActivateInset(cur, false)) return false; @@ -1041,30 +1021,6 @@ if (checkAndActivateInset(cur, true)) return false; - // next position is left of boundary, - // but go to next line for special cases like space, newline, linesep -#if 0 - // some effectless debug code to see the values in the debugger - int endpos = cur.textRow().endpos(); - int lastpos = cur.lastpos(); - int pos = cur.pos(); - bool linesep = cur.paragraph().isLineSeparator(cur.pos()); - bool newline = cur.paragraph().isNewline(cur.pos()); - bool sep = cur.paragraph().isSeparator(cur.pos()); - if (cur.pos() != cur.lastpos()) { - bool linesep2 = cur.paragraph().isLineSeparator(cur.pos()+1); - bool newline2 = cur.paragraph().isNewline(cur.pos()+1); - bool sep2 = cur.paragraph().isSeparator(cur.pos()+1); - } -#endif - if (cur.textRow().endpos() == cur.pos() + 1 && - cur.textRow().endpos() != cur.lastpos() && - !cur.paragraph().isNewline(cur.pos()) && - !cur.paragraph().isLineSeparator(cur.pos()) && - !cur.paragraph().isSeparator(cur.pos())) { - return setCursor(cur, cur.pit(), cur.pos() + 1, true, true); - } - // in front of RTL boundary? Stay on this side of the boundary because: // ab|cDDEEFFghi -> abc|DDEEFFghi if (bidi.isBoundary(cur.buffer(), cur.paragraph(), cur.pos() + 1)) {