Re: [Maposmatic-dev] [PATCH 1/4] Use Pango to render the text for the index

[Maxime Petazzoni]

Hi,

* Thomas Petazzoni <address@hidden> [2010-02-11 23:25:16]:

> -import sys, os, tempfile, psycopg2, re, math, cairo, locale, gzip, csv
> +import sys, os, tempfile, psycopg2, re, math, cairo, locale, gzip, csv, 
> pango, pangocairo

Don't be afraid to split your imports on multiple lines. Ideally, one
per line, alphabetically sorted (see PEP-8).

> +    def _get_font_parameters(self, cr, pc, fontsize):
> +        layout = pc.create_layout()
> +        fd = pango.FontDescription("DejaVu")
> +        fd.set_size(int(fontsize * 1.2 * pango.SCALE))
> +        f = layout.get_context().load_font(fd)
> +        heading_fm = f.get_metrics()

It looks like you're using the font name 'DejaVu' in several places
around the code. A constant would be nice (or better, a
self._get_index_font_name(), so we can make it more clever later).

The rest LGTM, except for one detail: DOCSTRINGS! I'm not even sure what
_get_font_parameters is really supposed to do by reading it. So please
comment all this. Here again, see PEP-8 for docstring formatting.

Otherwise, thanks for the work, Pango is a great enhancement for real
text rendering (all we'll get indexable indexes!).

- Maxime
-- 
Maxime Petazzoni <http://www.bulix.org>
 ``One by one, the penguins took away my sanity.''
Linux kernel and software developer at MontaVista Software

signature.asc
Description: Digital signature