msginit: Replace charset in Content-type with charset in locale #113

abetomo · 2025-01-22T08:38:21Z

getext is generated with charset=CHARSET.
Translation as is will result in an error because CHARSET is an invalid value.

Changed so that charset=CHARSET is replaced by the locale's charset.

`getext` is generated with `charset=CHARSET`. Translation as is will result in an error because `CHARSET` is an invalid value. Changed so that `charset=CHARSET` is replaced by the locale's charset.

kou · 2025-01-22T10:42:39Z

lib/gettext/po_parser.rb

Could you revert this?

kou · 2025-01-22T10:45:06Z

lib/gettext/tools/msginit.rb

        end
      end

+      CONTENT_TYPE_CHARSET = /(Content-Type: .+ charset=)CHARSET/


Suggested change

CONTENT_TYPE_CHARSET = /(Content-Type: .+ charset=)CHARSET/

CONTENT_TYPE_CHARSET = /^(Content-Type:.+ charset=)CHARSET/

kou · 2025-01-22T10:45:33Z

lib/gettext/tools/msginit.rb

+        if CONTENT_TYPE_CHARSET =~ @entry
+          @entry = @entry.gsub(CONTENT_TYPE_CHARSET, "\\1#{@charset}")
+        end


Suggested change

if CONTENT_TYPE_CHARSET =~ @entry

@entry = @entry.gsub(CONTENT_TYPE_CHARSET, "\\1#{@charset}")

end

@entry = @entry.gsub(CONTENT_TYPE_CHARSET, "\\1#{@charset}")

kou · 2025-01-22T10:45:43Z

test/po/ja/_.po

Could you revert this?

kou · 2025-01-22T10:48:01Z

test/tools/test_msginit.rb

  class TestLocale < self
-    def run_msginit(locale)
-      create_pot_file("test.pot")
+    def run_msginit(locale, charset = "UTF-8")


Suggested change

def run_msginit(locale, charset = "UTF-8")

def run_msginit(locale, charset=nil)

kou · 2025-01-22T10:52:07Z

test/tools/test_msginit.rb

+    def test_language_charset_with_replace_content_type
+      locale = "en"
+      assert_equal(po_header(locale, locale),
+                   run_msginit(locale, "CHARSET"))
+    end
+
+    def test_language_region_with_replace_content_type
+      locale = "en_US"
+      language = "en"
+      assert_equal(po_header(locale, language),
+                   run_msginit(locale, "CHARSET"))
+    end
+
+    def test_language_region_charset_with_replace_content_type
+      locale = "en_US"
+      language = "en"
+      charset = "UTF-8"
+      assert_equal(po_header(locale, language),
+                   run_msginit("#{locale}.#{charset}", "CHARSET"))
+    end


Why do we need them?
It seems that existing tests cover these cases.

The test was to check that charset is replaced as expected, even when --locale is specified.
This is because the value of language_tag changes with or without its option.

gettext/lib/gettext/tools/msginit.rb

Lines 169 to 173 in 17ad4cd

if @locale.nil?

language_tag = Locale.current

else

language_tag = Locale::Tag.parse(@locale)

end

OK.

Let's clarify it by test name and variable name:

-test_XXX_with_replace_content_type +test_XXX_with_template_charset

-run_msginit("XXX", "CHARSET") +template_charset = "CHARSET" +run_msginit("XXX", template_charset)

kou · 2025-01-22T10:52:27Z

test/tools/test_msginit.rb

+
+  class TestCurrentCharset < self
+    def run_msginit(charset)
+      create_pot_file("test.pot", :charset => charset)


Suggested change

create_pot_file("test.pot", :charset => charset)

create_pot_file("test.pot", charset: charset)

kou · 2025-01-22T10:53:18Z

test/tools/test_msginit.rb

+  end
+
+  class TestCurrentCharset < self
+    def run_msginit(charset)


Suggested change

def run_msginit(charset)

def run_msginit(pot_charset)

kou · 2025-01-22T10:53:56Z

test/tools/test_msginit.rb

+        assert_equal(po_header(:charset => "UTF-8"),
+                     run_msginit("CHARSET"))


Suggested change

assert_equal(po_header(:charset => "UTF-8"),

run_msginit("CHARSET"))

assert_equal(po_header(charset: "UTF-8"),

run_msginit("CHARSET"))

kou · 2025-01-22T10:54:08Z

test/tools/test_msginit.rb

+        assert_equal(po_header(:charset => "ASCII"),
+                     run_msginit("ASCII"))


Suggested change

assert_equal(po_header(:charset => "ASCII"),

run_msginit("ASCII"))

assert_equal(po_header(charset: "ASCII"),

run_msginit("ASCII"))

kou · 2025-01-23T00:08:45Z

test/tools/test_msginit.rb

+      super(current_locale, current_language, options)
+    end
+
+    def test_change


Suggested change

def test_change

def test_template_charset

kou · 2025-01-23T00:08:58Z

test/tools/test_msginit.rb

+                   run_msginit("CHARSET"))
+    end
+
+    def test_not_change


Suggested change

def test_not_change

def test_not_template_charset

kou · 2025-01-23T00:11:02Z

test/tools/test_msginit.rb

+    end
+
+    def test_change
+      assert_equal(po_header(charset: "UTF-8"),


Suggested change

assert_equal(po_header(charset: "UTF-8"),

assert_equal(po_header(charset: Locale.current.charset),

We may want to define current_charset like other current_* methods.

msginit: Replace charset in Content-type with charset in locale

d78b163

`getext` is generated with `charset=CHARSET`. Translation as is will result in an error because `CHARSET` is an invalid value. Changed so that `charset=CHARSET` is replaced by the locale's charset.

abetomo mentioned this pull request Jan 22, 2025

docs: build: Change to fail with sphinx-build warnings groonga/groonga#2093

Merged

kou reviewed Jan 22, 2025

View reviewed changes

abetomo added 3 commits January 23, 2025 08:31

Revert

a16e9bc

Improve regular expression

9f70ff1

test: Improve argument passing

29ad41d

kou reviewed Jan 23, 2025

View reviewed changes

Improve test name

fb300b5

kou merged commit 8b183c3 into ruby-gettext:master Jan 27, 2025
22 checks passed

abetomo deleted the replace-charset branch January 27, 2025 05:09

	CONTENT_TYPE_CHARSET = /(Content-Type: .+ charset=)CHARSET/
	CONTENT_TYPE_CHARSET = /^(Content-Type:.+ charset=)CHARSET/

	def run_msginit(locale, charset = "UTF-8")
	def run_msginit(locale, charset=nil)

	if @locale.nil?
	language_tag = Locale.current
	else
	language_tag = Locale::Tag.parse(@locale)
	end

	create_pot_file("test.pot", :charset => charset)
	create_pot_file("test.pot", charset: charset)

		assert_equal(po_header(:charset => "UTF-8"),
		run_msginit("CHARSET"))

		assert_equal(po_header(:charset => "ASCII"),
		run_msginit("ASCII"))

	assert_equal(po_header(charset: "UTF-8"),
	assert_equal(po_header(charset: Locale.current.charset),

msginit: Replace charset in Content-type with charset in locale #113

msginit: Replace charset in Content-type with charset in locale #113

Uh oh!

Conversation

abetomo commented Jan 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants