Repository: python-babel/babel
Pull request #1122 (2024-08-22 18:55:14)

Replace more alternate characters in `format_skeleton`
Related discussion: #1084 

Replaces some additional characters which never appear in resource files before matching the skeleton. This replicates the behaviour of ICU: https://github.com/unicode-org/icu/blob/5e22f0076ec9b55056cd8a84e9ef370632f44174/icu4j/main/core/src/main/java/com/ibm/icu/text/DateIntervalInfo.java#L1090-L1102

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/python-babel/babel/pull/1122?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 91.24%. Comparing base [(`cf7d223`)](https://app.codecov.io/gh/python-babel/babel/commit/cf7d22369cf40a8218cff1d3dc823eefa174aee0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) to head [(`bfb5e3d`)](https://app.codecov.io/gh/python-babel/babel/commit/bfb5e3d3c8a8715b7424a22a89587468e2030d7f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #1122      +/-   ##
==========================================
+ Coverage   91.22%   91.24%   +0.01%     
==========================================
  Files          27       27              
  Lines        4561     4569       +8     
==========================================
+ Hits         4161     4169       +8     
  Misses        400      400              
```

| [Flag](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | Coverage Δ | |
|---|---|---|
| [macos-12-3.10](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `90.04% <100.00%> (+0.01%)` | :arrow_up: |
| [macos-12-3.11](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `?` | |
| [macos-12-3.12](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `?` | |
| [macos-12-3.13-dev](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `?` | |
| [macos-12-3.8](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `?` | |
| [macos-12-3.9](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `?` | |
| [macos-12-pypy3.10](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `?` | |
| [ubuntu-22.04-3.10](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `90.06% <100.00%> (+0.01%)` | :arrow_up: |
| [ubuntu-22.04-3.11](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `89.99% <100.00%> (+0.01%)` | :arrow_up: |
| [ubuntu-22.04-3.12](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `?` | |
| [ubuntu-22.04-3.13-dev](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `89.69% <100.00%> (+0.01%)` | :arrow_up: |
| [ubuntu-22.04-3.8](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `89.99% <100.00%> (+0.01%)` | :arrow_up: |
| [ubuntu-22.04-3.9](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `89.99% <100.00%> (+0.01%)` | :arrow_up: |
| [ubuntu-22.04-pypy3.10](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `?` | |
| [windows-2022-3.10](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `90.18% <100.00%> (+0.01%)` | :arrow_up: |
| [windows-2022-3.11](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `?` | |
| [windows-2022-3.12](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `?` | |
| [windows-2022-3.13-dev](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `89.81% <100.00%> (+0.01%)` | :arrow_up: |
| [windows-2022-3.8](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `?` | |
| [windows-2022-3.9](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `90.11% <100.00%> (+0.01%)` | :arrow_up: |
| [windows-2022-pypy3.10](https://app.codecov.io/gh/python-babel/babel/pull/1122/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | `?` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/python-babel/babel/pull/1122?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel).


Patch:
diff --git a/babel/dates.py b/babel/dates.py
index 3373e06..30321b7 100644
--- a/babel/dates.py
+++ b/babel/dates.py
@@ -1232,7 +1232,7 @@ def parse_date(
 
     format_str = get_date_format(format=format, locale=locale).pattern.lower()
     year_idx = format_str.index('y')
-    month_idx = format_str.index('m')
+    month_idx = format_str.find('m')
     if month_idx < 0:
         month_idx = format_str.index('l')
     day_idx = format_str.index('d')
@@ -1277,7 +1277,7 @@ def parse_time(
 
     # TODO: try ISO format first?
     format_str = get_time_format(format=format, locale=locale).pattern.lower()
-    hour_idx = format_str.index('h')
+    hour_idx = format_str.find('h')
     if hour_idx < 0:
         hour_idx = format_str.index('k')
     min_idx = format_str.index('m')
@@ -1890,6 +1890,14 @@ def match_skeleton(skeleton: str, options: Iterable[str], allow_different_fields
 
     if 'z' in skeleton and not any('z' in option for option in options):
         skeleton = skeleton.replace('z', 'v')
+    if 'k' in skeleton and not any('k' in option for option in options):
+        skeleton = skeleton.replace('k', 'H')
+    if 'K' in skeleton and not any('K' in option for option in options):
+        skeleton = skeleton.replace('K', 'h')
+    if 'a' in skeleton and not any('a' in option for option in options):
+        skeleton = skeleton.replace('a', '')
+    if 'b' in skeleton and not any('b' in option for option in options):
+        skeleton = skeleton.replace('b', '')
 
     get_input_field_width = dict(t[1] for t in tokenize_pattern(skeleton) if t[0] == "field").get
     best_skeleton = None
diff --git a/babel/messages/pofile.py b/babel/messages/pofile.py
index 89a9242..5cd65d8 100644
--- a/babel/messages/pofile.py
+++ b/babel/messages/pofile.py
@@ -80,6 +80,50 @@ def denormalize(string: str) -> str:
         return unescape(string)
 
 
+def _extract_locations(line: str) -> list[str]:
+    """Extract locations from location comments.
+
+    Locations are extracted while properly handling First Strong
+    Isolate (U+2068) and Pop Directional Isolate (U+2069), used by
+    gettext to enclose filenames with spaces and tabs in their names.
+    """
+    if "\u2068" not in line and "\u2069" not in line:
+        return line.lstrip().split()
+
+    locations = []
+    location = ""
+    in_filename = False
+    for c in line:
+        if c == "\u2068":
+            if in_filename:
+                raise ValueError("location comment contains more First Strong Isolate "
+                                 "characters, than Pop Directional Isolate characters")
+            in_filename = True
+            continue
+        elif c == "\u2069":
+            if not in_filename:
+                raise ValueError("location comment contains more Pop Directional Isolate "
+                                 "characters, than First Strong Isolate characters")
+            in_filename = False
+            continue
+        elif c == " ":
+            if in_filename:
+                location += c
+            elif location:
+                locations.append(location)
+                location = ""
+        else:
+            location += c
+    else:
+        if location:
+            if in_filename:
+                raise ValueError("location comment contains more First Strong Isolate "
+                                 "characters, than Pop Directional Isolate characters")
+            locations.append(location)
+
+    return locations
+
+
 class PoFileError(Exception):
     """Exception thrown by PoParser when an invalid po file is encountered."""
 
@@ -269,7 +313,7 @@ class PoFileParser:
         self._finish_current_message()
 
         if line[1:].startswith(':'):
-            for location in line[2:].lstrip().split():
+            for location in _extract_locations(line[2:]):
                 pos = location.rfind(':')
                 if pos >= 0:
                     try:
@@ -307,7 +351,10 @@ class PoFileParser:
                 if line[1:].startswith('~'):
                     self._process_message_line(lineno, line[2:].lstrip(), obsolete=True)
                 else:
-                    self._process_comment(line)
+                    try:
+                        self._process_comment(line)
+                    except ValueError as exc:
+                        self._invalid_pofile(line, lineno, str(exc))
             else:
                 self._process_message_line(lineno, line)
 
@@ -474,6 +521,23 @@ def normalize(string: str, prefix: str = '', width: int = 76) -> str:
     return '""\n' + '\n'.join([(prefix + escape(line)) for line in lines])
 
 
+def _enclose_filename_if_necessary(filename: str) -> str:
+    """Enclose filenames which include white spaces or tabs.
+
+    Do the same as gettext and enclose filenames which contain white
+    spaces or tabs with First Strong Isolate (U+2068) and Pop
+    Directional Isolate (U+2069).
+    """
+    if " " not in filename and "\t" not in filename:
+        return filename
+
+    if not filename.startswith("\u2068"):
+        filename = "\u2068" + filename
+    if not filename.endswith("\u2069"):
+        filename += "\u2069"
+    return filename
+
+
 def write_po(
     fileobj: SupportsWrite[bytes],
     catalog: Catalog,
@@ -626,6 +690,7 @@ def generate_po(
 
             for filename, lineno in locations:
                 location = filename.replace(os.sep, '/')
+                location = _enclose_filename_if_necessary(location)
                 if lineno and include_lineno:
                     location = f"{location}:{lineno:d}"
                 if location not in locs:


Test patch:
diff --git a/tests/messages/test_pofile.py b/tests/messages/test_pofile.py
index d1a3e2d..c0ded12 100644
--- a/tests/messages/test_pofile.py
+++ b/tests/messages/test_pofile.py
@@ -19,6 +19,7 @@ import pytest
 from babel.core import Locale
 from babel.messages import pofile
 from babel.messages.catalog import Catalog, Message
+from babel.messages.pofile import _enclose_filename_if_necessary, _extract_locations
 from babel.util import FixedOffsetTimezone
 
 
@@ -438,6 +439,19 @@ msgstr[2] "Vohs [text]"
         assert message.string[1] == ''
         assert message.string[2] == 'Vohs [text]'
 
+    def test_with_location(self):
+        buf = StringIO('''\
+#: main.py:1 \u2068filename with whitespace.py\u2069:123
+msgid "foo"
+msgstr "bar"
+''')
+        catalog = pofile.read_po(buf, locale='de_DE')
+        assert len(catalog) == 1
+        message = catalog['foo']
+        assert message.string == 'bar'
+        assert message.locations == [("main.py", 1), ("filename with whitespace.py", 123)]
+
+
     def test_abort_invalid_po_file(self):
         invalid_po = '''
             msgctxt ""
@@ -841,6 +855,59 @@ msgstr ""'''
 msgid "foo"
 msgstr ""'''
 
+    def test_white_space_in_location(self):
+        catalog = Catalog()
+        catalog.add('foo', locations=[('main.py', 1)])
+        catalog.add('foo', locations=[('utils b.py', 3)])
+        buf = BytesIO()
+        pofile.write_po(buf, catalog, omit_header=True, include_lineno=True)
+        assert buf.getvalue().strip() == b'''#: main.py:1 \xe2\x81\xa8utils b.py\xe2\x81\xa9:3
+msgid "foo"
+msgstr ""'''
+
+    def test_white_space_in_location_already_enclosed(self):
+        catalog = Catalog()
+        catalog.add('foo', locations=[('main.py', 1)])
+        catalog.add('foo', locations=[('\u2068utils b.py\u2069', 3)])
+        buf = BytesIO()
+        pofile.write_po(buf, catalog, omit_header=True, include_lineno=True)
+        assert buf.getvalue().strip() == b'''#: main.py:1 \xe2\x81\xa8utils b.py\xe2\x81\xa9:3
+msgid "foo"
+msgstr ""'''
+
+    def test_tab_in_location(self):
+        catalog = Catalog()
+        catalog.add('foo', locations=[('main.py', 1)])
+        catalog.add('foo', locations=[('utils\tb.py', 3)])
+        buf = BytesIO()
+        pofile.write_po(buf, catalog, omit_header=True, include_lineno=True)
+        assert buf.getvalue().strip() == b'''#: main.py:1 \xe2\x81\xa8utils        b.py\xe2\x81\xa9:3
+msgid "foo"
+msgstr ""'''
+
+    def test_tab_in_location_already_enclosed(self):
+        catalog = Catalog()
+        catalog.add('foo', locations=[('main.py', 1)])
+        catalog.add('foo', locations=[('\u2068utils\tb.py\u2069', 3)])
+        buf = BytesIO()
+        pofile.write_po(buf, catalog, omit_header=True, include_lineno=True)
+        assert buf.getvalue().strip() == b'''#: main.py:1 \xe2\x81\xa8utils        b.py\xe2\x81\xa9:3
+msgid "foo"
+msgstr ""'''
+
+
+class RoundtripPoTestCase(unittest.TestCase):
+
+    def test_enclosed_filenames_in_location_comment(self):
+        catalog = Catalog()
+        catalog.add("foo", lineno=2, locations=[("main 1.py", 1)], string="")
+        catalog.add("bar", lineno=6, locations=[("other.py", 2)], string="")
+        catalog.add("baz", lineno=10, locations=[("main 1.py", 3), ("other.py", 4)], string="")
+        buf = BytesIO()
+        pofile.write_po(buf, catalog, omit_header=True, include_lineno=True)
+        buf.seek(0)
+        catalog2 = pofile.read_po(buf)
+        assert True is catalog.is_identical(catalog2)
 
 class PofileFunctionsTestCase(unittest.TestCase):
 
@@ -864,6 +931,51 @@ class PofileFunctionsTestCase(unittest.TestCase):
         assert expected_denormalized == pofile.denormalize(f'""\n{msgstr}')
 
 
+@pytest.mark.parametrize(("line", "locations"), [
+    ("\u2068file1.po\u2069", ["file1.po"]),
+    ("file1.po \u2068file 2.po\u2069 file3.po", ["file1.po", "file 2.po", "file3.po"]),
+    ("file1.po:1 \u2068file 2.po\u2069:2 file3.po:3", ["file1.po:1", "file 2.po:2", "file3.po:3"]),
+    ("\u2068file1.po\u2069:1 \u2068file\t2.po\u2069:2 file3.po:3",
+     ["file1.po:1", "file\t2.po:2", "file3.po:3"]),
+    ("file1.po  file2.po", ["file1.po", "file2.po"]),
+    ("file1.po \u2068\u2069 file2.po", ["file1.po", "file2.po"]),
+])
+def test_extract_locations_valid_location_comment(line, locations):
+    assert locations == _extract_locations(line)
+
+
+@pytest.mark.parametrize(("line",), [
+    ("\u2068file 1.po",),
+    ("file 1.po\u2069",),
+    ("\u2069file 1.po\u2068",),
+    ("\u2068file 1.po:1 \u2068file 2.po\u2069:2",),
+    ("\u2068file 1.po\u2069:1 file 2.po\u2069:2",),
+])
+def test_extract_locations_invalid_location_comment(line):
+    with pytest.raises(ValueError):
+        _extract_locations(line)
+
+
+@pytest.mark.parametrize(("filename",), [
+    ("file.po",),
+    ("file_a.po",),
+    ("file-a.po",),
+    ("file\n.po",),
+    ("\u2068file.po\u2069",),
+    ("\u2068file a.po\u2069",),
+])
+def test_enclose_filename_if_necessary_no_change(filename):
+    assert filename == _enclose_filename_if_necessary(filename)
+
+
+@pytest.mark.parametrize(("filename",), [
+    ("file a.po",),
+    ("file\ta.po",),
+])
+def test_enclose_filename_if_necessary_enclosed(filename):
+    assert "\u2068" + filename + "\u2069" == _enclose_filename_if_necessary(filename)
+
+
 def test_unknown_language_roundtrip():
     buf = StringIO(r'''
 msgid ""
diff --git a/tests/test_dates.py b/tests/test_dates.py
index 0e0c97d..549929b 100644
--- a/tests/test_dates.py
+++ b/tests/test_dates.py
@@ -17,7 +17,7 @@ import freezegun
 import pytest
 
 from babel import Locale, dates
-from babel.dates import NO_INHERITANCE_MARKER, UTC, _localize
+from babel.dates import NO_INHERITANCE_MARKER, UTC, _localize, parse_pattern
 from babel.util import FixedOffsetTimezone
 
 
@@ -619,6 +619,19 @@ def test_format_skeleton(timezone_getter):
     assert (dates.format_skeleton('EHm', dt, tzinfo=timezone_getter('Asia/Bangkok'), locale='th') == 'อา. 22:30 น.')
 
 
+@pytest.mark.parametrize(('skeleton', 'expected'), [
+    ('Hmz', 'Hmv'),
+    ('kmv', 'Hmv'),
+    ('Kmv', 'hmv'),
+    ('Hma', 'Hm'),
+    ('Hmb', 'Hm'),
+    ('zkKab', 'vHh'),
+])
+def test_match_skeleton_alternate_characters(skeleton, expected):
+    # https://github.com/unicode-org/icu/blob/5e22f0076ec9b55056cd8a84e9ef370632f44174/icu4j/main/core/src/main/java/com/ibm/icu/text/DateIntervalInfo.java#L1090-L1102
+    assert dates.match_skeleton(skeleton, (expected,)) == expected
+
+
 def test_format_timedelta():
     assert (dates.format_timedelta(timedelta(weeks=12), locale='en_US')
             == '3 months')
@@ -666,6 +679,24 @@ def test_parse_time(input, expected):
     assert dates.parse_time(input, locale='en_US') == expected
 
 
+def test_parse_time_alternate_characters(monkeypatch):
+    # 'K' can be used as an alternative to 'H'
+    def get_time_format(*args, **kwargs):
+        return parse_pattern('KK:mm:ss')
+    monkeypatch.setattr(dates, 'get_time_format', get_time_format)
+
+    assert dates.parse_time('9:30') == time(9, 30)
+
+
+def test_parse_date_alternate_characters(monkeypatch):
+    # 'l' can be used as an alternative to 'm'
+    def get_date_format(*args, **kwargs):
+        return parse_pattern('yyyy-ll-dd')
+    monkeypatch.setattr(dates, 'get_time_format', get_date_format)
+
+    assert dates.parse_date('2024-10-20') == date(2024, 10, 20)
+
+
 @pytest.mark.parametrize('case', ['', 'a', 'aaa'])
 @pytest.mark.parametrize('func', [dates.parse_date, dates.parse_time])
 def test_parse_errors(case, func):
@@ -754,5 +785,5 @@ def test_issue_892():
 
 
 def test_issue_1089():
-    assert dates.format_datetime(datetime.utcnow(), locale="ja_JP@mod")
-    assert dates.format_datetime(datetime.utcnow(), locale=Locale.parse("ja_JP@mod"))
+    assert dates.format_datetime(datetime.now(), locale="ja_JP@mod")
+    assert dates.format_datetime(datetime.now(), locale=Locale.parse("ja_JP@mod"))