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

Globalize #54

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

Globalize #54

wants to merge 4 commits into from

Conversation

Setogit
Copy link
Contributor

@Setogit Setogit commented Dec 25, 2015

connected to strongloop-internal/scrum-nodeops#1157

@Setogit Setogit self-assigned this Dec 25, 2015
@Setogit Setogit force-pushed the globalize branch 7 times, most recently from 35d35f7 to f2d4819 Compare December 29, 2015 01:00
@Setogit Setogit assigned sam-github and unassigned Setogit Dec 29, 2015
@@ -2,6 +2,7 @@

'use strict';

var GLB = require('strong-globalize');
Copy link
Contributor

Choose a reason for hiding this comment

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

camel case: glb

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

@Setogit Setogit force-pushed the globalize branch 2 times, most recently from f51a5c0 to 0063b05 Compare January 1, 2016 06:12
@@ -0,0 +1,31 @@
{
"all1": "",
Copy link
Contributor

Choose a reason for hiding this comment

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

why empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's one of the GPB issues. For es, fr, and pt, this string is translated to empty most likely due to the double curly braces. The issue has been reported to the GPB team.

@sam-github
Copy link
Contributor

#54 (comment) is unresolved (mentioning here because it is not displayed because of code updates)

@sam-github
Copy link
Contributor

re:
#54 (comment)

space is allowed in keys in json (any string is):

// fr/messages.json

{
    "hello, {person}, I am a string!": "bonjour, {person}, je suis un string!",
}

which could be used like: glb.log("hello, {person}, I am a string!", {person: "Tetsuo"});

I'm not sure how difficult to achieve this would be, but I'd like you to consider it more carefully. Strengths are:

  1. our source code is readable (as opposed to the current APIs in globalize, which make the log messages unreadable without a cumbersome lookup by a human of a key in a json file)
  2. unreadable source is error-prone source
  3. it facilitates automated detection of strings, calls to glb.log() (for example) in dev mode can write properties that are not already added into the messages dictionary into it, providing a clue that there is a new or changed message string

There might be some down sides.

What do other users of the globalize package do? Have you done some checking for blogs/design advice?

My experience may be biased by GNU gettext, in that I've worked on code that uses GNU internationalization, and other than seeing _(....)_ wrapped around strings, I didn't even notice the internationalization.

I may be biased, but translators are not going to see our source, we have to maintain and debug it all the time, we need to put effort into making it easier for us. Also, if its hard for us, we'll get it wrong, and then the best translatio won't help, because we will have bugs in our source.

@Setogit Setogit force-pushed the globalize branch 5 times, most recently from 49fcb95 to c474262 Compare January 3, 2016 06:02
@sam-github
Copy link
Contributor

@rmg @kraman PTAL

This is an example of what our code will look like once globalized. More than an example, this is the globalized strong-deploy.

Looks pretty good to me, minimally intrusive.

See https://github.com/strongloop/strong-globalize/blob/refactor-a/README.md for more information on the toolset.

Also, note that once the code is converted to use glb.*(), that there is a code scanner utility in strong-globalize to extract the messages, so the message JSON does not need manual maintenance.

.replace(/%MAIN%/g, $0)
.trim();

var USAGE = glb.t('sl-deploy.txt')
Copy link
Member

Choose a reason for hiding this comment

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

In this scenario it totally makes sense, but there's something weird to me about how it completely abstracts away the file opening and reading while still clearly operating on a specified file. I would expect there to have been a path resolver wrapped around the path string instead. I imagine it to look like this:

var USAGE = fs.readFileSync(glb.resolve('./sl-deploy.txt'), 'utf-8')
    .replace(/%MAIN%/g, $0)
    .trim();

If the logical resource is simply a string and the fact that it is in a file is an implementation detail, then I would expect the name to be something like 'USAGE' or something.

@rmg
Copy link
Member

rmg commented Jan 13, 2016

I expected strong-globalize to provide an API for operating on Strings, not replicate/wrap all the existing API's that current use Strings..

var t = require('strong-globalize').translate;
var f = require('strong-globalize').format;

console.log(t('Strings for life!'));
console.error(f('%j: %s', new Date(), process.argv[2]));

@Setogit
Copy link
Contributor Author

Setogit commented Jan 14, 2016

Also, all the followings work too:

var g = require('strong-globalize');

console.log(g.t('Strings for life!'));
g.log('Strings for life!');
console.error(g.t('%j: %s', new Date(), process.argv[2]));
console.error(g.t('%j: %s', g.d(new Date()), process.argv[2]));
console.error(g.t('%s: %s', g.d(new Date()), process.argv[2]));
g.error('%j: %s', new Date(), process.argv[2]);
g.error('%j: %s', g.d(new Date()), process.argv[2]);
g.error('%s: %s', g.d(new Date()), process.argv[2]);

Now that i've dog-fooded and globalized strong-deploy, strong-pm, strong-build, strong-mesh-models, strong-service-install and cli.js of strong-arc, i personally like the following because the key strokes i hit is least (and, it was even fun particularly because messages.json is auto-created and machine-translated to 9 languages (Russian is coming soon) in seconds). I don't mind writing code like this from the beginning:

var g = require('strong-globalize');

g.log('Strings for life!');
g.error('%s: %s', g.d(new Date()), process.argv[2]);

@Setogit
Copy link
Contributor Author

Setogit commented Jan 16, 2016

@sl-node test please

@Setogit
Copy link
Contributor Author

Setogit commented Jan 25, 2016

@sl-node test please

@Setogit Setogit assigned sam-github and unassigned Setogit Jan 25, 2016
);
g.error(
'Unable to detect service name, {{package.json}} ' +
'has no "name" property.\n' +
Copy link
Contributor

Choose a reason for hiding this comment

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

name has to be protected

@Setogit Setogit force-pushed the globalize branch 2 times, most recently from e8ae74e to 7ce5d6b Compare January 26, 2016 05:06
@Setogit
Copy link
Contributor Author

Setogit commented Jan 26, 2016

This requires strongloop/strong-globalize#5

@Setogit
Copy link
Contributor Author

Setogit commented Jan 26, 2016

@sl-node test please

1 similar comment
@Setogit
Copy link
Contributor Author

Setogit commented Jan 26, 2016

@sl-node test please

@sam-github
Copy link
Contributor

LGTM, but we can't merge until strong-globalize is published.

@sam-github sam-github assigned Setogit and unassigned sam-github Jan 27, 2016
@Setogit Setogit removed their assignment Jul 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants