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 interpolation variable matcher #15

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

Conversation

mixmix
Copy link

@mixmix mixmix commented Aug 15, 2014

here's something I'd love to use in devise-i18n
it check interpolation keys match in translations

based on something similar I made for loomio.
Please feel free to get me to rework anything that is unclear/ verbose

end

failure_for_should do |filepath|
"Expected #{filepath} interpolation variables to match #{default_locale_filepath}. \nProblems in:\n- " << @superset.join("\n- ")
Copy link
Author

Choose a reason for hiding this comment

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

I followed the pattern in another matcher file for failure_for_should

@@ -0,0 +1,10 @@
module I18nSpec
class Parse
Copy link
Author

Choose a reason for hiding this comment

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

I created a Parse class because this method didn't quite feel like it belonged in the LocaleFile class.
Let me know what you think

test_value = test_translations[key]

test_value.nil? ||
I18nSpec::Parse.for_interpolation_variables(default_value) == I18nSpec::Parse.for_interpolation_variables(test_value)
Copy link
Author

Choose a reason for hiding this comment

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

In this feature, I am declaring it a pass ✅ when:

A) the test locale string completely is empty / blank
OR
B) the test locale & the default locale have exactly the same interpolation variables

Is this ok?

@mixmix
Copy link
Author

mixmix commented Aug 30, 2014

@tigrish would love a merge or feedback on this when you've got time
:)

@deivid-rodriguez
Copy link
Collaborator

@mixmix Am I right assuming that #22 would make this PR unnecessary?

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

Successfully merging this pull request may close these issues.

2 participants