Review Request: Add smart clean up and buddy document architecture

Martin Heide martin.heide at gmx.net
Sun Mar 6 20:29:30 UTC 2011 


> On March 5, 2011, 10:45 a.m., Dmitry Risenberg wrote:
> > This does not apply cleanly on current master:
> > 
> > patching file sublime/container.h
> > Hunk #2 FAILED at 63.
> > 1 out of 2 hunks FAILED -- saving rejects to file sublime/container.h.rej
> > patching file sublime/container.cpp
> > Hunk #9 succeeded at 519 with fuzz 2 (offset 12 lines).
> > 
> > Please rebase on current master branch and re-send the patch.
> > 
> > Also the whitespace and newline changes make it hard to see what has actually changed. Some new trailing whitespaces are introduced - they should be removed.
> > 
> > Replace 'buddys' with 'buddies' - this is the correct spelling.
> > 
> > I think IBuddyDocumentFinder should be not in sublime, but in interfaces, because C++ support should not depend on the UI library (your other patch).
> > 
> > As a feature request: this should sometimes be triggered automatically, for example on closing KDevelop (whether to trigger or not can be configured in the settings).
> >

Ok sorry about that, we'll create a new patch.

Yes it's not optimal to have IBuddyDocumentFinder in sublime. The reason why we put it there:
Currently sublime does not depend on any other library of kdevplatform. We use IBuddyDocumentFinder and BuddyFinderRegistry in the tabbar cleanup, to keep foo.cpp always open when foo.h is still in use (and vice versa). The tabbar cleanup is in sublime as it is closely related to Sublime::Container, so we create a cleanup object in the constructor of Container, this seems to be the only proper way to add the cleanup feature.

So I see several possiblities:
a) Put the buddy functionality in interfaces. So the c++ plugin would not depend on sublime[1], but instead sublime would depend on interfaces.
b) Keep the buddy functionality in sublime.
c) Put the buddies in interfaces, and remove buddy integration in the cleanup, so keeping sublime independent of interfaces. I'd discourage this option, because someone who is editing foo.h will very likely also edit foo.cpp soon.
What do you think?
(Without our patch, the library dependencies are like this at the moment (correct me if I'm wrong):
shell -> sublime
shell -> interfaces
c++plugin -> interfaces)

The idea of triggering the cleanup on closing kdevelop is good, I think. As soon as our patch is accepted in general, we could add this.


[1] In fact, CppLanguageSupport::createActionsForMainWindow already has a parameter of type Sublime::MainWindow*, but it is not used, so perhaps it could be removed. At least, the plugin is not linked against sublime.


> On March 5, 2011, 10:45 a.m., Dmitry Risenberg wrote:
> > shell/documentcontroller.h, line 137
> > <http://git.reviewboard.kde.org/r/100795/diff/1/?file=10539#file10539line137>
> >
> >     Why does this belong to IDocumentController and not IBuddyDocumentFinder?

It belongs to DocumentController because here we search for a buddy document in all open documents, using ICore::self()->documentController()->openDocuments(). So currently, if we'd put it in IBuddyDocumentFinder (which is currently in sublime), sublime would depend on interfaces (and perhaps on shell, I'm not sure, as the real implementations of ICore and IDocumentController seem to be in the shell, in Core and DocumentController).
(see above, regarding the problem where to put IBuddyDocumentFinder)


- Martin


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


On March 4, 2011, 5:31 p.m., Yannick Motta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100795/
> -----------------------------------------------------------
> 
> (Updated March 4, 2011, 5:31 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 
>   kdevplatform.kdev4 f5f408c 
>   project/CMakeLists.txt a5b7f74 
>   shell/documentcontroller.h 8dc3706 
>   shell/documentcontroller.cpp bc0a3dd 
>   shell/settings/uiconfig.kcfg 60dccb2 
>   shell/settings/uiconfig.kcfgc 3c02f72 
>   shell/settings/uiconfig.ui 61f4b08 
>   shell/settings/uipreferences.cpp 43c63d6 
>   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 6b79df7 
>   sublime/container.cpp d45604f 
>   sublime/controller.h e8b4794 
>   sublime/controller.cpp a8dd38a 
>   sublime/ibuddydocumentfinder.h PRE-CREATION 
>   sublime/mainwindow.cpp 08ca648 
>   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/20110306/3e2e369d/attachment.html>

More information about the KDevelop-devel mailing list