<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/101794/">http://git.reviewboard.kde.org/r/101794/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hi,
I'm sorry that I've had to be a bit finicky with the review, but this is such a core part of calligra that some extra care is a good thing. In general, I like the idea a lot, bor the progress updating, I think something more in line with the way KoProgressUpdater/KoUpdater/KoProgressProxy were designed would be better. This is an untested, uncompiled diff against your patch which sort of shows what I mean:
diff --git a/libs/main/KoFilterManager.cpp b/libs/main/KoFilterManager.cpp
index 3dc76c4..e922587 100644
--- a/libs/main/KoFilterManager.cpp
+++ b/libs/main/KoFilterManager.cpp
@@ -64,13 +64,12 @@ KoFilterManager::KoFilterManager(const QString& url, const QByteArray& mimetypeH
d->batch = false;
}
-KoFilterManager::KoFilterManager(const QByteArray& mimeType,const QStringList &extraMimes, KoUpdater *updater) :
- m_document(0), m_parentChain(0), m_graph(""), d(new Private)
+KoFilterManager::KoFilterManager(const QByteArray& mimeType,const QStringList &extraMimes, KoProgressUpdater *updater) :
+ m_document(0), m_parentChain(0), m_graph(""), d(new Private(updater))
{
d->batch = false;
d->importMimeType = mimeType;
d->extraMimesType = extraMimes;
- d->updater = updater;
}
KoFilterManager::~KoFilterManager()
diff --git a/libs/main/KoFilterManager.h b/libs/main/KoFilterManager.h
index af721f9..8e29ad1 100644
--- a/libs/main/KoFilterManager.h
+++ b/libs/main/KoFilterManager.h
@@ -66,7 +66,7 @@ public:
* @param mimeType the mimetype to import to.
*/
explicit KoFilterManager(const QByteArray& mimeType,const QStringList &extraMimes,
- KoUpdater *updater = 0);
+ KoProgressUpdater *updater = 0);
/**
* Create a filter manager for a filter which wants to embed something.
diff --git a/libs/main/OutOfProcessLoad/KoFilterLoader.cpp b/libs/main/OutOfProcessLoad/KoFilterLoader.cpp
index 1226a07..688ebc4 100644
--- a/libs/main/OutOfProcessLoad/KoFilterLoader.cpp
+++ b/libs/main/OutOfProcessLoad/KoFilterLoader.cpp
@@ -28,6 +28,7 @@
#include <KoFilterManager.h>
#include <KoUpdater.h>
+#include <KoProgressUpdater.h>
KoFilterLoader::KoFilterLoader(QString &sourcefilePath, QString &sourcefilePathTypeName, QString& mimeType, QStringList &extraMimes)
: m_localSocket(0),
@@ -53,8 +54,7 @@ int KoFilterLoader::start()
m_localSocket = new QLocalSocket(this);
m_localSocket->connectToServer(QString::number(QCoreApplication::applicationPid()));
- m_updater = new KoUpdater(0);
- connect( m_updater, SIGNAL( sigProgress( int ) ), this, SLOT( setProgress( int ) ) );
+ m_updater = new KoProgressUpdater(this);
KoFilter::ConversionStatus status;
KoFilterManager* manager = new KoFilterManager( QByteArray(m_mimeType.toLatin1()), m_extraMimes, m_updater);
@@ -74,14 +74,14 @@ int KoFilterLoader::start()
return status;
}
-void KoFilterLoader::setProgress( int percent )
+void KoFilterLoader::setValue(int value)
{
QByteArray block;
uchar header = ProgressUpdate;
- uint dataSizeSize = sizeof(percent);
+ uint dataSizeSize = sizeof(value);
block.append((const char*)&header, sizeof(header));
block.append((const char*)&dataSizeSize, sizeof(dataSizeSize));
- block.append((const char*)&percent, sizeof(percent));
+ block.append((const char*)&value, sizeof(value));
m_localSocket->write(block);
m_localSocket->flush();
}
diff --git a/libs/main/OutOfProcessLoad/KoFilterLoader.h b/libs/main/OutOfProcessLoad/KoFilterLoader.h
index 3f84100..c2707df 100644
--- a/libs/main/OutOfProcessLoad/KoFilterLoader.h
+++ b/libs/main/OutOfProcessLoad/KoFilterLoader.h
@@ -23,12 +23,13 @@
#include <qobject.h>
#include <qstring.h>
#include <qstringlist.h>
+#include <KoProgressProxy.h>
class QLocalSocket;
-class KoUpdater;
+class KoProgressUpdater;
-class KoFilterLoader : public QObjectp
+class KoFilterLoader : public QObject, public KoProgressProxy
{
Q_OBJECT
public:
@@ -49,9 +50,16 @@ public:
QString m_sourcefilePathTypeName;
QString m_mimeType;
QStringList m_extraMimes;
- KoUpdater * m_updater;
-private slots:
-void setProgress( int percent );
+ KoProgressUpdater * m_updater;
+
+private:
+
+ // KoProgressProxy implementation
+ int maximum() const { return 100; }
+ void setValue(int value);
+ void setRange(int minimum, int maximum) {}
+ void setFormat(const QString &format) {}
+
};
#endif /* KOFILTERLOADER_H */
diff --git a/words/part/KWDocument.cpp b/words/part/KWDocument.cpp
index 77fb02f..154802f 100644
--- a/words/part/KWDocument.cpp
+++ b/words/part/KWDocument.cpp
@@ -94,6 +94,7 @@ KWDocument::KWDocument(QWidget *parentWidget, QObject *parent, bool singleViewMo
m_frameLayout(&m_pageManager, m_frameSets),
m_mainFramesetEverFinished(false)
{
+ setAsynchronousLoading(true);
m_frameLayout.setDocument(this);
resourceManager()->setOdfDocument(this);
</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/101794/diff/1/?file=25673#file25673line791" style="color: black; font-weight: bold; text-decoration: underline;">libs/main/KoDocument.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">791</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="n">setAsynchronousLoading</span><span class="p">(</span><span class="n">bool</span> <span class="n">asynchronousLoading</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">A better name would be, I think, "enableOutOfProcessLoading", because it isn't really asynchronous. Also, the getter/setter need documentation.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/101794/diff/1/?file=25674#file25674line137" style="color: black; font-weight: bold; text-decoration: underline;">libs/main/KoDocument.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">137</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">asynchronousLoading</span><span class="p">(</span><span class="kc">false</span><span class="p">),</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Also better rename this to "loadOutOfProcess"</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/101794/diff/1/?file=25674#file25674line204" style="color: black; font-weight: bold; text-decoration: underline;">libs/main/KoDocument.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">204</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QObject</span> <span class="o">*</span><span class="n">parent</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I don't think this is needed -- if you really need to access the parent of KoDocument, you can use the parent() method to retrieve the QObject parent of the KoDocument object.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/101794/diff/1/?file=25674#file25674line1509" style="color: black; font-weight: bold; text-decoration: underline;">libs/main/KoDocument.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">bool KoDocument::openFile()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1509</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span> <span class="n">d</span><span class="o">-></span><span class="n">asynchronousLoading</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">no space after the ! sign.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/101794/diff/1/?file=25675#file25675line41" style="color: black; font-weight: bold; text-decoration: underline;">libs/main/KoFilterChainLink.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">namespace</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">41</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">KoProgressU</span>pdater</span><span class="hl"> </span><span class="o"><span class="hl">*</span></span><span class="n"><span class="hl">pu</span></span> <span class="o">=</span> <span class="n">chain</span><span class="o">-></span><span class="n">manager</span><span class="p">()</span><span class="o">-></span><span class="n">progressUpdater</span><span class="p">();</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">41</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">u</span>pdater</span> <span class="o">=</span> <span class="n">chain</span><span class="o">-></span><span class="n">manager</span><span class="p">()</span><span class="o">-></span><span class="n">progressUpdater</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Why don't we have a subtask here anymore?</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/101794/diff/1/?file=25676#file25676line68" style="color: black; font-weight: bold; text-decoration: underline;">libs/main/KoFilterManager.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">68</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">explicit</span> <span class="n">KoFilterManager</span><span class="p">(</span><span class="k">const</span> <span class="n">QByteArray</span><span class="o">&</span> <span class="n">mimeType</span><span class="p"><span class="hl">);</span></span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">68</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">explicit</span> <span class="n">KoFilterManager</span><span class="p">(</span><span class="k">const</span> <span class="n">QByteArray</span><span class="o">&</span> <span class="n">mimeType</span><span class="p"><span class="hl">,</span></span><span class="k"><span class="hl">const</span></span><span class="hl"> </span><span class="n"><span class="hl">QStringList</span></span><span class="hl"> </span><span class="o"><span class="hl">&</span></span><span class="n"><span class="hl">extraMimes</span></span><span class="p"><span class="hl">,</span></span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'm not convinced that we need to pass a KoUpdater here -- it would be more consistent to pass a KoProgressUpdater, as in the other constructor.
Hm... and please add dox for extra params.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/101794/diff/1/?file=25676#file25676line88" style="color: black; font-weight: bold; text-decoration: underline;">libs/main/KoFilterManager.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">88</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c1">// KoFilterManager::KoFilterManager(const QString& url, const QByteArray& mimetypeHint,</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">just remove commented-out code.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/101794/diff/1/?file=25679#file25679line53" style="color: black; font-weight: bold; text-decoration: underline;">libs/main/KoUpdater.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class KoUpdaterPrivate;</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">53</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KoUpdater</span><span class="p">(</span><span class="n">KoUpdaterPrivate</span> <span class="o">*</span><span class="n">p</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I don't think this constructor should be public, it's only relevant for inheriting from KoUpdater, and shouldn't be publicly callable.
I see you want to construct a kind of "synthetic" KoUpdater instance in KoFilterLoader -- but it would be better to create a KoProgressUpdater instance there.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/101794/diff/1/?file=25680#file25680line34" style="color: black; font-weight: bold; text-decoration: underline;">libs/main/KoUpdater.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">34</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">p</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">A KoUpdater _always_ should have a d-pointer instance, so this check isn't needed. Having code that has a d-pointer that can be 0 will be very confusing because it conflicts with the pattern everywhere else.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/101794/diff/1/?file=25684#file25684line56" style="color: black; font-weight: bold; text-decoration: underline;">libs/main/OutOfProcessLoad/KoFilterLoader.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">int KoFilterLoader::start()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">56</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_updater</span> <span class="o">=</span> <span class="k">new</span> <span class="n">KoUpdater</span><span class="p">(</span><span class="mi">0</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">See the patch I included in the main review -- it touches this part.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/101794/diff/1/?file=25685#file25685line1" style="color: black; font-weight: bold; text-decoration: underline;">libs/main/OutOfProcessLoad/KoOutOfProcessLoad.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm">/* This file is part of the KDE project</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'd move this one directory up, since this isn't part of the little loader app, is it?</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/101794/diff/1/?file=25685#file25685line23" style="color: black; font-weight: bold; text-decoration: underline;">libs/main/OutOfProcessLoad/KoOutOfProcessLoad.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">23</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#endif </span><span class="c1">// KOOUTOFPROCESSLOAD_H</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Erm... wrong include guard?</pre>
</div>
<br />
<p>- Boudewijn</p>
<br />
<p>On June 28th, 2011, 12:46 p.m., Matus Hanzes wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Calligra.</div>
<div>By Matus Hanzes.</div>
<p style="color: grey;"><i>Updated June 28, 2011, 12:46 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It can happen that filters crash the application while loading invalid file.
Because the fixing would be to costly I have created this work around.
The idea is to place the loading code into another process and if the filter crashes the main application stays alive.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>libs/main/CMakeLists.txt <span style="color: grey">(5492748)</span></li>
<li>libs/main/KoDocument.h <span style="color: grey">(1cd759f)</span></li>
<li>libs/main/KoDocument.cpp <span style="color: grey">(7c2d8e3)</span></li>
<li>libs/main/KoFilterChainLink.cpp <span style="color: grey">(628ff95)</span></li>
<li>libs/main/KoFilterManager.h <span style="color: grey">(5e05b3b)</span></li>
<li>libs/main/KoFilterManager.cpp <span style="color: grey">(d4bc313)</span></li>
<li>libs/main/KoFilterManager_p.h <span style="color: grey">(f7a4485)</span></li>
<li>libs/main/KoUpdater.h <span style="color: grey">(f6406c3)</span></li>
<li>libs/main/KoUpdater.cpp <span style="color: grey">(484d7e4)</span></li>
<li>libs/main/KoUpdaterPrivate_p.cpp <span style="color: grey">(eae2e54)</span></li>
<li>libs/main/OutOfProcessLoad/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/main/OutOfProcessLoad/KoFilterLoader.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/main/OutOfProcessLoad/KoFilterLoader.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/main/OutOfProcessLoad/KoOutOfProcessLoad.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/main/OutOfProcessLoad/KoOutOfProcessLoad.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>libs/main/OutOfProcessLoad/main.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>words/part/KWDocument.cpp <span style="color: grey">(77fb02f)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101794/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>