Review Request: Add buddy document architecture to kdevplatform

[Martin Heide]

> On March 17, 2011, 9:34 p.m., Milian Wolff wrote:
> > 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!

kdevelop patch is updated now. sorry for the delay.


> On March 17, 2011, 9:34 p.m., Milian Wolff wrote:
> > interfaces/ibuddydocumentfinder.h, line 57
> > <http://git.reviewboard.kde.org/r/100874/diff/1/?file=11465#file11465line57>
> >
> >     idem? what does that mean?

it means "in the same way". I'll change it, sounds better anyway ;)


> On March 17, 2011, 9:34 p.m., Milian Wolff wrote:
> > shell/documentcontroller.cpp, line 388
> > <http://git.reviewboard.kde.org/r/100874/diff/1/?file=11469#file11469line388>
> >
> >     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?

Currently, it is implemented in the way that a parameter buddy != 0 won't be honored if the setting is disabled.

I think it's good like this, because if the user deactivates the setting, (s)he doesn't want it at all. So I think it's better to manage the setting centrally in the DocumentController. Like this, the plugins needn't to check for the setting, and the behavior is always consistent, even if some plugin forgets to check for the setting.

But I'm not sure. Any comments on this opinion? ^^


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100874/#review2021
-----------------------------------------------------------


On March 16, 2011, 5:58 p.m., Martin Heide wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100874/
> -----------------------------------------------------------
> 
> (Updated March 16, 2011, 5:58 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Summary
> -------
> 
> !! 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.
> 
> 
> 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 
> 
> Diff: http://git.reviewboard.kde.org/r/100874/diff
> 
> 
> 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").
> 
> 
> Screenshots
> -----------
> 
> New UiSettings
>   http://git.reviewboard.kde.org/r/100874/s/103/
> 
> 
> Thanks,
> 
> Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20110318/d18f7a4c/attachment.html>