Skip to content

[Translation] added option json_options to JsonFileDumper. #15756

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 11, 2015
Merged

[Translation] added option json_options to JsonFileDumper. #15756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Symfony/Component/Translation/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ CHANGELOG
-----

* deprecated Translator::getMessages(), rely on TranslatorBagInterface::getCatalogue() instead.
* added option `json_encoding` to JsonFileDumper
* added options `as_tree`, `inline` to YamlFileDumper
* added support for XLIFF target and tool attributes.
* added message parameters to DataCollectorTranslator.
Expand All @@ -13,7 +14,6 @@ CHANGELOG
so the class name is misleading. The `TargetOperation` class should be used for
this use-case instead.


2.7.0
-----

Expand Down
16 changes: 15 additions & 1 deletion src/Symfony/Component/Translation/Dumper/JsonFileDumper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,21 @@ class JsonFileDumper extends FileDumper
*/
public function format(MessageCatalogue $messages, $domain = 'messages')
{
return json_encode($messages->all($domain), defined('JSON_PRETTY_PRINT') ? JSON_PRETTY_PRINT : 0);
return $this->formatCatalogue($messages, $domain);
}

/**
* {@inheritdoc}
*/
protected function formatCatalogue(MessageCatalogue $messages, $domain, array $options = array())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolas-grekas see #15699

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protected methods must be set after public ones.

{
if (isset($options['json_encoding'])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe call this json_flags or json_options?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

json_flags sound good to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but can we take on #14750 (comment) ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, then fine for json_encoding...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverted, thanks!

$flags = $options['json_encoding'];
} else {
$flags = defined('JSON_PRETTY_PRINT') ? JSON_PRETTY_PRINT : 0;
}

return json_encode($messages->all($domain), $flags);
}

/**
Expand Down
28 changes: 24 additions & 4 deletions src/Symfony/Component/Translation/Tests/Dumper/JsonFileDumperTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,18 @@

class JsonFileDumperTest extends \PHPUnit_Framework_TestCase
{
private $tempDir;

protected function setUp()
{
$this->tempDir = sys_get_temp_dir();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add also tearDown() method to remove that dir after test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, I'd suggest removing $this->tempDir and setUp()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

}

protected function tearDown()
{
unlink($this->tempDir.'/messages.en.json');
}

public function testDump()
{
if (PHP_VERSION_ID < 50400) {
Expand All @@ -25,12 +37,20 @@ public function testDump()
$catalogue = new MessageCatalogue('en');
$catalogue->add(array('foo' => 'bar'));

$tempDir = sys_get_temp_dir();
$dumper = new JsonFileDumper();
$dumper->dump($catalogue, array('path' => $tempDir));
$dumper->dump($catalogue, array('path' => $this->tempDir));

$this->assertEquals(file_get_contents(__DIR__.'/../fixtures/resources.json'), file_get_contents($tempDir.'/messages.en.json'));
$this->assertEquals(file_get_contents(__DIR__.'/../fixtures/resources.json'), file_get_contents($this->tempDir.'/messages.en.json'));
}

public function testDumpWithCustomEncoding()
{
$catalogue = new MessageCatalogue('en');
$catalogue->add(array('foo' => '"bar"'));

$dumper = new JsonFileDumper();
$dumper->dump($catalogue, array('path' => $this->tempDir, 'json_encoding' => JSON_HEX_QUOT));

unlink($tempDir.'/messages.en.json');
$this->assertEquals(file_get_contents(__DIR__.'/../fixtures/resources.dump.json'), file_get_contents($this->tempDir.'/messages.en.json'));
}
}
1 change: 1 addition & 0 deletions src/Symfony/Component/Translation/Tests/fixtures/resources.dump.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"foo":"\u0022bar\u0022"}