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

2008-02-05 Thread rgheck



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_ptrInset 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));
-   

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

2008-02-05 Thread rgheck



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;

-
-   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));
-