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 support for dot notation #22

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

Conversation

jelhan
Copy link

@jelhan jelhan commented Jan 21, 2020

  • add basic support for dot notation
  • support usage of dot notation in a nested object, e.g. { "foo": { "bar.baz": "..." } }

@jelhan jelhan changed the title WIP: add support for dot notation add support for dot notation Jan 22, 2020
@snewcomer snewcomer added the enhancement New feature or request label Jan 23, 2020
@snewcomer
Copy link
Contributor

snewcomer commented Jan 24, 2020

@jelhan Phenomenal! I'm guessing you have tested this out on your own app(s) and it worked?

@jelhan
Copy link
Author

jelhan commented Jan 24, 2020

I tested mostly with the data in dummy/ directory. If run these steps to ensure that actual data and expected data are the same:

cd dummy
node ../bin/ember-i18n-intl-migrator.js --type=yaml
git status

Maybe we should setup TravisCI to run yarn test and something similar to instructions above? Could provide in another pull request if you like.

I've also tested with https://github.com/jelhan/croodle, which I need this for. But wasn't yet able to fully test cause some addition transformations need to be done manually (e.g. missing support for value invocation with 3-curlies to skip HTML escaping). Feel free to wait with merging this one until I could confirm that it helped me with the real-world scenario.

@snewcomer
Copy link
Contributor

Ok I'll wait for you to test on your real world scenario!

@jelhan
Copy link
Author

jelhan commented Jan 31, 2020

Was able to migrate my translations. Faced an edge case that I needed to fix manually:

export default {
  'foo': 'parent',
  'foo.bar': 'child',
}

This can not be transformed to YAML or nested JSON objects as the value of foo should be a string ("parent") and an object ({ bar: 'child' }) at the same time. The codemod does not warn in such a case. To be honest I was to lazy to add such a warning. I don't expect many people to use this codemod as hopefully most apps already migrated.

Ready to be merged from my side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants