>>>>> "Georg" == Georg Baum <[EMAIL PROTECTED]> writes:
Georg> Can anybody give me a hint please what goes wrong here: Georg> http://bugzilla.lyx.org/show_bug.cgi?id=2587 ? I guess this is Georg> DEPM playing tricks, but unfortunately I don't have any clue Georg> how it works. This bug is really annyoing if you work with Georg> keepempty layouts. I looked at this and there are several problems. I am not sure how to handle them Let's look at LyXText::erase (text.C) first. Basically, it move to next position (which may be next paragraph if at end) and invokes backspace(). There is an extra test on layouts that should be removed since backspace dies the test. So this part should be removed. There are also extra recordUndo calls that look suspicious. This is not too difficult to fix. So, it can be changed to be a very simple wrapper around backspace. This is what the patch cleanuperase.diff does. With this patch , erase looks like: bool LyXText::erase(LCursor & cur) { BOOST_ASSERT(this == cur.text()); if (cur.pos() != cur.lastpos()) { // move right, avoiding dEPM setCursorIntern(cur, cur.pit(), cur.pos() + 1, false, cur.boundary()); } else if (cur.pit() != cur.lastpit()) { // move to next paragraph, avoiding dEPM setCursorIntern(cur, cur.pit() + 1, 0, false, false); } else { // nothing to do return false; } // Delegate the real work to backspace. bool needsUpdate = backspace(cur); // if in CT mode, go one step to the right if (cur.buffer().params().tracking_changes && cur.paragraph().lookupChange(cur.pos()) == Change::DELETED) cur.posRight(); return needsUpdate; } I think it is very simple and correct. Comments? Did I get the CT part correctly? However, while we have removed an extra test for different layout, we are not done since the real problem is in backspacePos0 :( [since we are at the beginning of a paragraph, this is the function that triggers] A simple case, without any KeepEmpty paragraph, looks like [section] foo foo foo [subsection] |<------------ cursor is here [subsubsection] bar bar bar We have 3 different layouts and one of the paragraphs is empty. So how does backspace work? 1/ if paragraph is empty, it tries to just move the cursor and hope dEPM will trigger and remove the paragraph. This is nasty and does not work with KeepEmpty paragraphs. This is what you see if you are in second paragraph and hit backspace. Note that it does not work if the default layout is KeepEmpty (like in this bug). So, the example of the bug, backspace does not work on second paragraph, while it works in second paragraph of example above. This code using dEPM should be replaced by a plain removal of the paragraph where cursor is. Question is: how do I do that in a way that is correct wrt CT (Michael? Martin?) 2/ if paragraph is not empty (which is the case if you pressed delete while in paragraph 2, because cursor is in paragraph 3 and you do delete (lost yet?)), then it tests whether the two paragraphs have same layout and does a mergeParagraph. This does not work because the layouts are different. Even if we tested for this case, the result would be to give to paragraph 3 the layout subsection (this is how mergeParagraph work). So I need to know what is the clean code to erase a single paragraph, do the right undo stuff (what is Undo::DELETE exactly?) and do the right CT stuff. Could someone give me pointers here? Should I use eraseSelectionHelper, so that all the logic is in the same place? Or is there another function that is better adapted? Please help, I am lost. JMarc
Index: text.C =================================================================== --- text.C (revision 13835) +++ text.C (working copy) @@ -1580,31 +1580,26 @@ void LyXText::changeCase(LCursor & cur, bool LyXText::erase(LCursor & cur) { BOOST_ASSERT(this == cur.text()); - bool needsUpdate = false; if (cur.pos() != cur.lastpos()) { - recordUndo(cur, Undo::DELETE, cur.pit()); + // move right, avoiding dEPM setCursorIntern(cur, cur.pit(), cur.pos() + 1, false, cur.boundary()); - needsUpdate = backspace(cur); - if (cur.paragraph().lookupChange(cur.pos()) == Change::DELETED) - cur.posRight(); } else if (cur.pit() != cur.lastpit()) { - LCursor scur = cur; - + // move to next paragraph, avoiding dEPM setCursorIntern(cur, cur.pit() + 1, 0, false, false); - if (pars_[cur.pit()].layout() == pars_[scur.pit()].layout()) { - recordUndo(scur, Undo::DELETE, scur.pit()); - needsUpdate = backspace(cur); - if (cur.buffer().params().tracking_changes) { - // move forward after the paragraph break is DELETED - Paragraph & par = cur.paragraph(); - if (par.lookupChange(par.size()) == Change::DELETED) - setCursorIntern(cur, cur.pit() + 1, 0); - } - } else { - setCursorIntern(scur, scur.pit(), scur.pos(), false, scur.boundary()); - } + } else { + // nothing to do + return false; } + + // Delegate the real work to backspace. + bool needsUpdate = backspace(cur); + + // if in CT mode, go one step to the right + if (cur.buffer().params().tracking_changes && + cur.paragraph().lookupChange(cur.pos()) == Change::DELETED) + cur.posRight(); + return needsUpdate; }