Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a new CLI command to lint PO files #1168

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tomasr8
Copy link
Member

@tomasr8 tomasr8 commented Jan 11, 2025

Partially addresses #1011

This adds a new lint command to babel:

$ pybabel lint --help                                
Usage: pybabel lint [options] <input-paths>

check message catalogs for common problems

Options:
  -h, --help  show this help message and exit

Example output:

$ pybabel lint messages.po
messages.po:20: placeholders in msgid differ from placeholders in msgstr:
	placeholders in msgid but missing in msgstr: {foo}
	placeholders in msgstr but missing in msgid: {bar}

The implementation is based on my team's implementation in Indico.

Some notes:

  • Currently only f-string placeholders are checked. I didn't implement the old %-style formatting to keep the diff smaller, but I can add it later.
  • There are currently no CLI options besides the input-paths. We might want to consider adding a parameter to ignore a specific file/msgid but that can again be added later.

Feedback welcome!

(@akx would you mind taking a look?)

Copy link

codecov bot commented Jan 11, 2025

Codecov Report

Attention: Patch coverage is 93.24324% with 5 lines in your changes missing coverage. Please review.

Project coverage is 91.34%. Comparing base (9d25e67) to head (3248f82).
Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
babel/messages/frontend.py 93.24% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1168      +/-   ##
==========================================
- Coverage   91.47%   91.34%   -0.14%     
==========================================
  Files          27       27              
  Lines        4636     4728      +92     
==========================================
+ Hits         4241     4319      +78     
- Misses        395      409      +14     
Flag Coverage Δ
macos-14-3.10 90.37% <93.24%> (+0.12%) ⬆️
macos-14-3.11 90.31% <93.24%> (+0.12%) ⬆️
macos-14-3.12 90.52% <93.24%> (+0.12%) ⬆️
macos-14-3.13 90.52% <93.24%> (+0.12%) ⬆️
macos-14-3.8 90.24% <93.24%> (+0.06%) ⬆️
macos-14-3.9 90.30% <93.24%> (+0.12%) ⬆️
macos-14-pypy3.10 90.37% <93.24%> (+0.12%) ⬆️
ubuntu-24.04-3.10 90.39% <93.24%> (+0.12%) ⬆️
ubuntu-24.04-3.11 90.33% <93.24%> (+0.12%) ⬆️
ubuntu-24.04-3.12 90.54% <93.24%> (+0.12%) ⬆️
ubuntu-24.04-3.13 90.54% <93.24%> (+0.12%) ⬆️
ubuntu-24.04-3.8 90.26% <93.24%> (+0.06%) ⬆️
ubuntu-24.04-3.9 90.32% <93.24%> (+0.12%) ⬆️
ubuntu-24.04-pypy3.10 90.39% <93.24%> (+0.12%) ⬆️
windows-2022-3.10 90.40% <93.24%> (+0.01%) ⬆️
windows-2022-3.11 90.34% <93.24%> (+0.01%) ⬆️
windows-2022-3.12 90.55% <93.24%> (+0.01%) ⬆️
windows-2022-3.13 90.55% <93.24%> (+0.01%) ⬆️
windows-2022-3.8 90.38% <93.24%> (+0.06%) ⬆️
windows-2022-3.9 90.33% <93.24%> (+0.01%) ⬆️
windows-2022-pypy3.10 90.40% <93.24%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jun66j5
Copy link
Contributor

jun66j5 commented Jan 12, 2025

Why the new checker is not added to babel/messages/checkers.py? It could check using Message.check() method if added. Also, checker for %-styling formatting is already implemented.

(Side note: Trac provides check_catalog command via setup.py to check catalogs using the method.)

@tomasr8
Copy link
Member Author

tomasr8 commented Jan 12, 2025

Why the new checker is not added to babel/messages/checkers.py?

Yeah, that might be a good idea. I'll look into that :)

@tomasr8 tomasr8 marked this pull request as draft January 12, 2025 18:46
@tomasr8
Copy link
Member Author

tomasr8 commented Jan 12, 2025

Perhaps it might make more sense to start with implementing the python-brace-format flag

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants