>>>>> "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;
 }
 

Reply via email to