Reuse InitCatalog's guts in UpdateCatalog #1244
Conversation
Fixes #1139 (since `_init_catalog` creates directories on the way) Co-authored-by: lando <du33169@qq.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1244 +/- ##
==========================================
- Coverage 92.07% 92.06% -0.01%
==========================================
Files 27 27
Lines 4692 4687 -5
==========================================
- Hits 4320 4315 -5
Misses 372 372
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the catalog initialization logic to fix a bug where pybabel update --init-missing would fail if the target directory didn't exist. The refactoring extracts the catalog initialization code into a shared _init_catalog function that both InitCatalog and UpdateCatalog can use, ensuring consistent behavior including automatic directory creation.
Key changes:
- Introduced a new
_init_cataloghelper function that encapsulates catalog initialization logic and directory creation - Refactored
InitCatalogandUpdateCatalogcommands to use the shared function - Added test coverage for directory creation with
--init-missingflag
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| babel/messages/frontend.py | Extracted catalog initialization into _init_catalog function; updated InitCatalog and UpdateCatalog to use shared function with directory creation |
| tests/messages/frontend/test_cli.py | Added test to verify that pybabel update --init-missing creates parent directories when they don't exist |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| catalog.revision_date = datetime.datetime.now(LOCALTZ) | ||
| catalog.fuzzy = False | ||
|
|
||
| os.makedirs(os.path.dirname(output_file), exist_ok=True) |
There was a problem hiding this comment.
When output_file is just a filename without any directory component (e.g., "messages.po"), os.path.dirname(output_file) will return an empty string. Calling os.makedirs("", exist_ok=True) could lead to unexpected behavior or errors. Consider checking if the directory path is non-empty before calling os.makedirs, or use os.path.dirname(output_file) or "." to default to the current directory.
| os.makedirs(os.path.dirname(output_file), exist_ok=True) | |
| output_dir = os.path.dirname(output_file) | |
| if output_dir: | |
| os.makedirs(output_dir, exist_ok=True) |
| return mappings | ||
|
|
||
|
|
||
| def _init_catalog(*, input_file, output_file, locale: Locale, width: int) -> None: |
There was a problem hiding this comment.
The type hint for width parameter should be int | None instead of int, since width can be None when the no_wrap option is used. According to the write_po documentation, None is an acceptable value to disable line wrapping.
| def _init_catalog(*, input_file, output_file, locale: Locale, width: int) -> None: | |
| def _init_catalog(*, input_file, output_file, locale: Locale, width: int | None) -> None: |
Fixes #1139 (since
_init_catalogcreates directories on the way)Closes #1142 (supersedes it)