> 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" ?
>
> Alex Richardson wrote:
> 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.
>
> Albert Astals Cid wrote:
> 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.
>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?
- 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
