[Poppler-bugs] [Bug 55977] handling of rtl text inversion is too naive

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=55977

--- Comment #4 from Albert Astals Cid <tsdgeos at terra.es> ---
(In reply to comment #3)
> (In reply to comment #2)
> > Hi, thanks for the contribution!
> 
> hello, thanks a lot for considering my patch. and more for your work on this
> great project.
> > 
> > Some weak points of the patch (have not really looked at the contents yet,
> > just at the form):
> >  * You are missing support for the cmake build system
> i don't know cmake, but will happily add support for it. how and where?

Have a look at CMakeLists.txt and the various find modules at cmake/

> >  * You are exposing the #if HAVE_ICU in TextOutputDev.h (for a variable i
> > never see you assign or change)
> the reorderingMode enum has values taken from an icu header. indeed the enum
> declaration itself should depend on have_icu. will fix.

But you don't use the enum anywhere, do you? I mean i only see the same value
being passed everywhere

> >  * If i have icu and fribidi it will say that both are used, when that's not
> > true (please switch it to something like the cms check that says "with lcms"
> > or "with lcms2" depending on the library used)
> indeed, this might need some thought in configure.ac. like trying fribidi
> only if icu is missing. this is the behaviour in the TextOutputDev anyway.
> what would you think?

Yes, makes sense

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler-bugs/attachments/20121016/c1ae3d00/attachment.html>