> 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
> 
>

_______________________________________________
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Reply via email to