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

Fix parsing pofile with obsoletes #452

Merged
merged 3 commits into from
Nov 21, 2016

Conversation

mbirtwell
Copy link
Contributor

Fixes the handling of the unit before the obsolete unit. Previously it would mark the unit before an obsolete unit as obsolete also.

Fixes multi-line msgctxt statements. Previously it would just take the first line.

Some refactoring:

  • the transition between finishing one unit and starting the nextis clearer
  • separate the processing of keywords and continuation lines
  • combine the reset and initialisation code
  • Make the handling of strings consistent.
  • Add some nascent error handling, removed some errors in test inputs

@codecov-io
Copy link

codecov-io commented Oct 13, 2016

Current coverage is 90.14% (diff: 97.43%)

Merging #452 into master will increase coverage by 0.02%

@@             master       #452   diff @@
==========================================
  Files            24         24          
  Lines          3960       3979    +19   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           3569       3587    +18   
- Misses          391        392     +1   
  Partials          0          0          

Powered by Codecov. Last update 15365e2...7dfe6f9

@@ -79,7 +79,7 @@ def test_read_multiline(self):
message.id)

def test_fuzzy_header(self):
buf = StringIO(r'''\
buf = StringIO(r'''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity -- is the parser still able to read files that start with a newline, i.e. is this just cleanup? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the combination of r and triple quotes means that you can't escape new lines using "\". The original actually tested the string:

\
# Translations template for AReallyReallyLongNameForAProject.
...

My version:


# Translations template for AReallyReallyLongNameForAProject.
...

So in fact the test is now testing handling files which start with a newline. It looks like the original author intended there to be no blank line at the start of the file, but introduced an invalid character instead. The parser now prints a warning for lines it doesn't understand, so I removed the character to prevent that warning getting printed during unit tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough! Thank you for the explanation.

@akx
Copy link
Member

akx commented Nov 21, 2016

Could you fold the Python 2.6 changes into the relevant earlier commits?

Otherwise this looks nice :)

@akx akx self-assigned this Nov 21, 2016
Michael Birtwell added 3 commits November 21, 2016 14:32
Fixes the handling of the unit before the obsolete unit.
Previously it would mark the unit before an obsolete unit as obsolete
also.

Some refactoring:
* the transition between finishing one unit and starting the next
is clearer
* separate the processing of keywords and continuation lines
* combine the reset and initialisation code
* Make the handling of strings consistent.
* Add some nascent error handling, removed some errors in test inputs
@mbirtwell mbirtwell force-pushed the fix_parsing_pofile_with_obsoletes branch from bfbc48d to 7dfe6f9 Compare November 21, 2016 14:33
@akx akx merged commit ab56a43 into python-babel:master Nov 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants