From 49dc470a280ba8c506a77b5ae4d807345ec470e7 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager <mail@gerhard-schlager.at> Date: Fri, 18 Sep 2015 21:12:37 +0200 Subject: [PATCH 1/3] Add spec to check locale files for duplicate keys --- lib/locale_file_walker.rb | 48 +++++++++++++++++++++++++++++++++++++ spec/integrity/i18n_spec.rb | 37 ++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 lib/locale_file_walker.rb diff --git a/lib/locale_file_walker.rb b/lib/locale_file_walker.rb new file mode 100644 index 00000000000..0ac3146e84e --- /dev/null +++ b/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 diff --git a/spec/integrity/i18n_spec.rb b/spec/integrity/i18n_spec.rb index 2525668f542..e7b9c08fec9 100644 --- a/spec/integrity/i18n_spec.rb +++ b/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 From ade31c4468206240b3fba1e6cb4dd00d7c865a19 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager <mail@gerhard-schlager.at> Date: Fri, 18 Sep 2015 22:29:01 +0200 Subject: [PATCH 2/3] FIX: Remove duplicate keys from locale files --- config/locales/client.en.yml | 1 - config/locales/server.en.yml | 23 ++++++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 71a635f9afe..186c462884d 100644 --- a/config/locales/client.en.yml +++ b/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" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 1a91ce928d4..2cab867e202 100644 --- a/config/locales/server.en.yml +++ b/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: From d2d823186cf76461f649227474e6406d8fff2783 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager <mail@gerhard-schlager.at> Date: Fri, 18 Sep 2015 22:30:28 +0200 Subject: [PATCH 3/3] Add support for YAML aliases to pull_translations script The English locale files use aliases and anchors in order to prevent duplicate translations. Transifex doesn't support them. The script finds those aliases and anchors in the original locale files and adds them to the locale files pulled from Transifex. --- script/pull_translations.rb | 187 +++++++++++++++++++++++++++++++++--- 1 file changed, 175 insertions(+), 12 deletions(-) diff --git a/script/pull_translations.rb b/script/pull_translations.rb index 98da5ad419d..231482fcb26 100644 --- a/script/pull_translations.rb +++ b/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,19 +48,180 @@ 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) - lines.collect! {|line| line =~ /^[a-z_]+:$/i ? "#{language}:" : line} +# 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| - f.puts(YML_FILE_COMMENTS, '') unless lines[0][0] == '#' - f.puts(lines) + 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