Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#2539 closed task (fixed)

[PATCH] Port i18n scripts to Pology

Reported by: Adrián Chaves Owned by: leper
Priority: Should Have Milestone: Alpha 18
Component: Internationalization & Localization Keywords: patch
Cc: Patch:

Description

Our current i18n scripts are based on a fork (potter) of a piece of a Python library for a web framework.

Instead, we should make those scripts work with Pology (http://pology.nedohodnik.net//doc/user/en_US/ch-prog.html#sec-prfile). Pology is more reliable, more feature complete, and let us get rid of “potter” altogether.

Also, any improvements that we make to Pology due to our needs can be shared with a larger community. That said, I don’t think we will need to add anything to Pology, as our needs are very basic, and (apparently) completely covered by Pology’s feature set.

Attachments (4)

pology.diff (119.9 KB ) - added by leper 10 years ago.
WIP, still needs potter.util's relpath
pology2.diff (144.0 KB ) - added by leper 10 years ago.
Still needs a fix to pology to keep the source file ordering the same.
pology_change.diff (556 bytes ) - added by leper 10 years ago.
Change to pology to have a comment line for each source file (removes issue of weird wrapping with file paths with spaces)
pology_change1.diff (4.6 KB ) - added by leper 10 years ago.
Change to pology to add msgstr[i] for all i if we have plurals.

Download all attachments as: .zip

Change History (16)

comment:1 by leper, 10 years ago

Milestone: Alpha 17Alpha 18

comment:2 by leper, 10 years ago

Owner: set to leper
Status: newassigned

by leper, 10 years ago

Attachment: pology.diff added

WIP, still needs potter.util's relpath

by leper, 10 years ago

Attachment: pology2.diff added

Still needs a fix to pology to keep the source file ordering the same.

comment:3 by leper, 10 years ago

Keywords: patch review added
Summary: Port i18n scripts to Pology[PATCH] Port i18n scripts to Pology

The change to Pology itself still needs to be pushed upstream.

We keep potter's (actually babel's) jslexer, as there is nothing in Pology that could replace that and it's working.

comment:4 by leper, 10 years ago

Resolution: fixed
Status: assignedclosed

In 15894:

Switch i18n scripts to use Pology. Fixes #2539.
Still requires a patch to Pology to keep the source file ordering.

comment:5 by leper, 10 years ago

Keywords: review removed

comment:6 by Adrián Chaves, 10 years ago

leper, if the patch to send upstream is the last one, I see a # TODO added there that they will not like much. I can add the patch to the KDE Review Board nonetheless, and as soon as they accept it upload the changes to the upstream repository.

by leper, 10 years ago

Attachment: pology_change.diff added

Change to pology to have a comment line for each source file (removes issue of weird wrapping with file paths with spaces)

by leper, 10 years ago

Attachment: pology_change1.diff added

Change to pology to add msgstr[i] for all i if we have plurals.

in reply to:  6 comment:7 by leper, 10 years ago

Replying to Gallaecio: Both pology_change.diff and pology_change1.diff should be submitted. .The file descriptions here should probably be included for the reviewers. Two patches because the code changes aren't really related.

If they don't like pology_change.diff as is, I'd be interested if they'd accept it if I changed the patch to use an additional parameter (defaulting to the current behaviour) to get our wanted behaviour.

comment:8 by Adrián Chaves, 10 years ago

comment:9 by Adrián Chaves, 10 years ago

Upstream comment by Chusslove Illich, Pology maintainer:

to_lines method should only format the message as it is, and not modify it. So the change in the number of msgstr fields should happen elsewhere. Then, it is not clear to me when the number of msgstr fields should be adapted to the number given by plural header. But it most certainly shouldn't happen always.

So, now I committed the following, conservative addition. The sync method of Catalog has a new parameter, fitplural, which when set to True (default is False) will cause the number of msgstr fields to be adapted. But only on those plural messages case where all existing msgstr fields are empty. Can this satisfy the original use case?

I also read about the other patch on the given link, about source references where file names have spaces. The deeper issue is that in the PO format source references are separated by spaces. So even if they would be written out in one line by Pology, any subsequent read/write of the PO file (by Pology or another PO tool) would not be able to tell them apart from multiple source references. I suggest instead that the code which is using Pology to set these kind of source references, replaces spaces in file names with some other character. Maybe even with non-breaking space (u"\x00a0" in Python).

comment:10 by leper, 10 years ago

The fitplural way is perfect for our use case. Thanks.

Yes replacing spaces fixes the issue of parsing that, but that isn't the real problem here (I might still do that). But unless we replace spaces with some visible character (which could cause other issues) the source references just become unreadable if they are wrapping around and you have spaces in there that don't tell you anything. An option to have source references on one line each would be nice.

comment:11 by leper, 10 years ago

In 15913:

Use fitplural added in upstream revision 1404284. Refs #2539.

comment:12 by leper, 9 years ago

In 15993:

Replace spaces in source file paths with non-breaking spaces. Refs #2539.

This way our map file paths are not split due to word wrapping in the po files.

Note: See TracTickets for help on using tickets.