<html> <head> <base href="https://bugs.freedesktop.org/" /> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW --- - handling of rtl text inversion is too naive" href="https://bugs.freedesktop.org/show_bug.cgi?id=55977#c4">Comment # 4</a> on <a class="bz_bug_link bz_status_NEW " title="NEW --- - handling of rtl text inversion is too naive" href="https://bugs.freedesktop.org/show_bug.cgi?id=55977">bug 55977</a> from <a class="email" href="mailto:tsdgeos@terra.es" title="Albert Astals Cid <tsdgeos@terra.es>"> Albert Astals Cid</a> <pre>(In reply to <a href="show_bug.cgi?id=55977#c3">comment #3</a>) > (In reply to <a href="show_bug.cgi?id=55977#c2">comment #2</a>) > > 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</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the assignee for the bug.</li> </ul> </body> </html>