From 6fc3c30dc1f96d87a6f5a7bad7582fe7c7d3996a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 26 Mar 2022 14:56:18 +0530 Subject: [PATCH 1/4] refactor: make translate regex readable --- frappe/translate.py | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/frappe/translate.py b/frappe/translate.py index 0367d33d3b..04d1feea76 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -23,6 +23,35 @@ from frappe.utils import get_bench_path, is_html, strip, strip_html_tags from frappe.query_builder import Field, DocType from pypika.terms import PseudoColumn +TRANSLATE_PATTERN = re.compile( + r"_\(" # starts with literal `_(` work with both python / JS + + # BEGIN: message search + r"([\"']{,3})" # start of message string identifier - allows: ', ", """, '''; 1st capture group + r"(?P((?!\1).)*)" # Keep matching until string closing identifier is met which is same as 1st capture group + r"\1" # match exact string closing identifier + # END: message search + + # BEGIN: python context search + r"(\s*,\s*context\s*=\s*" # capture `context=` with ignoring whitespace + r"([\"'])" # start of context string identifier; 5th capture group + r"(?P((?!\5).)*)" # capture context string till closing id is found + r"\5" # match context string closure + r")*" # match one or more context string (?wat this should be 0 or 1) + # END: python context search + + # BEGIN: JS context search + r"(\s*,\s*(.)*?\s*(,\s*" # skip message format replacements: ["format", ...] | null | [] + r"([\"'])" # start of context string; 11th capture group + r"(?P((?!\11).)*)" # capture context string till closing id is found + r"\11" # match context string closure + r")*" + r")*" # match one or more context string + # END: JS context search + + r"\)" # Closing function call +) + def get_language(lang_list: List = None) -> str: """Set `frappe.local.lang` from HTTP headers at beginning of request @@ -651,9 +680,8 @@ def extract_messages_from_code(code): frappe.clear_last_message() messages = [] - pattern = r"_\(([\"']{,3})(?P((?!\1).)*)\1(\s*,\s*context\s*=\s*([\"'])(?P((?!\5).)*)\5)*(\s*,\s*(.)*?\s*(,\s*([\"'])(?P((?!\11).)*)\11)*)*\)" - for m in re.compile(pattern).finditer(code): + for m in TRANSLATE_PATTERN.finditer(code): message = m.group('message') context = m.group('py_context') or m.group('js_context') pos = m.start() From cf69b7f084848da2c7363f93442e7f773ecf0e21 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 26 Mar 2022 14:58:55 +0530 Subject: [PATCH 2/4] fix: only match 1 python context string at most --- frappe/translate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/translate.py b/frappe/translate.py index 04d1feea76..9869a89840 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -37,7 +37,7 @@ TRANSLATE_PATTERN = re.compile( r"([\"'])" # start of context string identifier; 5th capture group r"(?P((?!\5).)*)" # capture context string till closing id is found r"\5" # match context string closure - r")*" # match one or more context string (?wat this should be 0 or 1) + r")?" # match 0 or 1 context strings # END: python context search # BEGIN: JS context search From 92c563d193179d53ae2e9665ca830594244e720e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 26 Mar 2022 15:09:16 +0530 Subject: [PATCH 3/4] test: refactor translation extraction test --- frappe/tests/test_translate.py | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/frappe/tests/test_translate.py b/frappe/tests/test_translate.py index 1b96fb62c3..63c4e8928d 100644 --- a/frappe/tests/test_translate.py +++ b/frappe/tests/test_translate.py @@ -36,7 +36,18 @@ class TestTranslate(unittest.TestCase): def test_extract_message_from_file(self): data = frappe.translate.get_messages_from_file(translation_string_file) - self.assertListEqual(data, expected_output) + exp_filename = "apps/frappe/frappe/tests/translation_test_file.txt" + + self.assertEqual(len(data), len(expected_output), + msg=f"Mismatched output:\nExpected: {expected_output}\nFound: {data}") + + for extracted, expected in zip(data, expected_output): + ext_filename, ext_message, ext_context, ext_line = extracted + exp_message, exp_context, exp_line = expected + self.assertEqual(ext_filename, exp_filename) + self.assertEqual(ext_message, exp_message) + self.assertEqual(ext_context, exp_context) + self.assertEqual(ext_line, exp_line) def test_translation_with_context(self): try: @@ -107,13 +118,13 @@ class TestTranslate(unittest.TestCase): expected_output = [ - ('apps/frappe/frappe/tests/translation_test_file.txt', 'Warning: Unable to find {0} in any table related to {1}', 'This is some context', 2), - ('apps/frappe/frappe/tests/translation_test_file.txt', 'Warning: Unable to find {0} in any table related to {1}', None, 4), - ('apps/frappe/frappe/tests/translation_test_file.txt', "You don't have any messages yet.", None, 6), - ('apps/frappe/frappe/tests/translation_test_file.txt', 'Submit', 'Some DocType', 8), - ('apps/frappe/frappe/tests/translation_test_file.txt', 'Warning: Unable to find {0} in any table related to {1}', 'This is some context', 15), - ('apps/frappe/frappe/tests/translation_test_file.txt', 'Submit', 'Some DocType', 17), - ('apps/frappe/frappe/tests/translation_test_file.txt', "You don't have any messages yet.", None, 19), - ('apps/frappe/frappe/tests/translation_test_file.txt', "You don't have any messages yet.", None, 21) + ('Warning: Unable to find {0} in any table related to {1}', 'This is some context', 2), + ('Warning: Unable to find {0} in any table related to {1}', None, 4), + ("You don't have any messages yet.", None, 6), + ('Submit', 'Some DocType', 8), + ('Warning: Unable to find {0} in any table related to {1}', 'This is some context', 15), + ('Submit', 'Some DocType', 17), + ("You don't have any messages yet.", None, 19), + ("You don't have any messages yet.", None, 21) ] From 29ca4d7aaa75c20d0e7f796fdb217a1d5df36e88 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 26 Mar 2022 15:18:40 +0530 Subject: [PATCH 4/4] feat: allow splitting _() function call on multiple lines --- frappe/tests/test_translate.py | 5 ++++- frappe/tests/translation_test_file.txt | 16 +++++++++++++++- frappe/translate.py | 6 +++--- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/frappe/tests/test_translate.py b/frappe/tests/test_translate.py index 63c4e8928d..f5386914f7 100644 --- a/frappe/tests/test_translate.py +++ b/frappe/tests/test_translate.py @@ -125,6 +125,9 @@ expected_output = [ ('Warning: Unable to find {0} in any table related to {1}', 'This is some context', 15), ('Submit', 'Some DocType', 17), ("You don't have any messages yet.", None, 19), - ("You don't have any messages yet.", None, 21) + ("You don't have any messages yet.", None, 21), + ("Long string that needs its own line because of black formatting.", None, 24), + ("Long string with", "context", 28), + ("Long string with", "context on newline", 32), ] diff --git a/frappe/tests/translation_test_file.txt b/frappe/tests/translation_test_file.txt index 45f67a806b..7db71665ad 100644 --- a/frappe/tests/translation_test_file.txt +++ b/frappe/tests/translation_test_file.txt @@ -18,4 +18,18 @@ _('Submit', context="Some DocType") _("""You don't have any messages yet.""") -_('''You don't have any messages yet.''') \ No newline at end of file +_('''You don't have any messages yet.''') + +// allow newline in beginning +_( +"""Long string that needs its own line because of black formatting.""" +).format("blah") + +_( +"Long string with", context="context" +).format("blah") + +_( + "Long string with", + context="context on newline" +).format("blah") diff --git a/frappe/translate.py b/frappe/translate.py index 9869a89840..6e0fefd6fa 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -24,7 +24,7 @@ from frappe.query_builder import Field, DocType from pypika.terms import PseudoColumn TRANSLATE_PATTERN = re.compile( - r"_\(" # starts with literal `_(` work with both python / JS + r"_\([\s\n]*" # starts with literal `_(`, ignore following whitespace/newlines # BEGIN: message search r"([\"']{,3})" # start of message string identifier - allows: ', ", """, '''; 1st capture group @@ -33,7 +33,7 @@ TRANSLATE_PATTERN = re.compile( # END: message search # BEGIN: python context search - r"(\s*,\s*context\s*=\s*" # capture `context=` with ignoring whitespace + r"([\s\n]*,[\s\n]*context\s*=\s*" # capture `context=` with ignoring whitespace r"([\"'])" # start of context string identifier; 5th capture group r"(?P((?!\5).)*)" # capture context string till closing id is found r"\5" # match context string closure @@ -49,7 +49,7 @@ TRANSLATE_PATTERN = re.compile( r")*" # match one or more context string # END: JS context search - r"\)" # Closing function call + r"[\s\n]*\)" # Closing function call ignore leading whitespace/newlines )