From ca065e493e7a62f22cb330659e75f8567fa6131f Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Mon, 7 May 2018 16:04:11 +0200
Subject: Forbids combining named and unnamed variables in `gitlab.pot`

This would break with an argument error when interpolating using
`sprintf` in ruby.
---
 lib/gitlab/i18n/po_linter.rb           |  99 +++++++++++++++++++++-----------
 lib/gitlab/i18n/translation_entry.rb   |  20 ++++---
 spec/lib/gitlab/i18n/po_linter_spec.rb | 102 ++++++++++++++++++++-------------
 3 files changed, 140 insertions(+), 81 deletions(-)

diff --git a/lib/gitlab/i18n/po_linter.rb b/lib/gitlab/i18n/po_linter.rb
index 7d3ff8c7f58..ab79d4188dc 100644
--- a/lib/gitlab/i18n/po_linter.rb
+++ b/lib/gitlab/i18n/po_linter.rb
@@ -1,6 +1,8 @@
 module Gitlab
   module I18n
     class PoLinter
+      include Gitlab::Utils::StrongMemoize
+
       attr_reader :po_path, :translation_entries, :metadata_entry, :locale
 
       VARIABLE_REGEX = /%{\w*}|%[a-z]/.freeze
@@ -48,7 +50,7 @@ module Gitlab
 
         translation_entries.each do |entry|
           errors_for_entry = validate_entry(entry)
-          errors[join_message(entry.msgid)] = errors_for_entry if errors_for_entry.any?
+          errors[entry.msgid] = errors_for_entry if errors_for_entry.any?
         end
 
         errors
@@ -62,6 +64,7 @@ module Gitlab
         validate_newlines(errors, entry)
         validate_number_of_plurals(errors, entry)
         validate_unescaped_chars(errors, entry)
+        validate_translation(errors, entry)
 
         errors
       end
@@ -117,41 +120,73 @@ module Gitlab
       end
 
       def validate_variables_in_message(errors, message_id, message_translation)
-        message_id = join_message(message_id)
         required_variables = message_id.scan(VARIABLE_REGEX)
 
         validate_unnamed_variables(errors, required_variables)
-        validate_translation(errors, message_id, required_variables)
         validate_variable_usage(errors, message_translation, required_variables)
       end
 
-      def validate_translation(errors, message_id, used_variables)
+      def validate_translation(errors, entry)
+        Gitlab::I18n.with_locale(locale) do
+          if entry.has_plural?
+            translate_plural(entry)
+          else
+            translate_singular(entry)
+          end
+        end
+
+      # `sprintf` could raise an `ArgumentError` when invalid passing something
+      # other than a Hash when using named variables
+      #
+      # `sprintf` could raise `TypeError` when passing a wrong type when using
+      # unnamed variables
+      #
+      # FastGettext::Translation could raise `RuntimeError` (raised as a string),
+      # or as subclassess `NoTextDomainConfigured` & `InvalidFormat`
+      #
+      # `FastGettext::Translation` could raise `ArgumentError` as subclassess
+      # `InvalidEncoding`, `IllegalSequence` & `InvalidCharacter`
+      rescue ArgumentError, TypeError, RuntimeError => e
+        errors << "Failure translating to #{locale}: #{e.message}"
+      end
+
+      def translate_singular(entry)
+        used_variables = entry.msgid.scan(VARIABLE_REGEX)
         variables = fill_in_variables(used_variables)
 
-        begin
-          Gitlab::I18n.with_locale(locale) do
-            translated = if message_id.include?('|')
-                           FastGettext::Translation.s_(message_id)
-                         else
-                           FastGettext::Translation._(message_id)
-                         end
+        translation = if entry.msgid.include?('|')
+                        FastGettext::Translation.s_(entry.msgid)
+                      else
+                        FastGettext::Translation._(entry.msgid)
+                      end
 
-            translated % variables
-          end
+        translation % variables if used_variables.any?
+      end
+
+      def translate_plural(entry)
+        used_variables = entry.plural_id.scan(VARIABLE_REGEX)
+        variables = fill_in_variables(used_variables)
+
+        numbers_covering_all_plurals.map do |number|
+          translation = FastGettext::Translation.n_(entry.msgid, entry.plural_id, number)
+
+          translation % variables if used_variables.any?
+        end
+      end
+
+      def numbers_covering_all_plurals
+        @numbers_covering_all_plurals ||= Array.new(metadata_entry.expected_plurals) do |index|
+          number_for_pluralization(index)
+        end
+      end
+
+      def number_for_pluralization(counter)
+        pluralization_result = FastGettext.pluralisation_rule.call(counter)
 
-        # `sprintf` could raise an `ArgumentError` when invalid passing something
-        # other than a Hash when using named variables
-        #
-        # `sprintf` could raise `TypeError` when passing a wrong type when using
-        # unnamed variables
-        #
-        # FastGettext::Translation could raise `RuntimeError` (raised as a string),
-        # or as subclassess `NoTextDomainConfigured` & `InvalidFormat`
-        #
-        # `FastGettext::Translation` could raise `ArgumentError` as subclassess
-        # `InvalidEncoding`, `IllegalSequence` & `InvalidCharacter`
-        rescue ArgumentError, TypeError, RuntimeError => e
-          errors << "Failure translating to #{locale} with #{variables}: #{e.message}"
+        if pluralization_result.is_a?(TrueClass) || pluralization_result.is_a?(FalseClass)
+          counter
+        else
+          pluralization_result
         end
       end
 
@@ -172,14 +207,16 @@ module Gitlab
       end
 
       def validate_unnamed_variables(errors, variables)
-        if  variables.size > 1 && variables.any? { |variable_name| unnamed_variable?(variable_name) }
+        if variables.any? { |name| unnamed_variable?(name) } && variables.any? { |name| !unnamed_variable?(name) }
+          errors << 'is combining named variables with unnamed variables'
+        end
+
+        if variables.select { |variable_name| unnamed_variable?(variable_name) }.size > 1
           errors << 'is combining multiple unnamed variables'
         end
       end
 
       def validate_variable_usage(errors, translation, required_variables)
-        translation = join_message(translation)
-
         # We don't need to validate when the message is empty.
         # In this case we fall back to the default, which has all the the
         # required variables.
@@ -205,10 +242,6 @@ module Gitlab
       def validate_flags(errors, entry)
         errors << "is marked #{entry.flag}" if entry.flag
       end
-
-      def join_message(message)
-        Array(message).join
-      end
     end
   end
 end
diff --git a/lib/gitlab/i18n/translation_entry.rb b/lib/gitlab/i18n/translation_entry.rb
index e6c95afca7e..13c544aed27 100644
--- a/lib/gitlab/i18n/translation_entry.rb
+++ b/lib/gitlab/i18n/translation_entry.rb
@@ -11,11 +11,11 @@ module Gitlab
       end
 
       def msgid
-        entry_data[:msgid]
+        @msgid ||= Array(entry_data[:msgid]).join
       end
 
       def plural_id
-        entry_data[:msgid_plural]
+        @plural_id ||= Array(entry_data[:msgid_plural]).join
       end
 
       def has_plural?
@@ -23,12 +23,11 @@ module Gitlab
       end
 
       def singular_translation
-        all_translations.first if has_singular_translation?
+        all_translations.first.to_s if has_singular_translation?
       end
 
       def all_translations
-        @all_translations ||= entry_data.fetch_values(*translation_keys)
-                                .reject(&:empty?)
+        @all_translations ||= translation_entries.map { |translation| Array(translation).join }
       end
 
       def translated?
@@ -55,15 +54,15 @@ module Gitlab
       end
 
       def msgid_contains_newlines?
-        msgid.is_a?(Array)
+        entry_data[:msgid].is_a?(Array)
       end
 
       def plural_id_contains_newlines?
-        plural_id.is_a?(Array)
+        entry_data[:msgid_plural].is_a?(Array)
       end
 
       def translations_contain_newlines?
-        all_translations.any? { |translation| translation.is_a?(Array) }
+        translation_entries.any? { |translation| translation.is_a?(Array) }
       end
 
       def msgid_contains_unescaped_chars?
@@ -84,6 +83,11 @@ module Gitlab
 
       private
 
+      def translation_entries
+        @translation_entries ||= entry_data.fetch_values(*translation_keys)
+                                   .reject(&:empty?)
+      end
+
       def translation_keys
         @translation_keys ||= entry_data.keys.select { |key| key.to_s =~ /\Amsgstr(\[\d+\])?\z/ }
       end
diff --git a/spec/lib/gitlab/i18n/po_linter_spec.rb b/spec/lib/gitlab/i18n/po_linter_spec.rb
index 3a962ba7f22..9bbd51fe212 100644
--- a/spec/lib/gitlab/i18n/po_linter_spec.rb
+++ b/spec/lib/gitlab/i18n/po_linter_spec.rb
@@ -5,6 +5,25 @@ describe Gitlab::I18n::PoLinter do
   let(:linter) { described_class.new(po_path) }
   let(:po_path) { 'spec/fixtures/valid.po' }
 
+  def fake_translation(id:, translation:, plural_id: nil, plurals: [])
+    data = { msgid: id, msgid_plural: plural_id }
+
+    if plural_id
+      [translation, *plurals].each_with_index do |plural, index|
+        allow(FastGettext::Translation).to receive(:n_).with(plural_id, index).and_return(plural)
+        data.merge!("msgstr[#{index}]" => plural)
+      end
+    else
+      allow(FastGettext::Translation).to receive(:_).with(id).and_return(translation)
+      data[:msgstr] = translation
+    end
+
+    Gitlab::I18n::TranslationEntry.new(
+      data,
+      plurals.size + 1
+    )
+  end
+
   describe '#errors' do
     it 'only calls validation once' do
       expect(linter).to receive(:validate_po).once.and_call_original
@@ -155,9 +174,8 @@ 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}" }, 2
-      )
+      fake_invalid_entry = fake_translation(id: "Hello %{world}",
+                                            translation: "Bonjour %{monde}")
       allow(linter).to receive(:translation_entries) { [fake_invalid_entry] }
 
       expect(linter).to receive(:validate_entry)
@@ -177,6 +195,7 @@ describe Gitlab::I18n::PoLinter do
       expect(linter).to receive(:validate_newlines).with([], fake_entry)
       expect(linter).to receive(:validate_number_of_plurals).with([], fake_entry)
       expect(linter).to receive(:validate_unescaped_chars).with([], fake_entry)
+      expect(linter).to receive(:validate_translation).with([], fake_entry)
 
       linter.validate_entry(fake_entry)
     end
@@ -201,13 +220,12 @@ describe Gitlab::I18n::PoLinter do
   end
 
   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}' },
-        2
+    it 'validates both singular and plural in a pluralized string when the entry has a singular' do
+      pluralized_entry = fake_translation(
+        id: 'Hello %{world}',
+        translation: 'Bonjour %{world}',
+        plural_id: 'Hello all %{world}',
+        plurals: ['Bonjour tous %{world}']
       )
 
       expect(linter).to receive(:validate_variables_in_message)
@@ -221,11 +239,10 @@ describe Gitlab::I18n::PoLinter do
     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}' },
-        1
+      pluralized_entry = fake_translation(
+        id: 'Hello %{world}',
+        translation: 'Bonjour %{world}',
+        plural_id: 'Hello all %{world}'
       )
 
       expect(linter).to receive(:validate_variables_in_message)
@@ -235,10 +252,7 @@ describe Gitlab::I18n::PoLinter do
     end
 
     it 'validates the message variables' do
-      entry = Gitlab::I18n::TranslationEntry.new(
-        { msgid: 'Hello', msgstr: 'Bonjour' },
-        2
-      )
+      entry = fake_translation(id: 'Hello', translation: 'Bonjour')
 
       expect(linter).to receive(:validate_variables_in_message)
                           .with([], 'Hello', 'Bonjour')
@@ -251,21 +265,34 @@ describe Gitlab::I18n::PoLinter do
     it 'detects when a variables are used incorrectly' do
       errors = []
 
-      expected_errors = ['<hello %{world} %d> is missing: [%{hello}]',
-                         '<hello %{world} %d> is using unknown variables: [%{world}]',
-                         'is combining multiple unnamed variables']
+      expected_errors = ['<%d hello %{world} %s> is missing: [%{hello}]',
+                         '<%d hello %{world} %s> is using unknown variables: [%{world}]',
+                         'is combining multiple unnamed variables',
+                         'is combining named variables with unnamed variables']
 
-      linter.validate_variables_in_message(errors, '%{hello} world %d', 'hello %{world} %d')
+      linter.validate_variables_in_message(errors, '%d %{hello} world %s', '%d hello %{world} %s')
 
       expect(errors).to include(*expected_errors)
     end
+
+    it 'does not allow combining 1 `%d` unnamed variable with named variables' do
+      errors = []
+
+      linter.validate_variables_in_message(errors,
+                                           '%{type} detected %d vulnerability',
+                                           '%{type} detecteerde %d kwetsbaarheid')
+
+      expect(errors).not_to be_empty
+    end
   end
 
   describe '#validate_translation' do
+    let(:entry) { fake_translation(id: 'Hello %{world}', translation: 'Bonjour %{world}') }
+
     it 'succeeds with valid variables' do
       errors = []
 
-      linter.validate_translation(errors, 'Hello %{world}',  ['%{world}'])
+      linter.validate_translation(errors, entry)
 
       expect(errors).to be_empty
     end
@@ -275,43 +302,38 @@ describe Gitlab::I18n::PoLinter do
 
       expect(FastGettext::Translation).to receive(:_) { raise 'broken' }
 
-      linter.validate_translation(errors, 'Hello', [])
+      linter.validate_translation(errors, entry)
 
-      expect(errors).to include('Failure translating to en with []: broken')
+      expect(errors).to include('Failure translating to en: broken')
     end
 
     it 'adds an error message when translating fails when translating with context' do
+      entry = fake_translation(id: 'Tests|Hello', translation: 'broken')
       errors = []
 
       expect(FastGettext::Translation).to receive(:s_) { raise 'broken' }
 
-      linter.validate_translation(errors, 'Tests|Hello', [])
+      linter.validate_translation(errors, entry)
 
-      expect(errors).to include('Failure translating to en with []: broken')
+      expect(errors).to include('Failure translating to en: broken')
     end
 
     it "adds an error when trying to translate with incorrect variables when using unnamed variables" do
+      entry = fake_translation(id: 'Hello %s', translation: 'Hello %d')
       errors = []
 
-      linter.validate_translation(errors, 'Hello %d', ['%s'])
+      linter.validate_translation(errors, entry)
 
-      expect(errors.first).to start_with("Failure translating to en with")
+      expect(errors.first).to start_with("Failure translating to en")
     end
 
     it "adds an error when trying to translate with named variables when unnamed variables are expected" do
+      entry = fake_translation(id: 'Hello %s', translation: 'Hello %{thing}')
       errors = []
 
-      linter.validate_translation(errors, 'Hello %d', ['%{world}'])
-
-      expect(errors.first).to start_with("Failure translating to en with")
-    end
-
-    it 'adds an error when translated with incorrect variables using named variables' do
-      errors = []
-
-      linter.validate_translation(errors, 'Hello %{thing}', ['%d'])
+      linter.validate_translation(errors, entry)
 
-      expect(errors.first).to start_with("Failure translating to en with")
+      expect(errors.first).to start_with("Failure translating to en")
     end
   end
 
-- 
cgit v1.2.1


From 3b5ce6945dee059717855962271245bbb29f24e1 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Mon, 7 May 2018 17:56:18 +0200
Subject: Validate PO-variable usage in message ids

That way we can detect incorrect usage before the strings are added to
Crowdin for translation
---
 lib/gitlab/i18n/po_linter.rb           |  4 ++++
 spec/lib/gitlab/i18n/po_linter_spec.rb | 22 ++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/lib/gitlab/i18n/po_linter.rb b/lib/gitlab/i18n/po_linter.rb
index ab79d4188dc..ba08ef59776 100644
--- a/lib/gitlab/i18n/po_linter.rb
+++ b/lib/gitlab/i18n/po_linter.rb
@@ -109,10 +109,14 @@ module Gitlab
 
       def validate_variables(errors, entry)
         if entry.has_singular_translation?
+          validate_variables_in_message(errors, entry.msgid, entry.msgid)
+
           validate_variables_in_message(errors, entry.msgid, entry.singular_translation)
         end
 
         if entry.has_plural?
+          validate_variables_in_message(errors, entry.plural_id, entry.plural_id)
+
           entry.plural_translations.each do |translation|
             validate_variables_in_message(errors, entry.plural_id, translation)
           end
diff --git a/spec/lib/gitlab/i18n/po_linter_spec.rb b/spec/lib/gitlab/i18n/po_linter_spec.rb
index 9bbd51fe212..045916e6dc2 100644
--- a/spec/lib/gitlab/i18n/po_linter_spec.rb
+++ b/spec/lib/gitlab/i18n/po_linter_spec.rb
@@ -220,6 +220,10 @@ describe Gitlab::I18n::PoLinter do
   end
 
   describe '#validate_variables' do
+    before do
+      allow(linter).to receive(:validate_variables_in_message).and_call_original
+    end
+
     it 'validates both singular and plural in a pluralized string when the entry has a singular' do
       pluralized_entry = fake_translation(
         id: 'Hello %{world}',
@@ -259,6 +263,24 @@ describe Gitlab::I18n::PoLinter do
 
       linter.validate_variables([], entry)
     end
+
+    it 'validates variable usage in message ids' do
+      entry = fake_translation(
+        id: 'Hello %{world}',
+        translation: 'Bonjour %{world}',
+        plural_id: 'Hello all %{world}',
+        plurals: ['Bonjour tous %{world}']
+      )
+
+      expect(linter).to receive(:validate_variables_in_message)
+                          .with([], 'Hello %{world}', 'Hello %{world}')
+                          .and_call_original
+      expect(linter).to receive(:validate_variables_in_message)
+                          .with([], 'Hello all %{world}', 'Hello all %{world}')
+                          .and_call_original
+
+      linter.validate_variables([], entry)
+    end
   end
 
   describe '#validate_variables_in_message' do
-- 
cgit v1.2.1


From a9ebcfa5f55fe8444d9c0058904a9f1dbee7e32e Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Mon, 7 May 2018 18:36:38 +0200
Subject: Interpolate named counts in JS

Even though JS does the `%d` interpolation in `n__` using a string
replacement. This would not be compatible with the ruby
implementation.

Therefore using named variables instead.
---
 .../javascripts/ide/components/commit_sidebar/list_collapsed.vue | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/app/assets/javascripts/ide/components/commit_sidebar/list_collapsed.vue b/app/assets/javascripts/ide/components/commit_sidebar/list_collapsed.vue
index 2254271c679..d376a004e84 100644
--- a/app/assets/javascripts/ide/components/commit_sidebar/list_collapsed.vue
+++ b/app/assets/javascripts/ide/components/commit_sidebar/list_collapsed.vue
@@ -38,14 +38,17 @@ export default {
       return this.modifiedFilesLength ? 'multi-file-modified' : '';
     },
     additionsTooltip() {
-      return sprintf(n__('1 %{type} addition', '%d %{type} additions', this.addedFilesLength), {
+      return sprintf(n__('1 %{type} addition', '%{count} %{type} additions', this.addedFilesLength), {
         type: this.title.toLowerCase(),
+        count: this.addedFilesLength,
       });
     },
     modifiedTooltip() {
       return sprintf(
-        n__('1 %{type} modification', '%d %{type} modifications', this.modifiedFilesLength),
-        { type: this.title.toLowerCase() },
+        n__('1 %{type} modification', '%{count} %{type} modifications', this.modifiedFilesLength), {
+          type: this.title.toLowerCase(),
+          count: this.modifiedFilesLength,
+        },
       );
     },
     titleTooltip() {
-- 
cgit v1.2.1


From 9b5339371b500829b87602cfe7ba773e1027109f Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Mon, 7 May 2018 18:39:46 +0200
Subject: Validate if changed translations are checked in

---
 lib/tasks/gettext.rake | 26 ++++++++++++++++++++++++++
 lib/tasks/lint.rake    |  1 +
 locale/gitlab.pot      | 21 +++++++++++++++------
 3 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/lib/tasks/gettext.rake b/lib/tasks/gettext.rake
index 247d7be7d78..efe2997d78e 100644
--- a/lib/tasks/gettext.rake
+++ b/lib/tasks/gettext.rake
@@ -50,6 +50,32 @@ namespace :gettext do
     end
   end
 
+  task :updated_check do
+    # Removeing all pre-translated files speeds up `gettext:find` as the
+    # files don't need to be merged.
+    `rm locale/*/gitlab.po`
+
+    # `gettext:find` writes touches to temp files to `stderr` which would cause
+    # `static-analysis` to report failures. We can ignore these
+    silence_stream(STDERR) { Rake::Task['gettext:find'].invoke }
+
+    changed_files = `git diff --name-only`.lines.map(&:strip)
+
+    # reset the locale folder for potential next tasks
+    `git checkout locale`
+
+    if changed_files.include?('locale/gitlab.pot')
+      raise <<~MSG
+        Newly translated strings found, please add them to `gitlab.pot` by running:
+
+          bundle exec rake gettext:find; git checkout locale/*/gitlab.po;
+
+        Then check in the resulting `locale/gitlab.pot`
+
+      MSG
+    end
+  end
+
   def report_errors_for_file(file, errors_for_file)
     puts "Errors in `#{file}`:"
 
diff --git a/lib/tasks/lint.rake b/lib/tasks/lint.rake
index 8b86a5c72a5..b5a9cddaacb 100644
--- a/lib/tasks/lint.rake
+++ b/lib/tasks/lint.rake
@@ -27,6 +27,7 @@ unless Rails.env.production?
         scss_lint
         flay
         gettext:lint
+        gettext:updated_check
         lint:static_verification
       ].each do |task|
         pid = Process.fork do
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index e8a5a15f410..db5c183bac3 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -8,8 +8,8 @@ msgid ""
 msgstr ""
 "Project-Id-Version: gitlab 1.0.0\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2018-06-12 18:57+1000\n"
-"PO-Revision-Date: 2018-06-12 18:57+1000\n"
+"POT-Creation-Date: 2018-06-13 14:05+0200\n"
+"PO-Revision-Date: 2018-06-13 14:05+0200\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
 "Language: \n"
@@ -133,12 +133,12 @@ msgid "- show less"
 msgstr ""
 
 msgid "1 %{type} addition"
-msgid_plural "%d %{type} additions"
+msgid_plural "%{count} %{type} additions"
 msgstr[0] ""
 msgstr[1] ""
 
 msgid "1 %{type} modification"
-msgid_plural "%d %{type} modifications"
+msgid_plural "%{count} %{type} modifications"
 msgstr[0] ""
 msgstr[1] ""
 
@@ -2252,10 +2252,10 @@ msgstr ""
 msgid "Gitaly"
 msgstr ""
 
-msgid "Gitaly|Address"
+msgid "Gitaly Servers"
 msgstr ""
 
-msgid "Gitaly Servers"
+msgid "Gitaly|Address"
 msgstr ""
 
 msgid "Go Back"
@@ -2419,6 +2419,15 @@ msgstr ""
 msgid "If your HTTP repository is not publicly accessible, add authentication information to the URL: <code>https://username:password@gitlab.company.com/group/project.git</code>."
 msgstr ""
 
+msgid "ImageDiffViewer|2-up"
+msgstr ""
+
+msgid "ImageDiffViewer|Onion skin"
+msgstr ""
+
+msgid "ImageDiffViewer|Swipe"
+msgstr ""
+
 msgid "Import"
 msgstr ""
 
-- 
cgit v1.2.1


From f5d45e0632a2b7accbd5b2daae6966755865d50b Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Mon, 14 May 2018 15:20:38 +0200
Subject: Add Polish as a supported language

We already had some Polish translations, that were currently not
available to users. Adding it here because it helps me debug some
issues regarding language with more than one form.
---
 lib/gitlab/i18n.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/gitlab/i18n.rb b/lib/gitlab/i18n.rb
index 3772ef11c7f..343487bc361 100644
--- a/lib/gitlab/i18n.rb
+++ b/lib/gitlab/i18n.rb
@@ -21,7 +21,8 @@ module Gitlab
       'nl_NL' => 'Nederlands',
       'tr_TR' => 'Türkçe',
       'id_ID' => 'Bahasa Indonesia',
-      'fil_PH' => 'Filipino'
+      'fil_PH' => 'Filipino',
+      'pl_PL' => 'Polski'
     }.freeze
 
     def available_locales
-- 
cgit v1.2.1


From 17fc178cb5a68be787cb3fa243aed4cf1abb24a3 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Mon, 14 May 2018 15:28:39 +0200
Subject: Correctly translate all forms in tests

---
 lib/gitlab/i18n/metadata_entry.rb              | 11 +++-
 lib/gitlab/i18n/po_linter.rb                   | 59 +++++++++++++++------
 lib/gitlab/i18n/translation_entry.rb           |  6 +--
 lib/tasks/gettext.rake                         |  8 +--
 spec/lib/gitlab/i18n/metadata_entry_spec.rb    |  6 +--
 spec/lib/gitlab/i18n/po_linter_spec.rb         | 73 +++++++++++++++++++++-----
 spec/lib/gitlab/i18n/translation_entry_spec.rb |  6 +--
 7 files changed, 125 insertions(+), 44 deletions(-)

diff --git a/lib/gitlab/i18n/metadata_entry.rb b/lib/gitlab/i18n/metadata_entry.rb
index 35d57459a3d..36fc1bcdcb7 100644
--- a/lib/gitlab/i18n/metadata_entry.rb
+++ b/lib/gitlab/i18n/metadata_entry.rb
@@ -3,16 +3,25 @@ module Gitlab
     class MetadataEntry
       attr_reader :entry_data
 
+      # Avoid testing too many plurals if `nplurals` was incorrectly set.
+      # Based on info on https://www.gnu.org/software/gettext/manual/html_node/Plural-forms.html
+      # which mentions special cases for numbers ending in 2 digits
+      MAX_FORMS_TO_TEST = 101
+
       def initialize(entry_data)
         @entry_data = entry_data
       end
 
-      def expected_plurals
+      def expected_forms
         return nil unless plural_information
 
         plural_information['nplurals'].to_i
       end
 
+      def forms_to_test
+        @forms_to_test ||= [expected_forms, MAX_FORMS_TO_TEST].compact.min
+      end
+
       private
 
       def plural_information
diff --git a/lib/gitlab/i18n/po_linter.rb b/lib/gitlab/i18n/po_linter.rb
index ba08ef59776..d8e7269a2c2 100644
--- a/lib/gitlab/i18n/po_linter.rb
+++ b/lib/gitlab/i18n/po_linter.rb
@@ -36,7 +36,7 @@ module Gitlab
         end
 
         @translation_entries = entries.map do |entry_data|
-          Gitlab::I18n::TranslationEntry.new(entry_data, metadata_entry.expected_plurals)
+          Gitlab::I18n::TranslationEntry.new(entry_data, metadata_entry.expected_forms)
         end
 
         nil
@@ -84,25 +84,25 @@ module Gitlab
       end
 
       def validate_number_of_plurals(errors, entry)
-        return unless metadata_entry&.expected_plurals
+        return unless metadata_entry&.expected_forms
         return unless entry.translated?
 
-        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)}"
+        if entry.has_plural? && entry.all_translations.size != metadata_entry.expected_forms
+          errors << "should have #{metadata_entry.expected_forms} "\
+                    "#{'translations'.pluralize(metadata_entry.expected_forms)}"
         end
       end
 
       def validate_newlines(errors, entry)
-        if entry.msgid_contains_newlines?
+        if entry.msgid_has_multiple_lines?
           errors << 'is defined over multiple lines, this breaks some tooling.'
         end
 
-        if entry.plural_id_contains_newlines?
+        if entry.plural_id_has_multiple_lines?
           errors << 'plural is defined over multiple lines, this breaks some tooling.'
         end
 
-        if entry.translations_contain_newlines?
+        if entry.translations_have_multiple_lines?
           errors << 'has translations defined over multiple lines, this breaks some tooling.'
         end
       end
@@ -179,16 +179,41 @@ module Gitlab
       end
 
       def numbers_covering_all_plurals
-        @numbers_covering_all_plurals ||= Array.new(metadata_entry.expected_plurals) do |index|
-          number_for_pluralization(index)
+        @numbers_covering_all_plurals ||= calculate_numbers_covering_all_plurals
+      end
+
+      def calculate_numbers_covering_all_plurals
+        required_numbers = []
+        discovered_indexes = []
+        counter = 0
+
+        while discovered_indexes.size < metadata_entry.forms_to_test && counter < Gitlab::I18n::MetadataEntry::MAX_FORMS_TO_TEST
+          index_for_count = index_for_pluralization(counter)
+
+          unless discovered_indexes.include?(index_for_count)
+            discovered_indexes << index_for_count
+            required_numbers << counter
+          end
+
+          counter += 1
         end
+
+        required_numbers
       end
 
-      def number_for_pluralization(counter)
-        pluralization_result = FastGettext.pluralisation_rule.call(counter)
+      def index_for_pluralization(counter)
+        # This calls the C function that defines the pluralization rule, it can
+        # return a boolean (`false` represents 0, `true` represents 1) or an integer
+        # that specifies the plural form to be used for the given number
+        pluralization_result = Gitlab::I18n.with_locale(locale) do
+          FastGettext.pluralisation_rule.call(counter)
+        end
 
-        if pluralization_result.is_a?(TrueClass) || pluralization_result.is_a?(FalseClass)
-          counter
+        case pluralization_result
+        when false
+          0
+        when true
+          1
         else
           pluralization_result
         end
@@ -211,11 +236,13 @@ module Gitlab
       end
 
       def validate_unnamed_variables(errors, variables)
-        if variables.any? { |name| unnamed_variable?(name) } && variables.any? { |name| !unnamed_variable?(name) }
+        unnamed_variables, named_variables = variables.partition { |name| unnamed_variable?(name) }
+
+        if unnamed_variables.any? && named_variables.any?
           errors << 'is combining named variables with unnamed variables'
         end
 
-        if variables.select { |variable_name| unnamed_variable?(variable_name) }.size > 1
+        if unnamed_variables.size > 1
           errors << 'is combining multiple unnamed variables'
         end
       end
diff --git a/lib/gitlab/i18n/translation_entry.rb b/lib/gitlab/i18n/translation_entry.rb
index 13c544aed27..54adb98f42d 100644
--- a/lib/gitlab/i18n/translation_entry.rb
+++ b/lib/gitlab/i18n/translation_entry.rb
@@ -53,15 +53,15 @@ module Gitlab
         nplurals > 1 || !has_plural?
       end
 
-      def msgid_contains_newlines?
+      def msgid_has_multiple_lines?
         entry_data[:msgid].is_a?(Array)
       end
 
-      def plural_id_contains_newlines?
+      def plural_id_has_multiple_lines?
         entry_data[:msgid_plural].is_a?(Array)
       end
 
-      def translations_contain_newlines?
+      def translations_have_multiple_lines?
         translation_entries.any? { |translation| translation.is_a?(Array) }
       end
 
diff --git a/lib/tasks/gettext.rake b/lib/tasks/gettext.rake
index efe2997d78e..d52c419d66b 100644
--- a/lib/tasks/gettext.rake
+++ b/lib/tasks/gettext.rake
@@ -51,7 +51,7 @@ namespace :gettext do
   end
 
   task :updated_check do
-    # Removeing all pre-translated files speeds up `gettext:find` as the
+    # Removing all pre-translated files speeds up `gettext:find` as the
     # files don't need to be merged.
     `rm locale/*/gitlab.po`
 
@@ -62,15 +62,15 @@ namespace :gettext do
     changed_files = `git diff --name-only`.lines.map(&:strip)
 
     # reset the locale folder for potential next tasks
-    `git checkout locale`
+    `git checkout -- locale`
 
     if changed_files.include?('locale/gitlab.pot')
       raise <<~MSG
         Newly translated strings found, please add them to `gitlab.pot` by running:
 
-          bundle exec rake gettext:find; git checkout locale/*/gitlab.po;
+          bundle exec rake gettext:find; git checkout -- locale/*/gitlab.po;
 
-        Then check in the resulting `locale/gitlab.pot`
+        Then commit and push the resulting changes to `locale/gitlab.pot`.
 
       MSG
     end
diff --git a/spec/lib/gitlab/i18n/metadata_entry_spec.rb b/spec/lib/gitlab/i18n/metadata_entry_spec.rb
index ab71d6454a9..a399517cc04 100644
--- a/spec/lib/gitlab/i18n/metadata_entry_spec.rb
+++ b/spec/lib/gitlab/i18n/metadata_entry_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::I18n::MetadataEntry do
-  describe '#expected_plurals' do
+  describe '#expected_forms' do
     it 'returns the number of plurals' do
       data = {
         msgid: "",
@@ -22,7 +22,7 @@ describe Gitlab::I18n::MetadataEntry do
       }
       entry = described_class.new(data)
 
-      expect(entry.expected_plurals).to eq(2)
+      expect(entry.expected_forms).to eq(2)
     end
 
     it 'returns 0 for the POT-metadata' do
@@ -45,7 +45,7 @@ describe Gitlab::I18n::MetadataEntry do
       }
       entry = described_class.new(data)
 
-      expect(entry.expected_plurals).to eq(0)
+      expect(entry.expected_forms).to eq(0)
     end
   end
 end
diff --git a/spec/lib/gitlab/i18n/po_linter_spec.rb b/spec/lib/gitlab/i18n/po_linter_spec.rb
index 045916e6dc2..3dbc23d2aaf 100644
--- a/spec/lib/gitlab/i18n/po_linter_spec.rb
+++ b/spec/lib/gitlab/i18n/po_linter_spec.rb
@@ -1,20 +1,22 @@
 require 'spec_helper'
 require 'simple_po_parser'
 
+# Disabling this cop to allow for multi-language examples in comments
+# rubocop:disable Style/AsciiComments
 describe Gitlab::I18n::PoLinter do
   let(:linter) { described_class.new(po_path) }
   let(:po_path) { 'spec/fixtures/valid.po' }
 
-  def fake_translation(id:, translation:, plural_id: nil, plurals: [])
-    data = { msgid: id, msgid_plural: plural_id }
+  def fake_translation(msgid:, translation:, plural_id: nil, plurals: [])
+    data = { msgid: msgid, msgid_plural: plural_id }
 
     if plural_id
       [translation, *plurals].each_with_index do |plural, index|
-        allow(FastGettext::Translation).to receive(:n_).with(plural_id, index).and_return(plural)
+        allow(FastGettext::Translation).to receive(:n_).with(msgid, plural_id, index).and_return(plural)
         data.merge!("msgstr[#{index}]" => plural)
       end
     else
-      allow(FastGettext::Translation).to receive(:_).with(id).and_return(translation)
+      allow(FastGettext::Translation).to receive(:_).with(msgid).and_return(translation)
       data[:msgstr] = translation
     end
 
@@ -174,7 +176,7 @@ describe Gitlab::I18n::PoLinter do
 
   describe '#validate_entries' do
     it 'keeps track of errors for entries' do
-      fake_invalid_entry = fake_translation(id: "Hello %{world}",
+      fake_invalid_entry = fake_translation(msgid: "Hello %{world}",
                                             translation: "Bonjour %{monde}")
       allow(linter).to receive(:translation_entries) { [fake_invalid_entry] }
 
@@ -204,7 +206,7 @@ describe Gitlab::I18n::PoLinter do
   describe '#validate_number_of_plurals' do
     it 'validates when there are an incorrect number of translations' do
       fake_metadata = double
-      allow(fake_metadata).to receive(:expected_plurals).and_return(2)
+      allow(fake_metadata).to receive(:expected_forms).and_return(2)
       allow(linter).to receive(:metadata_entry).and_return(fake_metadata)
 
       fake_entry = Gitlab::I18n::TranslationEntry.new(
@@ -226,7 +228,7 @@ describe Gitlab::I18n::PoLinter do
 
     it 'validates both singular and plural in a pluralized string when the entry has a singular' do
       pluralized_entry = fake_translation(
-        id: 'Hello %{world}',
+        msgid: 'Hello %{world}',
         translation: 'Bonjour %{world}',
         plural_id: 'Hello all %{world}',
         plurals: ['Bonjour tous %{world}']
@@ -244,7 +246,7 @@ describe Gitlab::I18n::PoLinter do
 
     it 'only validates plural when there is no separate singular' do
       pluralized_entry = fake_translation(
-        id: 'Hello %{world}',
+        msgid: 'Hello %{world}',
         translation: 'Bonjour %{world}',
         plural_id: 'Hello all %{world}'
       )
@@ -256,7 +258,7 @@ describe Gitlab::I18n::PoLinter do
     end
 
     it 'validates the message variables' do
-      entry = fake_translation(id: 'Hello', translation: 'Bonjour')
+      entry = fake_translation(msgid: 'Hello', translation: 'Bonjour')
 
       expect(linter).to receive(:validate_variables_in_message)
                           .with([], 'Hello', 'Bonjour')
@@ -266,7 +268,7 @@ describe Gitlab::I18n::PoLinter do
 
     it 'validates variable usage in message ids' do
       entry = fake_translation(
-        id: 'Hello %{world}',
+        msgid: 'Hello %{world}',
         translation: 'Bonjour %{world}',
         plural_id: 'Hello all %{world}',
         plurals: ['Bonjour tous %{world}']
@@ -309,7 +311,7 @@ describe Gitlab::I18n::PoLinter do
   end
 
   describe '#validate_translation' do
-    let(:entry) { fake_translation(id: 'Hello %{world}', translation: 'Bonjour %{world}') }
+    let(:entry) { fake_translation(msgid: 'Hello %{world}', translation: 'Bonjour %{world}') }
 
     it 'succeeds with valid variables' do
       errors = []
@@ -330,7 +332,7 @@ describe Gitlab::I18n::PoLinter do
     end
 
     it 'adds an error message when translating fails when translating with context' do
-      entry = fake_translation(id: 'Tests|Hello', translation: 'broken')
+      entry = fake_translation(msgid: 'Tests|Hello', translation: 'broken')
       errors = []
 
       expect(FastGettext::Translation).to receive(:s_) { raise 'broken' }
@@ -341,7 +343,7 @@ describe Gitlab::I18n::PoLinter do
     end
 
     it "adds an error when trying to translate with incorrect variables when using unnamed variables" do
-      entry = fake_translation(id: 'Hello %s', translation: 'Hello %d')
+      entry = fake_translation(msgid: 'Hello %s', translation: 'Hello %d')
       errors = []
 
       linter.validate_translation(errors, entry)
@@ -350,13 +352,55 @@ describe Gitlab::I18n::PoLinter do
     end
 
     it "adds an error when trying to translate with named variables when unnamed variables are expected" do
-      entry = fake_translation(id: 'Hello %s', translation: 'Hello %{thing}')
+      entry = fake_translation(msgid: 'Hello %s', translation: 'Hello %{thing}')
       errors = []
 
       linter.validate_translation(errors, entry)
 
       expect(errors.first).to start_with("Failure translating to en")
     end
+
+    it 'tests translation for all given forms' do
+      # Fake a language that has 3 forms to translate
+      fake_metadata = double
+      allow(fake_metadata).to receive(:forms_to_test).and_return(3)
+      allow(linter).to receive(:metadata_entry).and_return(fake_metadata)
+      entry = fake_translation(
+        msgid: '%d exception',
+        translation: '%d uitzondering',
+        plural_id: '%d exceptions',
+        plurals: ['%d uitzonderingen', '%d uitzonderingetjes']
+      )
+
+      # Make each count use a different index
+      allow(linter).to receive(:index_for_pluralization).with(0).and_return(0)
+      allow(linter).to receive(:index_for_pluralization).with(1).and_return(1)
+      allow(linter).to receive(:index_for_pluralization).with(2).and_return(2)
+
+      expect(FastGettext::Translation).to receive(:n_).with('%d exception', '%d exceptions', 0).and_call_original
+      expect(FastGettext::Translation).to receive(:n_).with('%d exception', '%d exceptions', 1).and_call_original
+      expect(FastGettext::Translation).to receive(:n_).with('%d exception', '%d exceptions', 2).and_call_original
+
+      linter.validate_translation([], entry)
+    end
+  end
+
+  describe '#numbers_covering_all_plurals' do
+    it 'can correctly find all required numbers to translate to Polish' do
+      # Polish used as an example with 3 different forms:
+      # 0, all plurals except the ones ending in 2,3,4: Kotów
+      # 1: Kot
+      # 2-3-4: Koty
+      # So translating with [0, 1, 2] will give us all different posibilities
+      fake_metadata = double
+      allow(fake_metadata).to receive(:forms_to_test).and_return(4)
+      allow(linter).to receive(:metadata_entry).and_return(fake_metadata)
+      allow(linter).to receive(:locale).and_return('pl_PL')
+
+      numbers = linter.numbers_covering_all_plurals
+
+      expect(numbers).to contain_exactly(0, 1, 2)
+    end
   end
 
   describe '#fill_in_variables' do
@@ -380,3 +424,4 @@ describe Gitlab::I18n::PoLinter do
     end
   end
 end
+# rubocop:enable Style/AsciiComments
diff --git a/spec/lib/gitlab/i18n/translation_entry_spec.rb b/spec/lib/gitlab/i18n/translation_entry_spec.rb
index f68bc8feff9..b301e6ea443 100644
--- a/spec/lib/gitlab/i18n/translation_entry_spec.rb
+++ b/spec/lib/gitlab/i18n/translation_entry_spec.rb
@@ -109,7 +109,7 @@ describe Gitlab::I18n::TranslationEntry do
       data = { msgid: %w(hello world) }
       entry = described_class.new(data, 2)
 
-      expect(entry.msgid_contains_newlines?).to be_truthy
+      expect(entry.msgid_has_multiple_lines?).to be_truthy
     end
   end
 
@@ -118,7 +118,7 @@ describe Gitlab::I18n::TranslationEntry do
       data = { msgid_plural: %w(hello world) }
       entry = described_class.new(data, 2)
 
-      expect(entry.plural_id_contains_newlines?).to be_truthy
+      expect(entry.plural_id_has_multiple_lines?).to be_truthy
     end
   end
 
@@ -127,7 +127,7 @@ describe Gitlab::I18n::TranslationEntry do
       data = { msgstr: %w(hello world) }
       entry = described_class.new(data, 2)
 
-      expect(entry.translations_contain_newlines?).to be_truthy
+      expect(entry.translations_have_multiple_lines?).to be_truthy
     end
   end
 
-- 
cgit v1.2.1