Re: [Cvslog] r22782 - in /lyx-devel/trunk/src: CutAndPaste.cpp factory...

Tue, 05 Feb 2008 07:58:00 -0800 



Thanks for sorting this out, Stefan. I hadn't looked at this bug. But 
let me urge everyone to look at this patch. It reflects a change in 
TextClass handling that is important to understand.



The point, as Stefan says, is that anything that belongs to a text class 
needs to be "preserved" by keeping a TextClassPtr around. These are 
boost:shared_ptr's. The need for this is a consequence of the modules 
code. TextClass objects do not now represent layout files but rather the 
result of incorporating whatever modules the user loads. So they are 
unique to documents and can even change if the user loads a new module. 
So you cannot just represent a TextClass by an index into a list of 
layout files, as we used to. Rather, a TextClass is just an object, and 
it is inclined to disappear when the document that created it 
disappears. Now the problem is that, if you cut something, the cut 
material needs to know about its TextClass, since that is what defines 
its layout, etc. So the TextClass needs to "stay with" what's cut, since 
the document itself may be closed. And all references to layout-related 
stuff in, say, an inset need to go via the corresponding TextClassPtr.



There may be an argument here for doing things differently, a way Abdel 
suggested a while ago, namely, that we keep every TextClass we ever 
generate in (say) a std::vector and refer to them via indices into this 
vector. This change would not be terribly hard to make: It's just a 
matter of redefining TextClassPtr. The downside to this is that you 
could end up with a lot of TextClass's in memory that don't need to be 
there. But it would prevent this kind of problem.


Richard

[EMAIL PROTECTED] wrote:

Author: sts
Date: Tue Feb  5 11:34:01 2008
New Revision: 22782

URL: http://www.lyx.org/trac/changeset/22782
Log:
* Do not keep pointers to data structures around if you don't know
  whether the data structure outlives the pointer:

  InsetLayouts are really owned by the corresponding TextClass. So
  keep the TextClass alive by keeping a TextClassPtr around for the
  pointers lifetime. This fixes Bug #4538.

Modified:
    lyx-devel/trunk/src/CutAndPaste.cpp
    lyx-devel/trunk/src/factory.cpp
    lyx-devel/trunk/src/insets/InsetCollapsable.cpp
    lyx-devel/trunk/src/insets/InsetCollapsable.h
    lyx-devel/trunk/src/insets/InsetFlex.cpp
    lyx-devel/trunk/src/insets/InsetFlex.h

Modified: lyx-devel/trunk/src/CutAndPaste.cpp
URL: http://www.lyx.org/trac/file/lyx-devel/trunk/src/CutAndPaste.cpp?rev=22782
==============================================================================
--- lyx-devel/trunk/src/CutAndPaste.cpp (original)
+++ lyx-devel/trunk/src/CutAndPaste.cpp Tue Feb  5 11:34:01 2008
@@ -445,17 +445,15 @@
                if (inset->lyxCode() != FLEX_CODE)
                        // FIXME: Should we verify all InsetCollapsable?
                        continue;
-               docstring const name = inset->name();
-               InsetLayout const & il = tclass2.insetlayout(name);
-               inset->setLayout(il);
-               if (il.labelstring != from_utf8("UNDEFINED"))
+               inset->setLayout(c2);
+               if (inset->getLayout().labelstring != from_utf8("UNDEFINED"))
                        continue;
                // The flex inset is undefined in tclass2
                docstring const s = bformat(_(
                        "Flex inset %1$s is "
                        "undefined because of class "
                        "conversion from\n%2$s to %3$s"),
-                       name, from_utf8(tclass1.name()),
+                       inset->name(), from_utf8(tclass1.name()),
                        from_utf8(tclass2.name()));
                // To warn the user that something had to be done.
                errorlist.push_back(ErrorItem(

Modified: lyx-devel/trunk/src/factory.cpp
URL: http://www.lyx.org/trac/file/lyx-devel/trunk/src/factory.cpp?rev=22782
==============================================================================
--- lyx-devel/trunk/src/factory.cpp (original)
+++ lyx-devel/trunk/src/factory.cpp Tue Feb  5 11:34:01 2008
@@ -102,9 +102,7 @@

 
 		case LFUN_FLEX_INSERT: {

                        string s = cmd.getArg(0);
-                       TextClass const & tclass = params.getTextClass();
-                       InsetLayout const & il = 
tclass.insetlayout(from_utf8(s));
-                       return new InsetFlex(params, il);
+                       return new InsetFlex(params, params.getTextClassPtr(), 
s);
                }

 
 		case LFUN_NOTE_INSERT: {

@@ -384,8 +382,6 @@
        }

 
 	auto_ptr<Inset> inset;

-
-       TextClass const & tclass = buf.params().getTextClass();

 
 	lex.next();

        string tmptok = lex.getString();
@@ -480,8 +476,8 @@
                } else if (tmptok == "Flex") {
                        lex.next();
                        string s = lex.getString();
-                       InsetLayout const & il = 
tclass.insetlayout(from_utf8(s));
-                       inset.reset(new InsetFlex(buf.params(), il));

+			inset.reset(new InsetFlex(buf.params(), 
+				buf.params().getTextClassPtr(), s));

                } else if (tmptok == "Branch") {
                        inset.reset(new InsetBranch(buf.params(),
                                                    InsetBranchParams()));

Modified: lyx-devel/trunk/src/insets/InsetCollapsable.cpp
URL: 
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetCollapsable.cpp?rev=22782
==============================================================================
--- lyx-devel/trunk/src/insets/InsetCollapsable.cpp (original)
+++ lyx-devel/trunk/src/insets/InsetCollapsable.cpp Tue Feb  5 11:34:01 2008
@@ -74,10 +74,11 @@

 
 
 InsetCollapsable::InsetCollapsable(BufferParams const & bp,

-               CollapseStatus status, InsetLayout const * il)
-       : InsetText(bp), layout_(il), status_(status),
+               CollapseStatus status, TextClassPtr tc)
+       : InsetText(bp), status_(status),
          openinlined_(false), autoOpen_(false), mouse_hover_(false)
 {
+       setLayout(tc);
        setAutoBreakRows(true);
        setDrawFrame(true);
        setFrameColor(Color_collapsableframe);
@@ -86,6 +87,7 @@

 
 InsetCollapsable::InsetCollapsable(InsetCollapsable const & rhs)

        : InsetText(rhs),
+               textClass_(rhs.textClass_),
                layout_(rhs.layout_),
                labelstring_(rhs.labelstring_),
                button_dim(rhs.button_dim),
@@ -116,14 +118,20 @@

 
 void InsetCollapsable::setLayout(BufferParams const & bp)

 {
-       setLayout(bp.getTextClass().insetlayout(name()));
-}
-
-
-void InsetCollapsable::setLayout(InsetLayout const & il)
-{
-       layout_ = &il;
-       labelstring_ = layout_->labelstring;
+       setLayout(bp.getTextClassPtr());
+}
+
+
+void InsetCollapsable::setLayout(TextClassPtr tc)
+{
+       textClass_ = tc;
+       if ( tc.get() != 0 ) {
+               layout_ = &tc->insetlayout(name());
+               labelstring_ = layout_->labelstring;
+       } else {
+               layout_ = 0;
+               labelstring_ = _("UNDEFINED");
+       }

 
 	setButtonLabel();

 }

Modified: lyx-devel/trunk/src/insets/InsetCollapsable.h
URL: 
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetCollapsable.h?rev=22782
==============================================================================
--- lyx-devel/trunk/src/insets/InsetCollapsable.h (original)
+++ lyx-devel/trunk/src/insets/InsetCollapsable.h Tue Feb  5 11:34:01 2008
@@ -19,6 +19,8 @@
 #include "InsetText.h"

 
 #include "Box.h"

+#include "TextClass.h"
+#include "TextClassPtr.h"

 
 #include <string>
 
@@ -41,7 +43,7 @@

        InsetCollapsable(
                BufferParams const &,
                CollapseStatus status = Inset::Open,
-               InsetLayout const * il = 0
+               TextClassPtr tc = TextClassPtr((TextClass *)0)
                );
        ///
        InsetCollapsable(InsetCollapsable const & rhs);
@@ -52,10 +54,13 @@
        docstring name() const { return from_ascii("Collapsable"); }
        InsetLayout const & getLayout(BufferParams const &) const

 	{ return *layout_; } 
+	InsetLayout const & getLayout() const
+	{ return *layout_; } 
 	///

        void setLayout(BufferParams const &);
-       /// (Re-)set the character style parameters from \p il
-       void setLayout(InsetLayout const & il);
+       /// (Re-)set the character style parameters from \p tc according
+       /// to name()
+       void setLayout(TextClassPtr tc);
        ///
        void read(Buffer const &, Lexer &);
        ///
@@ -165,10 +170,11 @@
        ///
        virtual void resetParagraphsFont();

 
-protected:

-       ///
+private:
+       /// text class to keep the InsetLayout above in memory
+       TextClassPtr textClass_;
+       /// cache for the layout_. Make sure it is in sync with the text class!
        InsetLayout const * layout_;
-private:
        ///
        Dimension dimensionCollapsed() const;
        ///

Modified: lyx-devel/trunk/src/insets/InsetFlex.cpp
URL: 
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetFlex.cpp?rev=22782
==============================================================================
--- lyx-devel/trunk/src/insets/InsetFlex.cpp (original)
+++ lyx-devel/trunk/src/insets/InsetFlex.cpp Tue Feb  5 11:34:01 2008
@@ -43,12 +43,13 @@

 
 
 InsetFlex::InsetFlex(BufferParams const & bp,

-                               InsetLayout const & il)
-       : InsetCollapsable(bp, Collapsed, &il)
+       TextClassPtr tc, string const & layoutName)
+       : InsetCollapsable(bp, Collapsed, tc),
+       name_(layoutName)
 {
-       name_ = il.name;
-       packages_ = il.requires;
-       preamble_ = il.preamble;
+       setLayout(tc); // again, because now the name is initialized
+       packages_ = getLayout().requires;
+       preamble_ = getLayout().preamble;
 }

 
 
@@ -65,7 +66,7 @@
 
 bool InsetFlex::undefined() const

 {
-       return layout_->labelstring == from_utf8("UNDEFINED");
+       return getLayout().labelstring == from_utf8("UNDEFINED");
 }

 
 
@@ -118,8 +119,8 @@

        ParagraphList::const_iterator end = paragraphs().end();

 
 	if (!undefined())

-               sgml::openTag(os, layout_->latexname,
-                             par->getID(buf, runparams) + layout_->latexparam);
+               sgml::openTag(os, getLayout().latexname,
+                             par->getID(buf, runparams) + 
getLayout().latexparam);

 
 	for (; par != end; ++par) {

                par->simpleDocBookOnePar(buf, os, runparams,
@@ -128,7 +129,7 @@
        }

 
 	if (!undefined())

-               sgml::closeTag(os, layout_->latexname);
+               sgml::closeTag(os, getLayout().latexname);

 
 	return 0;

 }

Modified: lyx-devel/trunk/src/insets/InsetFlex.h
URL: 
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetFlex.h?rev=22782
==============================================================================
--- lyx-devel/trunk/src/insets/InsetFlex.h (original)
+++ lyx-devel/trunk/src/insets/InsetFlex.h Tue Feb  5 11:34:01 2008
@@ -15,6 +15,7 @@

 
 #include "InsetCollapsable.h"
 
+using std::string;
 
 namespace lyx {
 
@@ -25,7 +26,8 @@

 class InsetFlex : public InsetCollapsable {
 public:
        ///
-       InsetFlex(BufferParams const &, InsetLayout const &);
+       InsetFlex(BufferParams const &,
+                 TextClassPtr tc, string const & layoutName);
        ///
        docstring name() const { return from_utf8(name_); }

 



_______________________________________________
Cvslog mailing list
[EMAIL PROTECTED]
http://www.lyx.org/mailman/listinfo/cvslog

  
















    











					Reply via email to