-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
gh-44827: Improve error if BOM on first line of .po file #130187
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
Conversation
I'd like to reopen a discussion with regards to whether this should be an error or not. From my point of view, even though the original msgfmt is a Unix tool, I think in general we should try to match the behaviour of msgfmt as close as we can, but it'll never be 100% identical (as a matter of fact, msgfmt.py in its current form can't event compile There are also other tools in the stdlib that handle BOMs just fine, for example the |
@vstinner and @serhiy-storchaka are you still opposed to adding this functionality? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify, I'm not against this change in particular, I think it's an improvement compared to the current state, but long-term I don't see a reason to disallow files with BOMs |
Tools/i18n/msgfmt.py is a reimplementation in Python of the gettext gettext
|
I don't necessarily agree but I don't want to push this too much so I'll yield to you 🙂 |
Accidentally closed |
Thank you for your contribution, @StanFromIreland. |
@serhiy-storchaka No problem! |
Is a test necessary? I have one ready to commit if wanted.
@tomasr8 :-)