[patch] dialog provider for projectcontroller

[Andreas Pakulat]

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.