Abdelrazak Younes schrieb:
Abdelrazak Younes wrote:
Hello,
Here is the first draft of the model/view separation of the Paragraph
class. It is mostly working already execept for Copy&Paste and
MultipleView.
I've solve the C&P issue and many others. In single window mode, the
attached patch is very stable, I did extensive testing but could not
make it crash (including with DEPM action triggered). There's no
painting problem either. From this point I'd welcome any additional
testing. But I really think it is ready for inclusion.
So, objections?
Unfortunately, I am not able to do extensive tests right now :-(
However, I made a review of the patch:
(1) I think the idea behind the patch looks good and I think even
understood most parts of your code :-)
(2) I suggest removing "textRow()" from cursor.h - there is nothing
worse than unused code (which causes confusion in the future).
(3) Is there a need for two BufferView::parMetrics() methods? I don't
like methods that look very similar (but which are not fully identical).
Moreover, default values are the opposite of clarity.
(4) "if (pmc_it->second.rows().empty() && redo) {" - shouldn't && in
fact be || ???
(5) AFAICS, "BOOST_ASSERT(!pm.rows().empty());" in bufferview_funcs.C is
rather pointless if my assumption (4) holds (similar assertions are
given in other files, too).
(6) As soon as the patch is committed, I suggest aligning source files
and C++ classes (ParagraphMetrics).
(7) There are a couple of statements like this: "int const right_margin
= isMainText(buffer)? pm.rightMargin(buffer) : 0;". I wonder whether we
can encapsulate the condition.
I suggest committing the patch sooner than later. I guess minor problems
can be fixed afterwards. Creating a branch doesn't sound like a good
idea. In the past, branches haven't been very successful. Georg, what's
your opinion?
Excellent work, Abdel!
Michael
