<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/108632/">http://git.reviewboard.kde.org/r/108632/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 29th, 2013, 9:14 a.m. UTC, <b>Bernhard Beschow</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/108632/diff/1/?file=109457#file109457line95" style="color: black; font-weight: bold; text-decoration: underline;">src/plugins/runner/kml/KmlPlugin.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<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">95</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">connect</span><span class="p">(</span> <span class="n">m_model</span><span class="o">-></span><span class="n">fileManager</span><span class="p">(),</span> <span class="n">SIGNAL</span><span class="p">(</span><span class="n">fileRemoved</span><span class="p">(</span><span class="n">QString</span><span class="p">)),</span> <span class="k">this</span><span class="p">,</span> <span class="n">SLOT</span><span class="p">(</span><span class="n">cleanupTemporaryFiles</span><span class="p">(</span><span class="n">QString</span><span class="p">))</span> <span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This creates a circular dependency: FileManager already uses FileLoader which uses MarbleRunnerManager which uses the KML plugin. So if the KML plugin used FileLoader, a circle is created, which might call for trouble in the future.
To avoid the circular dependency, it should be possible to create a subclassed GeoDataDocument which removes the temporary file structure upon its deletion. GeoDataDocuments are created in the KmlParser class, which is fortunately part of the KML plugin. So this approach might actually work. In additon, this approach avoids the need to pass MarbleModel to the parsers.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'd rather call it a circular relation, not a dependency. Not a real argument against the approach in my opinion.
Still the idea to introduce a GeoDataDocument derived class to track the temporary files is very good: It reduces the complexity and amount of code changes needed for the implementation significantly.
</pre>
<br />
<p>- Dennis</p>
<br />
<p>On January 30th, 2013, 6:49 p.m. UTC, Dennis Nienhüser wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Marble.</div>
<div>By Dennis Nienhüser.</div>
<p style="color: grey;"><i>Updated Jan. 30, 2013, 6:49 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;">Extends the kml plugin to support opening .kmz files.
- ParseRunner gets access to MarbleModel
- KML plugin has an optional dependency on quazip
- When opening a .kmz file, quazip is used to extract the .kmz to a temporary location
- Last .kml file found in the .kmz is opened
- If the .kml is closed in Marble, the temporary files are removed
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">Opening some random .kmz files found on the Internet works.
TODO (for separate patches):
- Resolve files (images) in .kmz files correctly
- Fix Marble to delete plugins / close files correctly on shutdown
- Register .kmz files for opening with Marble
</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=310059">310059</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>cmake/modules/Findquazip.cmake <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/runner/kml/CMakeLists.txt <span style="color: grey">(35e00dc)</span></li>
<li>src/plugins/runner/kml/KmlDocument.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/runner/kml/KmlDocument.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/runner/kml/KmlParser.h <span style="color: grey">(a5e4c5f)</span></li>
<li>src/plugins/runner/kml/KmlParser.cpp <span style="color: grey">(b2ff585)</span></li>
<li>src/plugins/runner/kml/KmlPlugin.cpp <span style="color: grey">(fc23402)</span></li>
<li>src/plugins/runner/kml/KmlRunner.cpp <span style="color: grey">(3278c78)</span></li>
<li>src/plugins/runner/kml/KmzHandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/runner/kml/KmzHandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/108632/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>