<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/123363/">https://git.reviewboard.kde.org/r/123363/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On abril 17th, 2015, 10:02 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Sorry, but i don't see why such lot of changes are needed in the frameworks branch if this used to work, please explain what has changed and why all this is needed, since it would seem to me that "if it worked before it should work now" without any change.</p></pre>
</blockquote>
<p>On abril 17th, 2015, 11:55 p.m. UTC, <b>Alex Richardson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think this is all related to the use of generate_export_header() and the fact that previously consumers would <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">include_directories(${INCLUDE_DIR}/okular/core ${INCLUDE_DIR}/okular/interfaces)</code> so that the headers in interfaces/ would find the okularcore_export.h in core/.
We could add this to the INTERFACE_INCLUDE_DIRECTORIES, but I don't like the fact that this adds headers such as global.h, utils.h and version.h which might conflict with other headers. Ideally the user should #include okular/core/global.h instead. I think changing the interfaces #include to "../core/okularcore_export.h" is the cleaner solution here.</p></pre>
</blockquote>
<p>On abril 18th, 2015, 11:10 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">any reason it can't just be #include "core/okularcore_export.h" ?</p></pre>
</blockquote>
<p>On abril 18th, 2015, 11:36 p.m. UTC, <b>Alex Richardson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That all depends on how the Okular headers are supposed to be included by other applications: Is it 1. <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">#include <okular/core/document.h></code>, 2. <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">#include <core/document.h></code> or 3. <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">#include <document.h></code>?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As it is currently only $PREFIX/include is added to the include dirs (option 1). Of course the OkularConfig.cmake file could be changed to option 2, then it can be #include "core/okularcore_export.h" or we add include/okular/core to the INTERFACE_INCLUDE_DIRECTORIES then #nclude okularcore_export.h still works.
I would personally prefer if third party applications have to use #include <okular/core/document.h> and more importantly #include <okular/interfaces/configinterface.h> since there is e.g. ktexteditor/configinterface.h as well (although I think that needs to be included as <ktexteditor/configinterface.h> so there should be no problem).
Also #include <okular/core/utils.h> makes it explicit what library the header comes from as opposed to #include <core/utils.h> which could be anything.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">But I am happy to use whatever solution you prefer, I just want to get kile to compile again.</p></pre>
</blockquote>
<p>On abril 19th, 2015, 12:23 a.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">frameworks branch should be just a port of master to KF5, not have new features, changes or improvements, so applications are supposed to include Okular frameworks the same way they are supposed to include Okular master, which honestly i don't know which way it is :D
Now if you want to improve this in master, i can agree that certainly #include <okular/core/document.h> does make sense as it is more obvious on what is being included.</pre>
</blockquote>
<p>On abril 20th, 2015, 3:28 p.m. UTC, <b>Alex Richardson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">From looking at the source code master already had a relative include in interfaces/. This was removed in 5da7c5f77d50b08fd3e2dac020de41e0813d92fc so the way I see it this RR is just restoring the old behaviour from master.
Okay to commit?</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Ha! :D Sure</p></pre>
<br />
<p>- Albert</p>
<br />
<p>On abril 15th, 2015, 11:55 p.m. UTC, Alex Richardson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Okular.</div>
<div>By Alex Richardson.</div>
<p style="color: grey;"><i>Updated abr. 15, 2015, 11:55 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
okular
</div>
<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;">Fix the export header not being found in interfaces/*.h</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Kile compiles now.</p></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>CMakeLists.txt <span style="color: grey">(836b1912f29a5a83b52f40eb52ab6bfee327dd68)</span></li>
<li>interfaces/configinterface.h <span style="color: grey">(d5e64336e7015a4b3068e10b2ce1caea77015da0)</span></li>
<li>interfaces/guiinterface.h <span style="color: grey">(4b4dca0c839fb32eb28c3d710f7b06401e9c4440)</span></li>
<li>interfaces/printinterface.h <span style="color: grey">(460d33eb4e25e09e89a11bbbd126a5d610472f7d)</span></li>
<li>interfaces/saveinterface.h <span style="color: grey">(8ae1bea648ab6a0452ea0964dbf20ca3e9fb8fc6)</span></li>
<li>interfaces/viewerinterface.h <span style="color: grey">(c174059d78b5f4c329232406cacb5e5f49bf7f44)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123363/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>