[EMAIL PROTECTED] wrote:
Author: rgheck
Date: Sat Aug  9 18:56:30 2008
New Revision: 26112

URL: http://www.lyx.org/trac/changeset/26112
Log:
Fix crash noticed by Vincent. We don't know that insetText is non-null here!

Can someone check this stuff more closely? Something looks a bit fishy here. Viz:

Modified:
    lyx-devel/trunk/src/Text3.cpp

Modified: lyx-devel/trunk/src/Text3.cpp
URL: http://www.lyx.org/trac/file/lyx-devel/trunk/src/Text3.cpp?rev=26112
==============================================================================
--- lyx-devel/trunk/src/Text3.cpp (original)
+++ lyx-devel/trunk/src/Text3.cpp Sat Aug  9 18:56:30 2008
@@ -225,7 +225,7 @@
        cur.clearSelection(); // bug 393
        cur.finishUndo();
        InsetText * insetText = dynamic_cast<InsetText *>(inset);
-       if (insetText && !insetText->allowMultiPar() || cur.lastpit() == 0) {
+       if (insetText && (!insetText->allowMultiPar() || cur.lastpit() == 0)) {
                // reset first par to default
                cur.text()->paragraphs().begin()
                        ->setPlainOrDefaultLayout(bparams.documentClass());
@@ -238,9 +238,10 @@
        } else {
On this branch, InsetText may be null, and it may be non-null, so...
                cur.leaveInset(*inset);
is this harmless if we're not in an InsetText? Or should we not do it at all? And the same goes for what follows:
                // reset surrounding par to default
-               docstring const layoutname = insetText->usePlainLayout()
-                       ? bparams.documentClass().plainLayoutName()
-                       : bparams.documentClass().defaultLayoutName();
+               DocumentClass const & dc = bparams.documentClass();
+               docstring const layoutname = inset->usePlainLayout()
+                       ? dc.plainLayoutName()
+                       : dc.defaultLayoutName();
                text->setLayout(cur, layoutname);
        }
If it's not a text inset, then usePlainLayout() will always return false, since that doesn't really make sense for non-text insets. And actually, I don't really understand why we do this at all. Try this: Create a file with several lines, including a few lines in a row of quotation. Now select a few quotation lines, but not all of one of them, and Insert>Footnote. The part of the line you didn't select gets reset to standard. What's the point of that?

So the question is whether the attached patch is really what we want.

rh

Index: Text3.cpp
===================================================================
--- Text3.cpp	(revision 26112)
+++ Text3.cpp	(working copy)
@@ -224,8 +224,12 @@
 	cur.buffer().errors("Paste");
 	cur.clearSelection(); // bug 393
 	cur.finishUndo();
+
 	InsetText * insetText = dynamic_cast<InsetText *>(inset);
-	if (insetText && (!insetText->allowMultiPar() || cur.lastpit() == 0)) {
+	if (!insetText)
+		 return true;
+
+	if (!insetText->allowMultiPar() || cur.lastpit() == 0) {
 		// reset first par to default
 		cur.text()->paragraphs().begin()
 			->setPlainOrDefaultLayout(bparams.documentClass());
@@ -234,17 +238,8 @@
 		// Merge multiple paragraphs -- hack
 		while (cur.lastpit() > 0)
 			mergeParagraph(bparams, cur.text()->paragraphs(), 0);
-		cur.leaveInset(*inset);
-	} else {
-		cur.leaveInset(*inset);
-		// reset surrounding par to default
-		DocumentClass const & dc = bparams.documentClass();
-		docstring const layoutname = inset->usePlainLayout()
-			? dc.plainLayoutName()
-			: dc.defaultLayoutName();
-		text->setLayout(cur, layoutname);
 	}
-
+	cur.leaveInset(*inset);
 	return true;
 }
 

Reply via email to