mikeedwards / po2json

Pure Javascript implementation of Uniforum message translation. Based on a great gist.
https://gist.github.com/1739769
Other
178 stars 62 forks source link

[1-alpha] Over escaping braces #77

Closed maximelebastard closed 4 years ago

maximelebastard commented 6 years ago

Expected Behavior

I'd like my msgstr containing braces not being escaped

msgid "test"
msgstr "Hi %{firstname}"
{ "test": "Hi %{firstname}" }

Actual Behavior

msgid "test"
msgstr "Hi %{firstname}"
{ "test": "Hi %\\{firstname\\}" }

Steps to Reproduce

The first example is taken from the 0.x version, the second one is after an (unwanted) update of the library (through po-loader) to the 1-alpha version.

Additional info

Here is my po-loader webpack loader configuration

          {
            loader: require.resolve('po-loader'),
            options: {
              format: 'mf', // Clean key value output
            }
          },
mikeedwards commented 6 years ago

Hey @maximelebastard! Thanks for the bug report—this project lives and dies by the very specific issues our users find.

Not to kick you further upstream, but it looks like the original problem comes from gettext-to-messageformat. It's a great module that goes a lot further in effectively converting po files to mf than we were doing originally pre 1.0, but it does appear to be the source of your braces escaping issue.

Nevertheless, I have a PR up on this repo that addresses your issue directly by un-escaping the braces in the most brute-force way possible. Take a peek at #78 and see if that solves your problem. If it seems like it fixes things for you, great! It may be a decent stopgap until g2m resolves this, if that project feels it needs to.

JvJefke commented 4 years ago

Hey @mikeedwards,

It seems that you can manage the replacements through the options in gettext-to-messageformat parsePo function: See: https://github.com/messageformat/gettext-to-messageformat/blob/master/index.js

The first replacement causes the above issue I think. Is there a way to expose this option or to disable the first replacement by default?

mikeedwards commented 4 years ago

Hmm, that does look promising. My main concern would be if there are any unintended consequences in removing that escaping. Is this a problem that you're running into with your own projects? Would you mind testing a branch if I make one that fixes this by changing the g2m defaults? The big trick with making updates to po2json is that people have many different languages, uses etc. that I'm not always able to cover in the tests beforehand, so I like to check in with folks using it in the real world for updates like this.

JvJefke commented 4 years ago

@mikeedwards That is a valid concern. If implemented this way, this would mean a breaking change.

The alternative is to optionally expose that option in some form in the po2json options and let de default behaviour be like it is now.

Our frontend implementations use ngx-translate (https://github.com/ngx-translate/core). They use variable injection with the {...} placeholder. Using po2json breaks the variable injection.

mikeedwards commented 4 years ago

Hmm, yeah. Well, I just merged #84 which allows people using the module to push in g2m options that would fix the replacements or set them however they like. It won't help the command line users, but at least it's something. At some point, I just need to bite the bullet, move one major version up, and re-write this thing in modern JS, probably even TS, and toss out the old weird stuff that changing right now will break for a ton of folks. Someday...