KoSectionModel instead of KoSectionManager implemented
ClosedPublic

Authored by deniskuplyakov on Aug 6 2015, 5:52 PM.

Details

Summary

The main goal. Why it is done?
We need to store and maintain section tree in document. KoSectionManager were doing it lazy way: some changes can destroy its data and in that case you should call from code a special method to rebuild entirely whole tree of a document. This way comes more uncomfortable when we are going to visualize section tree (Author's outliner for example), as in that case you need to update widget by timer and in case of big documents such method will require high amount of computing.

So I decided to use Qts model as a base for the new KoSectionModel. New main aim: tree should be up-to-date always, and should be updated only by commiting local changes to it instead or rebuilding. Another pro of using Qt model that it is now super easy to show tree in UI with any data you want (check now QIdentityModel using with this in ConfigureSectionDialog).

Other changes

  1. I've found some bugs in handling of sections during deletion, unit-test reference data;
  2. Added more unit-tests for other types of sections operation, now also testing KoSectionModel;
  3. Small rewrites in a few places;
  4. Putted a detailed documentation on how sections handling now done to KoSectionModel.h;
  5. Introduced SplitSectionsCommand and corresponding tool that allows to insert paragraphs between directly nested sections, with that options user can create any section structure;

That's all that I remember, look at commit messages for details.

Connected future plans
I think that that SplitSections dialog UI should be reworked, some tips should be added to user about "why I need this?". Unfortunately, there is no place to look how it can be done, as LO doesn't allow such operation and there is no sections analog in MSO.

And good luck to reviewers: change is big enough ;)

Test Plan

What I have done

  • testKoTextEditor unit-test passing;
  • Manually checked split sections functionality.

What I want to reviewers make attention
There is a bunch of FIXME and TODO in code. Personally to boud: there is some unit-test I have commented in KoTextEditor, it is pointed with FIXME, check it plz.

Diff Detail

Repository
R8 Calligra
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
deniskuplyakov retitled this revision from to KoSectionModel instead of KoSectionManager implemented.
deniskuplyakov updated this object.
deniskuplyakov edited the test plan for this revision. (Show Details)
deniskuplyakov added reviewers: boemann, rempt.
deniskuplyakov set the repository for this revision to R8 Calligra.
deniskuplyakov added a project: Calligra: 3.0.

Still no reviews.... I know that everybody now is busy with code formatting / splitting repos and other important stuff, but I don't want my patch will be lost or need to be rewritten entirely. Can anybodu review, so I can push it before big changes?

rempt edited edge metadata.Aug 31 2015, 8:39 AM

I'll forward the phab mail to the calligra-devel list, it doesn't appear there by default, so people might have missed it.

Reviewed just the coding, some memory management, very good. Minor comments.

libs/kotext/KoSection.h
107

Is the error-prone reference really needed here?

children().insert() is used once in this patch, all other accesses look like const. Maybe insertChild() can be added and children() changed to QVector<KoSection *>?

The patch contains tabs. It could be good to replace tab to 4-spaces.

deniskuplyakov edited the test plan for this revision. (Show Details)
deniskuplyakov edited edge metadata.

Non-const reference children() removed.
Tabs fixed.

deniskuplyakov marked an inline comment as done.Aug 31 2015, 11:33 PM

Non-const reference children() removed.
Tabs fixed.

Pfff something strange with arc tool... Will fix it now.

Seems now it is good. Done it with arc diff master. Don't know why simple arc diff last time didn't made the trick

staniek accepted this revision.Sep 1 2015, 12:21 AM
staniek added a reviewer: staniek.

Minor stuff left. Please fix (if you have time) and push. Good job!

libs/kotext/KoSection.h
110

Hmm, the split to const/non-const versions is misleading.
Actually both are non-const: user can alter KoSection objects.
Traditional C++ constness paradox.

It would help me if I understand if this works: have only just this version:
QVector<KoSection *> children() const.

libs/kotext/KoSectionModel.cpp
190

not needed line

211

not needed line

This revision is now accepted and ready to land.Sep 1 2015, 12:21 AM
deniskuplyakov edited edge metadata.

Fixed latest issues

deniskuplyakov marked 3 inline comments as done.Sep 1 2015, 8:14 AM
This revision was automatically updated to reflect the committed changes.