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 matcher for checking missing interpolation keys #22

Closed
wants to merge 3 commits into from
Closed

Add matcher for checking missing interpolation keys #22

wants to merge 3 commits into from

Conversation

creasty
Copy link
Collaborator

@creasty creasty commented Mar 3, 2015

Wrote matcher for checking missing pairs of interpolation keys like:
'I am %{name}' and 'Soy %{nam3}'
'I am %{name}' and 'Soy name'

@aprescott
Copy link

This is a really nice addition. I think it would be good to actually include this as part of the functionality of the be_a_subset_of and be_a_complete_translation_of matchers. That way, not only are you asserting that translations aren't missing, you're also asserting that the interpolation keys didn't get altered or missed. What do you think?

@mixmix
Copy link

mixmix commented Mar 3, 2015

I built something similar a while ago : #15

From experience I recommend a test case which passes as long as the keys are all present, regardless of order

e.g expect
english: %{name} is now an admin of %{group}
bad_french: %{group} a un nouvel admin %{name}
to match

@creasty
Copy link
Collaborator Author

creasty commented Mar 3, 2015

@aprescott

Well, actually I did in that way at the first place.
But I've changed it, after I thought translations didn't always have to fully contain interpolation keys across all languages.
And as I18.t, it simply ignores binding for missing keys, instead of raising an error.

So it's okay if be_a_complete_translation_of to take second argument to enable this feature.
How about that?

@aprescott
Copy link

@creasty interesting. I'm trying to imagine a case where you wouldn't want an interpolation key in one language even though it's used in another. I suppose if that's a legitimate use case, then yeah, an option argument seems fine to me! (Not a maintainer or owner of this repo, I just would really like to have the extra guarantee as a user of i18n-spec. :))

@creasty
Copy link
Collaborator Author

creasty commented Mar 3, 2015

@aprescott aight, I'm gonna change it later

@creasty
Copy link
Collaborator Author

creasty commented Mar 3, 2015

@s01us Ah, sorry I might have misunderstood your advise.
What you meant to say was to write fixtures having different order of keys so that spec could guarantee of its behavior, isn't it.

@creasty
Copy link
Collaborator Author

creasty commented Mar 4, 2015

Added check_interpolation_key as the second argument to be_a_subset_of and be_a_complete_translation_of. And did a bit of refactoring.

@@ -1,14 +1,32 @@
RSpec::Matchers.define :be_a_complete_translation_of do |default_locale_filepath|
RSpec::Matchers.define :be_a_complete_translation_of do |default_locale_filepath, check_interpolation_key = false|

Choose a reason for hiding this comment

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

I don't know what the minimum version of Ruby is that is supported by this library, but it'd be great to use a keyword argument here of check_interpolation_key: false. I also have a feeling that a default value of true would be more useful.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Using keyword argument seems to me fine tho it's, in general, difficult problem to take Ruby version up to least 2.x unnecessarily.
And about the default value, I just want this feature to fail existing specs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just dropped support for EOL'd rubies, so we can actually apply this suggestion now 👍

@creasty
Copy link
Collaborator Author

creasty commented Mar 26, 2015

Quite uncertain whether @tigrish is alive...

@creasty
Copy link
Collaborator Author

creasty commented Apr 23, 2015

I'm tired of waiting.
I've extracted this feature into my own gem without depending on i18n-spec:
https://github.com/creasty/i18n_interpolation_spec

@deivid-rodriguez
Copy link
Collaborator

@creasty Are you still interested in getting this merged in? I think it would be a valuable addition!

@creasty creasty closed this Oct 11, 2020
@creasty creasty deleted the be_a_complete_interpolation_key_of branch October 11, 2020 18:37
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.

4 participants