Skip to content

Added PHP format#91

Merged
anthonynsimon merged 2 commits intoever-co:masterfrom
kjellspijker:feature/php-laravel-format
May 21, 2020
Merged

Added PHP format#91
anthonynsimon merged 2 commits intoever-co:masterfrom
kjellspijker:feature/php-laravel-format

Conversation

@kjellspijker
Copy link
Copy Markdown
Contributor

Closes #82

I chose the format used by the Laravel framework as base. This should be easy enough to include in different frameworks/projects as required.

Copy link
Copy Markdown
Contributor

@anthonynsimon anthonynsimon left a comment

Choose a reason for hiding this comment

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

Looks good, thank you for helping out! Please just review the comments below.

return jsonNestedParser(JSON.stringify(parse(php)));
};

export const phpExporter: Exporter = async (data: IntermediateTranslationFormat) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wouldn't you need to escape the term and translations in case there is quote in there?

For example in:

[  "term with \" quote" => "translation with \" quote"  ]

@anthonynsimon
Copy link
Copy Markdown
Contributor

@kjellspijker any update on this? :)

@kjellspijker
Copy link
Copy Markdown
Contributor Author

@anthonynsimon I've been a bit busy, so thanks for the reminder :)

I did manage to add the format to the import controller and I spent a bit of time looking at the case you presented with the quotes, but couldn't manage to get that part fixed.

As a side note, there is functionally a bit of difference between strings with double (") and single (') quotes in PHP, where double quotes allow string templating, which (in my opinion), you wouldn't want here. The same problem does still exist though, just with a single quote. Maybe you have some idea on how to fix this?

@anthonynsimon
Copy link
Copy Markdown
Contributor

@kjellspijker Hey thanks a lot for putting time into this :) If it's ok with you, I can try and incorporate the last fixes on my branch so that we can merge it.

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@anthonynsimon anthonynsimon merged commit 9f7b27e into ever-co:master May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

update on support for PHP strings

3 participants