From cdaf1072daecd628a89f019b701bc0a2fa27c20e Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Fri, 25 Aug 2017 11:23:48 +0200 Subject: [PATCH] Move detailed information of an entry into a separate class --- lib/gitlab/i18n/po_entry.rb | 66 ++++++++++++++++++++++++ lib/gitlab/i18n/po_linter.rb | 40 ++++----------- spec/lib/gitlab/i18n/po_entry_spec.rb | 71 ++++++++++++++++++++++++++ spec/lib/gitlab/i18n/po_linter_spec.rb | 10 ++-- 4 files changed, 153 insertions(+), 34 deletions(-) create mode 100644 lib/gitlab/i18n/po_entry.rb create mode 100644 spec/lib/gitlab/i18n/po_entry_spec.rb diff --git a/lib/gitlab/i18n/po_entry.rb b/lib/gitlab/i18n/po_entry.rb new file mode 100644 index 00000000000..aabb477bbea --- /dev/null +++ b/lib/gitlab/i18n/po_entry.rb @@ -0,0 +1,66 @@ +module Gitlab + module I18n + class PoEntry + attr_reader :entry_data + + def initialize(entry_data) + @entry_data = entry_data + end + + def msgid + entry_data[:msgid] + end + + def metadata? + msgid.empty? + end + + def plural_id + entry_data[:msgid_plural] + end + + def plural? + plural_id.present? + end + + def singular_translation + plural? ? entry_data['msgstr[0]'] : entry_data[:msgstr] + end + + def all_translations + @all_translations ||= entry_data.fetch_values(*translation_keys) + end + + def plural_translations + return [] unless plural? + + # The singular translation is used if there's only translation. This is + # the case for languages without plurals. + return all_translations if all_translations.size == 1 + + entry_data.fetch_values(*plural_translation_keys) + end + + def flag + entry_data[:flag] + end + + private + + def plural_translation_keys + @plural_translation_keys ||= translation_keys.select do |key| + plural_index = key.scan(/\d+/).first.to_i + plural_index > 0 + end + end + + def translation_keys + @translation_keys ||= if plural? + entry_data.keys.select { |key| key =~ /msgstr\[\d+\]/ } + else + [:msgstr] + end + end + end + end +end diff --git a/lib/gitlab/i18n/po_linter.rb b/lib/gitlab/i18n/po_linter.rb index 201d73cfe1d..2f6965a19aa 100644 --- a/lib/gitlab/i18n/po_linter.rb +++ b/lib/gitlab/i18n/po_linter.rb @@ -25,7 +25,7 @@ module Gitlab end def parse_po - @entries = SimplePoParser.parse(po_path) + @entries = SimplePoParser.parse(po_path).map { |data| Gitlab::I18n::PoEntry.new(data) } nil rescue SimplePoParser::ParserError => e @entries = [] @@ -36,11 +36,10 @@ module Gitlab errors = {} entries.each do |entry| - # Skip validation of metadata - next if entry[:msgid].empty? + next if entry.metadata? errors_for_entry = validate_entry(entry) - errors[join_message(entry[:msgid])] = errors_for_entry if errors_for_entry.any? + errors[join_message(entry.msgid)] = errors_for_entry if errors_for_entry.any? end errors @@ -57,27 +56,24 @@ module Gitlab end def validate_newlines(errors, entry) - message_id = join_message(entry[:msgid]) + message_id = join_message(entry.msgid) - if entry[:msgid].is_a?(Array) + if entry.msgid.is_a?(Array) errors << "<#{message_id}> is defined over multiple lines, this breaks some tooling." end - if translations_in_entry(entry).any? { |translation| translation.is_a?(Array) } + if entry.all_translations.any? { |translation| translation.is_a?(Array) } errors << "<#{message_id}> has translations defined over multiple lines, this breaks some tooling." end end def validate_variables(errors, entry) - if entry[:msgid_plural].present? - validate_variables_in_message(errors, entry[:msgid], entry['msgstr[0]']) + validate_variables_in_message(errors, entry.msgid, entry.singular_translation) - # Validate all plurals - entry.keys.select { |key_name| key_name =~ /msgstr\[[1-9]\]/ }.each do |plural_key| - validate_variables_in_message(errors, entry[:msgid_plural], entry[plural_key]) + if entry.plural? + entry.plural_translations.each do |translation| + validate_variables_in_message(errors, entry.plural_id, translation) end - else - validate_variables_in_message(errors, entry[:msgid], entry[:msgstr]) end end @@ -168,26 +164,12 @@ module Gitlab end def validate_flags(errors, entry) - if flag = entry[:flag] - errors << "is marked #{flag}" - end + errors << "is marked #{entry.flag}" if entry.flag end def join_message(message) Array(message).join end - - def translations_in_entry(entry) - if entry[:msgid_plural].present? - entry.fetch_values(*plural_translation_keys_in_entry(entry)) - else - [entry[:msgstr]] - end - end - - def plural_translation_keys_in_entry(entry) - entry.keys.select { |key| key =~ /msgstr\[\d*\]/ } - end end end end diff --git a/spec/lib/gitlab/i18n/po_entry_spec.rb b/spec/lib/gitlab/i18n/po_entry_spec.rb new file mode 100644 index 00000000000..0317caedbff --- /dev/null +++ b/spec/lib/gitlab/i18n/po_entry_spec.rb @@ -0,0 +1,71 @@ +require 'spec_helper' + +describe Gitlab::I18n::PoEntry do + describe '#singular_translation' do + it 'returns the normal `msgstr` for translations without plural' do + data = { msgid: 'Hello world', msgstr: 'Bonjour monde' } + entry = described_class.new(data) + + expect(entry.singular_translation).to eq('Bonjour monde') + end + + it 'returns the first string for entries with plurals' do + data = { + msgid: 'Hello world', + msgid_plural: 'Hello worlds', + 'msgstr[0]' => 'Bonjour monde', + 'msgstr[1]' => 'Bonjour mondes' + } + entry = described_class.new(data) + + expect(entry.singular_translation).to eq('Bonjour monde') + end + end + + describe '#all_translations' do + it 'returns all translations for singular translations' do + data = { msgid: 'Hello world', msgstr: 'Bonjour monde' } + entry = described_class.new(data) + + expect(entry.all_translations).to eq(['Bonjour monde']) + end + + it 'returns all translations when including plural translations' do + data = { + msgid: 'Hello world', + msgid_plural: 'Hello worlds', + 'msgstr[0]' => 'Bonjour monde', + 'msgstr[1]' => 'Bonjour mondes' + } + entry = described_class.new(data) + + expect(entry.all_translations).to eq(['Bonjour monde', 'Bonjour mondes']) + end + end + + describe '#plural_translations' do + it 'returns all translations if there is only one plural' do + data = { + msgid: 'Hello world', + msgid_plural: 'Hello worlds', + 'msgstr[0]' => 'Bonjour monde' + } + entry = described_class.new(data) + + expect(entry.plural_translations).to eq(['Bonjour monde']) + end + + it 'returns all translations except for the first one if there are multiple' do + data = { + msgid: 'Hello world', + msgid_plural: 'Hello worlds', + 'msgstr[0]' => 'Bonjour monde', + 'msgstr[1]' => 'Bonjour mondes', + 'msgstr[2]' => 'Bonjour tous les mondes' + } + entry = described_class.new(data) + + expect(entry.plural_translations).to eq(['Bonjour mondes', 'Bonjour tous les mondes']) + end + end +end diff --git a/spec/lib/gitlab/i18n/po_linter_spec.rb b/spec/lib/gitlab/i18n/po_linter_spec.rb index a8e9e4377b6..2e155511076 100644 --- a/spec/lib/gitlab/i18n/po_linter_spec.rb +++ b/spec/lib/gitlab/i18n/po_linter_spec.rb @@ -127,13 +127,13 @@ describe Gitlab::I18n::PoLinter do describe '#validate_entries' do it 'skips entries without a `msgid`' do - allow(linter).to receive(:entries) { [{ msgid: "" }] } + allow(linter).to receive(:entries) { [Gitlab::I18n::PoEntry.new({ msgid: "" })] } expect(linter.validate_entries).to be_empty end it 'keeps track of errors for entries' do - fake_invalid_entry = { msgid: "Hello %{world}", msgstr: "Bonjour %{monde}" } + fake_invalid_entry = Gitlab::I18n::PoEntry.new({ msgid: "Hello %{world}", msgstr: "Bonjour %{monde}" }) allow(linter).to receive(:entries) { [fake_invalid_entry] } expect(linter).to receive(:validate_entry) @@ -158,12 +158,12 @@ describe Gitlab::I18n::PoLinter do describe '#validate_variables' do it 'validates both signular and plural in a pluralized string' do - pluralized_entry = { + pluralized_entry = Gitlab::I18n::PoEntry.new({ msgid: 'Hello %{world}', msgid_plural: 'Hello all %{world}', 'msgstr[0]' => 'Bonjour %{world}', 'msgstr[1]' => 'Bonjour tous %{world}' - } + }) expect(linter).to receive(:validate_variables_in_message) .with([], 'Hello %{world}', 'Bonjour %{world}') @@ -174,7 +174,7 @@ describe Gitlab::I18n::PoLinter do end it 'validates the message variables' do - entry = { msgid: 'Hello', msgstr: 'Bonjour' } + entry = Gitlab::I18n::PoEntry.new({ msgid: 'Hello', msgstr: 'Bonjour' }) expect(linter).to receive(:validate_variables_in_message) .with([], 'Hello', 'Bonjour')