Merge pull request #289 from sebastianludwig/286-287-django

Django formatter improvements

lib/twine/formatters/django.rb

@@ -1,5 +1,6 @@
 module Twine
   module Formatters
+    # For a description of the .po file format, see https://www.gnu.org/software/gettext/manual/html_node/PO-Files.html
     class Django < Abstract
       def format_name
         'django'
@@ -14,9 +15,9 @@ module Twine
       end
 
       def read(io, lang)
-        comment_regex = /#\. *"?(.*)"?$/
-        key_regex = /msgid *"(.*)"$/
-        value_regex = /msgstr *"(.*)"$/m
+        comment_regex = /^\s*#\. *"?(.*)"?$/
+        key_regex = /^msgid *"(.*)"$/
+        value_regex = /^msgstr *"(.*)"$/m
 
         while line = io.gets          
           comment_match = comment_regex.match(line)
@@ -53,11 +54,12 @@ module Twine
       end
 
       def format_header(lang)
-        "##\n # Django Strings File\n # Generated by Twine #{Twine::VERSION}\n # Language: #{lang}\nmsgid \"\"\nmsgstr \"\"\n\"Content-Type: text/plain; charset=UTF-8\\n\""
+        # see https://www.gnu.org/software/trans-coord/manual/gnun/html_node/PO-Header.html for details
+        "# Django Strings File\n# Generated by Twine #{Twine::VERSION}\n# Language: #{lang}\nmsgid \"\"\nmsgstr \"\"\n\"Content-Type: text/plain; charset=UTF-8\\n\""
       end
 
       def format_section_header(section)
-        "#--------- #{section.name} ---------#\n"
+        "# --------- #{section.name} --------- #\n"
       end
 
       def format_definition(definition, lang)

lib/twine/placeholders.rb

@@ -72,5 +72,11 @@ module Twine
     def convert_placeholders_from_flash_to_twine(input)
       input.gsub /\{\d+\}/, '%@'
     end
+
+    # Python supports placeholders in the form of `%(amount)03d`
+    # see https://docs.python.org/3/library/stdtypes.html#printf-style-string-formatting
+    def contains_python_specific_placeholder(input)
+      /%\([a-zA-Z0-9_-]+\)#{PLACEHOLDER_PARAMETER_FLAGS_WIDTH_PRECISION_LENGTH}#{PLACEHOLDER_TYPES}/.match(input) != nil
+    end
   end
 end

lib/twine/runner.rb

@@ -241,6 +241,7 @@ module Twine
       duplicate_keys = Set.new
       keys_without_tags = Set.new
       invalid_keys = Set.new
+      keys_with_python_only_placeholders = Set.new
       valid_key_regex = /^[A-Za-z0-9_]+$/
 
       @twine_file.sections.each do |section|
@@ -253,6 +254,8 @@ module Twine
           keys_without_tags.add(definition.key) if definition.tags == nil or definition.tags.length == 0
 
           invalid_keys << definition.key unless definition.key =~ valid_key_regex
+
+          keys_with_python_only_placeholders << definition.key if definition.translations.values.any? { |v| Placeholders.contains_python_specific_placeholder(v) }
         end
       end
 
@@ -275,6 +278,10 @@ module Twine
         errors << "Found key(s) with invalid characters:\n#{join_keys.call(invalid_keys)}"
       end
 
+      unless keys_with_python_only_placeholders.empty?
+        errors << "Found key(s) with placeholders that are only supported by Python:\n#{join_keys.call(keys_with_python_only_placeholders)}"
+      end
+
       raise Twine::Error.new errors.join("\n\n") unless errors.empty?
 
       Twine::stdout.puts "#{@options[:twine_file]} is valid."

test/fixtures/formatter_django.po

@@ -1,12 +1,11 @@
-##
- # Django Strings File
- # Generated by Twine <%= Twine::VERSION %>
- # Language: en
+# Django Strings File
+# Generated by Twine <%= Twine::VERSION %>
+# Language: en
 msgid ""
 msgstr ""
 "Content-Type: text/plain; charset=UTF-8\n"
 
-#--------- Section 1 ---------#
+# --------- Section 1 --------- #
 
 #. comment key1
 # base translation: "value1-english"
@@ -18,7 +17,7 @@ msgid "key2"
 msgstr "value2-english"
 
 
-#--------- Section 2 ---------#
+# --------- Section 2 --------- #
 
 # base translation: "value3-english"
 msgid "key3"

test/test_formatters.rb

@@ -554,6 +554,19 @@ class TestDjangoFormatter < FormatterTest
     language = %w(en-GB de fr).sample
     assert_equal language, @formatter.determine_language_given_path("/output/#{language}/#{@formatter.default_file_name}")
   end
+
+  def test_ignores_commented_out_strings
+    content = <<-EOCONTENT
+      #~ msgid "foo"
+      #~ msgstr "This should be ignored"
+    EOCONTENT
+
+    io = StringIO.new(content)
+
+    @formatter.read io, 'en'
+
+    assert_nil @empty_twine_file.definitions_by_key["foo"]
+  end
 end
 
 class TestFlashFormatter < FormatterTest

test/test_placeholders.rb

@@ -122,4 +122,21 @@ class PlaceholderTest < TwineTest
       assert_equal "some %@ more %@ text %@", from_flash("some {0} more {1} text {2}")
     end
   end
+
+  class PythonPlaceholder < PlaceholderTest
+    def test_negative_for_regular_placeholders
+      assert_equal false, Twine::Placeholders.contains_python_specific_placeholder(placeholder)
+    end
+
+    def test_positive_for_named_placeholders
+      inputs = [
+        "%(language)s has",
+        "For %(number)03d quotes",
+        "bought on %(app_name)s"
+      ]
+      inputs.each do |input|
+        assert_equal true, Twine::Placeholders.contains_python_specific_placeholder(input)
+      end
+    end
+  end
 end

test/test_validate_twine_file.rb

@@ -58,4 +58,12 @@ class TestValidateTwineFile < CommandTest
       Twine::Runner.new(@options.merge(pedantic: true), @twine_file).validate_twine_file
     end
   end
+
+  def test_reports_python_specific_placeholders
+    random_definition.translations["en"] = "%(python_only)s"
+
+    assert_raises Twine::Error do
+      Twine::Runner.new(@options, @twine_file).validate_twine_file
+    end
+  end
 end