On 06/09/2012 08:35 AM, Tommaso Cucinotta wrote: > On 08/06/12 04:15, Richard Heck wrote: >> On 6/7/12 6:15 PM, Tommaso Cucinotta wrote: >>> Do u mind explaining what does this code do ? >>> >>> +void BufferView::makeDocumentClass() >>> +{ >>> + DocumentClassConstPtr olddc = >>> buffer_.params().documentClassPtr(); >>> + buffer_.params().makeDocumentClass(); >>> + updateDocumentClass(olddc); >>> +} >>> >> Here's how to think of it. Suppose you add a new module to your >> document. Then we need to do two things: (i) Load the module's code; >> (ii) Adapt the Buffer to the presence of the new module. This code >> does those two steps, whenever you do anything that requires them. >> The BufferParams::makeDocumentClass() routine takes the base >> TextClass (layout file) and loads modules and local layout into it, >> creating a new DocumentClass. But we then have to make sure that the >> Buffer itself uses this DocumentClass and, e.g., that each paragraph >> references *its* layouts as opposed to the ones in the now discarded >> DocumentClass. That's what updateDocumentClass() does. > > Ok, so it seems to me that, at the level of the document model, > params().makeDocumentClass() and BufferView::updateDocumentClass() > (or, perhaps, cap::switchBetweenClasses()) should happen atomically > together, i.e., makeDocumentClass() might call > cap::switchBetweenClasses(), or maybe better have a > Buffer::makeDocumentClass() that calls both. Later, if the view > happens to have further objects referencing the old class (e.g., > cursor ?), then it would be its own responsibility to update those > further pointers. > > For example, I noticed that readHeader() calls makeDocumentClass() but > apparently no further update is done on paragraphs. Now, > BufferView.cpp calls readHeader() and afterwards updateDocumentClass() > so it seems ok. However, GuiApplication.cpp, in SAVE_AS_DEFAULT, calls > readHeader() but not updateDocumentClass() afterwards. > Also, what about other calls to params().makeDocumentClass() ? > > $ find src/ -name '*.cpp' -exec grep -Hn makeDocumentClass {} \; > src/Buffer.cpp:848: params().makeDocumentClass(); > src/BufferParams.cpp:353: makeDocumentClass(); > src/BufferParams.cpp:2023:void BufferParams::makeDocumentClass() > src/BufferView.cpp:944:void BufferView::makeDocumentClass() > src/BufferView.cpp:947: buffer_.params().makeDocumentClass(); > src/BufferView.cpp:1263: makeDocumentClass(); > src/BufferView.cpp:1279: makeDocumentClass(); > src/BufferView.cpp:1308: makeDocumentClass(); > src/BufferView.cpp:1332: makeDocumentClass(); > src/frontends/qt4/FindAndReplace.cpp:506: dest_bv.makeDocumentClass(); > src/frontends/qt4/GuiDocument.cpp:2056: bp_.makeDocumentClass(); > src/frontends/qt4/GuiDocument.cpp:2220: bp_.makeDocumentClass(); > src/frontends/qt4/GuiDocument.cpp:2337: > param_copy.makeDocumentClass(); > > The calls in GuiDocument.cpp seem to never be followed by an > updateDocumentClass(), but perhaps these calls operate on temporary > BufferParams and no document's paragraphs updates are needed. > I looked through these, but should probably do so again. The ones in GuiDocument aren't a problem, since the BufferParams objects there aren't actually associated with a document; they're representing the BufferParams of a document, but they're just a copy. So we don't need to change any paragraphs. The one in Buffer itself is just part of reading the file, and the actual DocumentClass gets set later, if necessary. I agree this is sub-standard, and everything ought to be done at once. But especially in GuiDocument, this isn't possible. Richard