-
Notifications
You must be signed in to change notification settings - Fork 266
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 check-consistent-interpolations #304
Conversation
@glebm I was not able to figure out why |
2f068d2
to
682adc8
Compare
Does |
args: %i[locales out_format] | ||
|
||
def check_consistent_interpolations(opt = {}) | ||
forest = i18n.wrong_interpolations(opt.slice(:locales, :base_locale)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename wrong_interpolations
to inconsistent_interpolation
throughout. This is because may check the arguments passed in code in the future, so I'd prefer the name of this check and associated methods to be specific for clarity.
base = base_locale || self.base_locale | ||
tree = empty_forest | ||
|
||
data[base].key_values.each do |key, node| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename node
to value
.
data[base].key_values.each do |key, node| | ||
next unless node.is_a?(String) | ||
|
||
base_variables = node.scan(VARIABLE_REGEX) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set.new(node.scan(VARIABLE_REGEX))
so that the sameness check is faster.
|
||
private | ||
|
||
def check_in_other_locale(tree, key, base_variables, locale) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Private module methods from multiple modules will, unfortunately, conflict with each other (https://repl.it/@glebm1/Ruby-Private-module-methods-conflict).
I suggest inlining the private methods into the public one (they will be tiny once the other comments have been addressed).
private | ||
|
||
def check_in_other_locale(tree, key, base_variables, locale) | ||
node = data[locale].first.children[key] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be simplified to:
data[locale][key]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data[locale][key]
does not do the same thing as data[locale].first.children[key]
, I have picked this pattern from equal_values_tree
in missing_keys.rb
end | ||
|
||
def select_key(key, locale) | ||
data[locale].select_keys(root: false) { |search_key| search_key == key } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is rather inefficient as it traverses the whole tree. Consider something like this instead:
tree.append!(
node.walk_to_root.reduce(nil) { |p, c| [p.derive(children: c)] }.tap do |root|
root.data[:type] = :inconsistent_interpolation
end
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That code does not work as is, and I do not understand walk_to_root/derive enough to figure out why. Won't append also fail when multiple nodes come from the same locale e.g. es
?
35241b2
to
f3c38ad
Compare
Verify that non-base translations are using variable names defined in the base translation. Fix glebm#303
OK, I was not able to use the faster traversal method, and I was not able to simplify the one method below the rubocop threshold, but otherwise it works now. |
Released in v0.9.26! I've renamed some methods and the ignore setting before releasing. |
* upstream/master: (177 commits) ✏️ Typo 📝 Add example of usage for "Key pattern syntax" Silence warnings for common leaf -> tree expansion scenario. Update README and post installation instructions message (glebm#320) Bump to v0.9.28 Do not consider a plural node if it only has one child with no count (glebm#312) Include file name in CommandError raised during unsuccessful file load (glebm#313) Use separate trees per locale in missing_plural_forest (glebm#311) Missing plural keys: Extract plural_nodes method Missing plural keys: minor optimization Missing plural keys: do not modify data nodes Check for missing plural keys (glebm#309) Fix interpolations with repeated vars readme [ci skip] changelog s/inconsistent_interpolation/inconsistent_interpolations Bump to v0.9.26 s/Commands::Inconsistent/Commands::Interpolations/ DeeplTranslator: Fix CommandError Follow-up to glebm#304 ...
Verify that non-base translations are using variable names defined in the base translation.
Fix #303