Generate l10n per locale - #3585
Draft
cheweaaron609-png wants to merge 35 commits into
Draft
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a Python script that builds a consolidated holidays JSON from gettext .po files, includes a representative sample JSON output, and appends one contributor to CONTRIBUTORS. ChangesLocalization Infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/l10n/json_builder.py`:
- Around line 45-57: The iteration over locale_path and PO files is
non-deterministic (locale_path.iterdir(), lc_messages.glob("*.po")), causing
unstable output; modify the loops to iterate in a deterministic order by using
sorted() on locale_path.iterdir() and on lc_messages.glob("*.po") (or otherwise
sort by name) when constructing lang_folder and po_file lists, and normalize the
countries collection (the list built from country_code/appends) by
deduplicating/sorting it (e.g., convert to a set then sort) before writing the
final JSON to ensure stable ordering across runs.
- Line 71: The current id generation (clean_id = re.sub(r'[^a-z0-9]+', '_',
comment.lower()).strip('_')) can produce duplicates; implement collision
handling by tracking used IDs (e.g., a set or dict) and, when a generated
clean_id already exists, append a numeric suffix like _2, _3, … until it's
unique; apply this change where clean_id is created (both the occurrence at the
current assignment and the logic around lines 90-96 that also generates ids) and
ensure the chosen unique id is stored in the used-IDs tracker before being
emitted.
- Around line 65-66: The grouping key currently prefers entry.tcomment which can
split/merge records across locales; change the assignment so the canonical
grouping key uses entry.comment first (e.g., set comment = entry.comment or
entry.tcomment or "" then comment = comment.strip()) and ensure any grouping
logic that uses the local variable comment continues to reference this updated
value so tcomment is only a fallback metadata source.
- Around line 76-81: Replace the locale-string heuristics used to set
is_default_file with an authoritative check against the PO file headers: read
the PO metadata (e.g., po.metadata.get('Language') and
po.metadata.get('X-Source-Language')) and determine is_default_file by comparing
those header values to the target lang/country_code (normalize case) instead of
relying on COUNTRY_LANG_MAP, lang_parts, or startswith('en'); if metadata is
absent, you may optionally fall back to the existing heuristic, but primary
decision must come from the PO headers.
- Around line 59-62: The code currently swallows all errors when calling
polib.pofile(str(po_file), ...) in the try/except around po_file; change the
bare except to catch specific errors (e.g., except (IOError, OSError,
UnicodeDecodeError, polib.POFileError) as e:) and log the failure including the
po_file path and the exception (use the module logger or
logger.exception/logger.error(..., exc_info=True)) before continuing; ensure the
variable names po_file and po are left intact and preserve the continue after
logging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a290741e-dbe0-4753-8b26-b3a45c1f577b
📒 Files selected for processing (2)
CONTRIBUTORSscripts/l10n/json_builder.py
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
6 issues found across 2 files
Confidence score: 2/5
- High-confidence issues in
scripts/l10n/json_builder.pysuggest real regression risk: fatal failures useexit()without a non-zero status, which can report success in CI while localization generation actually fails. - The most severe behavior risk is data integrity: broad
Exceptionhandling can silently drop locale files and produce incomplete JSON output, and normalized-ID collisions can break the uniqueness guarantee. - There are also correctness concerns in grouping/default logic (
entry.tcommentvs canonical comments, and broaden*default-file detection) that can misgroup entries or backfill translations incorrectly for non-default locales. - Pay close attention to
scripts/l10n/json_builder.py- error signaling, collision handling, and locale/grouping logic can lead to silent bad output.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/l10n/json_builder.py">
<violation number="1" location="scripts/l10n/json_builder.py:16">
P1: `exit()` without a status code exits with success (0), masking this fatal error in CI/automation</violation>
<violation number="2" location="scripts/l10n/json_builder.py:45">
P3: Sort locale/PO traversal (and normalize `countries` ordering) so the generated JSON remains deterministic across runs.</violation>
<violation number="3" location="scripts/l10n/json_builder.py:61">
P1: Avoid catching `Exception` and continuing silently here; parse/read failures currently drop locale files without any visibility and can produce incomplete JSON output.</violation>
<violation number="4" location="scripts/l10n/json_builder.py:65">
P2: Use `entry.comment` as the grouping key here; preferring `entry.tcomment` can group entries by translator notes instead of the canonical extracted comment.</violation>
<violation number="5" location="scripts/l10n/json_builder.py:71">
P1: No collision handling for normalized IDs: different comments can normalize to the same `clean_id`, violating the PR's claim of unique identifiers.</violation>
<violation number="6" location="scripts/l10n/json_builder.py:79">
P2: This default-file heuristic is too broad: treating every `en*` locale as default can backfill missing `msgstr` with `msgid` for files that are not true source-language defaults.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/l10n/sample_output.json`:
- Line 99: The Thai string "ชดเชย%s" is incorrectly used for both s_estimated
and s_in_lieu; update the s_estimated Thai value (not the s_in_lieu one) to the
correct distinct translation for "(estimated)" (e.g., use the Thai phrase for
"estimated") and regenerate or re-run the sample_output.json generation to
ensure s_estimated and s_in_lieu no longer collide; locate the entry with "th":
"ชดเชย%s" and the JSON keys s_estimated / s_in_lieu to make this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5839dbe5-f05d-47a8-b4f7-58791ed81950
📒 Files selected for processing (1)
scripts/l10n/sample_output.json
Contributor
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/l10n/json_builder.py">
<violation number="1" location="scripts/l10n/json_builder.py:45">
P3: Sort locale/PO traversal (and normalize `countries` ordering) so the generated JSON remains deterministic across runs.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/l10n/json_builder.py`:
- Around line 118-119: The current logic in scripts/l10n/json_builder.py
silently keeps the first translation for a given (comment, lang) pair and
depends on unsorted lc_messages.glob("*.po") order; change it so when building
grouped (use variables grouped, comment, lang, and entry.msgstr) you sort the
list of .po paths deterministically before iterating and, on encountering an
existing grouped[comment]["messages"][lang], compare the new translation against
the stored value and, if they differ, emit an error or warning that includes the
involved .po filenames and both strings (rather than silently skipping); ensure
the deterministic sort dictates precedence if you must choose one.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: eb8c6b11-4900-4a47-a422-3f0463ce6e9a
📒 Files selected for processing (1)
scripts/l10n/json_builder.py
KJhellico
reviewed
May 26, 2026
KJhellico
reviewed
May 26, 2026
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (4)
scripts/l10n/json_builder.py (4)
66-78:⚠️ Potential issue | 🟠 Major | ⚡ Quick winID de-duplication can still produce collisions.
_deduplicate_ids()can rename a duplicatefootofoo_1even when another entry already has base IDfoo_1, so uniqueness is still not guaranteed.Suggested fix
`@staticmethod` def _deduplicate_ids(grouped: dict) -> None: - """Ensure all IDs are unique by appending a counter to duplicates.""" - id_counts: dict = {} - for entry in grouped.values(): - id_counts[entry["id"]] = id_counts.get(entry["id"], 0) + 1 - - id_seen: dict = {} + """Ensure all IDs are unique by appending a counter to duplicates.""" + used_ids: set[str] = set() + next_suffix: dict[str, int] = {} for entry in grouped.values(): base_id = entry["id"] - if id_counts[base_id] > 1: - id_seen[base_id] = id_seen.get(base_id, 0) + 1 - entry["id"] = f"{base_id}_{id_seen[base_id]}" + candidate = base_id + if candidate in used_ids: + idx = next_suffix.get(base_id, 1) + while f"{base_id}_{idx}" in used_ids: + idx += 1 + candidate = f"{base_id}_{idx}" + next_suffix[base_id] = idx + 1 + entry["id"] = candidate + used_ids.add(candidate)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/l10n/json_builder.py` around lines 66 - 78, _deduplicate_ids currently appends a counter like "foo_1" to duplicates but doesn't check whether that new ID already exists, causing possible collisions; change the logic in _deduplicate_ids to first build a set of used_ids from all current entry["id"] values, then for each entry whose base_id is duplicated, loop incrementing a suffix (start at 1) until base_id_suffix is not in used_ids, assign entry["id"] = base_id_suffix and add it to used_ids; you can drop or keep id_counts/id_seen but ensure the uniqueness check uses the used_ids set to avoid collisions.
108-111:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t silently ignore PO parse errors.
Line 110 catches all exceptions and drops the file quietly, which can hide missing locale data in output.
Suggested fix
-from polib import pofile +import sys +from polib import POFileError, pofile @@ - except Exception: + except (OSError, UnicodeDecodeError, POFileError) as exc: + print(f"WARN: Skipping unreadable PO file {po_file_path}: {exc}", file=sys.stderr) return🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/l10n/json_builder.py` around lines 108 - 111, The try/except around pofile(str(po_file_path), encoding="utf-8") currently swallows all exceptions and returns silently; change it to catch the exception as a variable (e.g., except Exception as e) and emit a clear error (include po_file_path and the exception message/traceback) instead of silently returning so PO parse failures are visible; keep the same control flow (skip the bad file) but log the failure using the module's logger or stderr before continuing.
82-82:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
entry.commentas the canonical grouping key.Line 82 currently prefers
entry.tcomment, which can split/merge groups unexpectedly when grouping is intended by msgcomment.Suggested fix
- comment = (entry.tcomment or entry.comment or "").strip() + comment = (entry.comment or entry.tcomment or "").strip()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/l10n/json_builder.py` at line 82, Change the grouping key so msgcomment uses entry.comment as the canonical source: replace the current preference of entry.tcomment with entry.comment when computing the local variable comment (used for grouping) — i.e., compute comment from entry.comment first, falling back to entry.tcomment, then strip; update the assignment where comment is created to reference entry.comment before entry.tcomment so grouping uses msgcomment consistently.
98-99:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTranslation precedence is order-dependent and lossy.
Line 98 keeps first-write-wins for
(comment, lang). Combined with unsorted traversal at Line 123, different runs can pick different winners. Also, countries ordering should be normalized before write.Suggested fix
- if lang not in grouped[comment]["messages"]: - grouped[comment]["messages"][lang] = translation + existing = grouped[comment]["messages"].get(lang) + if existing is None: + grouped[comment]["messages"][lang] = translation + elif existing != translation: + print( + f"WARN: Conflicting translation for comment={comment!r}, lang={lang}, " + f"country={country_code}: keeping {existing!r}, skipping {translation!r}" + ) @@ - for po_file_path in locale_path.rglob("*.po"): + for po_file_path in sorted(locale_path.rglob("*.po"), key=lambda p: p.as_posix()): lang = po_file_path.parents[1].name IntermediateJsonBuilder._process_po_file( po_file_path, lang, grouped) @@ + for item in grouped.values(): + item["countries"].sort() output = sorted(grouped.values(), key=lambda x: x["comment"])Also applies to: 123-126, 129-129
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/l10n/json_builder.py` around lines 98 - 99, The code currently uses first-write-wins for grouped[comment]["messages"][lang] with an unsorted traversal, producing non-deterministic and lossy results; change the logic to be deterministic by iterating sources/countries in a stable, sorted order and then explicitly setting (overwriting) grouped[comment]["messages"][lang] = translation so precedence is controlled and repeatable. Locate code manipulating grouped, comment, lang, translation and the loop over countries (the blocks around grouped[comment]["messages"][lang] and the traversal at lines ~123-129) and: sort the list/keys used to drive that traversal (normalize country ordering) and replace the conditional "if lang not in grouped[comment]['messages']" with an unconditional assignment (or explicit precedence check) so the winner is deterministic across runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@scripts/l10n/json_builder.py`:
- Around line 66-78: _deduplicate_ids currently appends a counter like "foo_1"
to duplicates but doesn't check whether that new ID already exists, causing
possible collisions; change the logic in _deduplicate_ids to first build a set
of used_ids from all current entry["id"] values, then for each entry whose
base_id is duplicated, loop incrementing a suffix (start at 1) until
base_id_suffix is not in used_ids, assign entry["id"] = base_id_suffix and add
it to used_ids; you can drop or keep id_counts/id_seen but ensure the uniqueness
check uses the used_ids set to avoid collisions.
- Around line 108-111: The try/except around pofile(str(po_file_path),
encoding="utf-8") currently swallows all exceptions and returns silently; change
it to catch the exception as a variable (e.g., except Exception as e) and emit a
clear error (include po_file_path and the exception message/traceback) instead
of silently returning so PO parse failures are visible; keep the same control
flow (skip the bad file) but log the failure using the module's logger or stderr
before continuing.
- Line 82: Change the grouping key so msgcomment uses entry.comment as the
canonical source: replace the current preference of entry.tcomment with
entry.comment when computing the local variable comment (used for grouping) —
i.e., compute comment from entry.comment first, falling back to entry.tcomment,
then strip; update the assignment where comment is created to reference
entry.comment before entry.tcomment so grouping uses msgcomment consistently.
- Around line 98-99: The code currently uses first-write-wins for
grouped[comment]["messages"][lang] with an unsorted traversal, producing
non-deterministic and lossy results; change the logic to be deterministic by
iterating sources/countries in a stable, sorted order and then explicitly
setting (overwriting) grouped[comment]["messages"][lang] = translation so
precedence is controlled and repeatable. Locate code manipulating grouped,
comment, lang, translation and the loop over countries (the blocks around
grouped[comment]["messages"][lang] and the traversal at lines ~123-129) and:
sort the list/keys used to drive that traversal (normalize country ordering) and
replace the conditional "if lang not in grouped[comment]['messages']" with an
unconditional assignment (or explicit precedence check) so the winner is
deterministic across runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 31355ecb-e5a4-45b7-9065-37a7b5a3c78e
📒 Files selected for processing (1)
scripts/l10n/json_builder.py
Contributor
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
KJhellico
reviewed
May 27, 2026
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
KJhellico
reviewed
May 28, 2026
KJhellico
left a comment
KJhellico
left a comment
Collaborator
There was a problem hiding this comment.
Last refactoring, and you can start analyzing the collected data. ;)
|
❌ The last analysis has failed. |
Collaborator
|
I'm not sure what the best way to organize translation discussions is, so for now I will simply share my suggestions.
|
Collaborator
|
Also we need to think about the technical aspects of making these changes. In particular:
|
9 tasks
KJhellico
reviewed
Jun 10, 2026
KJhellico
reviewed
Jun 10, 2026
KJhellico
reviewed
Jun 10, 2026
cheweaaron609-png
force-pushed
the
generate-l10n-per-locale
branch
from
June 14, 2026 09:09
f86b056 to
613beb5
Compare
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Confidence score: 4/5
- In
scripts/l10n/replace_tr_strings.py, removing--previewmeans every run now edits files in-place, which raises the chance of unintended bulk changes across hundreds of localization files and makes review/rollback harder—reintroduce a dry-run/preview mode (or equivalent diff-only path) before write operations.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/l10n/replace_tr_strings.py">
<violation number="1" location="scripts/l10n/replace_tr_strings.py:125">
P3: The `--preview` flag was removed, so the script now always writes changes in-place with no safe-preview path. Users can no longer inspect what would change before 666+ files are modified.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| for start, end, new_val in sorted(spans, key=lambda c: c[0], reverse=True): | ||
| source = source[:start] + new_val + source[end:] | ||
|
|
||
| return source, len(changes) |
Contributor
There was a problem hiding this comment.
P3: The --preview flag was removed, so the script now always writes changes in-place with no safe-preview path. Users can no longer inspect what would change before 666+ files are modified.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/l10n/replace_tr_strings.py, line 125:
<comment>The `--preview` flag was removed, so the script now always writes changes in-place with no safe-preview path. Users can no longer inspect what would change before 666+ files are modified.</comment>
<file context>
@@ -84,22 +96,38 @@ def to_char_offset(lineno, byte_col):
+ source = source[:start] + new_val + source[end:]
- return source, replacements
+ return source, len(changes)
</file context>
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Not reviewed (too large): scripts/l10n/holidays_l10n.json (~8,123 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
KJhellico
reviewed
Jul 28, 2026
| for lang, val in entry["messages"].items(): | ||
| if isinstance(val, str): | ||
| candidates.setdefault(val, set()).add(msgid) | ||
| return {val: next(iter(msgids)) for val, msgids in candidates.items() if len(msgids) == 1} |
Collaborator
There was a problem hiding this comment.
It's wrong to ignore translations for which there is more than one msgid. If this string doesn't appear in .py files, that's fine, but if it's actually there, at least a warning should be displayed.
KJhellico
reviewed
Jul 28, 2026
Comment on lines
+106
to
+108
| comment_lineno = arg.lineno - 2 | ||
| if comment_lineno < 0: | ||
| continue |
Collaborator
There was a problem hiding this comment.
Check multi-line comments to multi-line messages (e.g., in Germany); they are not processed.
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Confidence score: 3/5
- In
scripts/l10n/replace_tr_strings.py, the broadstartswith("tr(")match can misclassify single-linetr("...")calls as multi-line openers when they appear near commented calls, which risks incorrect localization replacements or misplaced comments in generated output; tighten the detection to distinguish single-line vs multi-linetr()patterns (and add a regression test for the two-lines-above comment case).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/l10n/replace_tr_strings.py">
<violation number="1" location="scripts/l10n/replace_tr_strings.py:121">
P1: The `startswith("tr(")` check is too broad: it matches both multi-line `tr(` openers and single-line `tr("...")` calls. When a previous single-line `tr()` call with its own comment sits exactly two lines above the current string, the code incorrectly treats it as a multi-line opener and backs up one more line—replacing the wrong comment. This can corrupt source file l10n metadata.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| comment_line = lines[comment_end_lineno] | ||
| stripped = comment_line.lstrip() | ||
| # If line above string is tr( itself, look one more line up | ||
| if stripped.startswith("tr(") or stripped == "tr(": |
Contributor
There was a problem hiding this comment.
P1: The startswith("tr(") check is too broad: it matches both multi-line tr( openers and single-line tr("...") calls. When a previous single-line tr() call with its own comment sits exactly two lines above the current string, the code incorrectly treats it as a multi-line opener and backs up one more line—replacing the wrong comment. This can corrupt source file l10n metadata.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/l10n/replace_tr_strings.py, line 121:
<comment>The `startswith("tr(")` check is too broad: it matches both multi-line `tr(` openers and single-line `tr("...")` calls. When a previous single-line `tr()` call with its own comment sits exactly two lines above the current string, the code incorrectly treats it as a multi-line opener and backs up one more line—replacing the wrong comment. This can corrupt source file l10n metadata.</comment>
<file context>
@@ -117,6 +117,13 @@ def to_char_offset(lineno, byte_col):
comment_line = lines[comment_end_lineno]
stripped = comment_line.lstrip()
+ # If line above string is tr( itself, look one more line up
+ if stripped.startswith("tr(") or stripped == "tr(":
+ comment_end_lineno -= 1
+ if comment_end_lineno < 0:
</file context>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
This script collects and systematizes all translatable strings from the existing .po files across the entire holidays library as the first step of centralizing the localization architecture.
The script (scripts/l10n/json_builder.py) reads all 666 .po files across 157 language folders, deduplicates entries using msgcomment as the grouping key, and produces one JSON entry per unique holiday containing a unique snake_case id, the English comment as the identifier, all translations per language, and all countries that observe the holiday.
Total unique holidays collected: 1,890
Part of GSoC 2026 project: Generate l10n file on per locale basis.
Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.