[patch] dialog provider for projectcontroller

[Manuel Breugelmans]

On Monday 25 August 2008 19:25:57 Andreas Pakulat wrote:
> 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_..."

Right, then I'll get rid of the I-version altogether. Btw addition of new 
virtual slots to this provider is BC, yes? Techbase seems to confirm this but 
only in a little vague comment at the bottom.

> > 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.

A standard reload also acts weird here. The reload gets through but the 
project-manager toolview gets removed, probably not that hard to fix.

> >      QPointer<QAction> m_closeAllProjects;
> > +    IProjectDialogProvider* dialogs;
>
> This should use singular form, its not a list or some such.

ok

> > +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? 

ah yes, that can be removed

> And why that using declaration?
> Its not like writing the QTest:: in the macro saves you a lot of code :)

kWaitForSignal is used somewhere else as well. Basicly I found 
QTest::kWaitForSignal( object, SIGNAL( mySignalMemberFunction( ) ), 20000 ) a 
tad verbose.

> > +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. 

Correct


Manuel