> 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 > >
_______________________________________________ Okular-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/okular-devel
