[Okular-devel] Review Request 109021: Font selector for TextDocumentGenerator

[Albert Astals Cid]

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

Why you need the private class to inherit from QObject to make         void initializeGenerator( TextDocumentConverter *converter ); private?


- Albert


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


On May 16, 2013, 8:36 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, 8:36 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20130516/8b29f7fc/attachment.html>