diff --git a/CHANGES.md b/CHANGES.md index 3f6cfec7..4195e488 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,9 +1,11 @@ ## 0.8.6 (unreleased) -* `health`, `missing`, and `unused` now exit with code `1` if the respective keys are detected [#151](https://github.com/glebm/i18n-tasks/issues/151). -* XLSX report compatibility with OSX Numbers App [#159](https://github.com/glebm/i18n-tasks/issues/159). -* RSpec template compatibility with `config.expose_dsl_globally = false` [#148](https://github.com/glebm/i18n-tasks/issues/148). -* `bundle show vagrant` example in the config template is no longer interpolated [#161](https://github.com/glebm/i18n-tasks/issues/161). +* Keys missing in the source are now reported in all locales from the missing task. [#162](https://github.com/glebm/i18n-tasks/issues/162) +* Fixed `data-remove` task. [#140](https://github.com/glebm/i18n-tasks/issues/140) +* `health`, `missing`, and `unused` now exit with code `1` if the respective keys are detected. [#151](https://github.com/glebm/i18n-tasks/issues/151) +* XLSX report compatibility with the OSX Numbers App. [#159](https://github.com/glebm/i18n-tasks/issues/159) +* RSpec template compatibility with `config.expose_dsl_globally = false`. [#148](https://github.com/glebm/i18n-tasks/issues/148) +* `bundle show vagrant` example in the config template is no longer interpolated .[#161](https://github.com/glebm/i18n-tasks/issues/161) ## 0.8.5 diff --git a/config/locales/en.yml b/config/locales/en.yml index 186c7fd4..3d78f73b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -15,7 +15,7 @@ en: key_pattern_to_rename: Full key (pattern) to rename. Required locale: Locale locale_to_translate_from: Locale to translate from - locales_filter: "Locale(s) to process. Special: base" + locales_filter: 'Locale(s) to process. Special: base' missing_types: 'Filter by types: %{valid}' new_key_name: New name, interpolates original name as %{key}. Required nostdin: Do not read from stdin @@ -90,7 +90,7 @@ en: none: No translations are missing. remove_unused: confirm: - one: One translations will be removed from %{locales}. + one: "%{count} translation will be removed from %{locales}." other: "%{count} translation will be removed from %{locales}." noop: No unused keys to remove removed: Removed %{count} keys diff --git a/lib/i18n/tasks/command/commands/missing.rb b/lib/i18n/tasks/command/commands/missing.rb index cf6c1d4c..87a1423e 100644 --- a/lib/i18n/tasks/command/commands/missing.rb +++ b/lib/i18n/tasks/command/commands/missing.rb @@ -49,11 +49,6 @@ def translate_missing(opt = {}) def add_missing(opt = {}) forest = i18n.missing_keys(opt).set_each_value!(opt[:value]) i18n.data.merge! forest - # missing keys detected in the source are only returned in the base locale tree - # merge again in case such keys have been added to add them to other locales - forest_2 = i18n.missing_keys(opt).set_each_value!(opt[:value]) - i18n.data.merge! forest_2 - forest.merge! forest_2 log_stderr t('i18n_tasks.add_missing.added', count: forest.leaves.count) print_forest forest, opt end diff --git a/lib/i18n/tasks/command/commands/usages.rb b/lib/i18n/tasks/command/commands/usages.rb index aa9e4d9c..faabf4a5 100644 --- a/lib/i18n/tasks/command/commands/usages.rb +++ b/lib/i18n/tasks/command/commands/usages.rb @@ -52,7 +52,7 @@ def remove_unused(opt = {}) def confirm_remove_unused!(unused_keys, opt) return if ENV['CONFIRM'] || opt[:confirm] - locales = bold(opt[:locales] * ', ') + locales = bold(unused_keys.flat_map { |root| root.key.split('+') }.sort.uniq * ', ') msg = [ red(t('i18n_tasks.remove_unused.confirm', count: unused_keys.leaves.count, locales: locales)), yellow(t('i18n_tasks.common.continue_q')), diff --git a/lib/i18n/tasks/console_context.rb b/lib/i18n/tasks/console_context.rb index 330946d6..9441980b 100644 --- a/lib/i18n/tasks/console_context.rb +++ b/lib/i18n/tasks/console_context.rb @@ -36,13 +36,12 @@ def guide green(bold "i18n-tasks IRB Quick Start guide") + "\n" + <<-TEXT #{yellow 'Data as trees'} tree(locale) - missing_tree(locale, compared_to = base_locale) used_tree(source_occurrences: false, key_filter: nil) unused_tree(locale) build_tree('es' => {'hello' => 'Hola'}) #{yellow 'Traversal'} - tree = missing_tree(base_locale) + tree = missing_diff_tree('es') tree.nodes { |node| } tree.nodes.to_a tree.leaves { |node| } diff --git a/lib/i18n/tasks/data/tree/node.rb b/lib/i18n/tasks/data/tree/node.rb index 5bfeeaee..bef0143b 100644 --- a/lib/i18n/tasks/data/tree/node.rb +++ b/lib/i18n/tasks/data/tree/node.rb @@ -63,7 +63,7 @@ def parent? end def children? - children && children.any? + children && !children.empty? end def data @@ -119,6 +119,13 @@ def walk_from_root(&visitor) end end + def set(full_key, node) + (@children ||= Siblings.new(parent: self)).set(full_key, node) + dirty! + node + end + alias []= set + def to_nodes Nodes.new([self]) end diff --git a/lib/i18n/tasks/data/tree/nodes.rb b/lib/i18n/tasks/data/tree/nodes.rb index 2f607263..b8604302 100644 --- a/lib/i18n/tasks/data/tree/nodes.rb +++ b/lib/i18n/tasks/data/tree/nodes.rb @@ -79,7 +79,7 @@ def merge!(nodes) alias + merge! def children(&block) - return to_enum(:children) { map { |c| c.children.size }.reduce(:+) } unless block + return to_enum(:children) { map { |c| c.children ? c.children.size : 0 }.reduce(:+) } unless block each do |node| node.children.each(&block) if node.children? end diff --git a/lib/i18n/tasks/data/tree/siblings.rb b/lib/i18n/tasks/data/tree/siblings.rb index dac57d5d..52b0d216 100644 --- a/lib/i18n/tasks/data/tree/siblings.rb +++ b/lib/i18n/tasks/data/tree/siblings.rb @@ -1,11 +1,15 @@ # coding: utf-8 +require 'set' +require 'i18n/tasks/split_key' require 'i18n/tasks/data/tree/nodes' module I18n::Tasks::Data::Tree # Siblings represents a subtree sharing a common parent # in case of an empty parent (nil) it represents a forest # siblings' keys are unique class Siblings < Nodes + include ::I18n::Tasks::SplitKey + attr_reader :parent, :key_to_node def initialize(opts = {}) @@ -44,8 +48,6 @@ def replace_node!(node, new_node) key_to_node[new_node.key] = new_node end - include ::I18n::Tasks::SplitKey - # @return [Node] by full key def get(full_key) first_key, rest = split_key(full_key.to_s, 2) @@ -121,15 +123,12 @@ def merge(nodes) end def subtract_keys(keys) - exclude = {} + to_remove = Set.new keys.each do |full_key| - if (node = get full_key) - exclude[node] = true - end + node = get full_key + to_remove << node if node end - select_nodes { |node| - not exclude[node] || node.children.try(:all?) { |c| exclude[c] } - } + remove_nodes_collapsing_emptied_ancestors to_remove end def subtract_by_key(other) @@ -143,8 +142,6 @@ def set_root_key!(new_key, data = nil) self end - private - def merge_node!(node) if key_to_node.key?(node.key) our = key_to_node[node.key] @@ -164,6 +161,27 @@ def merge_node!(node) end end + # @param [Enumerable] nodes. Modified in-place. + def remove_nodes_collapsing_emptied_ancestors(nodes) + add_ancestors_that_only_contain_nodes! nodes + select_nodes { |node| !nodes.include?(node) } + end + + # @param [Enumerable] nodes. Modified in-place. + def remove_nodes_collapsing_emptied_ancestors!(nodes) + add_ancestors_that_only_contain_nodes! nodes + select_nodes! { |node| !nodes.include?(node) } + end + + private + + # @param [Set] nodes. Modified in-place. + def add_ancestors_that_only_contain_nodes!(nodes) + levels.reverse_each do |level_nodes| + level_nodes.each { |node| nodes << node if node.children? && node.children.all? { |c| nodes.include?(c) } } + end + end + def warn_add_children_to_leaf(node) ::I18n::Tasks::Logging.log_warn "'#{node.full_key}' was a leaf, now has children (value <- scope conflict)" end diff --git a/lib/i18n/tasks/data/tree/traversal.rb b/lib/i18n/tasks/data/tree/traversal.rb index 48afb947..0b518501 100644 --- a/lib/i18n/tasks/data/tree/traversal.rb +++ b/lib/i18n/tasks/data/tree/traversal.rb @@ -18,14 +18,15 @@ def leaves(&visitor) def levels(&block) return to_enum(:levels) unless block - nodes = to_nodes + nodes = to_nodes unless nodes.empty? block.yield nodes - if nodes.children.size == 1 - first.children + if nodes.size == 1 + node = first + node.children.levels(&block) if node.children? else - Nodes.new(nodes: nodes.children) - end.levels(&block) + Nodes.new(nodes: nodes.children).levels(&block) + end end self end @@ -76,7 +77,8 @@ def root_key_values(sort = false) #-- modify / derive - # @return Siblings + # Select the nodes for which the block returns true. Pre-order traversal. + # @return [Siblings] a new tree def select_nodes(&block) tree = Siblings.new each do |node| @@ -90,6 +92,22 @@ def select_nodes(&block) tree end + # Select the nodes for which the block returns true. Pre-order traversal. + # @return [Siblings] self + def select_nodes!(&block) + to_remove = [] + each do |node| + if block.yield(node) + node.children.select_nodes!(&block) if node.children + else + # removing during each is unsafe + to_remove << node + end + end + to_remove.each { |node| remove! node } + self + end + # @return Siblings def select_keys(opts = {}, &block) root = opts.key?(:root) ? opts[:root] : false diff --git a/lib/i18n/tasks/missing_keys.rb b/lib/i18n/tasks/missing_keys.rb index 0312fd40..62131d9d 100644 --- a/lib/i18n/tasks/missing_keys.rb +++ b/lib/i18n/tasks/missing_keys.rb @@ -1,4 +1,5 @@ # coding: utf-8 +require 'set' module I18n::Tasks module MissingKeys @@ -48,20 +49,10 @@ def missing_diff_forest(locales, base = base_locale) tree end - def missing_used_forest(locales, base = base_locale) - if locales.include?(base) - missing_used_tree(base) - else - empty_forest - end - end - - def missing_tree(locale, compared_to) - if locale == compared_to - missing_used_tree locale - else - missing_diff_tree locale, compared_to - end + def missing_used_forest(locales, _base = base_locale) + locales.inject(empty_forest) { |forest, locale| + forest.merge! missing_used_tree(locale) + } end # keys present in compared_to, but not in locale @@ -96,5 +87,32 @@ def equal_values_tree(locale, compare_to = base_locale) def locale_key_missing?(locale, key) !key_value?(key, locale) && !ignore_key?(key, :missing) end + + # @param [::I18n::Tasks::Data::Tree::Siblings] forest + def collapse_missing_used_locales!(forest) + locales_and_nodes_by_key = {} + to_remove = [] + forest.each do |root| + locale = root.key + root.leaves { |node| + if node.data[:type] == :missing_used + (locales_and_nodes_by_key[node.full_key(root: false)] ||= []) << [locale, node] + to_remove << node + end + } + end + forest.remove_nodes_collapsing_emptied_ancestors! to_remove + keys_and_nodes_by_locale = {} + locales_and_nodes_by_key.each { |key, locales_and_nodes| + locales = locales_and_nodes.map(&:first).sort.join('+') + (keys_and_nodes_by_locale[locales] ||= []) << [key, locales_and_nodes[0][1]] + } + keys_and_nodes_by_locale.map { |locales, keys_nodes| + keys_nodes.each { |(key, node)| + forest["#{locales}.#{key}"] = node + } + } + forest + end end end diff --git a/lib/i18n/tasks/reports/base.rb b/lib/i18n/tasks/reports/base.rb index ace119db..8eac4578 100644 --- a/lib/i18n/tasks/reports/base.rb +++ b/lib/i18n/tasks/reports/base.rb @@ -51,5 +51,14 @@ def forest_to_attr(forest) {key: key, value: node.value, type: node.data[:type], locale: node.root.key, data: node.data} } end + + def format_locale(locale) + return '' unless locale + if locale.split('+') == task.locales.sort + 'all' + else + locale.tr '+', ' ' + end + end end end diff --git a/lib/i18n/tasks/reports/spreadsheet.rb b/lib/i18n/tasks/reports/spreadsheet.rb index de1c4c34..77954493 100644 --- a/lib/i18n/tasks/reports/spreadsheet.rb +++ b/lib/i18n/tasks/reports/spreadsheet.rb @@ -20,17 +20,19 @@ def save_report(path, opts) private def add_missing_sheet(wb) - tree = task.missing_keys + forest = task.missing_keys + forest = task.collapse_plural_nodes!(forest) + forest = task.collapse_missing_used_locales!(forest) wb.styles do |s| type_cell = s.add_style :alignment => {:horizontal => :center} locale_cell = s.add_style :alignment => {:horizontal => :center} regular_style = s.add_style - wb.add_worksheet(name: missing_title(tree)) { |sheet| + wb.add_worksheet(name: missing_title(forest)) { |sheet| sheet.page_setup.fit_to :width => 1 sheet.add_row [I18n.t('i18n_tasks.common.type'), I18n.t('i18n_tasks.common.locale'), I18n.t('i18n_tasks.common.key'), I18n.t('i18n_tasks.common.base_value')] style_header sheet - tree.keys do |key, node| - locale, type = node.root.data[:locale], node.data[:type] + forest.keys do |key, node| + locale, type = format_locale(node.root.data[:locale]), node.data[:type] sheet.add_row [missing_type_info(type)[:summary], locale, key, task.t(key)], styles: [type_cell, locale_cell, regular_style, regular_style] end diff --git a/lib/i18n/tasks/reports/terminal.rb b/lib/i18n/tasks/reports/terminal.rb index bb288b5c..2cce5059 100644 --- a/lib/i18n/tasks/reports/terminal.rb +++ b/lib/i18n/tasks/reports/terminal.rb @@ -9,13 +9,14 @@ class Terminal < Base def missing_keys(forest = task.missing_keys) forest = task.collapse_plural_nodes!(forest) + forest = task.collapse_missing_used_locales!(forest) if forest.present? print_title missing_title(forest) print_table headings: [cyan(bold(I18n.t('i18n_tasks.common.locale'))), cyan(bold I18n.t('i18n_tasks.common.key')), I18n.t('i18n_tasks.missing.details_title')] do |t| t.rows = sort_by_attr!(forest_to_attr(forest)).map do |a| - [{value: cyan(a[:locale]), alignment: :center}, cyan(a[:key]), missing_key_info(a)] + [{value: cyan(format_locale(a[:locale])), alignment: :center}, cyan(a[:key]), missing_key_info(a)] end end else diff --git a/spec/i18n_tasks_spec.rb b/spec/i18n_tasks_spec.rb index 5e911a40..e463ec0c 100644 --- a/spec/i18n_tasks_spec.rb +++ b/spec/i18n_tasks_spec.rb @@ -44,31 +44,36 @@ end describe 'missing' do - let (:expected_missing_keys) { - %w( en.used_but_missing.key - en.relative.index.missing - es.missing_in_es.a - en.present_in_es_but_not_en.a - en.hash.pattern_missing.a - en.hash.pattern_missing.b - en.missing_symbol_key - en.missing_symbol.key_two - en.missing_symbol.key_three - es.missing_in_es_plural_1.a - es.missing_in_es_plural_2.a - en.missing-key-with-a-dash.key - en.missing-key-question?.key - en.fn_comment - en.only_in_es - en.events.show.success - ) + let (:expected_missing_keys_in_source) { + %w( + used_but_missing.key + relative.index.missing + hash.pattern_missing.a + hash.pattern_missing.b + missing_symbol_key + missing_symbol.key_two + missing_symbol.key_three + missing-key-with-a-dash.key + missing-key-question?.key + fn_comment + events.show.success + ) + } + let (:expected_missing_keys_diff) { + %w( + es.missing_in_es.a + en.present_in_es_but_not_en.a + es.missing_in_es_plural_1.a + es.missing_in_es_plural_2.a + en.only_in_es + ) } it 'detects missing' do - es_keys = expected_missing_keys.grep(/^es\./) + es_keys = expected_missing_keys_diff.grep(/^es\./) + expected_missing_keys_in_source.map { |k| "es.#{k}" } out, result = run_cmd_capture_stdout_and_result 'missing' expect(result).to eq :exit_1 - expect(out).to be_i18n_keys expected_missing_keys - # locale argument + expect(out).to be_i18n_keys(expected_missing_keys_diff + + expected_missing_keys_in_source.map { |k| "all.#{k}" }) expect(run_cmd 'missing', '-les').to be_i18n_keys es_keys expect(run_cmd 'missing', 'es').to be_i18n_keys es_keys end diff --git a/spec/support/i18n_tasks_output_matcher.rb b/spec/support/i18n_tasks_output_matcher.rb index 184df2c3..4bb3d320 100644 --- a/spec/support/i18n_tasks_output_matcher.rb +++ b/spec/support/i18n_tasks_output_matcher.rb @@ -8,14 +8,13 @@ def extract_keys(actual) actual = Term::ANSIColor.uncolor(actual).split("\n").map(&:presence).compact actual = actual[3..-2] actual = actual.map { |row| - next if row =~ /^\|\s+\|/ - row.gsub(/(?:\s|^)\|(?:\s|$)/, ' ').gsub(/\s+/, ' ').strip.split(' ').map(&:presence).compact + row[1..-1].gsub(/(?:\s+|^)\|(?:\s+|$)/, '|').gsub(/\s+/, ' ').strip.split(/\s*\|\s*/) }.compact return [] if actual.empty? locale_col = 0 key_col = 1 actual.map { |row| - key = "#{row[locale_col]}.#{row[key_col]}" + key = [row[locale_col], row[key_col]].map(&:presence).compact.join('.') key = key[0..-2] if key.end_with?(':') key }.compact