pofile.py: Added new exception called PoFileError and thrown if flagged #532
Conversation
Bedrock02
force-pushed
the
invalid-po-issue-531
branch
3 times, most recently
from
7f54e8e
to
4ac2700
Compare
Codecov Report
@@ Coverage Diff @@
## master #532 +/- ##
==========================================
- Coverage 90.15% 88.86% -1.29%
==========================================
Files 24 24
Lines 4033 4033
==========================================
- Hits 3636 3584 -52
- Misses 397 449 +52
Continue to review full report at Codecov.
|
babel/messages/pofile.py
Outdated
| @@ -18,6 +18,8 @@ | |||
| from babel.util import wraptext | |||
| from babel._compat import text_type | |||
|
|
|||
| __all__ = ['PoFileError'] | |||
There was a problem hiding this comment.
Remove this, please. We haven't had an __all__ stanza in this file, so adding one that just exports a single name will break from babel.messages.pofile import * imports in client code.
| @@ -73,6 +75,10 @@ def denormalize(string): | |||
| return unescape(string) | |||
|
|
|||
|
|
|||
| class PoFileError(Exception): | |||
| """Exception thrown by PoParser when an invalid po file is encountered.""" | |||
There was a problem hiding this comment.
It would be nice if this was a more full-fledged exception, to make it easier to use programmatically by client code:
class PoFileError(Exception):
def __init__(self, message, catalog, line, lineno):
super(PoFileError, self).__init__('{message} on {lineno}'.format(message=message, lineno=lineno))
self.catalog = catalog
self.line = line
self.lineno = linenoand naturally the raise below would be
raise PoFileError(msg, self.catalog, line, lineno)This way client code can introspect the error without having to try to parse the message.
What do you think?
There was a problem hiding this comment.
Sounds like a good idea, will implement soon
babel/messages/pofile.py
Outdated
| @@ -276,11 +283,13 @@ def parse(self, fileobj): | |||
| self._add_message() | |||
|
|
|||
| def _invalid_pofile(self, line, lineno, msg): | |||
| if self.abort_invalid: | |||
| raise PoFileError("Invalid POFile encountered line_number={}, msg={}".format(lineno, msg)) | |||
There was a problem hiding this comment.
See the discussion above about a better exception :)
tests/messages/test_pofile.py
Outdated
| msgid "" | ||
| "Thank you very much for your time.\n" | ||
| "If you have any questions regarding this survey, please contact Tim " | ||
| "at andrea.raynorhentschel@dfo.ca." |
There was a problem hiding this comment.
Please make absolutely sure there aren't any emails or names belonging to real people in the test data. (Consider this duplicated for all the names and emails here.)
|
@akx thanks for the quick feedback. I applied the suggested changes in a separate commit. After changes come to a final state I could squash to one commit. |
Bedrock02
force-pushed
the
invalid-po-issue-531
branch
from
2e3668a
to
7520b0e
Compare
|
@Bedrock02 This looks good now :) Mind rebasing on top of current master and squashing the two commits while you're at it? |
Bedrock02
force-pushed
the
invalid-po-issue-531
branch
2 times, most recently
from
fe4a31c
to
0abde9f
Compare
|
@akx branch has been rebased on top of the current master and I squashed the two commits. Let me know if there is anything else that I should do. |
|
@Bedrock02 Can you rebase once more on top of the current master so we get rid of the CI failure (as since #546 was just merged we don't support Python 2.6 anymore)? Thank you! |
This new exception is thrown when the po parser finds an invalid pofile. This helps handle invalid po files that are parsed. Invalid po files may cause other possible errors such as a UnicodeEncodeError. Closes python-babel#531
Bedrock02
force-pushed
the
invalid-po-issue-531
branch
from
0abde9f
to
9ba950c
Compare
|
@akx 👍 |
|
Thank you :) |
This new exception is thrown when the po parser finds an invalid pofile.
This helps handle invalid po files that are parsed. Invalid po files may cause
other possible errors such as a UnicodeEncodeError.
Closes #531