Merge pull request #3787 from gschlager/locale-keys

FIX: Some strings in locale files were not translatable

config/locales/client.en.yml

@@ -153,7 +153,6 @@ en:
     sign_up: "Sign Up"
     log_in: "Log In"
     age: "Age"
-    last_post: "Last Post"
     joined: "Joined"
     admin_title: "Admin"
     flags_title: "Flags"

config/locales/server.en.yml

@@ -20,17 +20,21 @@ en:
     short_date: "D MMM, YYYY"
     long_date: "MMMM D, YYYY h:mma"
 
-  datetime: &datetime
-    month_names:
-      [~, January, February, March, April, May, June, July, August, September, October, November, December]
+  datetime_formats: &datetime_formats
     formats:
+      # Format directives: http://ruby-doc.org/core-2.2.0/Time.html#method-i-strftime
       short: "%m-%d-%Y"
+      # Format directives: http://ruby-doc.org/core-2.2.0/Time.html#method-i-strftime
       short_no_year: "%B %-d"
+      # Format directives: http://ruby-doc.org/core-2.2.0/Time.html#method-i-strftime
       date_only: "%B %-d, %Y"
   date:
-    <<: *datetime
+    # Do not remove the brackets and commas and do not translate the first month name. It should be "null".
+    month_names:
+      [~, January, February, March, April, May, June, July, August, September, October, November, December]
+    <<: *datetime_formats
   time:
-    <<: *datetime
+    <<: *datetime_formats
 
   title: "Discourse"
   topics: "Topics"
@@ -62,8 +66,10 @@ en:
       exclusion: is reserved
       greater_than: must be greater than %{count}
       greater_than_or_equal_to: must be greater than or equal to %{count}
+      has_already_been_used: "has already been used"
       inclusion: is not included in the list
       invalid: is invalid
+      is_invalid: "is invalid; try to be a little more descriptive"
       less_than: must be less than %{count}
       less_than_or_equal_to: must be less than or equal to %{count}
       not_a_number: is not a number
@@ -101,9 +107,6 @@ en:
   activemodel:
     errors:
       <<: *errors
-  activerecord:
-    errors:
-      <<: *errors
 
   bulk_invite:
     file_should_be_csv: "The uploaded file should be of csv or txt format."
@@ -280,9 +283,7 @@ en:
       user:
         ip_address: ""
     errors:
-      messages:
-        is_invalid: "is invalid; try to be a little more descriptive"
-        has_already_been_used: "has already been used"
+      <<: *errors
       models:
         topic:
           attributes:

lib/locale_file_walker.rb

@@ -0,0 +1,48 @@
+require 'psych'
+
+class LocaleFileWalker
+  protected
+
+  def handle_document(document)
+    # we want to ignore the language (first key), so let's start at -1
+    handle_nodes(document.root.children, -1, [])
+  end
+
+  def handle_nodes(nodes, depth, parents)
+    if nodes
+      consecutive_scalars = 0
+      nodes.each do |node|
+        consecutive_scalars = handle_node(node, depth, parents, consecutive_scalars)
+      end
+    end
+  end
+
+  def handle_node(node, depth, parents, consecutive_scalars)
+    node_is_scalar = node.is_a?(Psych::Nodes::Scalar)
+
+    if node_is_scalar
+      handle_scalar(node, depth, parents) if valid_scalar?(depth, consecutive_scalars)
+    elsif node.is_a?(Psych::Nodes::Alias)
+      handle_alias(node, depth, parents)
+    elsif node.is_a?(Psych::Nodes::Mapping)
+      handle_mapping(node, depth, parents)
+      handle_nodes(node.children, depth + 1, parents.dup)
+    end
+
+    node_is_scalar ? consecutive_scalars + 1 : 0
+  end
+
+  def valid_scalar?(depth, consecutive_scalars)
+    depth >= 0 && consecutive_scalars.even?
+  end
+
+  def handle_scalar(node, depth, parents)
+    parents[depth] = node.value
+  end
+
+  def handle_alias(node, depth, parents)
+  end
+
+  def handle_mapping(node, depth, parents)
+  end
+end

script/pull_translations.rb

@@ -6,6 +6,7 @@
 # team will pull them in.
 
 require 'open3'
+require_relative '../lib/locale_file_walker'
 
 if `which tx`.strip.empty?
   puts '', 'The Transifex client needs to be installed to use this script.'
@@ -17,10 +18,11 @@ if `which tx`.strip.empty?
   exit 1
 end
 
-locales = Dir.glob(File.expand_path('../../config/locales/client.*.yml', __FILE__)).map {|x| x.split('.')[-2]}.select {|x| x != 'en'}.sort.join(',')
+languages = Dir.glob(File.expand_path('../../config/locales/client.*.yml', __FILE__))
+              .map { |x| x.split('.')[-2] }.select { |x| x != 'en' }.sort
 
 puts 'Pulling new translations...', ''
-command = "tx pull --mode=developer --language=#{locales} #{ARGV.include?('force') ? '-f' : ''}"
+command = "tx pull --mode=developer --language=#{languages.join(',')} #{ARGV.include?('force') ? '-f' : ''}"
 
 Open3.popen2e(command) do |stdin, stdout_err, wait_thr|
   while (line = stdout_err.gets)
@@ -46,20 +48,181 @@ END
 YML_DIRS = ['config/locales',
             'plugins/poll/config/locales',
             'vendor/gems/discourse_imgur/lib/discourse_imgur/locale']
+YML_FILE_PREFIXES = ['server', 'client']
 
-# Add comments to the top of files
-['client', 'server'].each do |base|
-  YML_DIRS.each do |dir|
-    Dir.glob(File.expand_path("../../#{dir}/#{base}.*.yml", __FILE__)).each do |file_name|
-      language = File.basename(file_name).match(Regexp.new("#{base}\\.([^\\.]*)\\.yml"))[1]
+def yml_path(dir, prefix, language)
+  path = "../../#{dir}/#{prefix}.#{language}.yml"
+  path = File.expand_path(path, __FILE__)
+  File.exists?(path) ? path : nil
+end
 
-      lines = File.readlines(file_name)
+# Add comments to the top of files and replace the language (first key in YAML file)
+def update_file_header(filename, language)
+  lines = File.readlines(filename)
   lines.collect! {|line| line =~ /^[a-z_]+:$/i ? "#{language}:" : line}
 
-      File.open(file_name, 'w+') do |f|
+  File.open(filename, 'w+') do |f|
     f.puts(YML_FILE_COMMENTS, '') unless lines[0][0] == '#'
     f.puts(lines)
   end
+end
+
+class YamlAliasFinder < LocaleFileWalker
+  def initialize
+    @anchors = {}
+    @aliases = Hash.new { |hash, key| hash[key] = [] }
+  end
+
+  def parse_file(filename)
+    document = Psych.parse_file(filename)
+    handle_document(document)
+    {anchors: @anchors, aliases: @aliases}
+  end
+
+  private
+
+  def handle_alias(node, depth, parents)
+    @aliases[node.anchor] << parents.dup
+  end
+
+  def handle_mapping(node, depth, parents)
+    if node.anchor
+      @anchors[parents.dup] = node.anchor
+    end
+  end
+end
+
+class YamlAliasSynchronizer < LocaleFileWalker
+  def initialize(original_alias_data)
+    @anchors = original_alias_data[:anchors]
+    @aliases = original_alias_data[:aliases]
+    @used_anchors = Set.new
+
+    calculate_required_keys
+  end
+
+  def add_to(filename)
+    stream = Psych.parse_stream(File.read(filename))
+    stream.children.each { |document| handle_document(document) }
+
+    add_aliases
+    write_yaml(stream, filename)
+  end
+
+  private
+
+  def calculate_required_keys
+    @required_keys = {}
+
+    @aliases.each_value do |key_sets|
+      key_sets.each do |keys|
+        until keys.empty?
+          add_needed_node(keys)
+          keys = keys.dup
+          keys.pop
+        end
+      end
+    end
+
+    add_needed_node([]) unless @required_keys.empty?
+  end
+
+  def add_needed_node(keys)
+    @required_keys[keys] = {mapping: nil, scalar: nil, alias: nil}
+  end
+
+  def write_yaml(stream, filename)
+    yaml = stream.to_yaml(nil, {:line_width => -1})
+
+    File.open(filename, 'w') do |file|
+      file.write(yaml)
+    end
+  end
+
+  def handle_scalar(node, depth, parents)
+    super(node, depth, parents)
+
+    if @required_keys.has_key?(parents)
+      @required_keys[parents][:scalar] = node
+    end
+  end
+
+  def handle_alias(node, depth, parents)
+    if @required_keys.has_key?(parents)
+      @required_keys[parents][:alias] = node
+    end
+  end
+
+  def handle_mapping(node, depth, parents)
+    if @anchors.has_key?(parents)
+      node.anchor = @anchors[parents]
+      @used_anchors.add(node.anchor)
+    end
+
+    if @required_keys.has_key?(parents)
+      @required_keys[parents][:mapping] = node
+    end
+  end
+
+  def add_aliases
+    @used_anchors.each do |anchor|
+      @aliases[anchor].each do |keys|
+        parents = []
+        parent_node = @required_keys[[]]
+
+        keys.each_with_index do |key, index|
+          parents << key
+          current_node = @required_keys[parents]
+          is_last = index == keys.size - 1
+          add_node(current_node, parent_node, key, is_last ? anchor : nil)
+          parent_node = current_node
+        end
+      end
+    end
+  end
+
+  def add_node(node, parent_node, scalar_name, anchor)
+    parent_mapping = parent_node[:mapping]
+    parent_mapping.children ||= []
+
+    if node[:scalar].nil?
+      node[:scalar] = Psych::Nodes::Scalar.new(scalar_name)
+      parent_mapping.children << node[:scalar]
+    end
+
+    if anchor.nil?
+      if node[:mapping].nil?
+        node[:mapping] = Psych::Nodes::Mapping.new
+        parent_mapping.children << node[:mapping]
+      end
+    elsif node[:alias].nil?
+      parent_mapping.children << Psych::Nodes::Alias.new(anchor)
+    end
+  end
+end
+
+def get_english_alias_data(dir, prefix)
+  filename = yml_path(dir, prefix, 'en')
+  filename ? YamlAliasFinder.new.parse_file(filename) : nil
+end
+
+def add_anchors_and_aliases(english_alias_data, filename)
+  if english_alias_data
+    YamlAliasSynchronizer.new(english_alias_data).add_to(filename)
+  end
+end
+
+YML_DIRS.each do |dir|
+  YML_FILE_PREFIXES.each do |prefix|
+    english_alias_data = get_english_alias_data(dir, prefix)
+
+    languages.each do |language|
+      filename = yml_path(dir, prefix, language)
+
+      if filename
+        add_anchors_and_aliases(english_alias_data, filename)
+        update_file_header(filename, language)
+      end
     end
   end
 end

spec/integrity/i18n_spec.rb

@@ -1,4 +1,5 @@
 require 'spec_helper'
+require 'locale_file_walker'
 
 describe "i18n integrity checks" do
 
@@ -57,4 +58,40 @@ describe "i18n integrity checks" do
     end
   end
 
+  describe 'keys in English locale files' do
+    locale_files = ['config/locales', 'plugins/**/locales']
+                     .product(['server.en.yml', 'client.en.yml'])
+                     .collect { |dir, filename| Dir["#{Rails.root}/#{dir}/#{filename}"] }
+                     .flatten
+                     .map { |path| Pathname.new(path).relative_path_from(Rails.root) }
+
+    class DuplicateKeyFinder < LocaleFileWalker
+      def find_duplicates(filename)
+        @keys_with_count = {}
+
+        document = Psych.parse_file(filename)
+        handle_document(document)
+
+        @keys_with_count.delete_if { |key, count| count <= 1 }.keys
+      end
+
+      protected
+
+      def handle_scalar(node, depth, parents)
+        super(node, depth, parents)
+
+        key = parents.join('.')
+        @keys_with_count[key] = @keys_with_count.fetch(key, 0) + 1
+      end
+    end
+
+    locale_files.each do |path|
+      context path do
+        it 'has no duplicate keys' do
+          duplicates = DuplicateKeyFinder.new.find_duplicates("#{Rails.root}/#{path}")
+          expect(duplicates).to be_empty
+        end
+      end
+    end
+  end
 end