OOP & PSR-0-ify gettext .po file parsing and generation

File format:
http://www.gnu.org/savannah-checkouts/gnu/gettext/manual/html_node/PO-Fi...
http://docs.webtranslateit.com/file_formats/gettext_po/

A few gettext related classes to get inspiration from:
http://svn.automattic.com/wordpress-i18n/tools/trunk
http://pypi.python.org/pypi/Translate%20Toolkit (python)

One more gettext class:
http://codeigniter.com/wiki/Category:Internationalization::Gettext

Wordpress does generate po files: http://svn.automattic.com/wordpress-i18n/tools/trunk/makepot.php
Found another, Pear: http://pear.php.net/package/File_Gettext/

formatString() in #13 works fine, except it removes the trailing space from every string part. An additional parameter for wordwrap() restores the space:

    $parts = explode("\n", wordwrap(str_replace('\n', "\\n\n", $string), 70, " \n"));

My first wobbly steps in Drupal OOP. Be kind on me, but hard on the code ;) Added a translations database iterator to accompany to Gábor's (#13) Writer class.
I sure hope it is worth to use an iterator here because processing db records takes twice as much time as the usual db_query + foreach. Yes I see the benefits of seek based batch imports, but do we need all this flexibility when reading translations from the database?
The patch requires #532512: Plural string storage is broken, editing UI is missing.

One more reference: Gettext and locale API project.
I've started to sketch out the code and expect to have some results in the following days. Just in time for the sprint ;)

The sketch code I'd like to share. It covers the procedural part of the gettext import/export handling, the batch processing and the required methods from a GettextData class. No executable code yet, but big steps. Leaving out all fault handling, exceptions and user experience. Next is to sketch out the Gettext class(es).

The patch contains:

• abstract class Reader
• abstract class Writer
• class PoWriter extends Writer
• interface GettextInterface
• class GettextFileInterface implements GettextInterface

I need review and advise on how to connect a (generic) data source/target to the reader/writer. And whether to use an abstract class, use an traversable or interator, use a steam wrapper.

A quick UML of the Writer + GettextInterface classes: https://skitch.com/erikstielstra/8csg9/gettext-uml.graffle

For the architecture, you may ignore the gettext.sketch.inc file in the beginning of the patch. This was written before I started working on the classes. It should read $file = new GettextFileInterface('public://test-1.po.txt'); Below you find a piece of testing code I used for the PoWrite class:

    use Drupal\Core\Gettext\GettextFileInterface;
    use Drupal\Core\Gettext\PoWriter;
     
    function test_gettext() {
     
      $langcode = 'nl';
      $translations = array(
        (object) array(
          'source' => 'home',
          'translation' => 'thuis',
          'plural' => 0,
          'context' => '',
        ),
        (object) array(
          'source' => 'delete',
          'translation' => 'verwijderen',
          'plural' => 0,
          'context' => '',
        ),
        (object) array(
          'source' => array('1 day', '@count days'),
          'translation' => array('1 dag', '@count dagen'),
          'plural' => 1,
          'context' => '',
        ),
      );
     
      $file = new GettextFileInterface('public://test-1.po.txt');
      $target = new PoWriter($file, $langcode);
      $header = array(
          'authors' => array('me', 'you'),
          'po_date' => date("Y-m-d H:iO"),
          'plurals' => 'nplurals=2; plural=(n > 1);',
        );
      $target->setMetaData($header, $langcode);
      foreach ($translations as $translation) {
        $target->write($translation);
      }
    }

For simplicity I only focused on the Write side. Architecturally the Read side should be similar. Write is an abstract class, PoWrite, DatabaseWrite are derived from that and handle the syntax specific Writing. Translations are transferred between Read and Write as an (translation) object. PoWrite recieves a translations object and passes a set of formatted strings to the data interface. But this is where my problems begin. I thought to use the stream wrapper interface for file handling. But could not make a smooth connection with it. In the patch the GettextFileInterface class uses the PublicStream. This should be replaced by a TranslationStream class which uses the translation directory instead of the files directory.

Reader and PoReader class added.
high level functions in gettext.sketch.inc modified to match the current architecture.

Attached is a tiny update of some classes + inclusion of a test-script + a naive Iterator example of a PoInfiniteProducer which never fails to produce yet another translation structure.

I'm still puzzled by the use-case of Stream versus Iterator. And waiting for fuhby's lexer :)

To test run

drush @drupal.d8 php-script core/lib/Drupal/Core/Gettext/testGettext.php

Next check the public file system for new files.

I'll jump back on it when I can contribute.

Just some notes:

As we are now using symfony I checked their code. A simple parser: http://trac.symfony-project.org/ticket/9582 lead to inclusion into https://github.com/symfony/Translation/tree/master/Loader (unfortunately master).

Symfony PoDumper https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Tra... has some nifty escaping

Also the plural processing is nice: https://github.com/symfony/Translation/blob/master/PluralizationRules.php

Should we spent time on these?

It looks like the Symfony implementation is not stream aware thus excluding it's use for us right now.

But the other bits (escaping + plural) could help us I guess.

I'm waiting for fuhby implementation of his stream PO parser to review (or match my thought against :p)

The entire symfony translator library looks really interesting and I think we could do many really awesome things with that. It basically covers most of what I talked to you, Gabor, so enthusiastically about during our sprint. Please take a couple of minutes to look at this https://github.com/symfony/Translation in general .... The Parser, Dumper, etc. in this library are indeed very simplistic and we are going to need more (and I already have more) but the other stuff there looks really sweet at a first glance.

I've got a lot of work to catch up with due to DCon and won't be able to finish this before the weekend. I am looking forward to digging even deeper into this... Sorry for the delay.

I'm more confused after reading #569004: Add support for seek based batch import of .po files and project http://drupal.org/project/gettextapi referred in that and this issue.

From @Sutharsan diagram and @fubhy intention to write a parser to _read_ .po files the title (and history) of this issue suggest something different: _write_ .po files.

Considering the Symfony way (Having a Dumper, Writer, Parser all in one Class tree) I still expect something similar in this patch.

So am I missing something important here?

(I'm still grasping StreamWrapper which are renamed through #1475020: Convert stream_wrappers.inc to PSR-0)

Concerning Symfony translation component:

Translation
- we cannot get access to the read entries so reading and then stuff items into the database is a nogo. The translation is done by echo $translator->trans('Hello World!') . "\n"; .

PoFileLoader is missing features we probably need:
- it cannot parse our files ... ours are missing a white-space before each entry (http://www.gnu.org/software/gettext/manual/gettext.html#PO-Files). I checked ie drupal-7.11.ca.po and commerce_kickstart-7.x-1.3.da.po

- as documented by PoFileLoader::parse()

     * - No support for comments spanning multiple lines.
     * - Translator and extracted comments are treated as being the same type.
     * - Message IDs are allowed to have other encodings as just US-ASCII.

The following example is translation in action with a whitespace fixed po-file. And a prepared vendor tree with two added components:

ls -l core/vendor/Symfony/Component/
...
Config -> ../../../../../symfony/src/Symfony/Component/Config
Translation -> ../../../../../../symfony/Translation

drush php-script ../../code/translation-test.php 
Home : Inici
Enable !title : Habilita !title
Fields type(s) in use : Tipus de camp(s) en ús

use Symfony\Component\Translation\Translator;
use Symfony\Component\Translation\MessageSelector;
use Symfony\Component\Translation\Loader\ArrayLoader;

$filename = "drupal-7.11.ca.po";
$fullpath = dirname(__FILE__) . '/' . $filename;

$translator = new Translator('ca', new MessageSelector());
$translator->addLoader('po', new Symfony\Component\Translation\Loader\PoFileLoader());
$translator->addResource('po', $fullpath, 'ca');

$lookup = array(
  'Home',
  'Enable !title',
  'Fields type(s) in use',
);
foreach ($lookup as $english) {
  echo "$english : " . $translator->trans($english) . "\n";
}

My conclusion for now:

1. I do not have enough knowledge about resource handling by Symfony thus cannot compare that with our Drupal needs. The resources are loaded when needed (see the Translator::trans code)
2. All different file formats do look interesting enough if and when Drupal needs this.
3. The PoFileLoader cannot load Drupal .po files
4. The FileLoaders are not Batch aware as I guess there is no needs for this within Symfony context (?).
5. We must write our own Batch aware readers and writers. (duh)

After puzzling a lot how to manage state (and streams work) I want to propose to build the following together with fuhby:

1. BatchStreamManager class

- This manage any Stream for read/write operation.
- Knows how to recover a stream for the next batch op.

2. POStreamHandler class

- Implements get/setHeader to read or write the PO Header
- read/write PO items
- Uses the stream managed by BatchStreamManager

3. POItem class

4. POHeader class

An small code example would look like the following.

//Batch setup:

// The file from either public:// private:// temporary:// or even http://
$uri = 'public://text-file.txt.po';

// $mode = read / write
$bsm = new BatchStreamManager()
$bsm->open($uri, $mode, $options);
// uses file_stream_wrapper_get_instance_by_uri($uri);

// The stream to work on.
$my_s = $bsm->getStream();
$po = new POStreamProcessor($my_s);

// read header
$po->getHeader();

// write mode
$po->setHeader($my_header);

// Read next item
$po_item = $po->read();
// read 10 items
$po_items = $po->read(10);

// Write next item(s)
$po->write($po_item);

// before ending batch iteration
$state = $bsm->getBatchState();
// Store state
batch_set_state($state);

next batch iteration

// Recove state
$state = batch_get_state();
$bsm = new BatchStreamManager()
$bsm->setBatchState($state);

$my_s = $b->getStream();
$po = new POStreamProcessor($my_s);
$po->read(20);

For the DB side of this issue we should have an abstraction for reading/writing POItems to the database as well.

For this to work POStreamProcessor must set the seek position to the end of the last read item and not just read chunks of bytes.

What we gain is:

1. We decoupled Streams from managing Batched (the batch just uses the stream)
2. We decoupled POProcessor from Streams (just uses the stream)
3. A unified BatchStreamManager for whatever stream we can get.
4. A PO reader/writer
5. A clear definition of what a POItem / POHeader is.

@fubhy: What do you think of this decoupling? You need to provide for POItem and POHeader (sorry about that :p)

I agree on the separation between stream interface and po lexer/generator. The stream object could be swapped for a database interface when translations come from the database, and the po generator can be swapped for a pot generator when we download a translation template. But I doubt the usefulness of a header and a po object.

The po header is almost empty. The plural formula is the only valuable part. The rest of the po header consist of static data and the header properties are not used in our context.
I have thought about a po object too. It can be usefull as a definition of translation data, but I can't think of any methods in this object. It is just transferred from the reader to the writer. Thats all. Further I have concerns about the performance impact of instantiating this object thousends of times.

The plural formula is the only valuable part.

Don't forget about number of plurals

Rereading #42 - #45 and trying to build the BatchStreamManager I think we can do it with just a new Interface and follow the original thoughts of @Sutharsan having a POReader / POWriter as having a POProcessor would require different state management.

Thus a StreamStateInterface allowing for a POReader / POWriter to implement.

interface StreamStateInterface {
  public function setStreamState( array state);
  public function getStreamState();
}

Each POReader / POWriter implements StreamStateInterface and are capable to open a Stream by using

class POReader implements StreamStateInterface {
  public function open($uri, $mode, $options, &$opened_url) {
    $s = file_stream_wrapper_get_instance_by_uri($uri);
    if ($s) {
...
  }

  /**
   * Read a PO Item by reading severals lines from the file.
   *
   * @return a structured array | object?
   */
  public function readItem() {
  }
}

fubhy can now build this POReader and the Batch can set the state cleanly.

This seems way cleaner then #42 or #29

What do you think?

@clemens, your proposal looks good. Indeed more straight forward that our earlier ideas. Remember that besides POReader and POWriter we need and a DBReader and DBWriter. Readers can have the same base class or interface and the Writers likewise.

Sorry for disappearing from this issue for so long... Coming back from Denver I found an overwhelming amount of work on my desk and it took all of my focus to get that stuff over with as soon as possible. Anyways, here are my conclusions and opinions from the last couple of comments on this issue:

Even if this might be obvious: Let's first clarify what "Reading" / "Writing" / "Parsing" means.

Reading: Reading means (in my mind): "Find a msgid in the database / file (in general "source") to identify the respective translated strings". This is more like a "query" behavior (query for a specific condition "msgid" or even something else) to retrieve a subset of the available translated strings from whatever source we are querying. In case of normal t() invocations this would be a query for a "msgid" for example with just one return value.

In a more complex scenario (and if we had that data available in the database) we could also query for the context of a translated string or whatever else might be of interest. Consequently "Reading" is, by default, not a line-by-line reading ("batchable") process. It is really more like a single query (I believe that this is very important).

Hypothetically speaking "reading" from a .po file (querying) could, in a custom environment, replace the currently used db query for t() invocations and instead directly query for specific msgid's from a .po file (really just hypothetically speaking). So in order to make the Interface very solid and clean the "read" operation should just provide a "query for one, return one" functionality.

Parsing: This is basically the same as "reading" but doesn't have a conditional query component. It is a plain iterational procedure where we extract one "Gettext 'node' object" after the other from the data source until it is depleated (EOF is reached or all database record have been returned).

When parsing a .po file (exporting from a .po file) or exporting from the database we could have a special case operation where we don't query / read for a specific msgid but instead repeatedly ask for the next "Gettext 'node' object" in the source (file stream vs. db connection). This operation relies on an internal pointer in the data source (record id in the database vs. current file pointer / position in a file stream). This operation is batchable!

$nodes = array();

$stream = new DrupalPublicStreamWrapper();
$stream->setUri('some-po-file.po');

$pohandler= new GettextPoHandler($stream);

// Or maybe even set the data source in a separate method.
$pohandler->setDataSource($stream); 

// The current position (position of last node object that was parsed).
$position = 5 // E.g. the number of the node that we parsed in the last iteration.
$nodes += $pohandler->getNext($position);

// Or parse a specific number of nodes with a start and offset argument.
$offset = 10;
$nodes += $pohandler->getNodes($position, $offset); // Parses node 6-15.

// After we got a couple of nodes we can write them into the database.
$dbhandler = new GettextDbHandler(); // Defaults to the current db connection by default but also accepts a custom db connection as an argument in the constructor?!
$dbhandler ->writeNodes($nodes);

// Now that we inserted all these nodes into the database we can query for them in the db.
$result = $dbhandler->query('This is a msgid that I want to have translated', 'fr'); // 'fr' to override the currently active language in our environment to french (hypothetically) :).

So let's please make a distinction between "reading" and "parsing" :).

Looking at the bigger picture we can probably get to a point where importing / exporting easily works both ways (export from .po file => import into database // export from database => import into .po file).

If you ask me we dont even need two separate classes for reading / writing (the source & destination are the same (db <=> db / .po file <=> .po file) and both operate on that one source (file stream vs. database connection). So why make a disctinction? Just because the "direction" of the operations are different (get vs. set)? So.. Let's get to the "writing"

Writing: This is pretty obvious. Either write a "Gettext 'node' object" to a .po file or create a record for it in the database.

While "exporting" is a repeated "read" operation "importing" basically is a repeated "write" operation.

Practically speaking this is what we could do:

1. Extract node objects from your environment (export from db) => Iterate over the generated "Gettext 'node' objects" and write (import) them into a (newly generated) .po file via our famous stream wrapper.
2. Parse a whole .po file (export data from the .po file in a node-by-node manner) => Iterate over the generated "Gettext 'node' objects" and write (import) each one of them into the database.

On a side note: I think the title of this issue is indeed slightly outdated if we start to discuss complex proposals like the stuff we have been discussing in the last couple of replies. We should either rename the title of this issue or split this discussion into multiple issues (although the whole architecture [as I would like to see it] is so tightly integrated with each other that we cant really separate it if we really want to build it like this [yes, lets do it!]). :)

Looks to me that we all have the same architecture in mind now. A source interface class for database or po file and a reader/parser/writer which works with the source to move data in and out the source interface.

I have no problem with combining the read and the write functionality into one class, theoretically it would be sane. It is that I ran into difficulties when building the class that I decided to split it. Also I saw no use case where we need both reading and writing in the same context. But if you manage to get the two into one without too much hassle, by all means go ahead.

But I have serious concerns of using the gettext classes for t(). t() should be light weight. Using a gettext interface class is theoretically correct and may therefore be preferable. But we risk performance as t() is called very often. If any I would give it the lowest priority in the development.

Time for code.

Well for one this is not trivial :p

To run this from drupal root.

php core/lib/Drupal/Core/Gettext/testGettext.php

I did some more testing on drupal-7.11.ca.po file which is now partly parse by POReader in a crappy way.

Things to fix are:
- it does not stop reading.
- some plural lines are wrong parsed .. see TODO in POReader (set breakpoint in saveOneString on ksort line)
- the header is now an array ... I really like to have the full header structure
- POReader::readLine() has state related variables lying around. These must be defined in the class header.
- GettextFileInterface is opening a public stream ... this should not be its responsabiliy ... it should only manage a stream. We can test for is_local when assigning it a stream.

This to refactor/implement
- state management
- class structure

Failures from the drupal-7.11.ca.po file.

clemens$ php core/lib/Drupal/Core/Gettext/testGettext.php | grep Array | grep source | sort -u
    [source] => Array
    [source] => Array%archive_file contains versions of modules or themes that are not compatible with Drupal !version: %names
    [source] => Array%type is used by @count pieces of content on your site. If you remove %type, you will not be able to edit the %type content and it may not display correctly.
    [source] => Array<strong>Warning:</strong> There are currently @count menu links in %title. They will be deleted (system-defined links will be reset).
    [source] => Array@count pending updates (@number_applied to be applied, @number_incompatible skipped)
    [source] => Array@count translation strings were skipped because they contain disallowed HTML.
    [source] => ArrayInstalling modules and themes requires access to your server via one of the following methods: <strong>@backends</strong>. See the <a href="@handbook_url">handbook</a> for other installation methods.
    [source] => ArraySorry, there have been more than @count failed login attempts for this account. It is temporarily blocked. Try again later or <a href="@url">request a new password</a>.
    [source] => ArrayThe @module module is missing, so the following modules will be disabled: @depends.
    [source] => ArrayTo uninstall @module, the following modules must be uninstalled first: @required_modules
    [source] => ArrayUpdating modules and themes requires access to your server via one of the following methods: <strong>@backends</strong>. See the <a href="@handbook_url">handbook</a> for other update methods.
    [source] => ArrayYou must include at least one positive keyword with @count characters or more.

I fixed the parser for multi-line plural source lines as that was the problem dumped in #52 and mentioned in #51. I'm puzzled whether the parsing code was that buggy?!? Should we report that elsewhere?

The command now produces a nice dump of public://test.po.txt which was a copy of drupal-7.11.ca.po ... I'd like to test for more complex po files.

php core/lib/Drupal/Core/Gettext/testGettext.php

seems to work with code like this

function testPoReader() {
  $uri = 'public://test.po.txt';
  echo "========================================\n";
  echo "Reading : $uri\n";
  echo "------------- PROCESSING ---------------\n";
  echo "File contents first 500 bytes\n";
  $contents = file_get_contents($uri);
  echo substr($contents, 0, 500) . "\n";
  echo "------------- PROCESSING ---------------\n";
  $file = new GettextFileInterface($uri);

  $reader = new Drupal\Core\Gettext\PoReader($file);

  // Reading the first items result in reading the header first
  // TODO : add method readHeader
  $translation = $reader->read();
  print_r($reader->header);
  print_r($translation);

  while ($translation = $reader->read()) {
    echo "...\n";
    print_r($translation);
  }

  echo "$line\n";
  echo "---------------- DONE ------------------\n";
}

so we can use this through a batch as the context+state variabled are now stored outside the PoReader::read() method. This needs some cleanup.

The code must now get converted to something mentioned in #42.

For our db read/writer we need to read an item

(Locale module, line 520):

   $translation = db_query("SELECT s.lid, t.translation, s.version FROM {locales_source} s LEFT JOIN {locales_target} t ON s.lid = t.lid AND t.language = :language WHERE s.source = :source AND s.context = :context", array(
     ':language' => $langcode,
     ':source' => $string,
     ':context' => (string) $context,
   ))->fetchObject();

Or the full monty

_locale_export_get_strings($language = NULL, $options = array())

Writing a translation to DB (locales.pages.inc):

locale_translate_edit_form_submit()

Write the full monty

_locale_import_one_string_db(&$report, $langcode, $context, $source, $translation, $location, $overwrite_options, $customized = LOCALE_NOT_CUSTOMIZED)

Nice

Thanks @Sutharsan

@clemens,

I'm puzzled whether the parsing code was that buggy?

You worked from my quick and dirty migration of _locale_import_read_po(). You should compare your findings with that code.

@Sutharsan I worked from your version in POReader.

"that buggy" was to hard. Sorry for that. But I had to fix it's ending somehow. I'll check for the mentioned function later on.

Can you add some PO files to the issue summary with known(?) issues or with complex plural forms.

Attached patch is a little improved version. But it still has some issues. It introduces a POItem and POHeader class as I think that's the interface we should deal with. (if garbage collections is an issue we can refactor into a pool of unused objects)
But one benefit is we can now do

echo $myPOHeader . "\n";
// not yet
echo $myPOItem . "\n";

I don't like the names POHeader / POItem so better names are welcome.

I hope to get a Writer in place tomorrow and a new UML diagram. Next refactor into that model.

Just run php core/lib/Drupal/Core/Gettext/testGettext.php from drupal root.

@clemens, This will keep you busy ;)
Drupal core 7.11, Arabic
Drupal core 7.11, Russian
Drupal core 7.11, Croatian

Tested with Arabic and Russian, the strings '1 hour' and '1 day'. The plural string are presented in the right order and all translations in l.d.o are present in the parsed po data. For reference: '1 day' in Arabic at l.d.o and '1 day' in Russian at l.d.o

Add parent

Running the test script I now manage to import ar, ca, nl from the commandline :)

What it does is Remotely download and write these to public:// next read from public:// and write to Db

function pumpAround($langcode) {
  $uri = getRemoteUri($langcode);

  $reader = new PoFileReader($uri, $langcode);
  $header = $reader->getHeader();

  $uri = getPublicUri(__FUNCTION__, $langcode);
  $writer = new PoFileWriter($uri, $header);
  processN($writer, $reader, -1);

  $reader = new PoFileReader($uri, $langcode);
  $writer = new PODbWriter($langcode);
  processN($writer, $reader, -1);
}

I moved around a lot of code snippets from @Sutharsan effort ... not sure whether I missed ... guess I did :(

I created new classes POHeader and POItem and moved code into these clases.

This still needs a lot of work:

• Interface or abstract class: I created an interface for POReader and POWriter but am in doubt.
• When to open a POReader, POWriter
• When to close a POReader, POWriter
• DBReader state management fails somehow ... the prepared statement needs some feedback. @see DBReader::buildQuery()
• The is Drupal code in POHeader
• Documentation
• Test : the tests from testGettext.php need to go into Unit and or Drupal test cases.
• A new UML diagram as promised from #56 is still to come

+++ b/core/lib/Drupal/Core/Gettext/PODbReader.php
@@ -0,0 +1,169 @@
+    $query->orderBy('s.lid');
+    $query->condition('s.lid', ':lid', '>');
+
+    $this->_result = $query->execute(array('lid' => $this->_lid));

This seems not to work.

Calling getState() after some reading shows the lid increasing but it is not set for the next 'batch'

(I cannot find a test in testGettext.php .. darn.

Do we need to read the DB in batches? Batch handling was introduced to cope with performance bottlenecks and unusual situations like installing Drupal. Does this apply to DB'es? Also the po export to file is no bottleneck, although I don't know about requirements for export at l.d.o. As far as I can see there is no need for batch handling the db read and file write.

@Sutharsan I'm not sure what you mean:

- this issue is about making gettext abstract thus PODbReader should follow an interface. But this does not mean PODbWriter can not finish it's job in one go.

- In #39 I said I was confused about #569004: Add support for seek based batch import of .po files. That why I want you on the phone :)

- Having POFileReader batch aware means PODbWriter must be batch aware too. Ie whether or not writing the header or zapping all db entries. I guess we don't have these features now but shouldn't we?

- The other way around PODbReader + POFileWriter one could argue we don't need this batch but that would make the design asymmetric making PODbReader special which I don't like.

The -1 below is about writing ALL to whichever writer.

  processN($writer, $reader, -1);

and the result from a db_select->execute() below is an Iterator too.

class PODbReader implements BatchStateInterface, PoReaderInterface {
...
  private function buildQuery() {
...
    $query->condition('s.lid', $this->_lid, '>');
...
    $this->_result = $query->execute();
}

Would it better be adding readItems()?

interface PoReaderInterface {
...
  function readItems($count =  10);
}

Regarding Symfony Translation (not for this issue but the near future):
- Do we need it's rich file format support like .mo .csv .xliff to name a few?
- I intend to contribute our POFileParsing POFileLoader
- Should we adhere to their naming: Dumper / Loader
- As Symfony lacks a Batch (afaik) right now I (maybe) want to start a feature request over there.

Get @Sutharsan on the Phone :p

(to be continued)

Attached patch fixed the get/setState of PODBReader mentioned in #59 and makes

pumpAround('nl); 

download from http:// to public:// into database next out into a new public://

I'll try to:

- get rid of gettext.inc
- have tests Unit + Web TestCases
- add UML diagram.
- ?

I talked with @Sutharsan about the context of this issue and that helped much :)

Note to self: read #48 and update issue summary.

Attached patch is only for the testbot ... to see what more I missed :p Strategy I choose for now is to rewrite gettext.inc gradually as @Sutharsan sketched in #32.

This patch only has

_locale_import_po()

rewritten. It imported nicely the nl-po file. But the rewrite needs a lot of header related fixes.
It showed me we have also a POMemoryReader|Writer sort of.

I think we need to leave ability for batch import like #569004: Add support for seek based batch import of .po files

@andypost ? ... this issue has (in theory) batch support.

$reader = new POFileReader();
$writer = new POBDWriter();
$writer->writeItems($reader, 10);

$states(
  'reader' => $reader->getState(),
  'writer' => $writer->getState(),
);
batch-save-state($states);

next batch

$states = batch-get-state();

$reader = new POFileReader();
$reader->setState($states['reader']);

$writer = new POBDWriter();
$writer->setState($states['writer']);

$writer->writeItems($reader, 10);

But I first want to make the testbot happy.

Added all functions from gettext.inc

Not all import errors are fixed.
- I'm puzzled with why importing a French .po should change the Plural-forms? Plural numbers changed. locale.test 1011
- How should this work : Language file automatically imported. locale.test 1087
- Other 2 errs are prob related to the above?

More gettext.inc functions are moved. See issue summary for details.

Next steps are:
PO Exports : Fix test errors to begin with.
POMemoryWriter (?+ POMemoryReader) : To obsolete function _locale_import_read_po()

Note: the PoDbWriter relates to #851362: Add hash column to {locales_source} to query faster locale strings

Still lots of work.

That's weird. On my laptop I have when testing throught UI I get only failures related to 'Translation import'.

135 passes, 4 fails, 0 exceptions, and 45 debug messages.

From the testbot log

Path alias functionality 132 passes, 0 fails, and 0 exceptions
exception 'PDOException' with message 'SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupaltestbotmysql.locales_source' doesn't exist' in /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Statement.php:58
Stack trace:
#0 /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Statement.php(58): PDOStatement->execute(Array)
#1 /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Connection.php(506): Drupal\Core\Database\Statement->execute(Array, Array)
#2 /var/lib/drupaltestbot/sites/default/files/checkout/core/includes/database.inc(215): Drupal\Core\Database\Connection->query('SELECT s.lid, t...', Array, Array)
#3 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/locale/locale.module(524): db_query('SELECT s.lid, t...', Array)
#4 /var/lib/drupaltestbot/sites/default/files/checkout/core/includes/bootstrap.inc(1380): locale('1 pass?@count p...', '', 'fr')
#5 /var/lib/drupaltestbot/sites/default/files/checkout/core/includes/common.inc(1779): t('1 pass?@count p...', Array, Array)
#6 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/simpletest/simpletest.module(107): format_plural(37, '1 pass', '@count passes')
#7 /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh(373): _simpletest_format_summary_line(Array)
#8 /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh(22): simpletest_script_run_one_test('1', 'PathMonolingual...')
#9 {main}FATAL PathTaxonomyTermTestCase: test runner returned a non-zero error code (1).
OpenID helper functions 45 passes, 0 fails, and 0 exceptions

Any idea?

#70: 1189184-gettext-parsing-70.patch queued for re-testing.

I added #1497230: Use Dependency Injection to handle object definitions to the patch from #70 as the test definitely will not work.

I refactored a lot by adding
- Interfaces: PoInterface, PoStreamInterface, PoReaderInterface, PoWriterInterface
- Class : POMemoryWriter
- Cleaned gettext.inc more

I changed locale.bulk.inc to use _locale_import_po straightaway ... I hope understood the batch intention enough.

Changed install.inc to use _locale_files_to_memory() as an initial test. But where is that tests?

This still needs lots of work.

I intend to do much more but it takes time :(

Let's see what the testbot thinks now.

Hmmm ... I ran the tests locally on Mac OSX
- 17005 passes, 18 fails, 2 exceptions, and 3681 debug messages

Page cache test
Enable the page cache and test it with various HTTP requests.
73 passes, 6 fails, 2 exceptions, and 19 debug messages

drupal_render()
Performs functional tests on drupal_render().
50 passes, 2 fails, 0 exceptions, and 14 debug messages

Image dimensions
Tests that images have correct dimensions when styled.
47 passes, 9 fails, 0 exceptions, and 8 debug messages

Language dependency injection
Compares the default language from $GLOBALS against the dependency injected language object.
17 passes, 1 fail, and 0 exceptions

Testbot reports (from #76 )
- 35,271 pass(es), 64 fail(s), and 3 exception(s)

ExpandPlural handling (LocalePluralFormatTest) [Locale] 73 39 0
ExpandTranslation export (LocaleExportFunctionalTest) [Locale] 43 2 0
ExpandTranslation import (LocaleImportFunctionalTest) [Locale] 106 23 3

This is too weird for me ... I zap my drupal install then start all over with the patch from #76 .

Can someone please test the patch locally? I suspect something messing with this new Classes.

This patch fixes the installer ... just did an Afrikaans installation with the PoMemoryWriter :-)

The POFileReader had some bugs too with plural which are hopefully fixed now too.

What bothers me still are the exceptions from the testbot.

(sorry)

[edit]Test bot missed #79
I missed browser refresh :/ [/edit]

Running locally (output shortened)

sudo -u _www php ./core/scripts/run-tests.sh --url http://drupal.d8 --php /usr/bin/php Locale

Drupal test run
---------------

Tests to be run:
 - Browser language detection (LocaleBrowserDetectionTest)
...
 - Translation export (LocaleExportFunctionalTest)
 - Translation import (LocaleImportFunctionalTest)
...
 - Plural handling (LocalePluralFormatTest)
...

Test run started:
 Wednesday, April 25, 2012 - 11:23

Test summary
------------
...
Translation import 139 passes, 2 fails, 1 exception, and 46 debug messages
...

As suggested by @Gábor Hojtsy on irc i should solve the local problems first as the testbot could be picky about NOTICES and others.

Wow ... some subtle changes were needed.

$writer->setLangcode($langcode);
-  $writer->setHeader($header);
   $options = array(
     'overwrite_options' => $overwrite_options,
     'customized' => $customized,
   );
+  // It's vital options are set first.
   $writer->setOptions($options);
+  // TODO: is this a design problem?
+  $writer->setHeader($header);

The order of setters is imported for parsing the header. On could say the design of PODbWriter is bad. But this is necessary for the batch interface. We cannot have constructors with arguments as the next batch does not know how to recreate the object right?

Next the Plural-Forms was empty when writing a POHeader. Fixed that.

function locale_translate_batch_import($filepath, &$context) {
@@ -298,6 +301,8 @@ function locale_translate_batch_import($filepath, &$context) {
   // we can extract the language code to use for the import from the end.
   if (preg_match('!(/|\.)([^\./]+)\.po$!', $filepath, $langcode)) {
     $file = (object) array('filename' => drupal_basename($filepath), 'uri' => $filepath);
+    // We need only the last match
+    $langcode = array_pop($langcode);
     _locale_import_po($file, $langcode, array(), LOCALE_NOT_CUSTOMIZED, TRUE);
     $context['results'][] = $filepath;
   }

This was tricky ... the $langcode is an array ... not sure why other tests did not fail.

Lastly I reordered the 'Plural number changed' to match the po upload better. There was a unrelated drupalPost in between.

Attached current UML.

Note the three main Interfaces: BatchState, POReader|Writer, POStream

What is missing is a POMemoryReader. I haven't found a usecase for it. POMemoryWriter has a getData method which breaks symmetry.

I haven't got time to implement the batch due to the fails _ALL_ of the testbot. I suspect the testsuite not to find the new Gettext components somehow.

From irc ([edit:with some additions])

jthorson: ClemensTolboom:  Something to try ... run the tests locally using run-tests.sh with --concurrency set to something other than 1 ... this could help better mimic the testbot operation.
...
jthorson: ClemensTolboom:  I think http://drupal.org/node/346844#comment-1258161 is related to what I was looking for.
...
ClemensTolboom : jthorson: I guess so for that particular exception … but does that explain the import/export failures too?
jthorson : notes that the recurring locale-related issues are one item that he consistently hasn't been able to get a true grasp of the underlying problem.
...
jthorson : ClemensTolboom:  Will see if randy has any thoughts ... otherwise, I think manually testing on a testbot and then inspecting the state of the 'site-under-test' may be our next step.  That approach also allows us to manually start adding debug code to the particular test and re-triggering  the testbot run to see where it's getting hung up.
...
ClemensTolboom : jthorson: I don't want to waste your time if I can do better
...
jthorson : 	ClemensTolboom:  Another random thought ... have you run the tests locally on a site with locale not enabled to start? 19:12

ClemensTolboom : jthorson: I have a script in the patch that does a lot of test and that failed when locale.module was not enabled but apart from that no? 19:14

jthorson : ClemensTolboom:  Just thinking that could be one more difference between your local run-tests.sh execution and the testbot's. 19:15
ClemensTolboom : jthorson: ic … ok I'll start run-tests later on with that one disabled 19:15
jthorson : If that triggers the fails, let me know.  

So need to

• read http://drupal.org/node/346844#comment-1258161 #346844: Clear language statics to remove locale module errors
• Test without locale enabled
• Give feedback to jthorson

Some related issues:
#1337628: Enhance language select form with textbox and other tools
#568986: Dynamically update standard language list from localization server

Unable to troubleshoot tonight, as my manual bot is running into APC errors ... if you could open an issue in the https://drupal.org/project/issues/testbot queue, that will help remind me to come back and re-visit this once I've got it behaving again.

I created #1548366: Locale related Exception in Review log of test. as requested by @jthorson

http://drupal.org/project/testbot states testbot runs with clean url turned off so when testing locally one should test without clean url too.

In #1548366: Locale related Exception in Review log of test. I added a comment #1539072: Support for disabled languages broken, drop it has the same exception.

I have (now) no time to run the full test-suite as that takes ages (it ran for 4+ hours :/ )... will do after May 5th.

Attached patch has some documentation added.

Why I think the classloader could be causing this err is when developing and debugging with Netbeans the classloader does not load the file.

...
namespace Symfony\Component\ClassLoader;
...
class UniversalClassLoader
{
...
    public function loadClass($class)
    {
        if ($file = $this->findFile($class)) {
            require $file;
        }
    }
...
}

which assigned nada (NULL, FALSE?) to my $reader = new PoFileReader(); when not properly declared. I have to retry that after May 5th.

Please make sure that all your namespaces and classes are using the same and sane letter-casing.

Some classes/namespaces in this patch are using "PO" while some others are using "Po".

PSR-0 is case-sensitive. Only use "Po" everywhere.

Yup ... the PDO Exception is a red herring, introduced by http://drupal.org/node/1497230 (and present in all D8 tests ever since, despite them passing).

Patch modified to use "Po" everywhere as suggested by sun.

Thanks - #95 passed the testbot now.

The patch still contains plenty of notes and @todos, is missing comments, phpDoc, etc and thus, overall doesn't seem to be ready for reviews yet.

However, some early feedback:

+++ b/core/includes/gettext.inc
@@ -6,7 +6,28 @@
+use Drupal\Core\Gettext\PoDbReader;
+use Drupal\Core\Gettext\PoDbWriter;
+use Drupal\Core\Gettext\PoFileReader;
+use Drupal\Core\Gettext\PoFileWriter;
+use Drupal\Core\Gettext\PoMemoryWriter;

1) That looks like a crazy level of abstraction... I'd rather expect:

Gettext\DatabaseController
Gettext\FileController
Gettext\MemoryController

but if that level of abstraction is required, I'll let you guys roll...

2) Even when not using controllers but individual reader/writer classes, the class names could be simplified. They also should not contain abbreviations ("Db" vs. "Database").

Just a short reply @sun about class names.

I'm looking @ http://symfony.com/doc/current/book/translation.html and esp. https://github.com/symfony/symfony/tree/master/src/Symfony/Component/Tra... their implementation where there are Loaders and Dumpers.

I like the design of Symfony but still don't get their whole picture regarding translation.

@sun: I can install drupal in ie af / ar etc through the PoMemoryWriter but I'm not sure whether we test this. Do you have some hints?

@others : Should we target using Symfony translation framework? What impact could that have on our way of handling translations? See links mentioned above.

Oh, wow. I wasn't aware of the Translation component in Symfony. I quickly glanced through it and couldn't find anything that would look problematic.

Seems to revolve around reading and writing gettext strings in various formats (storage is separate), and also translating strings at runtime (with plural support, though hard-coded plural rules). I don't think we have any intention to change our runtime translation t() for D8, but I wonder whether we could only leverage the reader and writer parts of it?

If we want to explore that, we might want to create a parallel issue to this (so this one isn't hi-jacked in case it doesn't work out).

Why is patch from #95 pass tests now. Is my local env doing different?

I couldn't apply patch from #95 due to one line in gettext.inc concerning menu_router_rebuild() (committed May 3) ... renaming files in a patch made some more work too for comparing changes. How should one take care of that when working on issue branch?

(so this patch is just to make sure testbot does not fail again)

I just did an symfony patch to see how our parser behaves in their environment. See https://github.com/symfony/symfony/pull/4235

Some notes:

Regarding PoItem + PoHeader these classes are nicer then just arrays but they are tied to the Gettext interface / file format and thus cluttering the PoDbWriter. I think we should talk about 'LC_MESSAGES' instead.

Symfony uses a message structure between all their loaders. I was bothered by the fact all our Classes needed to know what a POHeader/POItem is which felt bad.

We should define a datastructure and document that. PoMemoryWriter somehow uses one.

// Singular
$messages[$context][$source] = $translation
// plural
$messages[implode(LOCALE_PLURAL_DELIMITER, source)] = implode(LOCALE_PLURAL_DELIMITER, $translation);

Symfony uses

// For singural
$messages[$source] = $translation

// For plural two entries are added: the singular form and the plural containing an index + | concat
$messages["foo"]= "bar";
$messages["foos"]= "{0} bar|{1} bars"

I'll create a new issue regarding symfony the moment that pull request is pushed to their master. And thus try to stop talking about symfony within this issue :p

From our office hours @irc #drupal-i18n http://groups.drupal.org/node/229008 :

We could try to write a StreamWrapper mimicking multiple files served one by one on each batch iteration to the symfony Translation\PoFileLoader component. That sounds like a nice solution.

But for this to work we need to adhere to symfony's message structure (see #102) ... I don't like the '|' separator used by symfony so we need to convince them to use our LOCALE_PLURAL_DELIMITER.

Symfony's plural handling http://symfony.com/doc/current/book/translation.html

$this->get('translator')->transChoice($source, $count, $args)

construct is lacking our http://api.drupal.org/api/drupal/includes!bootstrap.inc/function/format_... options.

I need to chew on this a little.

Do the trans() and transChoice() methods matter at all?

We're not going to replace our actual t() string translation with Symfony (yet). The idea is to merely leverage the Translation component for gettext import/export/parsing operations.

EDIT: That said, I'd still recommend to create a new issue for the Symfony based approach. 'cos if that doesn't work out, this issue won't be totally hi-jacked.

@sun Agreed

I start working on #1570346: Let Symfony Translation component handle language messages from now to see how that could work for Drupal.

This issue is now postponed in favor off #1570346: Let Symfony Translation component handle language messages.

As #1570346: Let Symfony Translation component handle language messages is now closed as it was to farfetched to implement within Symfony framework yet.

As I understood from @Sutharsan by a phone call he is sort of blocked by this issue as there is no API to code against. So the issue title does not match that goal either.

(I'll try to re-roll if needed the patch and get familiar with this issue before the sprint.)

This patch needs a controller which stored translations in memory (with a batch interface?!?)

We should not name it Gettext or something but rather MessageCatalogController using a MessageCatalog (aka Symfony). I'm not sure about this but let's make the controller our new API.

The controller can then load/dump the messages into:
- load from file (*.po)
- dump to memory for the installer
- load from db
- dump to cache (weird thing)
- dump to file (*.po)

Needs feedback!

@Gábor Hojtsy

The memory constraint puzzles me:
- Is ie install.nl.po supposed to be small? That would mean we get the Symfony domain 'problem'.
- As this is not about gettext msgctxt but a separate PO file (Symfony calls it domain) for the installer?
- Is http://localize.drupal.org taking care of that in some way?
- How should the message extract work?

You have a good point about PoItem if object memory is much bigger then an array.

#100: 1189184-gettext-parsing-100.patch queued for re-testing.

@Gábor Hojtsy could you please give some feedback on the installer questions from #111 ... guess you missed it as we commented within the minute :)

Note to self
... cannot merge with head
... not sure what git tries to tell here.
See #100

<<<<<<< HEAD
  // Rebuild the menu, strings may have changed.
  menu_router_rebuild();
=======
  if (!$is_batch) {
    // Clear cache and force refresh of JavaScript translations.
    _locale_invalidate_js($langcode);
    cache()->deletePrefix('locale:');
>>>>>>> Fixed POMemoryReader and POFileReader parsing problems.

    // Rebuild the menu, strings may have changed.
    menu_rebuild();

Just a rebase patch to feed the bot

I've reread the whole issue and noted a lot of issues for this issue.
- the issue summary sucks
- a lot of comment from others are not addressed
- some question I asked are not answered.
So these thinks needs to be taken care of before moving this forward.

This issue was postponed May 10 #105 so I need to learn it again :(

So my battleplan for today is:
- read the code and hopefully fix the tests (morning).
- summarize all open comments and questions (afternoon).

There was an old menu_rebuild() ... so testbot be nice :)

Changed summary for pointing out that downloading the translations is required on installing.

Download to sites/default/files/translations the Dutch (nl), Catalan (ca) and Arabic (ar) translations from l.d.o and be sure that they are named install.{lang}.po.

The patch removed nplural knowledge from POItem which took me too much time to discover.

It used variable_get to iterate over its translation which is so against Separation of concern.

We must make sure somewhere higher up the chain POItem contains the correct data.

Thanks @penyaskito for your help :)

Download install po files before installing the site on testing section

I've updated the issue summary to a v0.2 (that means more to come)

Changed POHeader and POItem to PoHeader and PoItem for consistency. Let's see if it's already green.

Don't panic, I think that I used wrong revisions when creating the patch.

This is the last attempt, if it fails I get back to @clemens.tolboom tomorrow.

Feed the testbot.

Removed the references to PoWriter, that was renamed to PoWriterInterface.

Per the quotes at

The component must be capable of reading from/writing to *.po files, reading/writing to database, 'writing' to memory.

, looking that Symfony uses Loader and Dumper (#1189184-35: OOP & PSR-0-ify gettext .po file parsing and generation by @clemens.tolboom) and the terminology that @fubhy explains at #1189184-48: OOP & PSR-0-ify gettext .po file parsing and generation, maybe we should consider replacing the suffixes Writer and Reader to Loader and Dumper. Eventually, we could split Loader in Loader and Reader for sequential loads if needed.

Feeding the testbot. Added Gettext class used by the installer by Clemens and some refactoring done by Clemens and me.

(notes)
When we start working on the Batch interface we could try to do it for testing manually
- install_import_translations which uses "locale_translate_batch_import"
This code below is probably obsolete? (related to: Why are files imported twice?)

  $batch = locale_translate_batch_import_files($langcode);
  if (!empty($batch)) {
    return $batch;
  }

as install_import_translations_remaining does something similar.

function install_import_translations_remaining(&$install_state) {
  include_once drupal_get_path('module', 'locale') . '/locale.bulk.inc';
  return locale_translate_batch_import_files($install_state['parameters']['langcode']);
}

But for debugging we can better use the UI interface for importing.

(v0.2) A rewrite with problems mapped to solutions.

Deleted code snippets which cluttered the text.

Some layout improvements

I updated the issue summary a little:
- removed irrelevant code snippets
- changed some styling
- added BatchStateInterface interface (does it help?!?)

Rephrased BatchStateInterface a little (not sure whether that helps)

I'm not sure the Batch interpretation currently done is what we need:
- Say we have 10 *.po files to load by a module x
- Now each .po file should be loaded into smaller chunks say 5
The module does not care the files are loaded in batches and we don't care about multiple files.

That doesn't sound to good. So any help is welcome. What should we do?

Summary of my talk with Clemens in random order:

1. POFileReader checks for BOM, but doesn't check if the file is UTF8, does it have to do it?
2. Try to get rid of gettext.inc
3. Don't export if there's nothing to export
4. Show message if import succeeded

#1630524: Don't create a .po file if it's empty

@atttiks thanks for the summary.

1. The BOM / Byte Order Marker is copy/paste code. (for me for now: won't fix :/)

2. I hope today with @attiks and @penyaskito to remove all export function. And hopefully the _locale_import_po too. Then it _is_ empty :)

To make that work we need a way to get 'rid' of the drupal_set_message stuff. Yesterday I tried to create a Gettext::fileToDatabase() to discover the caller needs both the PoFileReader (for failures on reading) as PoDatabaseWriter (for failure and statistics). So we should discuss this a little with the three of us.

3. Did we talk about that? Is this solved now?

4. See the statistics from 2

Your last patch is empty. Have you asked @Gábor Hojtsy for git access?

1/ see #1630568: Validate that uploaded .po files are UTF8
2/ todo
3/ fixed
4/ is a temp fix, will have a look

trying to import the same po file twice:
Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'temporary://modules-block.nl_.po' for key 'uri': INSERT INTO {file_managed} (fid, uid, filename, uri, langcode, filemime, filesize, status, timestamp) VALUES (default, :db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7); Array ( [:db_insert_placeholder_0] => 1 [:db_insert_placeholder_1] => modules-block.nl_.po [:db_insert_placeholder_2] => temporary://modules-block.nl_.po [:db_insert_placeholder_3] => und [:db_insert_placeholder_4] => application/octet-stream [:db_insert_placeholder_5] => 15250 [:db_insert_placeholder_6] => 0 [:db_insert_placeholder_7] => 1339578176 ) in drupal_write_record() (line 483 of /var/aegir/platforms/i18n/core/includes/schema.inc).

ignore #142, file permission problem

Remove hasResults from PoDbReader as 'emptiness' is a business rule like

$r = new PoDbReader();
....

$item = $r->getItem();
// We don't want to write an empty reader
if (!empty($item)) {
  $w = new PoFileWriter();
  $w->setX();

  $w->writeItem($item);
  $w->writeItems($r);
}

[edit] Think of PoFile|Database|Memory as Iterators which 'just' dry out.

My idea for the batch is adding the following to Gettext.php

  static function getStates($reader, $writer) {
    return array(
      'reader' => $reader->getState(),
      'writer' => $writer->getState(),
    );
  }
  
  static function createFromStates($states) {
    $reader = new $state['reader']['__CLASS__'];
    $reader->setState($state['reader']['__CLASS__']);
    $writer = new $state['writer']['__CLASS__'];
    $writer->setState($state['writer']['__CLASS__']);
    
    return array($reader, $writer);
  }

Clemens: the current batch is working, why do you want to change it?

Things to check for are:

What _is_ the new API

I somehow expect to have a single point in the code with a factory method to produce a reader or writer.

Issues named in this issue

- #1 #147915: Refactor locale import/export API to support non-local files and non-gettext formats (non-gettext formats) (we can load from remote)
- #2 #361597: CRUD API for locale source and locale target strings (crud would be cool especially when save multiple at once aka $writer->writeItems())
- #3 #420510: Possibly misleading documentation/func naming for developers on locale.inc functions (we have still a documentation problem over here)
- #6 #1219236: Locale module includes 1350+ lines of unneeded code on all page loads
- #21 #532512: Plural string storage is broken, editing UI is missing (I haven't seen that interface)

Possible design and bugs

- #18 Do we have a test for multilines
- #55 Our parser is from a refactoring try. We should check with the current code version.
- #69 Batch support is underway.
- Code is now located in code/lib/Drupal/Core but it should be in core/lib/Drupal/Component I guess

That's it for now.

Fixed into {language}

Let's see some green here in the meantime.

One thing that bothers me is that for exporting a PO/POT file we are now creating the physical file on the server and then reading and flushing it to the client.

Should we implement a PoStringWriter for this instead?

@penyaskito #149 that's performance related a good idea.

But why don't we do a redirect to the file written that is use a public:// file? Then it's up to apache to serve it.

Ok, seems that having all the string in memory is not a good idea. We can continue the clean up and consider that later if needed.

Saving the file in public:// doesn't look good to me, it could be using disk space that can be restricted in some environments, and the file will always be regenerated. But the way, you reminded me that we should delete it once it is downloaded, I will commit this to the sandbox.

tests added for multiline and allowed/blocked tags

I started the refactoring _locale_import_one_string_db. We need to get rid of the dependency from Gettext to locale module.

Note to self:

* Answer Gábor.
* Check with attiks why the 2 new tests are failing. I fixed them with

diff --git a/core/modules/locale/lib/Drupal/locale/Tests/LocaleImportFunctionalTest.php b/core/modules/locale/lib/Drupal/locale/Tests/LocaleImportFunctionalTest.php
index e321be3..8415572 100644
--- a/core/modules/locale/lib/Drupal/locale/Tests/LocaleImportFunctionalTest.php
+++ b/core/modules/locale/lib/Drupal/locale/Tests/LocaleImportFunctionalTest.php
@@ -212,7 +212,7 @@ class LocaleImportFunctionalTest extends WebTestBase {
     $search = array(
       'string' => 'HTTP Result Code: !status',
       'language' => $langcode,
-      'translation' => 'translated',
+      'translation' => 'all',
     );
     $this->drupalPost('admin/config/regional/translate/translate', $search, t('Filter'));
     $this->assertNoText(t('No strings available.'), t('String successfully imported.'));
@@ -221,7 +221,7 @@ class LocaleImportFunctionalTest extends WebTestBase {
     $search = array(
       'string' => 'xyzzy',
       'language' => $langcode,
-      'translation' => 'translated',
+      'translation' => 'all',
     );
     $this->drupalPost('admin/config/regional/translate/translate', $search, t('Filter'));
     $this->assertNoText(t('No strings available.'), t('String successfully imported.'));

but not sure if the strings contained at files imported automatically should be imported marked as 'translated'. If the answer is not, the problem is at the test.

Some pending tasks:

#156: We have to move PoDbReader and PoDbWriter to the Locale module.

#159: We should take a look at the importing functionality when adding a new language, I think that we are not flagging the strings as 'translation'.

#159: If I'm right with my previous affirmation, I'm scared that this issue didn't appeared before. Maybe we need more tests?

#*: We should git mv core/lib/Drupal/Core/Gettext to core/lib/Drupal/Component/Gettext

#158 is right
#18 test for multilines added
#55 ok
#69 Batch support is working, batch inside batch is handled in a follow-up
#6 ok
#1 is out of scope for this issue

so list in #160 wraps it up (for now ;p)

slightly off topic: i created an overview off the current behaviour of the translation interface so we know which tests can pass: https://docs.google.com/spreadsheet/ccc?key=0Ar95pTGHJsoxdDMzbUl1Uk5aTGh...

Ok, so all tests at green now. Those failing tests were new, and the behavior is not what @attiks nor I expected to be. @attiks gathered together that list, that maybe we can handle in a DX follow-up task. @attiks, could you create it and attach the document?

So now, #160 is fixed

Let's see a green. Maybe?

Rebased on branch 8.x.