Review Request: Add smart clean up and buddy document architecture

[Milian Wolff]

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


please split buddy/cleanup review requests

and I'm missing the code that defines IBuddyDocumentFinder - seems to be not included in the patchset

furthermore I don't see *any* reason for putting the IBuddy stuff into sublime, make it kdevplatform interfaces and put the implementation into shell and use it in the document controller

imo for sublime the only change required is the "open view after view x"


shell/documentcontroller.cpp
<http://git.reviewboard.kde.org/r/100795/#comment1604>

    please use consisting formatting, preferrably put the closing { behind the ) on the same line (if the condition is on a single line)



shell/documentcontroller.cpp
<http://git.reviewboard.kde.org/r/100795/#comment1605>

    double whitespaces



shell/documentcontroller.cpp
<http://git.reviewboard.kde.org/r/100795/#comment1606>

    put that on a single line or format it like this:
    
    const QString mime = doc->mimeType()->name();                        Sublime::IBuddyDocumentFinder* buddyFinder = Sublime::BuddyFinderRegistry::getByMimeType(mime);



shell/documentcontroller.cpp
<http://git.reviewboard.kde.org/r/100795/#comment1607>

    hm you say here buddies are not applicable but there is still code using buddyFinder below?



shell/documentcontroller.cpp
<http://git.reviewboard.kde.org/r/100795/#comment1608>

    single line please



sublime/buddyfinderregistry.h
<http://git.reviewboard.kde.org/r/100795/#comment1610>

    rename: finderForMimetype



sublime/buddyfinderregistry.h
<http://git.reviewboard.kde.org/r/100795/#comment1611>

    public exported class, please PIMPL



sublime/cleanup.h
<http://git.reviewboard.kde.org/r/100795/#comment1613>

    this seems unused.
    
    if it's supposed to be used from plugins or so it requires to be exported and put into a proper standalone icleanupstrategy.h header
    
    anyways could you please split up the review requests? this seems unrelated to the buddy stuff above and I'd rather review it separately


- Milian


On March 10, 2011, 7:25 p.m., Yannick Motta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100795/
> -----------------------------------------------------------
> 
> (Updated March 10, 2011, 7:25 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Summary
> -------
> 
> 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 with BuddyFinderRegistry.
> An option was added to open documents side by side with its buddies.
> 
> Add smart cleanup feature to allow user to easily close outdated tabs related to each tab-bar.
> Outdated tabs are files that are not currently used.
> This feature is configurable, user can choose the time after which smart cleanup considers an unused tab outdated.
> An document is considered active if it is active at least five seconds.
> Last five documents are never outdated. 
> If an outdated document has an active buddy, it is immune to cleanup.
> User must manually start cleanup feature on the context menu of the tab-bar.
> A dialog opens and user can see which documents are considered outdated. User can remove documents from the list.
> If the user clicks "ok", tabs will be close.
> 
> Add an option to hide close buttons on tabs in order to free some space.
> 
> 
> Diffs
> -----
> 
>   interfaces/idocumentcontroller.h d036afa 
>   shell/documentcontroller.h 8dc3706 
>   shell/documentcontroller.cpp bc0a3dd 
>   shell/settings/uiconfig.kcfg 60dccb2 
>   shell/settings/uiconfig.kcfgc 3c02f72 
>   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/CMakeLists.txt f9aaaeb 
>   sublime/area.h df0e2bb 
>   sublime/area.cpp efa8025 
>   sublime/buddyfinderregistry.h PRE-CREATION 
>   sublime/buddyfinderregistry.cpp PRE-CREATION 
>   sublime/cleanup.h PRE-CREATION 
>   sublime/cleanup.cpp PRE-CREATION 
>   sublime/cleanupdialog.h PRE-CREATION 
>   sublime/cleanupdialog.cpp PRE-CREATION 
>   sublime/cleanupdialog.ui PRE-CREATION 
>   sublime/container.h 1633a9f 
>   sublime/container.cpp f858575 
>   sublime/controller.h e8b4794 
>   sublime/controller.cpp a8dd38a 
>   sublime/ibuddydocumentfinder.h PRE-CREATION 
>   sublime/mainwindow.cpp a2d1aab 
>   sublime/tests/CMakeLists.txt 410412b 
>   sublime/tests/areaoperationtest.h 0e0f08e 
>   sublime/tests/areaoperationtest.cpp 28dbeff 
>   sublime/tests/cleanuptest.h PRE-CREATION 
>   sublime/tests/cleanuptest.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/100795/diff
> 
> 
> Testing
> -------
> 
> We have added two unit tests :
> sublime-cleanuptest
> shell-buddytest
> both pass without error.
> manual test seems also working as expected.
> 
> 
> Screenshots
> -----------
> 
> New ui settings
>   http://git.reviewboard.kde.org/r/100795/s/90/
> Cleanup dialog and hided close buttons
>   http://git.reviewboard.kde.org/r/100795/s/91/
> Tab bar context menu
>   http://git.reviewboard.kde.org/r/100795/s/92/
> 
> 
> Thanks,
> 
> Yannick
> 
>

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