[Okular-devel] Review Request: Settings separation: observer.h install and compilation correction on Windows

[Albert Astals Cid]

> On Nov. 1, 2012, 4:36 p.m., Albert Astals Cid wrote:
> > I am sincerely not sure observer.h is ready for prime time installation, i mean, it has a global list of ids/priorities hardcoded in it, maybe we should rework the ids so that you can register yourself and get your id back? About the priorities i guess we could rename them to be less "okularpart" specific? What do you think?
> 
> Bogdan Cristea wrote:
>     Before I start to code, I would like to discuss the best approach. Hard coded IDs should be removed, so I propose to use instead Observers defined as singletons. When the observer is created an unique ID is generated (since all observers have a common base, one could use a static data member defined in the base class in order to generate unique IDs).

Singletons are something i try to use as few as possible, I'd go for a more simple API

For what i see, it seems the observerId is only used inside the Document class, so i'd add a Document::registerObserver(DocumentObserver *observer) that takes the DocumentObserver and sets it the next "free" id from the document. That'd mean adding an int to DocumentPrivate, making DocumentObserver::observerId non virtual, add it as int inside DocumentObserver::Private (making Document be friend of DocumentObserver).

What do you think? 

Wait we already have a Document::addObserver so forget about the registerObserver and use that one?


> On Nov. 1, 2012, 4:36 p.m., Albert Astals Cid wrote:
> > conf/settings.kcfgc, line 7
> > <http://git.reviewboard.kde.org/r/107047/diff/2/?file=92392#file92392line7>
> >
> >     SettingsCore is part of libokularcore, so using OKULAR_EXPORT is fine, no?
> 
> Bogdan Cristea wrote:
>     settings_core.h exposes methods found in okularcore library, while settings.h exposes methods found in okularpart library. In Windows, when the second library is build, settings_core.h should import its symbols, while settings.h should export them. For Linux there is no problem since all library symbols are exported by default.

Oh sorry, this is for settings.kcfgc, not for settings_core.kcfgc

Still, we are not installing settings.h so can't we just drop the visibility? I.e. other classes of the part like part.h have no visibility definition, no?


- Albert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107047/#review21327
-----------------------------------------------------------


On Oct. 26, 2012, 7:57 a.m., Bogdan Cristea wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107047/
> -----------------------------------------------------------
> 
> (Updated Oct. 26, 2012, 7:57 a.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> -------
> 
> This patch is related to settings separation for the frontend and the backend. It proposes the installation of core/observer.h and corrects compilation of okular on Windows (using KDE on windows):
> - after separation, settings.h and settings_core.h need to use different precompiler switches for exporting/importing symbols
> - add definitions needed to activate these switches on Windows
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 063778f 
>   active/components/CMakeLists.txt 19b759e 
>   conf/settings.kcfgc 5e2ec95 
>   okular_part_export.h PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/107047/diff/
> 
> 
> Testing
> -------
> 
> yes
> 
> 
> Thanks,
> 
> Bogdan Cristea
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20121113/216f013f/attachment.html>