[Okular-devel] Review Request 123363: Fix the export header not being found in interfaces/*.h

[Alex Richardson]

> On April 17, 2015, 11:02 p.m., Albert Astals Cid wrote:
> > 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.
> 
> Alex Richardson wrote:
>     I think this is all related to the use of generate_export_header() and the fact that previously consumers would `include_directories(${INCLUDE_DIR}/okular/core ${INCLUDE_DIR}/okular/interfaces)` 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.
> 
> Albert Astals Cid wrote:
>     any reason it can't just be #include "core/okularcore_export.h" ?

That all depends on how the Okular headers are supposed to be included by other applications: Is it 1. `#include <okular/core/document.h>`, 2. `#include <core/document.h>` or 3. `#include <document.h>`?

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.

But I am happy to use whatever solution you prefer, I just want to get kile to compile again.


- Alex


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123363/#review79148
-----------------------------------------------------------


On April 16, 2015, 12:55 a.m., Alex Richardson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123363/
> -----------------------------------------------------------
> 
> (Updated April 16, 2015, 12:55 a.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> -------
> 
> Fix the export header not being found in interfaces/*.h
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 836b1912f29a5a83b52f40eb52ab6bfee327dd68 
>   interfaces/configinterface.h d5e64336e7015a4b3068e10b2ce1caea77015da0 
>   interfaces/guiinterface.h 4b4dca0c839fb32eb28c3d710f7b06401e9c4440 
>   interfaces/printinterface.h 460d33eb4e25e09e89a11bbbd126a5d610472f7d 
>   interfaces/saveinterface.h 8ae1bea648ab6a0452ea0964dbf20ca3e9fb8fc6 
>   interfaces/viewerinterface.h c174059d78b5f4c329232406cacb5e5f49bf7f44 
> 
> Diff: https://git.reviewboard.kde.org/r/123363/diff/
> 
> 
> Testing
> -------
> 
> Kile compiles now.
> 
> 
> Thanks,
> 
> Alex Richardson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20150418/5bd9a286/attachment.html>