Skip to content

Honor the encoding when reading lines in reverse - #463

Closed
codewithfourtix wants to merge 1 commit into
mahmoud:masterfrom
codewithfourtix:fix-reverse-lines-encoding
Closed

codewithfourtix wants to merge 1 commit into
mahmoud:masterfrom
codewithfourtix:fix-reverse-lines-encoding

Conversation

@codewithfourtix

Copy link
Copy Markdown
Contributor

reverse_iter_lines() overwrites a text stream's encoding with UTF-8 and drops an explicit encoding for binary streams. Use the supplied encoding, falling back to the stream's encoding when available.

Adds Latin-1 text-stream and binary-stream regression tests.

Copilot AI lite review requested due to automatic review settings September 5, 2026 17:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The change is small, directly addresses the stated bug, and includes targeted regression tests for both affected scenarios (text and binary streams).

Pull request overview

This PR fixes reverse_iter_lines() so it no longer forces UTF-8 for text streams and no longer discards an explicitly provided encoding for binary streams, ensuring reverse line iteration decodes using the caller-supplied encoding (or the stream’s own encoding when available).

Changes:

  • Update reverse_iter_lines() to select encoding from the explicit argument first, then fall back to file_obj.encoding when present.
  • Add regression tests covering Latin-1 decoding for both a text stream (with an encoding) and a binary stream (with an explicit encoding).
File summaries
File Description
tests/test_jsonutils.py Adds regression tests verifying encoding is honored for both text and binary streams.
boltons/jsonutils.py Fixes encoding selection logic to preserve caller/stream encodings instead of forcing UTF-8.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

mahmoud added a commit that referenced this pull request Sep 7, 2026
Fold #463, #466, #476, #478, #460, #459, and #477 with focused regressions and 26.2.0 release notes.

Verified 672 local tests and the full GitHub Actions matrix before merge.

Co-authored-by: Ali Zulfiqar <[email protected]>
Co-authored-by: Guo Jiarui <[email protected]>
Co-authored-by: Nikolaus Schuetz <[email protected]>
Co-authored-by: Raiyan Zaman <[email protected]>
@mahmoud

mahmoud commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Folded into #481 with credit (Co-authored-by). Thanks!

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.

3 participants