Skip to content
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.

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

Add additional argument for generating msgstr in order to create *.po files #45

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

pospi
Copy link

@pospi pospi commented Oct 22, 2016

No description provided.

@pospi
Copy link
Author

pospi commented Oct 22, 2016

@evenchange4
Copy link
Owner

Would you mind providing more details about your translation workflow?
To be honest I could not figure out why skipping the 'pot' step. 🤔

@pospi
Copy link
Author

pospi commented Oct 24, 2016

Well, I'm sure you know a *.pot file is just a *.po file with translations missing- they're the same format, one is just a template. I'm not advocating only doing the *.po step, but I would like to generate both a *.pot template and a ready-to-go translation file for the default language the app is built in, using the defaultMessage feature of react-intl. Mainly because I can then pipe that into other tools to generate merged translation files for the default language that aren't spread out through defaultMessage attributes in your app. Maybe it's a rare case but that's why I avoided disturbing the default behaviour. Others could concievably have different app source directories with different languages built into them as the default language which this would assist with too.

@evenchange4
Copy link
Owner

evenchange4 commented Oct 24, 2016

Just a reminder that there is already a built in algorithm working internally of react-intl. It will automatically fallback to defaultMessage if the message id is not matched in your translation files. (Maybe not the only solution.)

Anyway, thanks for sharing the process of your project, and it's ok for me to merge this PR if it is ready. :)

@pospi
Copy link
Author

pospi commented Oct 25, 2016

Yeah, so many options aren't there! Ready to merge if you're happy to have these features in the core project (:

@pospi
Copy link
Author

pospi commented Oct 25, 2016

Yeah, so many options aren't there! Well I'm ready if you are and you're
happy for this feature to be part of the core (:

On Mon, Oct 24, 2016 at 8:41 PM Michael Hsu notifications@github.com
wrote:

Just a reminder that there is already a built in algorithm
https://github.com/yahoo/react-intl/wiki/API#message-formatting-fallbacks
working internally of react-intl. It will automatically fallback to
defaultMessage if the message id is not matched in your translation
files. (Maybe not the only solution.)

Anyway, thanks for sharing the process of your project, and I am okay to
merge this PR if it is ready. :)


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#45 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAatJHRech5KhDm4ne6oVHBVjARNFpQ4ks5q3ItOgaJpZM4Kdzab
.

});

test('should return pot formatted string', t => {
t.is(
potFormater({
potFormater(null)({
Copy link
Owner

Choose a reason for hiding this comment

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

Add another test case for custom messageValue will be great. :)

Object.keys(messageObject) // return array of id
.sort()
.map(id => `${potCommentsFormater(messageObject[id])}msgid "${id}"\nmsgstr ""\n`)
.map(id => `${potCommentsFormater(messageObject[id])}msgid "${id}"\nmsgstr "${messageValue ? messageObject[id][0][messageValue] : ''}"\n`)
Copy link
Owner

Choose a reason for hiding this comment

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

There is an eslint error here, please fix it or alternatively just adding one line eslint-disable for pretty display. :)

ps: Rebase onto master branch which now using node v6 and npm v3 and just run npm run lint locally.

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.

None yet

2 participants