> On May 13, 2013, 10:48 p.m., Albert Astals Cid wrote: > > core/textdocumentgenerator.h, line 217 > > <http://git.reviewboard.kde.org/r/109021/diff/3/?file=142972#file142972line217> > > > > This should ideally be a function of the private class > > Azat Khuzhin wrote: > Could you explain why you think so? > > Fabio D'Urso wrote: > I haven't followed the discussion, but I think I can answer. > Because Okular tries to expose as little as possible to external modules, > in order to minimize chances of ABI breakage (the compiler doesn't know that > initializeGenerator exists so it can't bind your code on it). > > Azat Khuzhin wrote: > Sure I can do this, but as far as I know, ABI won't be broken, because > here I just add new method, like a new constructor. > > Albert Astals Cid wrote: > Actually think about it the other way around. Is there any need for it to > be in the public class? If not, shouldn't be there. > > Azat Khuzhin wrote: > We have one unresolved issue. > > I don't wanted to inheriting private class from QObject, do you think it > really need to do? > Also I suggest to move this into another patch. > > Albert Astals Cid wrote: > Why you need the private class to inherit from QObject to make > void initializeGenerator( TextDocumentConverter *converter ); private? > > Albert Astals Cid wrote: > Err, i mean to move void initializeGenerator( > TextDocumentConverter *converter ); to the private class, sorry :D > > Azat Khuzhin wrote: > Because it has some slots, example addAction: > Q_PRIVATE_SLOT( d_func(), void addAction( Action*, int, int ) ) > > And I don't want to call this using Q_Q(), because this add more relation > between private/public classes. > > Even if we do this (drop Q_PRIVATE_SLOT(), and add private slots into > private class, and inherit it from QObject), > we still have more methods/slots/signals from public class that we must > use from private: > setFeature() > SIGNAL(error()) > e.t.c. > > If you ok with that, I have to change it. > > Albert Astals Cid wrote: > I think I'm not explaining myself correctly. No worries, i'll do that > part myself :-)
Seems that I misunderstood you, I will look into final patches in repository for the right explanation. Thanks. - Azat ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109021/#review32466 ----------------------------------------------------------- On May 16, 2013, 10:02 p.m., Azat Khuzhin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109021/ > ----------------------------------------------------------- > > (Updated May 16, 2013, 10:02 p.m.) > > > Review request for Okular, Albert Astals Cid and Eike Hein. > > > Description > ------- > > Development history: > https://github.com/azat/okular/compare/master...font-selector-for-plain-text-formats > > http://quickgit.kde.org/?p=clones%2Fokular%2Fazatkhuzhin%2Fokular.git&a=commitdiff&h=font-selector-for-plain-text-formats&hp=master > (but it has some delay, about a day or so while this clone will be updated, > maybe because of mirrors?) > > Link to thread from mailing list: > http://comments.gmane.org/gmane.comp.kde.devel.okular/13279 > > > Diffs > ----- > > CMakeLists.txt 543e6df > conf/textdocumentsettings.ui PRE-CREATION > core/textdocumentgenerator.h dd75c5c > core/textdocumentgenerator.cpp f370ded > core/textdocumentgenerator_p.h 749d6f2 > core/textdocumentsettings.h PRE-CREATION > core/textdocumentsettings.cpp PRE-CREATION > core/textdocumentsettings_p.h PRE-CREATION > generators/epub/CMakeLists.txt f076ed9 > generators/epub/generator_epub.h cef2879 > generators/epub/generator_epub.cpp 59bb2bf > generators/epub/libokularGenerator_epub.desktop 5c853a3 > generators/fictionbook/generator_fb.h a898397 > generators/fictionbook/generator_fb.cpp 2317083 > generators/fictionbook/libokularGenerator_fb.desktop 099268c > generators/ooo/converter.cpp 1124e2a > generators/ooo/generator_ooo.h 3441c7a > generators/ooo/generator_ooo.cpp 793ee58 > generators/ooo/libokularGenerator_ooo.desktop 328ae26 > generators/txt/CMakeLists.txt 5a126b7 > generators/txt/generator_txt.h 5c15ec4 > generators/txt/generator_txt.cpp 93ca4aa > generators/txt/libokularGenerator_txt.desktop 235e23d > > Diff: http://git.reviewboard.kde.org/r/109021/diff/ > > > Testing > ------- > > Tested manually > > > Thanks, > > Azat Khuzhin > >
_______________________________________________ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
