[reportlab-users] Hebrew Support Patch

[Moshe Wagner]

Thanks for all your comments.

I'm starting to like python too, although I'm still overwhelmed by the
fact that the same variable could be of different types without any
warning.

Besides "warnings.warn" that didn't seem to work (or did I
misunderstand you?), I tried to follow your suggestions.

I'll post the new code in my reply to Andy's email.

Thanks again,
Moshe

On Fri, Jun 5, 2009 at 3:54 PM, Marius Gedminas<marius at gedmin.as> wrote:

> On Fri, Jun 05, 2009 at 10:02:57AM +0300, Moshe Wagner wrote:

>> Thanks for your corrections.

>> As I said, I'm new to python, so I didn't realize you could use loops

>> that way.

>

> Welcome.  I think you'll enjoy Python.  I do.

>

>> This line:

>> "if type(line) in (FragLine, ParaLines):"

>>  didn't seem to work, as type(line) always returned "instance", so I

>> kept it the way I had it.

>

> Old-style classes FTW.

>

> isinstance(line, (FragLine, ParaLines)) should work, and is better style

> anyway.

>

>> Another thing I did wrong is implementing '.reverse()' manually, not

>> realizing it exists.

>>

>> So here's the new code. Clearly looks better:

>>

>> ######################

>> # Hebrew text patch, Moshe Wagner, June 2009

>> # <moshe.wagner at gmail.com>

>>

>> #This code fixes paragraphs with RTL text

>>

>> # It does it by flipping each line seperatly.

>> #     (Depending on the type of the line)

>>

>> # If fribidi cant be imported, it does nothing

>>

>> try:

>>       import pyfribidi

>

> Please don't use tabs for indentation.  4 spaces are traditional.  (The

> Python coding style guidelines are almost universally accepted; you can

> find them at http://www.python.org/dev/peps/pep-0008/)

>

> Although last time I checked Reportlab didn't follow PEP-8 faithfully.

>

>>       for line in blPara.lines:

>>               if line.__class__.__name__ in ('FragLine', 'ParaLines'):

>

> isinstance(line, (FragLine, ParaLines))

>

>>                       """When the line is a FragLine or ParaLines, It's

>>                       text attribute of each of it's words is flipped.

>>                       Then, the order of the words is flipped too,

>>                       So that 2 word parts on the same line

>>                       will be in the right order """

>

> Comments are written like this:

>

>                        # When the line is a FragLine or ParaLines, It's

>                        # text attribute of each of it's words is flipped.

>                        # Then, the order of the words is flipped too,

>                        # So that 2 word parts on the same line

>                        # will be in the right order

>

> Also, it's "its", not "It's".

>

>>                       for word in line.words:

>>                               word.text = pyfribidi.log2vis (word.text, base_direction = pyfribidi.ON)

>

> PEP-8 recommends no space in front of ( following a function name, and

> no spaces around = when used to pass keyword arguments.

>

>>

>>                       line.words.reverse()

>>

>>               elif line.__class__.__name__ == 'tuple':

>

> isinstance(line, tuple)

>

>>                       """When the line is just a tuple whose second value is the text.

>>                       since I coulden't directly change it's value,

>>                       it's done by merging the words, flipping them,

>>                       and re-entering them one by one to the second attribute """

>>

>>                       s = ' '.join(line[1])

>>                       s = pyfribidi.log2vis( s, base_direction=pyfribidi.ON)

>>                       line[1][:] = s.split()

>>               else:

>>                       print line.__class__.__name__

>

> This is for debugging purposes, right?

>

>>                       #print self.blPara.lines[i][1][1:]

>> except ImportError:

>>       print "Fribidi module not found; You will not have rtl support for

>> this paragraph"

>

> It's typically better to keep the part between try: and except: as small

> as possible, thus it's idiomatic to write

>

>    try:

>        import pyfribidi

>    except ImportError:

>        print >> sys.stderr, "Fribidi module not found ..."

>    else:

>        # code

>

> Printing to sys.stdout (or even sys.stderr) from a library is perhaps

> bad style, but I'm not sure what's the right solution here.  Either the

> logging or the warnings module would be better.  E.g.

>

>    try:

>        import pyfribidi

>    except ImportError:

>        warnings.warn("Fribidi module not found ...")

>    else:

>        # code

>

>> ######################

>>

>> Is there any chance this could be added to the official code?

>

> I'm not affiliated with Reportlab in any way (other than being a happy

> user and a some-time contributor a long time ago), but I'd say yes.  The

> chances would be better if you added some tests for the new code.

>

> Marius Gedminas

> --

> We have enough youth, how about a fountain of SMART?

>

> -----BEGIN PGP SIGNATURE-----

> Version: GnuPG v1.4.9 (GNU/Linux)

>

> iD8DBQFKKRV6kVdEXeem148RAmRgAJ9dhfak9mqݪ揕燼Ʒ领鸣쐻

> TWooIPPcqwnXOOM6XqZVh7E=

> =DBGB

> -----END PGP SIGNATURE-----

>

> _______________________________________________

> reportlab-users mailing list

> reportlab-users at reportlab.com

> http://two.pairlist.net/mailman/listinfo/reportlab-users

>

>