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

Normalize with pattern router may leave duplicate entries. #263

Open
vsakarov opened this issue Sep 24, 2017 · 6 comments
Open

Normalize with pattern router may leave duplicate entries. #263

vsakarov opened this issue Sep 24, 2017 · 6 comments
Labels

Comments

@vsakarov
Copy link

vsakarov commented Sep 24, 2017

If you use normalize to move translations between files you may end with duplicate entries in different files.

diff --git a/spec/i18n_tasks_spec.rb b/spec/i18n_tasks_spec.rb
index 7ba464a..9d97ba8 100644
--- a/spec/i18n_tasks_spec.rb
+++ b/spec/i18n_tasks_spec.rb
@@ -178,6 +178,8 @@ RSpec.describe 'i18n-tasks' do
         expect(File).to_not exist 'config/locales/devise.en.yml'
         run_cmd 'normalize', '--pattern_router'
         expect(YAML.load_file('config/locales/devise.en.yml')['en']['devise']['a']).to eq 'EN_TEXT'
+        # Old value should be removed
+        expect(File).not_to exist 'config/locales/old_devise.en.yml'
       end
     end
   end
@@ -325,7 +327,6 @@ resolved_reference_target.a (resolved ref)
         },
         'numeric'                => { 'a' => v_num },
         'plural'                 => { 'a' => { 'one' => v, 'other' => "%{count} #{v}s" } },
-        'devise'                 => { 'a' => v },
         'scoped'                 => { 'x' => v },
         'very'                   => { 'scoped' => { 'x' => v } },
         'used'                   => { 'a' => v },
@@ -362,6 +363,8 @@ resolved_reference_target.a (resolved ref)
     fs = fixtures_contents.merge(
       'config/locales/en.yml' => { 'en' => en_data }.to_yaml,
       'config/locales/es.yml' => { 'es' => es_data }.to_yaml,
+      'config/locales/old_devise.en.yml' => { 'en' => { 'devise' => { 'a' => 'EN_TEXT' } } }.to_yaml,
+      'config/locales/old_devise.es.yml' => { 'es' => { 'devise' => { 'a' => 'ES_TEXT' } } }.to_yaml,
       'config/locales/unused.en.yml' => { 'en' => { 'unused' => { 'file' => 'EN_TEXT' } } }.to_yaml,
       'config/locales/unused.es.yml' => { 'es' => { 'unused' => { 'file' => 'ES_TEXT' } } }.to_yaml,
       'config/locales/not-in-write/unused.en.yml' =>
@glebm glebm added the bug label Sep 24, 2017
@glebm glebm added this to the v0.9.19 milestone Sep 24, 2017
glebm added a commit that referenced this issue Sep 24, 2017
@glebm
Copy link
Owner

glebm commented Sep 24, 2017

Should be fixed by the commit linked above, PTAL

@vsakarov
Copy link
Author

vsakarov commented Sep 24, 2017

Thanks again for the quick responses and fixes. It seems to work fine, but there is one minor problem. If you already have some leftovers they will not be deleted.

P.S. I've made some comments in the commit, hope they are useful.

@glebm
Copy link
Owner

glebm commented Sep 24, 2017

If you already have some leftovers they will not be deleted.

What do you mean? Can you give an example?

@vsakarov
Copy link
Author

vsakarov commented Sep 25, 2017

If you have ran normalize before this fix:

diff --git a/spec/i18n_tasks_spec.rb b/spec/i18n_tasks_spec.rb
index 9d97ba8..6d07048 100644
--- a/spec/i18n_tasks_spec.rb
+++ b/spec/i18n_tasks_spec.rb
@@ -365,6 +365,8 @@ resolved_reference_target.a (resolved ref)
       'config/locales/es.yml' => { 'es' => es_data }.to_yaml,
       'config/locales/old_devise.en.yml' => { 'en' => { 'devise' => { 'a' => 'EN_TEXT' } } }.to_yaml,
       'config/locales/old_devise.es.yml' => { 'es' => { 'devise' => { 'a' => 'ES_TEXT' } } }.to_yaml,
+      'config/locales/devise.en.yml' => { 'en' => { 'devise' => { 'a' => 'EN_TEXT' } } }.to_yaml,
+      'config/locales/devise.es.yml' => { 'es' => { 'devise' => { 'a' => 'ES_TEXT' } } }.to_yaml,
       'config/locales/unused.en.yml' => { 'en' => { 'unused' => { 'file' => 'EN_TEXT' } } }.to_yaml,
       'config/locales/unused.es.yml' => { 'es' => { 'unused' => { 'file' => 'ES_TEXT' } } }.to_yaml,
       'config/locales/not-in-write/unused.en.yml' =>

And then run it again with the fix, and you still got your duplicates.

Not sure whether this should be fixed by normalize or some kind of consistency check. If this has happened long ago you may have different translations for the same keys. Still this is a minor issue as when you move translations around you should carefully examine your diffs in the first place.

@glebm
Copy link
Owner

glebm commented Sep 25, 2017

This should probably just issue a warning when loading the data (like we do for other pathological cases).

@vsakarov
Copy link
Author

Sounds good.

@glebm glebm modified the milestones: v0.9.19, v0.9.20 Nov 1, 2017
@glebm glebm modified the milestones: v0.9.20, v0.9.21 Jan 16, 2018
@glebm glebm removed this from the v0.9.21 milestone Sep 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants