Decode msgctxt using the catalog's charset in read_mo() - #1295
Open
agu2347 wants to merge 1 commit into
Open
Conversation
read_mo() extracted the message context (msgctxt) as raw bytes and used it directly, while msgid/msgstr a few lines later are properly decoded via catalog.charset. write_mo() already expects (and calls .encode(catalog.charset) on) a plain str context when serializing, so this asymmetry broke round-tripping a catalog through write_mo() then read_mo(): a catalog written with a str context came back from read_mo() with a bytes context instead, and made it impossible to use Catalog.get()/Catalog.__getitem__ with a plain str context on the result without first manually re-encoding it to match. Decode ctxt via catalog.charset immediately after splitting it off, matching how msg/tmsg are decoded a few lines below and matching what write_mo() already expects on the way out. Verified with a real write_mo() -> read_mo() round-trip: confirmed the context comes back as a plain str matching the original value, and that Catalog.get() finds the message using that same plain str context without needing any manual re-encoding -- the reporter's exact use case. Also confirmed messages without a context are unaffected. Added a regression test performing this exact round-trip. Confirmed the test fails with the original code (context comes back as bytes) and passes with the fix. Ran the full existing test_mofile.py suite (5 passed: 4 baseline + 1 new) and the broader messages test suite (test_catalog.py, test_pofile.py: 82 passed total), no regressions. Fixes python-babel#1147
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1147.
read_mo()extracted the message context (msgctxt) as raw bytes and used it directly, whilemsgid/msgstra few lines later are properly decoded viacatalog.charset.write_mo()already expects (and calls.encode(catalog.charset)on) a plainstrcontext when serializing, so this asymmetry broke round-tripping a catalog throughwrite_mo()thenread_mo(): a catalog written with astrcontext came back fromread_mo()with abytescontext instead, and made it impossible to useCatalog.get()/Catalog.__getitem__with a plainstrcontext on the result without first manually re-encoding it to match -- exactly as described in the issue.Fix: decode
ctxtviacatalog.charsetimmediately after splitting it off, matching howmsg/tmsgare decoded a few lines below and matching whatwrite_mo()already expects on the way out.Testing: verified with a real
write_mo()->read_mo()round-trip: confirmed the context comes back as a plainstrmatching the original value, and thatCatalog.get()finds the message using that same plainstrcontext without needing any manual re-encoding -- the reporter's exact use case. Also confirmed messages without a context are unaffected.Added a regression test performing this exact round-trip. I confirmed the test fails with the original code (context comes back as
bytes) and passes with the fix. Ran the full existingtest_mofile.pysuite (5 passed: 4 baseline + 1 new) and the broader messages test suite (test_catalog.py,test_pofile.py: 82 passed total), no regressions.