Review Request: Add buddy document architecture to kdevplatform

[floris]

On Thu, 2011-03-17 at 21:34 +0000, Milian Wolff wrote:
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100874/ 
> 
> seems to be very good work, please update the kdevelop patch and I'll do some actual testing instead of just looking at the code.
> 
> but I'll probably give my OK if I don't spot anything important then. Below are some small nitpicks, would be cool if you could attend to those.
> 
> ps: thanks for not just writing new code but also documenting it *and* writing unit tests, very good!
> 
> interfaces/ibuddydocumentfinder.h (Diff revision 1) 
> 
> same IBuddyDocumentFinder for several mimetypes.
> 
> 
>                57
> Idem, in your destructor, you'll call
> idem? what does that mean?
> 
idem is latin for "the same"
> interfaces/ibuddydocumentfinder.h (Diff revision 1) 
> 
> same IBuddyDocumentFinder for several mimetypes.
> 
> 
>                73
> /** @return true, if the two documents are buddies.
> could you unify the indentation in the doxygen comments, make them all look like the one for e.g. addFinder
> 
> thanks
> 
> interfaces/idocumentcontroller.h (Diff revision 1) 
> public Q_SLOTS:
> 
> 
>               107
> A buddy document can be specified. Typically, the document view will be opened
> add a newline before, doxygen is always
> 
> brief single line
> 
> long description
> 
> also please not that the buddy document can be supplied optionally and - if it's 0 - the registered finders will be queried automatically
> 
> interfaces/idocumentcontroller.h (Diff revision 1) 
> public Q_SLOTS:
> 
> 
>               122
> A buddy document can be specified. Typically, the document view will be opened
> same as above
> 
> shell/documentcontroller.cpp (Diff revision 1) 
> struct DocumentControllerPrivate {
> 
> 
>               388
> if(Core::self()->uiControllerInternal()->arrangeBuddies()) {
> what if this setting is false and a buddy was passed to openDocument directly?
> 
> I'd assume the config only decides whether to look for buddies automatically. if some plugin passes a buddy manually it should be always honored. or what do you think?
> 
> shell/settings/uiconfig.ui (Diff revision 1) 
> 
> 
> 
>               167
> <string>Arrange buddy documents side by side</string>
> in a user visible string, I'd prefer "related documents" instead of buddy documents.
> 
> the tooltip should also be more descriptive:
> 
> "When enabled, plugins can group related files side by side. For example a header file will be opened next to the implementation file."
> 
> 
> 
> - Milian
> 
> 
> On March 16th, 2011, 5:58 p.m., Martin Heide wrote:
> 
> Review request for KDevelop.
> By Martin Heide.
> Updated March 16, 2011, 5:58 p.m.
> 
> 
> Description 
> !! To apply patch, use (instead of git apply)
> patch -p1 < kdevplatform-buddy-1.patch
> (bug in reviewboard, sorry) !!
> 
> (This review request is based on https://git.reviewboard.kde.org/r/100795/ , to separate cleanup and buddy document features)
> Works together with the review request https://git.reviewboard.kde.org/r/100796/ in kdevelop. Apply them both before compiling.
> 
> Add document buddy architecture :
> Allow kdevelop language plugins to manage related documents (like foo.h and foo.cpp for cpp plugin)
> Language plugins may implement IBuddyDocumentFinder to define the rules (how to know if two documents are buddies).
> Language plugins that implement IBuddyDocumentFinder  must register themselves.
> An option was added to the UIConfig to open documents side by side with its buddies.
> 
> Buddy documents work like this: Assume that the option is enabled in the config, and there exist the following tabs:
> | foo.h | bar.cpp |
> If now the user opens foo.cpp, then this tab will be placed to the right of foo.h.
> 
> Another feature: Assume that both buddy documents and the option "Open new tab after current" are enabled. There are the following tabs:
> | *foo.h* | foo.cpp | bar.h |  (foo.h is the active tab)
> Normally, if the user opens somefile.cpp now, the tab would be placed between foo.h and foo.cpp, which would separate the buddies. So we changed the bahavior: the new tab will be placed after foo.cpp instead.
> Testing 
> Created shell-buddytest and sublime-documentcontrollertest. Both pass.
> Fixed an error in sublime-areaoperationtest (which was not related to the buddy document feature, but to the option "Open new tab after current").
> Diffs 
>       * interfaces/CMakeLists.txt (170cf63)
>       * interfaces/ibuddydocumentfinder.h (PRE-CREATION)
>       * interfaces/ibuddydocumentfinder.cpp (PRE-CREATION)
>       * interfaces/idocumentcontroller.h (d036afa)
>       * shell/documentcontroller.h (8dc3706)
>       * shell/documentcontroller.cpp (bc0a3dd)
>       * shell/settings/uiconfig.kcfg (60dccb2)
>       * shell/settings/uiconfig.ui (61f4b08)
>       * shell/tests/CMakeLists.txt (b093565)
>       * shell/tests/documentcontrollertest.h (PRE-CREATION)
>       * shell/tests/documentcontrollertest.cpp (PRE-CREATION)
>       * shell/tests/shellbuddytest.h (PRE-CREATION)
>       * shell/tests/shellbuddytest.cpp (PRE-CREATION)
>       * sublime/area.h (df0e2bb)
>       * sublime/area.cpp (efa8025)
>       * sublime/controller.h (e8b4794)
>       * sublime/controller.cpp (a8dd38a)
>       * sublime/tests/areaoperationtest.h (0e0f08e)
>       * sublime/tests/areaoperationtest.cpp (28dbeff)
> 
> View Diff
> 
> 
> Screenshots 
> New UiSettings