[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: KDE/kdepim/akregator/src
From:       David Faure <faure () kde ! org>
Date:       2009-09-18 17:35:30
Message-ID: 200909181935.31038.faure () kde ! org
[Download RAW message or body]

On Sunday 06 September 2009, Michael Jansen wrote:
> So inheriting from BrowserRun - which is allowed - is kind of guesswork.

No it's not. It just requires reading the source code from KRun and BrowserRun 
:-)

> Some assertions would probably help after calling it.
>     - is the timer started for example?

Well, if that was the only problem, we could just do this

--- krun.cpp    2009-09-01 19:51:56.000000000 +0200
+++ krun.cpp    2009-09-18 19:29:00.000000000 +0200
@@ -1359,6 +1359,8 @@ void KRun::mimeTypeDetermined(const QStr

     foundMimeType(mimeType);

+    startTimer(0);
+
     d->m_showingDialog = false;
 }

But the problem is that just starting the timer (in order to "trigger the next 
step") is not enough, we also need to define what the next step is...
If neither setFinished nor setError was called, then, hmm, it will just 
silently die (assuming autodelete, which is the default). OK, maybe not
as bad (no memory leak), but contract broken still (finishing without emitting
a signal). Maybe we can set m_bFinished = true before calling foundMimeType 
since we know this is the last step anyway. And then we can simplify quite
some code in the 3 subclasses. I like that. I'll -do- that ;)

> Or redesigning it :(

Yes. The current design is not a design, it's only the result of a conversion 
between protected members and setter methods; but they don't make much sense 
the way they are.

So, yes for redesigning. See you around KDE-5 :-)

-- 
David Faure, faure@kde.org, sponsored by Nokia to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).
[prev in list] [next in list] [prev in thread] [next in thread]