Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Escape all characters above \u007f at the i18n directory #2418

Closed
wants to merge 2 commits into from

Conversation

lgalfaso
Copy link
Contributor

Modify the script that writes the locales so all characters above \u007f are escaped
Run the modified script and update the locales

Closes #2417

Modify the script that writes the locales so all characters above \u007f are escaped
Run the modified script and update the locales

Closes angular#2417
@petebacondarwin
Copy link
Contributor

This needs a unit test. To help with this I would suggest that you extract out the serialization of the content into its own function:

function serializeContent(localObj) {
  return JSON.stringify(localeObj, canonicalizeForJsonStringify, '  ')
    .replace(/\¤/g, '\\u00A4')
    .replace(new RegExp('[\\u007f-\\uffff]', 'g'), function(c) { return '\\u'+('0000'+c.charCodeAt(0).toString(16)).slice(-4); })
    .replace(/"@@|@@"/g, '');
});

Then call this in outputLocale:

...
  var content = serializeContent(localeInfo[localeID]);
...

Then write a unit test that can go into i18n/spec/closureI18nExtractorSpec.js

@petebacondarwin
Copy link
Contributor

  • Contributor signed CLA now or in the past (if you just signed, leave a comment here with your real name)
  • PR doesn't introduce new api
  • PR doesn't contain a breaking change
  • PR contains unit tests
  • PR contains e2e tests (if suitable)
  • PR contains documentation update (if suitable)
  • PR passes all tests on Travis (sanity)
  • PR passes all tests on ci.angularjs.org (cross-browser compatibility)
  • PR is rebased against recent master
  • PR is squashed into one commit per logical change
  • PR's commit messages are descriptive and allows us to autogenerate release notes (required commit message format)
  • All changes requested in review have been implemented

Can you check that all these tasks have been completed? Thanks.

Refactor the serialization to be able to add unit tests
Add unit tests

Closes angular#2418
@lgalfaso
Copy link
Contributor Author

Did the refactor to be able to create unit tests. Added two new unit tests.
BTW, already signed the CLA

@petebacondarwin
Copy link
Contributor

Thanks @lgalfaso for a great PR. Merged at: 695c54c

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Encoding issues with i18n other than english
2 participants