[patch] dialog provider for projectcontroller
Andreas Pakulat
apaku at gmx.de
Mon Aug 25 17:25:57 UTC 2008
On 25.08.08 18:02:16, Manuel Breugelmans wrote:
> This patch introduces an extra class, IProjectDialogProvider.
> ProjectController delegates dialog creation to this one, instead of inline'd
> KDialog:: etc calls. Also some cleanups and extensions for
> ProjectControllerTest.
>
> The reason I think this is useful:
+1 from me for the idea.
> Currently it resides in shell but maybe it makes more sense to have this in
> interfaces ...
I don't think thats useful. I can't think of a good reason for a plugin
to change that dialog provider, the testability-case is covered by
having the API available in shell. BTW: No need to put the
dialog-provider declarations into the .cpp files, you can just as well
put them into the .h so you don't need the #include "moc_..."
> Btw, Andreas, project-reloading is broken (part of this is exposed through
> the extra test in this patch)
I guess you're talking about re-opening while a project loads? Yeap
thats an untested case basically.
> QPointer<QAction> m_closeAllProjects;
> + IProjectDialogProvider* dialogs;
This should use singular form, its not a list or some such.
> +using QTest::kWaitForSignal;
> +#define WAIT_FOR_OPEN_SIGNAL \
> +if (1) {\
Whats the if(1) for, just using {} makes sure you can use the macro
multiple times within the same block? And why that using declaration?
Its not like writing the QTest:: in the macro saves you a lot of code :)
> +void ProjectControllerTest::reopenWhileLoading()
> +{
> + // Open the same project again while the first is still
> + // loading. The second open request should be blocked.
> + m_projCtrl->setDialogProvider(new DialogProviderFake);
> + QSignalSpy* spy = createOpenedSpy();
> + QVERIFY(m_projCtrl->openProject(m_projFileUrl));
> + QVERIFY(!m_projCtrl->openProject(m_projFileUrl));
I guess here it fails currently? Thats because the controller doesn't
keep track of projects that have been started to open but are not
finished yet. Feel free to implement that :)
Andreas
--
Your ignorance cramps my conversation.
More information about the KDevelop-devel
mailing list