From c47e1896194720343b0f7ab6496e0db1485992c0 Mon Sep 17 00:00:00 2001
From: Jonathan Allard <jonathan@allard.io>
Date: Sat, 13 Feb 2016 16:32:50 -0500
Subject: [PATCH 1/2] I18n: Switch locale helpers and I18n paths

---
 middleman-core/features/i18n_paths.feature    |  33 ++++
 .../fixtures/i18n-test-app/locales/en.yml     |   3 +-
 .../fixtures/i18n-test-app/locales/es.yml     |   3 +-
 .../i18n-test-app/source/layouts/layout.erb   |   6 +-
 .../source/localizable/morning.es.html.erb    |   1 -
 .../i18n-test-app/source/manana.es.html.erb   |   4 +
 .../{localizable => }/morning.en.html.erb     |   0
 .../source/{localizable => }/one.en.html.md   |   0
 .../source/{localizable => }/one.es.html.md   |   0
 .../middleman-core/core_extensions/i18n.rb    | 154 ++++++++++++------
 10 files changed, 152 insertions(+), 52 deletions(-)
 create mode 100644 middleman-core/features/i18n_paths.feature
 delete mode 100644 middleman-core/fixtures/i18n-test-app/source/localizable/morning.es.html.erb
 create mode 100644 middleman-core/fixtures/i18n-test-app/source/manana.es.html.erb
 rename middleman-core/fixtures/i18n-test-app/source/{localizable => }/morning.en.html.erb (100%)
 rename middleman-core/fixtures/i18n-test-app/source/{localizable => }/one.en.html.md (100%)
 rename middleman-core/fixtures/i18n-test-app/source/{localizable => }/one.es.html.md (100%)

diff --git a/middleman-core/features/i18n_paths.feature b/middleman-core/features/i18n_paths.feature
new file mode 100644
index 000000000..cab9171e3
--- /dev/null
+++ b/middleman-core/features/i18n_paths.feature
@@ -0,0 +1,33 @@
+Feature: i18n Paths
+
+  Scenario: Linking to a page in a language-agnostic way
+    Given a fixture app "i18n-test-app"
+    And a file named "config.rb" with:
+      """
+      activate :i18n
+      """
+    Given the Server is running at "i18n-test-app"
+    When I go to "/"
+    Then I should see '<a href="iframe.php?url=https%3A%2F%2Fgithub.com%2Fmorning.html">'
+    When I go to "/es"
+    Then I should see '<a href="iframe.php?url=https%3A%2F%2Fgithub.com%2Fes%2Fmanana.html">'
+    When I go to "/morning.html"
+    Then I should see "Good morning"
+    When I go to "/es/manana.html"
+    Then I should see "Buenos días"
+
+  Scenario: Locale-switching link
+    Given a fixture app "i18n-test-app"
+    And a file named "config.rb" with:
+      """
+      activate :i18n
+      """
+    Given the Server is running at "i18n-test-app"
+    When I go to "/"
+    Then I should see '<a href="iframe.php?url=https%3A%2F%2Fgithub.com%2Fes%2F">es</a>'
+    When I go to "/es"
+    Then I should see '<a href="iframe.php?url=https%3A%2F%2Fgithub.com%2F">en</a>'
+    When I go to "/morning.html"
+    Then I should see '<a href="iframe.php?url=https%3A%2F%2Fgithub.com%2Fes%2Fmanana.html">es</a>'
+    When I go to "/es/manana.html"
+    Then I should see '<a href="iframe.php?url=https%3A%2F%2Fgithub.com%2Fmorning.html">en</a>'
diff --git a/middleman-core/fixtures/i18n-test-app/locales/en.yml b/middleman-core/fixtures/i18n-test-app/locales/en.yml
index 91519342d..5085beb2c 100644
--- a/middleman-core/fixtures/i18n-test-app/locales/en.yml
+++ b/middleman-core/fixtures/i18n-test-app/locales/en.yml
@@ -1,4 +1,5 @@
 ---
 en:
   greetings: "Howdy"
-  hi: "Hello"
\ No newline at end of file
+  hi: "Hello"
+  morning: "Morning"
diff --git a/middleman-core/fixtures/i18n-test-app/locales/es.yml b/middleman-core/fixtures/i18n-test-app/locales/es.yml
index a33215463..6b90aa6e9 100644
--- a/middleman-core/fixtures/i18n-test-app/locales/es.yml
+++ b/middleman-core/fixtures/i18n-test-app/locales/es.yml
@@ -4,6 +4,7 @@ es:
     hello: "hola"
     morning: "manana"
     one: "una"
-  
+
   greetings: "Como Esta?"
   hi: "Hola"
+  morning: "Manana"
diff --git a/middleman-core/fixtures/i18n-test-app/source/layouts/layout.erb b/middleman-core/fixtures/i18n-test-app/source/layouts/layout.erb
index cb465e99d..8d78e2919 100644
--- a/middleman-core/fixtures/i18n-test-app/source/layouts/layout.erb
+++ b/middleman-core/fixtures/i18n-test-app/source/layouts/layout.erb
@@ -3,6 +3,10 @@
     <%= stylesheet_link_tag :site %>
   </head>
   <body>
+    <nav>
+      <%= link_to t(:morning), "/morning.html" %>
+      <%= link_to other_locale, current_page, locale: other_locale %>
+    </nav>
     <%= yield %>
   </body>
-</html>
\ No newline at end of file
+</html>
diff --git a/middleman-core/fixtures/i18n-test-app/source/localizable/morning.es.html.erb b/middleman-core/fixtures/i18n-test-app/source/localizable/morning.es.html.erb
deleted file mode 100644
index 30d65605e..000000000
--- a/middleman-core/fixtures/i18n-test-app/source/localizable/morning.es.html.erb
+++ /dev/null
@@ -1 +0,0 @@
-Buenos días
diff --git a/middleman-core/fixtures/i18n-test-app/source/manana.es.html.erb b/middleman-core/fixtures/i18n-test-app/source/manana.es.html.erb
new file mode 100644
index 000000000..aa365663e
--- /dev/null
+++ b/middleman-core/fixtures/i18n-test-app/source/manana.es.html.erb
@@ -0,0 +1,4 @@
+---
+id: morning
+---
+Buenos días
diff --git a/middleman-core/fixtures/i18n-test-app/source/localizable/morning.en.html.erb b/middleman-core/fixtures/i18n-test-app/source/morning.en.html.erb
similarity index 100%
rename from middleman-core/fixtures/i18n-test-app/source/localizable/morning.en.html.erb
rename to middleman-core/fixtures/i18n-test-app/source/morning.en.html.erb
diff --git a/middleman-core/fixtures/i18n-test-app/source/localizable/one.en.html.md b/middleman-core/fixtures/i18n-test-app/source/one.en.html.md
similarity index 100%
rename from middleman-core/fixtures/i18n-test-app/source/localizable/one.en.html.md
rename to middleman-core/fixtures/i18n-test-app/source/one.en.html.md
diff --git a/middleman-core/fixtures/i18n-test-app/source/localizable/one.es.html.md b/middleman-core/fixtures/i18n-test-app/source/one.es.html.md
similarity index 100%
rename from middleman-core/fixtures/i18n-test-app/source/localizable/one.es.html.md
rename to middleman-core/fixtures/i18n-test-app/source/one.es.html.md
diff --git a/middleman-core/lib/middleman-core/core_extensions/i18n.rb b/middleman-core/lib/middleman-core/core_extensions/i18n.rb
index 156f2694d..f364fc687 100644
--- a/middleman-core/lib/middleman-core/core_extensions/i18n.rb
+++ b/middleman-core/lib/middleman-core/core_extensions/i18n.rb
@@ -11,6 +11,7 @@ class Middleman::CoreExtensions::Internationalization < ::Middleman::Extension
 
   # Exposes `locales` to templates
   expose_to_template :locales, :langs, :locale, :lang
+  attr_reader :lookup
 
   def initialize(*)
     super
@@ -53,7 +54,6 @@ def after_configuration
     # parsing config.rb
     app.files.on_change(:locales, &method(:on_file_changed))
 
-    @maps = {}
     @mount_at_root = options[:mount_at_root].nil? ? locales.first : options[:mount_at_root]
 
     configure_i18n
@@ -69,27 +69,58 @@ def t(*args)
     def url_for(path_or_resource, options={})
       locale = options.delete(:locale) || ::I18n.locale
 
+      # Backup options in case there's an error
       opts = options.dup
 
-      should_relativize = opts.key?(:relative) ? opts[:relative] : config[:relative_links]
 
-      opts[:relative] = false
+      should_relativize = if opts.key?(:relative)
+        opts[:relative]
+      else
+        config[:relative_links]
+      end
 
-      href = super(path_or_resource, opts)
+      opts[:relative] = false
+      # We will call super at first without relative
+      # until we figure out what's going on
+
+
+      # Me too, I have no idea if what I got is a path or resource.
+      # But I need the potential resource to figure out the page_id,
+      # if there's one, and if so, see if there's a localization
+      # available
+
+      # Look if this stringish designates a resource
+      # Copied from M::Util::Paths#url_for:153
+      if path_or_resource.is_a?(String) || path_or_resource.is_a?(Symbol)
+        if r = app.sitemap.find_resource_by_page_id(path_or_resource)
+          path_or_resource = r
+        elsif r = app.sitemap.find_resource_by_path(path_or_resource)
+          path_or_resource = r
+        end
+      end
 
-      final_path = if result = extensions[:i18n].localized_path(href, locale)
-        result
+      # If stringish designates a resource, transform p_o_r into
+      # the path with locale
+      if path_or_resource.is_a?(::Middleman::Sitemap::Resource)
+        page_id = path_or_resource.page_id
+
+        final_path =
+          if result = extensions[:i18n].localized_path(page_id, locale)
+            result
+          else
+            # Should we log the missing file?
+            path_or_resource
+          end
       else
-        # Should we log the missing file?
-        href
+        final_path = path_or_resource
       end
 
       opts[:relative] = should_relativize
 
       begin
         super(final_path, opts)
-      rescue RuntimeError
-        super(path_or_resource, options)
+      # rescue RuntimeError
+      #   super(path_or_resource, options)
       end
     end
 
@@ -119,6 +150,19 @@ def locate_partial(partial_name, try_static=false)
           super
       end
     end
+
+    def other_locale
+      if extensions[:i18n].langs.count > 2
+        raise "There is more than one other locale to choose"\
+          "from. Use `other_locales` to get them all."
+      end
+
+      other_locales[0]
+    end
+
+    def other_locales
+      extensions[:i18n].langs - [I18n.locale]
+    end
   end
 
   Contract ArrayOf[Symbol]
@@ -142,9 +186,10 @@ def locale
   Contract ResourceList => ResourceList
   def manipulate_resource_list(resources)
     new_resources = []
+    @lookup ||= {}
 
     file_extension_resources = resources.select do |resource|
-      parse_locale_extension(resource.path)
+      parse_locale_extension(resource.path)[0]
     end
 
     localizable_folder_resources = resources.select do |resource|
@@ -153,36 +198,35 @@ def manipulate_resource_list(resources)
 
     # If it's a "localizable template"
     localizable_folder_resources.each do |resource|
-      page_id = File.basename(resource.path, File.extname(resource.path))
+      # Remove folder name
+      path = resource.path.sub(options[:templates_dir], '')
+      page_id = resource.page_id.sub(options[:templates_dir] + '/', '')
+
       locales.each do |locale|
-        # Remove folder name
-        path = resource.path.sub(options[:templates_dir], '')
         new_resources << build_resource(path, resource.path, page_id, locale)
       end
 
       resource.ignore!
 
-      # This is for backwards compatibility with the old provides_metadata-based code
-      # that used to be in this extension, but I don't know how much sense it makes.
+      # This is for backwards compatibility with the old
+      # provides_metadata-based code that used to be in this extension,
+      # but I don't know how much sense it makes.
       # next if resource.options[:locale]
 
       # $stderr.puts "Defaulting #{resource.path} to #{@mount_at_root}"
-      # resource.add_metadata options: { locale: @mount_at_root }, locals: { locale: @mount_at_root }
-    end
+      # resource.add_metadata options: { locale: @mount_at_root },
+      # locals: { locale: @mount_at_root }
+  end
 
     # If it uses file extension localization
     file_extension_resources.each do |resource|
-      result = parse_locale_extension(resource.path)
-      ext_locale, path, page_id = result
-      new_resources << build_resource(path, resource.path, page_id, ext_locale)
+      ext_locale, path = parse_locale_extension(resource.path)
+      _, page_id = parse_locale_extension(resource.page_id, has_last: false)
 
-      resource.ignore!
-    end
+      new_resources << build_resource(path, resource.path,
+                                      page_id, ext_locale)
 
-    @lookup = new_resources.each_with_object({}) do |desc, sum|
-      abs_path = desc.source_path.sub(options[:templates_dir], '')
-      sum[abs_path] ||= {}
-      sum[abs_path][desc.locale] = '/' + desc.path
+      resource.ignore!
     end
 
     new_resources.reduce(resources) do |sum, r|
@@ -191,11 +235,8 @@ def manipulate_resource_list(resources)
   end
 
   Contract String, Symbol => Maybe[String]
-  def localized_path(path, locale)
-    lookup_path = path.dup
-    lookup_path << app.config[:index_file] if lookup_path.end_with?('/')
-
-    @lookup[lookup_path] && @lookup[lookup_path][locale]
+  def localized_path(page_id, locale)
+    @lookup[page_id] && @lookup[page_id][locale]
   end
 
   Contract Symbol => String
@@ -242,26 +283,38 @@ def known_locales
     end
   end
 
-  # Parse locale extension filename
-  # @return [locale, path, basename]
+  # Parse locale extension filename or page_id when implicit
+  #
+  # Page id could have no extension if set manually
+  #
+  # @return [locale, result]
   # will return +nil+ if no locale extension
-  Contract String => Maybe[[Symbol, String, String]]
-  def parse_locale_extension(path)
-    path_bits = path.split('.')
-    return nil if path_bits.size < 3
+  Contract String, Hash => [Maybe[Symbol], String]
+  def parse_locale_extension(pathish, has_last: true)
+    dirname = File.dirname(pathish)
+    basename = File.basename(pathish)
+
+    path_bits = basename.split('.')
 
-    locale = path_bits.delete_at(-2).to_sym
-    return nil unless locales.include?(locale)
+    locale_index = has_last ? -2 : -1
+    locale = path_bits[locale_index].try(:to_sym)
 
-    path = path_bits.join('.')
-    basename = File.basename(path_bits[0..-2].join('.'))
-    [locale, path, basename]
+    if locales.include?(locale)
+      path_bits.delete_at(locale_index)
+    else
+      locale = nil
+    end
+
+    pathish = path_bits.join('.')
+    pathish = File.join(dirname, pathish) unless dirname == '.'
+
+    [locale, pathish]
   end
 
-  LocalizedPageDescriptor = Struct.new(:path, :source_path, :locale) do
+  LocalizedPageDescriptor = Struct.new(:path, :source_path, :page_id, :locale) do
     def execute_descriptor(app, resources)
       r = ::Middleman::Sitemap::ProxyResource.new(app.sitemap, path, source_path)
-      r.add_metadata options: { locale: locale }
+      r.add_metadata page: {id: page_id}, options: { locale: locale }
       resources + [r]
     end
   end
@@ -270,12 +323,14 @@ def execute_descriptor(app, resources)
   def build_resource(path, source_path, page_id, locale)
     old_locale = ::I18n.locale
     ::I18n.locale = locale
-    localized_page_id = ::I18n.t("paths.#{page_id}", default: page_id, fallback: [])
+    localized_page_id = ::I18n.t("paths.#{page_id}", default: page_id,
+                                 fallback: [])
 
     partially_localized_path = ''
 
     File.dirname(path).split('/').each do |path_sub|
-      next if path_sub == ''
+      next if ['', '.'].include?(path_sub)
+
       partially_localized_path = "#{partially_localized_path}/#{::I18n.t("paths.#{path_sub}", default: path_sub)}"
     end
 
@@ -290,8 +345,11 @@ def build_resource(path, source_path, page_id, locale)
 
     path = path.sub(options[:templates_dir] + '/', '')
 
+    @lookup[page_id] ||= {}
+    @lookup[page_id][locale] = '/' + path
+
     ::I18n.locale = old_locale
 
-    LocalizedPageDescriptor.new(path, source_path, locale)
+    LocalizedPageDescriptor.new(path, source_path, page_id, locale)
   end
 end

From e67608dea557978050d9afa14e0ca81a160be779 Mon Sep 17 00:00:00 2001
From: Jonathan Allard <jonathan@allard.io>
Date: Sat, 18 Feb 2017 14:46:13 -0500
Subject: [PATCH 2/2] I18n: Refactor resource manipulation

---
 .../middleman-core/core_extensions/i18n.rb    | 158 ++++++++++--------
 1 file changed, 90 insertions(+), 68 deletions(-)

diff --git a/middleman-core/lib/middleman-core/core_extensions/i18n.rb b/middleman-core/lib/middleman-core/core_extensions/i18n.rb
index f364fc687..8ddced684 100644
--- a/middleman-core/lib/middleman-core/core_extensions/i18n.rb
+++ b/middleman-core/lib/middleman-core/core_extensions/i18n.rb
@@ -185,51 +185,15 @@ def locale
   # @return Array<Middleman::Sitemap::Resource>
   Contract ResourceList => ResourceList
   def manipulate_resource_list(resources)
-    new_resources = []
     @lookup ||= {}
 
-    file_extension_resources = resources.select do |resource|
-      parse_locale_extension(resource.path)[0]
-    end
+    new_resources = []
 
-    localizable_folder_resources = resources.select do |resource|
-      !file_extension_resources.include?(resource) && File.fnmatch?(File.join(options[:templates_dir], '**'), resource.path)
+    [FileExtensionResource, LocalizableResource].map do |type|
+      new_resources << type.find_in(resources, i18n: self).map(&:build)
     end
 
-    # If it's a "localizable template"
-    localizable_folder_resources.each do |resource|
-      # Remove folder name
-      path = resource.path.sub(options[:templates_dir], '')
-      page_id = resource.page_id.sub(options[:templates_dir] + '/', '')
-
-      locales.each do |locale|
-        new_resources << build_resource(path, resource.path, page_id, locale)
-      end
-
-      resource.ignore!
-
-      # This is for backwards compatibility with the old
-      # provides_metadata-based code that used to be in this extension,
-      # but I don't know how much sense it makes.
-      # next if resource.options[:locale]
-
-      # $stderr.puts "Defaulting #{resource.path} to #{@mount_at_root}"
-      # resource.add_metadata options: { locale: @mount_at_root },
-      # locals: { locale: @mount_at_root }
-  end
-
-    # If it uses file extension localization
-    file_extension_resources.each do |resource|
-      ext_locale, path = parse_locale_extension(resource.path)
-      _, page_id = parse_locale_extension(resource.page_id, has_last: false)
-
-      new_resources << build_resource(path, resource.path,
-                                      page_id, ext_locale)
-
-      resource.ignore!
-    end
-
-    new_resources.reduce(resources) do |sum, r|
+    new_resources.flatten.reduce(resources) do |sum, r|
       r.execute_descriptor(app, sum)
     end
   end
@@ -249,6 +213,91 @@ def path_root(locale)
     end
   end
 
+  # Those are 'source' resource not destination
+  class I18nResource
+    attr_reader :resource, :i18n
+
+    def initialize(resource, i18n)
+      @resource = resource
+      @i18n = i18n
+    end
+  end
+
+  class LocalizableResource < I18nResource
+    def self.find_in(resources, i18n:)
+      resources.each_with_object([]) do |resource, output|
+        is_localizable = File.fnmatch?(
+          File.join(i18n.options[:templates_dir], '**'), resource.path
+        )
+
+        output << new(resource, i18n) if !resource.ignored? && is_localizable
+      end
+    end
+
+    def build
+      # Remove folder name
+      path = resource.path.sub(i18n.options[:templates_dir], '')
+      page_id = resource.page_id.sub(i18n.options[:templates_dir] + '/', '')
+
+      result = i18n.locales.map do |locale|
+        i18n.send(:build_resource, *[path, resource.path, page_id, locale])
+      end
+
+      resource.ignore!
+
+      result
+    end
+  end
+
+  class FileExtensionResource < I18nResource
+    def self.find_in(resources, i18n:)
+      # File extension resources are the ones that return a true parse
+      resources.each_with_object([]) do |resource, output|
+        output << new(resource, i18n) if parse_locale_extension(resource.path, i18n: i18n)[0]
+      end
+    end
+
+    def build
+      ext_locale, path = self.class.parse_locale_extension(resource.path, i18n: i18n)
+      _, page_id = self.class.parse_locale_extension(resource.page_id, i18n: i18n, has_last: false)
+
+      # result = i18n.build_resource(path, resource.path, page_id, ext_locale)
+      result = i18n.send(:build_resource, *[path, resource.path, page_id, ext_locale])
+      resource.ignore!
+
+      result
+    end
+
+    # Parse locale extension filename or page_id when implicit
+    #
+    # Page id could have no extension if set manually
+    #
+    # @return [locale, result]
+    # will return +nil+ if no locale extension
+
+    # Contract String, Hash => [Maybe[Symbol], String]
+    def self.parse_locale_extension(pathish, i18n:, has_last: true)
+      dirname = File.dirname(pathish)
+      basename = File.basename(pathish)
+
+      path_bits = basename.split('.')
+
+      locale_index = has_last ? -2 : -1
+      locale = path_bits[locale_index].try(:to_sym)
+
+      if i18n.locales.include?(locale)
+        path_bits.delete_at(locale_index)
+      else
+        locale = nil
+      end
+
+      pathish = path_bits.join('.')
+      pathish = File.join(dirname, pathish) unless dirname == '.'
+
+      [locale, pathish]
+    end
+  end
+
   private
 
   def on_file_changed(_updated_files, _removed_files)
@@ -283,34 +332,6 @@ def known_locales
     end
   end
 
-  # Parse locale extension filename or page_id when implicit
-  #
-  # Page id could have no extension if set manually
-  #
-  # @return [locale, result]
-  # will return +nil+ if no locale extension
-  Contract String, Hash => [Maybe[Symbol], String]
-  def parse_locale_extension(pathish, has_last: true)
-    dirname = File.dirname(pathish)
-    basename = File.basename(pathish)
-
-    path_bits = basename.split('.')
-
-    locale_index = has_last ? -2 : -1
-    locale = path_bits[locale_index].try(:to_sym)
-
-    if locales.include?(locale)
-      path_bits.delete_at(locale_index)
-    else
-      locale = nil
-    end
-
-    pathish = path_bits.join('.')
-    pathish = File.join(dirname, pathish) unless dirname == '.'
-
-    [locale, pathish]
-  end
-
   LocalizedPageDescriptor = Struct.new(:path, :source_path, :page_id, :locale) do
     def execute_descriptor(app, resources)
       r = ::Middleman::Sitemap::ProxyResource.new(app.sitemap, path, source_path)
@@ -319,6 +340,7 @@ def execute_descriptor(app, resources)
     end
   end
 
+  # Build destination resource
   Contract String, String, String, Symbol => LocalizedPageDescriptor
   def build_resource(path, source_path, page_id, locale)
     old_locale = ::I18n.locale