rgheck wrote:

There's a bug in the code that sets the layout for the first paragraph of an InsetCollapsable. If you start LyX from a terminal and create a branch, you'll see some error messages about PlainLayout not being found. This is because the first paragraph is being set to PlainLayout in the InsetCollapsable constructor, whereas InsetBranch takes Standard.

[snip]

The right place to do this would be the InsetText constructor. Is there a better way around the "Can't call virtual functions in a constructor" problem?


The attached patch implements the solution to this problem that is suggested in _Effective C++_, namely, to pass the necessary information up the line from subclass to superclass.

It could be simplified by making the useplain parameter optional, but it's easiest, at first, not to do it that way, to make sure everything is right. If this seems OK, I'll commit a simplifying patch next.

Comments? OK to commit?

rh

Index: src/insets/InsetOptArg.cpp
===================================================================
--- src/insets/InsetOptArg.cpp	(revision 27131)
+++ src/insets/InsetOptArg.cpp	(working copy)
@@ -22,7 +22,7 @@
 
 
 InsetOptArg::InsetOptArg(Buffer const & buf)
-	: InsetCollapsable(buf)
+	: InsetCollapsable(buf, true)
 {}
 
 
Index: src/insets/InsetCaption.cpp
===================================================================
--- src/insets/InsetCaption.cpp	(revision 27131)
+++ src/insets/InsetCaption.cpp	(working copy)
@@ -48,13 +48,11 @@
 
 
 InsetCaption::InsetCaption(Buffer const & buf)
-	: InsetText(buf)
+	: InsetText(buf, true) // use plain layout
 {
 	setAutoBreakRows(true);
 	setDrawFrame(true);
 	setFrameColor(Color_captionframe);
-	// There will always be only one
-	paragraphs().back().setLayout(buf.params().documentClass().plainLayout());
 }
 
 
Index: src/insets/InsetFlex.cpp
===================================================================
--- src/insets/InsetFlex.cpp	(revision 27131)
+++ src/insets/InsetFlex.cpp	(working copy)
@@ -34,7 +34,7 @@
 
 
 InsetFlex::InsetFlex(Buffer const & buf, string const & layoutName)
-	: InsetCollapsable(buf), name_(layoutName)
+	: InsetCollapsable(buf, true), name_(layoutName)
 {
 	// again, because now the name is initialized
 	setLayout(buf.params().documentClassPtr());
Index: src/insets/InsetTabular.cpp
===================================================================
--- src/insets/InsetTabular.cpp	(revision 27131)
+++ src/insets/InsetTabular.cpp	(working copy)
@@ -487,7 +487,6 @@
 	  inset(new InsetTableCell(buf))
 {
 	inset->setBuffer(const_cast<Buffer &>(buf));
-	inset->paragraphs().back().setLayout(buf.params().documentClass().plainLayout());
 }
 
 
@@ -2787,7 +2786,7 @@
 /////////////////////////////////////////////////////////////////////
 
 InsetTableCell::InsetTableCell(Buffer & buf)
-	: InsetText(buf), isFixedWidth(false),
+	: InsetText(buf, true), isFixedWidth(false),
 	  contentAlign(LYX_ALIGN_CENTER)
 {}
 
Index: src/insets/InsetBranch.cpp
===================================================================
--- src/insets/InsetBranch.cpp	(revision 27131)
+++ src/insets/InsetBranch.cpp	(working copy)
@@ -40,7 +40,7 @@
 namespace lyx {
 
 InsetBranch::InsetBranch(Buffer const & buf, InsetBranchParams const & params)
-	: InsetCollapsable(buf), params_(params)
+	: InsetCollapsable(buf, false), params_(params)
 {}
 
 
Index: src/insets/InsetNote.cpp
===================================================================
--- src/insets/InsetNote.cpp	(revision 27131)
+++ src/insets/InsetNote.cpp	(working copy)
@@ -116,7 +116,7 @@
 /////////////////////////////////////////////////////////////////////
 
 InsetNote::InsetNote(Buffer const & buf, string const & label)
-	: InsetCollapsable(buf)
+	: InsetCollapsable(buf, true)
 {
 	params_.type = notetranslator().find(label);
 }
Index: src/insets/InsetBox.cpp
===================================================================
--- src/insets/InsetBox.cpp	(revision 27131)
+++ src/insets/InsetBox.cpp	(working copy)
@@ -96,7 +96,8 @@
 /////////////////////////////////////////////////////////////////////////
 
 InsetBox::InsetBox(Buffer const & buffer, string const & label)
-	: InsetCollapsable(buffer), params_(label)
+	: InsetCollapsable(buffer, false), // this will be reset shortly
+	  params_(label)
 {
 	if (forcePlainLayout())
 		paragraphs().back().setLayout(buffer.params().documentClass().plainLayout());
Index: src/insets/InsetInfo.cpp
===================================================================
--- src/insets/InsetInfo.cpp	(revision 27131)
+++ src/insets/InsetInfo.cpp	(working copy)
@@ -83,7 +83,7 @@
 
 	
 InsetInfo::InsetInfo(Buffer const & buf, string const & name) 
-	: InsetCollapsable(buf), type_(UNKNOWN_INFO), name_()
+	: InsetCollapsable(buf, true), type_(UNKNOWN_INFO), name_()
 {
 	setAutoBreakRows(true);
 	setInfo(name);
Index: src/insets/InsetListings.cpp
===================================================================
--- src/insets/InsetListings.cpp	(revision 27131)
+++ src/insets/InsetListings.cpp	(working copy)
@@ -53,7 +53,7 @@
 	"!*()-=+|;:'\"`,<.>/?QWERTYUIOPASDFGHJKLZXCVBNMqwertyuiopasdfghjklzxcvbnm";
 
 InsetListings::InsetListings(Buffer const & buf, InsetListingsParams const & par)
-	: InsetCollapsable(buf)
+	: InsetCollapsable(buf, true)
 {
 	status_ = par.status();
 }
Index: src/insets/InsetFloat.cpp
===================================================================
--- src/insets/InsetFloat.cpp	(revision 27131)
+++ src/insets/InsetFloat.cpp	(working copy)
@@ -114,7 +114,7 @@
 
 
 InsetFloat::InsetFloat(Buffer const & buf, string const & type)
-	: InsetCollapsable(buf), name_(from_utf8(type))
+	: InsetCollapsable(buf, true), name_(from_utf8(type))
 {
 	setLabel(_("float: ") + floatName(type, buf.params()));
 	params_.type = type;
Index: src/insets/InsetIndex.cpp
===================================================================
--- src/insets/InsetIndex.cpp	(revision 27131)
+++ src/insets/InsetIndex.cpp	(working copy)
@@ -43,7 +43,7 @@
 
 
 InsetIndex::InsetIndex(Buffer const & buf)
-	: InsetCollapsable(buf)
+	: InsetCollapsable(buf, true)
 {}
 
 
Index: src/insets/InsetFootlike.cpp
===================================================================
--- src/insets/InsetFootlike.cpp	(revision 27131)
+++ src/insets/InsetFootlike.cpp	(working copy)
@@ -25,7 +25,7 @@
 
 
 InsetFootlike::InsetFootlike(Buffer const & buf)
-	: InsetCollapsable(buf)
+	: InsetCollapsable(buf, true)
 {}
 
 
Index: src/insets/InsetCollapsable.cpp
===================================================================
--- src/insets/InsetCollapsable.cpp	(revision 27131)
+++ src/insets/InsetCollapsable.cpp	(working copy)
@@ -76,8 +76,8 @@
 }
 
 
-InsetCollapsable::InsetCollapsable(Buffer const & buf)
-	: InsetText(buf), status_(Inset::Open),
+InsetCollapsable::InsetCollapsable(Buffer const & buf, bool useplain)
+	: InsetText(buf, useplain), status_(Inset::Open),
 	  openinlined_(false), autoOpen_(false), mouse_hover_(false)
 {
 	DocumentClass const & dc = buf.params().documentClass();
Index: src/insets/InsetText.h
===================================================================
--- src/insets/InsetText.h	(revision 27131)
+++ src/insets/InsetText.h	(working copy)
@@ -34,9 +34,11 @@
  */
 class InsetText : public Inset {
 public:
+	/// \param useplain whether to use the plain layout
+	/// this is needed because we cannot call the virtual function
+	/// usePlainLayout() from within the constructor.
+	explicit InsetText(Buffer const & buffer, bool useplain);
 	///
-	explicit InsetText(Buffer const & buffer);
-	///
 	InsetText(InsetText const &);
 	///
 	void setBuffer(Buffer &);
@@ -169,7 +171,7 @@
 	void doDispatch(Cursor & cur, FuncRequest & cmd);
 private:
 	///
-	void initParagraphs();
+	void initParagraphs(bool useplain);
 	///
 	void setParagraphOwner();
 	///
Index: src/insets/InsetWrap.cpp
===================================================================
--- src/insets/InsetWrap.cpp	(revision 27131)
+++ src/insets/InsetWrap.cpp	(working copy)
@@ -43,7 +43,7 @@
 namespace lyx {
 
 InsetWrap::InsetWrap(Buffer const & buf, string const & type)
-	: InsetCollapsable(buf)
+	: InsetCollapsable(buf, true)
 {
 	setLabel(_("wrap: ") + floatName(type, buf.params()));
 	params_.type = type;
Index: src/insets/InsetText.cpp
===================================================================
--- src/insets/InsetText.cpp	(revision 27131)
+++ src/insets/InsetText.cpp	(working copy)
@@ -71,11 +71,11 @@
 
 /////////////////////////////////////////////////////////////////////
 
-InsetText::InsetText(Buffer const & buf)
+InsetText::InsetText(Buffer const & buf, bool useplain)
 	: drawFrame_(false), frame_color_(Color_insetframe)
 {
 	setBuffer(const_cast<Buffer &>(buf));
-	initParagraphs();
+	initParagraphs(useplain);
 }
 
 
@@ -99,12 +99,15 @@
 }
 
 
-void InsetText::initParagraphs()
+void InsetText::initParagraphs(bool useplain)
 {
 	LASSERT(paragraphs().empty(), /**/);
 	paragraphs().push_back(Paragraph());
 	Paragraph & ourpar = paragraphs().back();
 	ourpar.setInsetOwner(this);
+	DocumentClass const & dc = buffer_->params().documentClass();
+	Layout const & lay = useplain ? dc.plainLayout() : dc.defaultLayout();
+	ourpar.setLayout(lay);
 }
 
 
Index: src/insets/InsetCollapsable.h
===================================================================
--- src/insets/InsetCollapsable.h	(revision 27131)
+++ src/insets/InsetCollapsable.h	(working copy)
@@ -33,7 +33,7 @@
 class InsetCollapsable : public InsetText {
 public:
 	///
-	InsetCollapsable(Buffer const &);
+	InsetCollapsable(Buffer const &, bool useplain);
 	///
 	InsetCollapsable(InsetCollapsable const & rhs);
 	///
Index: src/factory.cpp
===================================================================
--- src/factory.cpp	(revision 27131)
+++ src/factory.cpp	(working copy)
@@ -523,7 +523,7 @@
 		} else if (tmptok == "Tabular") {
 			inset.reset(new InsetTabular(const_cast<Buffer &>(buf)));
 		} else if (tmptok == "Text") {
-			inset.reset(new InsetText(buf));
+			inset.reset(new InsetText(buf, false));
 		} else if (tmptok == "VSpace") {
 			inset.reset(new InsetVSpace);
 		} else if (tmptok == "Foot") {
Index: src/CutAndPaste.cpp
===================================================================
--- src/CutAndPaste.cpp	(revision 27131)
+++ src/CutAndPaste.cpp	(working copy)
@@ -160,7 +160,7 @@
 		}
 	}
 
-	InsetText in(buffer);
+	InsetText in(buffer, false);
 	// Make sure there is no class difference.
 	in.paragraphs().clear();
 	// This works without copying any paragraph data because we have
Index: src/Buffer.cpp
===================================================================
--- src/Buffer.cpp	(revision 27131)
+++ src/Buffer.cpp	(working copy)
@@ -262,7 +262,7 @@
 {
 	LYXERR(Debug::INFO, "Buffer::Buffer()");
 
-	d->inset = new InsetText(*this);
+	d->inset = new InsetText(*this, false);
 	d->inset->setAutoBreakRows(true);
 	d->inset->getText(0)->setMacrocontextPosition(par_iterator_begin());
 }

Reply via email to