Take nplurals
into account when validating translations.
This commit is contained in:
parent
2c4f9b7a73
commit
abe198723d
9 changed files with 120 additions and 112 deletions
|
@ -1,13 +1,16 @@
|
|||
module Gitlab
|
||||
module I18n
|
||||
class MetadataEntry < PoEntry
|
||||
class MetadataEntry
|
||||
attr_reader :entry_data
|
||||
|
||||
def initialize(entry_data)
|
||||
@entry_data = entry_data
|
||||
end
|
||||
|
||||
def expected_plurals
|
||||
return nil unless plural_information
|
||||
|
||||
nplurals = plural_information['nplurals'].to_i
|
||||
if nplurals > 0
|
||||
nplurals
|
||||
end
|
||||
plural_information['nplurals'].to_i
|
||||
end
|
||||
|
||||
private
|
||||
|
|
|
@ -1,19 +0,0 @@
|
|||
module Gitlab
|
||||
module I18n
|
||||
class PoEntry
|
||||
def self.build(entry_data)
|
||||
if entry_data[:msgid].empty?
|
||||
MetadataEntry.new(entry_data)
|
||||
else
|
||||
TranslationEntry.new(entry_data)
|
||||
end
|
||||
end
|
||||
|
||||
attr_reader :entry_data
|
||||
|
||||
def initialize(entry_data)
|
||||
@entry_data = entry_data
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
|
@ -25,12 +25,19 @@ module Gitlab
|
|||
end
|
||||
|
||||
def parse_po
|
||||
entries = SimplePoParser.parse(po_path).map { |data| Gitlab::I18n::PoEntry.build(data) }
|
||||
entries = SimplePoParser.parse(po_path)
|
||||
|
||||
# The first entry is the metadata entry if there is one.
|
||||
# This is an entry when empty `msgid`
|
||||
@metadata_entry = entries.shift if entries.first.is_a?(Gitlab::I18n::MetadataEntry)
|
||||
@translation_entries = entries
|
||||
if entries.first[:msgid].empty?
|
||||
@metadata_entry = Gitlab::I18n::MetadataEntry.new(entries.shift)
|
||||
else
|
||||
return 'Missing metadata entry.'
|
||||
end
|
||||
|
||||
@translation_entries = entries.map do |entry_data|
|
||||
Gitlab::I18n::TranslationEntry.new(entry_data, metadata_entry.expected_plurals)
|
||||
end
|
||||
|
||||
nil
|
||||
rescue SimplePoParser::ParserError => e
|
||||
|
@ -64,7 +71,7 @@ module Gitlab
|
|||
return unless metadata_entry&.expected_plurals
|
||||
return unless entry.translated?
|
||||
|
||||
if entry.plural? && entry.all_translations.size != metadata_entry.expected_plurals
|
||||
if entry.has_plural? && entry.all_translations.size != metadata_entry.expected_plurals
|
||||
errors << "should have #{metadata_entry.expected_plurals} "\
|
||||
"#{'translations'.pluralize(metadata_entry.expected_plurals)}"
|
||||
end
|
||||
|
@ -85,11 +92,11 @@ module Gitlab
|
|||
end
|
||||
|
||||
def validate_variables(errors, entry)
|
||||
if entry.has_singular?
|
||||
if entry.has_singular_translation?
|
||||
validate_variables_in_message(errors, entry.msgid, entry.singular_translation)
|
||||
end
|
||||
|
||||
if entry.plural?
|
||||
if entry.has_plural?
|
||||
entry.plural_translations.each do |translation|
|
||||
validate_variables_in_message(errors, entry.plural_id, translation)
|
||||
end
|
||||
|
@ -161,8 +168,8 @@ module Gitlab
|
|||
translation = join_message(translation)
|
||||
|
||||
# We don't need to validate when the message is empty.
|
||||
# Translations could fallback to the default, or we could be validating a
|
||||
# language that does not have plurals.
|
||||
# In this case we fall back to the default, which has all the the
|
||||
# required variables.
|
||||
return if translation.empty?
|
||||
|
||||
found_variables = translation.scan(VARIABLE_REGEX)
|
||||
|
|
|
@ -1,6 +1,13 @@
|
|||
module Gitlab
|
||||
module I18n
|
||||
class TranslationEntry < PoEntry
|
||||
class TranslationEntry
|
||||
attr_reader :nplurals, :entry_data
|
||||
|
||||
def initialize(entry_data, nplurals)
|
||||
@entry_data = entry_data
|
||||
@nplurals = nplurals
|
||||
end
|
||||
|
||||
def msgid
|
||||
entry_data[:msgid]
|
||||
end
|
||||
|
@ -9,16 +16,17 @@ module Gitlab
|
|||
entry_data[:msgid_plural]
|
||||
end
|
||||
|
||||
def plural?
|
||||
def has_plural?
|
||||
plural_id.present?
|
||||
end
|
||||
|
||||
def singular_translation
|
||||
plural? ? entry_data['msgstr[0]'] : entry_data[:msgstr]
|
||||
all_translations.first if has_singular_translation?
|
||||
end
|
||||
|
||||
def all_translations
|
||||
@all_translations ||= entry_data.fetch_values(*translation_keys).reject(&:empty?)
|
||||
@all_translations ||= entry_data.fetch_values(*translation_keys)
|
||||
.reject(&:empty?)
|
||||
end
|
||||
|
||||
def translated?
|
||||
|
@ -26,25 +34,22 @@ module Gitlab
|
|||
end
|
||||
|
||||
def plural_translations
|
||||
return [] unless plural?
|
||||
return [] unless has_plural?
|
||||
return [] unless translated?
|
||||
|
||||
# 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)
|
||||
@plural_translations ||= if has_singular_translation?
|
||||
all_translations.drop(1)
|
||||
else
|
||||
all_translations
|
||||
end
|
||||
end
|
||||
|
||||
def flag
|
||||
entry_data[:flag]
|
||||
end
|
||||
|
||||
# When a translation is a plural, but only has 1 translation, we could be
|
||||
# talking about a language in which plural and singular is the same thing.
|
||||
# In which case we always translate as a plural.
|
||||
def has_singular?
|
||||
!plural? || all_translations.size > 1
|
||||
def has_singular_translation?
|
||||
nplurals > 1 || !has_plural?
|
||||
end
|
||||
|
||||
def msgid_contains_newlines?
|
||||
|
@ -61,15 +66,8 @@ module Gitlab
|
|||
|
||||
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?
|
||||
@translation_keys ||= if has_plural?
|
||||
entry_data.keys.select { |key| key =~ /msgstr\[\d+\]/ }
|
||||
else
|
||||
[:msgstr]
|
||||
|
|
4
spec/fixtures/missing_metadata.po
vendored
Normal file
4
spec/fixtures/missing_metadata.po
vendored
Normal file
|
@ -0,0 +1,4 @@
|
|||
msgid "1 commit"
|
||||
msgid_plural "%d commits"
|
||||
msgstr[0] "1 cambio"
|
||||
msgstr[1] "%d cambios"
|
|
@ -24,5 +24,28 @@ describe Gitlab::I18n::MetadataEntry do
|
|||
|
||||
expect(entry.expected_plurals).to eq(2)
|
||||
end
|
||||
|
||||
it 'returns 0 for the POT-metadata' do
|
||||
data = {
|
||||
msgid: "",
|
||||
msgstr: [
|
||||
"",
|
||||
"Project-Id-Version: gitlab 1.0.0\\n",
|
||||
"Report-Msgid-Bugs-To: \\n",
|
||||
"PO-Revision-Date: 2017-07-13 12:10-0500\\n",
|
||||
"Language-Team: Spanish\\n",
|
||||
"Language: es\\n",
|
||||
"MIME-Version: 1.0\\n",
|
||||
"Content-Type: text/plain; charset=UTF-8\\n",
|
||||
"Content-Transfer-Encoding: 8bit\\n",
|
||||
"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n",
|
||||
"Last-Translator: Bob Van Landuyt <bob@gitlab.com>\\n",
|
||||
"X-Generator: Poedit 2.0.2\\n"
|
||||
]
|
||||
}
|
||||
entry = described_class.new(data)
|
||||
|
||||
expect(entry.expected_plurals).to eq(0)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,18 +0,0 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe Gitlab::I18n::PoEntry do
|
||||
describe '.build' do
|
||||
it 'builds a metadata entry when the msgid is empty' do
|
||||
entry = described_class.build(msgid: '')
|
||||
|
||||
expect(entry).to be_kind_of(Gitlab::I18n::MetadataEntry)
|
||||
end
|
||||
|
||||
it 'builds a translation entry when the msgid is empty' do
|
||||
entry = described_class.build(msgid: 'Hello world')
|
||||
|
||||
expect(entry).to be_kind_of(Gitlab::I18n::TranslationEntry)
|
||||
end
|
||||
|
||||
end
|
||||
end
|
|
@ -69,6 +69,14 @@ describe Gitlab::I18n::PoLinter do
|
|||
end
|
||||
end
|
||||
|
||||
context 'with missing metadata' do
|
||||
let(:po_path) { 'spec/fixtures/missing_metadata.po' }
|
||||
|
||||
it 'returns the an error' do
|
||||
is_expected.to include('PO-syntax errors' => a_kind_of(Array))
|
||||
end
|
||||
end
|
||||
|
||||
context 'with a valid po' do
|
||||
it 'parses the file' do
|
||||
expect(linter).to receive(:parse_po).and_call_original
|
||||
|
@ -90,7 +98,7 @@ describe Gitlab::I18n::PoLinter do
|
|||
context 'with missing plurals' do
|
||||
let(:po_path) { 'spec/fixtures/missing_plurals.po' }
|
||||
|
||||
it 'has no errors' do
|
||||
it 'has errors' do
|
||||
is_expected.not_to be_empty
|
||||
end
|
||||
end
|
||||
|
@ -136,8 +144,7 @@ describe Gitlab::I18n::PoLinter do
|
|||
describe '#validate_entries' do
|
||||
it 'keeps track of errors for entries' do
|
||||
fake_invalid_entry = Gitlab::I18n::TranslationEntry.new(
|
||||
msgid: "Hello %{world}",
|
||||
msgstr: "Bonjour %{monde}"
|
||||
{ msgid: "Hello %{world}", msgstr: "Bonjour %{monde}" }, 2
|
||||
)
|
||||
allow(linter).to receive(:translation_entries) { [fake_invalid_entry] }
|
||||
|
||||
|
@ -169,9 +176,8 @@ describe Gitlab::I18n::PoLinter do
|
|||
allow(linter).to receive(:metadata_entry).and_return(fake_metadata)
|
||||
|
||||
fake_entry = Gitlab::I18n::TranslationEntry.new(
|
||||
msgid: 'the singular',
|
||||
msgid_plural: 'the plural',
|
||||
'msgstr[0]' => 'the singular'
|
||||
{ msgid: 'the singular', msgid_plural: 'the plural', 'msgstr[0]' => 'the singular' },
|
||||
2
|
||||
)
|
||||
errors = []
|
||||
|
||||
|
@ -183,27 +189,31 @@ describe Gitlab::I18n::PoLinter do
|
|||
|
||||
describe '#validate_variables' do
|
||||
it 'validates both signular and plural in a pluralized string when the entry has a singular' do
|
||||
pluralized_entry = Gitlab::I18n::TranslationEntry.new({
|
||||
msgid: 'Hello %{world}',
|
||||
msgid_plural: 'Hello all %{world}',
|
||||
'msgstr[0]' => 'Bonjour %{world}',
|
||||
'msgstr[1]' => 'Bonjour tous %{world}'
|
||||
})
|
||||
pluralized_entry = Gitlab::I18n::TranslationEntry.new(
|
||||
{ msgid: 'Hello %{world}',
|
||||
msgid_plural: 'Hello all %{world}',
|
||||
'msgstr[0]' => 'Bonjour %{world}',
|
||||
'msgstr[1]' => 'Bonjour tous %{world}' },
|
||||
2
|
||||
)
|
||||
|
||||
expect(linter).to receive(:validate_variables_in_message)
|
||||
.with([], 'Hello %{world}', 'Bonjour %{world}')
|
||||
.and_call_original
|
||||
expect(linter).to receive(:validate_variables_in_message)
|
||||
.with([], 'Hello all %{world}', 'Bonjour tous %{world}')
|
||||
.and_call_original
|
||||
|
||||
linter.validate_variables([], pluralized_entry)
|
||||
end
|
||||
|
||||
it 'only validates plural when there is no separate singular' do
|
||||
pluralized_entry = Gitlab::I18n::TranslationEntry.new({
|
||||
msgid: 'Hello %{world}',
|
||||
msgid_plural: 'Hello all %{world}',
|
||||
'msgstr[0]' => 'Bonjour %{world}'
|
||||
})
|
||||
pluralized_entry = Gitlab::I18n::TranslationEntry.new(
|
||||
{ msgid: 'Hello %{world}',
|
||||
msgid_plural: 'Hello all %{world}',
|
||||
'msgstr[0]' => 'Bonjour %{world}' },
|
||||
1
|
||||
)
|
||||
|
||||
expect(linter).to receive(:validate_variables_in_message)
|
||||
.with([], 'Hello all %{world}', 'Bonjour %{world}')
|
||||
|
@ -213,8 +223,8 @@ describe Gitlab::I18n::PoLinter do
|
|||
|
||||
it 'validates the message variables' do
|
||||
entry = Gitlab::I18n::TranslationEntry.new(
|
||||
msgid: 'Hello',
|
||||
msgstr: 'Bonjour'
|
||||
{ msgid: 'Hello', msgstr: 'Bonjour' },
|
||||
2
|
||||
)
|
||||
|
||||
expect(linter).to receive(:validate_variables_in_message)
|
||||
|
|
|
@ -4,7 +4,7 @@ describe Gitlab::I18n::TranslationEntry 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)
|
||||
entry = described_class.new(data, 2)
|
||||
|
||||
expect(entry.singular_translation).to eq('Bonjour monde')
|
||||
end
|
||||
|
@ -16,7 +16,7 @@ describe Gitlab::I18n::TranslationEntry do
|
|||
'msgstr[0]' => 'Bonjour monde',
|
||||
'msgstr[1]' => 'Bonjour mondes'
|
||||
}
|
||||
entry = described_class.new(data)
|
||||
entry = described_class.new(data, 2)
|
||||
|
||||
expect(entry.singular_translation).to eq('Bonjour monde')
|
||||
end
|
||||
|
@ -25,7 +25,7 @@ describe Gitlab::I18n::TranslationEntry do
|
|||
describe '#all_translations' do
|
||||
it 'returns all translations for singular translations' do
|
||||
data = { msgid: 'Hello world', msgstr: 'Bonjour monde' }
|
||||
entry = described_class.new(data)
|
||||
entry = described_class.new(data, 2)
|
||||
|
||||
expect(entry.all_translations).to eq(['Bonjour monde'])
|
||||
end
|
||||
|
@ -37,7 +37,7 @@ describe Gitlab::I18n::TranslationEntry do
|
|||
'msgstr[0]' => 'Bonjour monde',
|
||||
'msgstr[1]' => 'Bonjour mondes'
|
||||
}
|
||||
entry = described_class.new(data)
|
||||
entry = described_class.new(data, 2)
|
||||
|
||||
expect(entry.all_translations).to eq(['Bonjour monde', 'Bonjour mondes'])
|
||||
end
|
||||
|
@ -50,7 +50,7 @@ describe Gitlab::I18n::TranslationEntry do
|
|||
msgid_plural: 'Hello worlds',
|
||||
'msgstr[0]' => 'Bonjour monde'
|
||||
}
|
||||
entry = described_class.new(data)
|
||||
entry = described_class.new(data, 1)
|
||||
|
||||
expect(entry.plural_translations).to eq(['Bonjour monde'])
|
||||
end
|
||||
|
@ -63,21 +63,21 @@ describe Gitlab::I18n::TranslationEntry do
|
|||
'msgstr[1]' => 'Bonjour mondes',
|
||||
'msgstr[2]' => 'Bonjour tous les mondes'
|
||||
}
|
||||
entry = described_class.new(data)
|
||||
entry = described_class.new(data, 3)
|
||||
|
||||
expect(entry.plural_translations).to eq(['Bonjour mondes', 'Bonjour tous les mondes'])
|
||||
end
|
||||
end
|
||||
|
||||
describe '#has_singular?' do
|
||||
describe '#has_singular_translation?' do
|
||||
it 'has a singular when the translation is not pluralized' do
|
||||
data = {
|
||||
msgid: 'hello world',
|
||||
msgstr: 'hello'
|
||||
}
|
||||
entry = described_class.new(data)
|
||||
entry = described_class.new(data, 2)
|
||||
|
||||
expect(entry).to have_singular
|
||||
expect(entry).to have_singular_translation
|
||||
end
|
||||
|
||||
it 'has a singular when plural and singular are separately defined' do
|
||||
|
@ -87,9 +87,9 @@ describe Gitlab::I18n::TranslationEntry do
|
|||
"msgstr[0]" => 'hello world',
|
||||
"msgstr[1]" => 'hello worlds'
|
||||
}
|
||||
entry = described_class.new(data)
|
||||
entry = described_class.new(data, 2)
|
||||
|
||||
expect(entry).to have_singular
|
||||
expect(entry).to have_singular_translation
|
||||
end
|
||||
|
||||
it 'does not have a separate singular if the plural string only has one translation' do
|
||||
|
@ -98,34 +98,34 @@ describe Gitlab::I18n::TranslationEntry do
|
|||
msgid_plural: 'hello worlds',
|
||||
"msgstr[0]" => 'hello worlds'
|
||||
}
|
||||
entry = described_class.new(data)
|
||||
entry = described_class.new(data, 1)
|
||||
|
||||
expect(entry).not_to have_singular
|
||||
expect(entry).not_to have_singular_translation
|
||||
end
|
||||
end
|
||||
|
||||
describe '#msgid_contains_newlines'do
|
||||
describe '#msgid_contains_newlines' do
|
||||
it 'is true when the msgid is an array' do
|
||||
data = { msgid: %w(hello world) }
|
||||
entry = described_class.new(data)
|
||||
entry = described_class.new(data, 2)
|
||||
|
||||
expect(entry.msgid_contains_newlines?).to be_truthy
|
||||
end
|
||||
end
|
||||
|
||||
describe '#plural_id_contains_newlines'do
|
||||
describe '#plural_id_contains_newlines' do
|
||||
it 'is true when the msgid is an array' do
|
||||
data = { plural_id: %w(hello world) }
|
||||
entry = described_class.new(data)
|
||||
data = { msgid_plural: %w(hello world) }
|
||||
entry = described_class.new(data, 2)
|
||||
|
||||
expect(entry.plural_id_contains_newlines?).to be_truthy
|
||||
end
|
||||
end
|
||||
|
||||
describe '#translations_contain_newlines'do
|
||||
describe '#translations_contain_newlines' do
|
||||
it 'is true when the msgid is an array' do
|
||||
data = { msgstr: %w(hello world) }
|
||||
entry = described_class.new(data)
|
||||
entry = described_class.new(data, 2)
|
||||
|
||||
expect(entry.translations_contain_newlines?).to be_truthy
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue