Lars Gullik Bjønnes a écrit :
Abdelrazak Younes <[EMAIL PROTECTED]> writes:
| Guys,
|
| I have tested this new approach pretty extensively:
| - opened all my lyx documents+lyx help doc
| - played with them, added/erased paragraph randomly,
| - cut/copy/past large selection, math, graphics,
| - undo/redo, export to latex, export to text.
|
| So far, everything seems fine. What else should I test?
You seem to have covered it.
Be sure to do the tests inside/mixed/outside insets as well.
If you mean inside Notes, Foot Notes, Floats and the likes, then I've
tested that as well.
| I really think that there are some problems in output_*.C that my
| patch reveals. The responsible of that portion should maybe check
| (-dbg action will gives the relevant warning).
Can you expand a bit on the problems you see?
Extract from a previous mail:
For those interested, here is a second version of the it_vector class.
With former one an assertion is triggered whenever you try to export to
latex. It seems that the at() method is used beyond the size() limit. I
am really not sure but there is maybe a hidden bug somewhere in the
"export to latex" functionality. I see a lot of "boost::next" inside
output_latex.C there's maybe one that goes beyond the edges. This is
just a guess.
This new version accept the call to at(i) with i=size() but this is only
temporary in order to let the export working:
inline value_type & at(size_t pos) {
if (pos >= ItVector_.size()) {
lyxerr[Debug::ACTION]
<< "trying to access element " << pos
<< " while size is "<< size() << endl;
return *ItVector_[size()-1];
}
BOOST_ASSERT(pos < ItVector_.size());
return *ItVector_[pos];
}
| Anyway, I attached again the last unchanged patch and the new
| "it_vector.h" header. Jean-Marc, if you could test this after 1.4.0,
| I think it could be a good candidate for 1.4.1.
I must admit that I'd prefere it to be for 1.5 only. My guess is that
the implementation suited for 1.4 will diverge quickly from what we
will have in trunk.
If tested and proved correct why not take this patch as-is for 1.4.x and
let it diverge for 1.5.x so we can hit the perfect, ideal, STL-ish
BOOST-ish solution. In the mean time, my pragmatic side says that it is
good enough and there are other things to cure for 1.4.x. And the code
is that that ugly compared to some other code inside lyx, is it? ;-)
| Angus, I did removed the "for_each" methods but I let the inner class
| iterator and const_iterator because they could be useful in the
| future. Beside that they provide already "operator+=". I am still open
| on that issue.
I'll probably have an opinion on this.
| 1) Try to reduce the time of rebuilding the tree in case of a change
| in "it_vector.h". A priori, the main limiting factor is that
| "LyxText.h" is included at many place in the code.
One possibility (you won't like this),
Indeed.
is to change it_vector.h to
_be_ paragraphlist.[hC]. I.e. make it a non-template class and move
most methods into the source file.
You will need to play with the header in any case if you want to modify
the private members. In any case, the recompilation is not that awful
(at least with Qt4) it is the final linking stage that takes time. So I
really prefer to let it stay a template class and put it in
"src/support" instead of "src/".
A related option is pimpl, but might make things a bit too slow.
Or a base virtual class that will fix the interface... (maybe that's
what pimpl do?)
| 2) simplify the use of ParagraphList wherever possible by using
| some of the convenience interface methods in it_vector (my original
| patch did some simplifications already).
I think use of splice should be investigated, especially in relation
to cut/paste, but also undo.
Agreed, right now we have a lot unneeded copies of ParagraphList objects
in Undo and CopyAndPaste. But I am not sure of what you want, use of use
of splice inside it_vector or outside?
| 3) optimize "updateCounter" call inside "text.C":BreakParagraph(...).
| In this case we don't need the full updateCounter, something around
| current cursor should suffice. But we need the constructor
| "ParIterator(InsetBase &, lyx::pit_type pit)" to be implemented first.
Hmm... is this directly related to the ParagraphList work?
No. But it's related to paragraph insertion/removal optimisation as
stated in the title.
| 4) Rename ParagraphList into paragraph_container.
This I probably just disagree with. (against our naming scheme for one
thing)
OK. ParagraphContainer then?
| 5) Transform some functions that uses ParagraphList into ParagraphList
| member methods (in particular those in paragraph_func.C) or at least
| create helper method for classes that uses ParagraphList (Undo,
| CutAndPaste, etc).
If a function does not need the private parts of an object/class it
should not be a member function.
I am sure we will end-up with some members that will be best hidden as
private member. But, in general, I agree with you.
| 6) See if some other classes could benefit from it_vector (InsetList
| and FontList come to mind).
Probably not... those lists are small and per-paragraph. But possibly.
We'll see. That was just a suggestion seeing that "InsetList"
encapsulate much of the same interface as "it_vector". And there is at
least one use case where there would be one benefit: multiple graphics
in the same paragraph. Actually, I am pretty sure that this is the one
remaining issue with Helge's use case of Lyx over ADSL.
I can provide a patch to test this if you are interested.
Thanks for commenting Lars,
Abdel.
