-
-
Notifications
You must be signed in to change notification settings - Fork 323
fix(provider): use encoding settings in config #1857
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1857 +/- ##
=======================================
Coverage 97.98% 97.98%
=======================================
Files 60 60
Lines 2684 2686 +2
=======================================
+ Hits 2630 2632 +2
Misses 54 54 ☔ View full report in Codecov by Sentry. |
|
let me check whether the new tests are strong enough. I expect some of the new tests should fail before the base_provider change |
|
The new test indeed fails in #1858 |
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.
Pull request overview
Updates file-based version providers to respect the configured encoding when reading version files, addressing Windows decode errors for UTF-8 pyproject.toml and other non-ASCII content (Fixes #1636).
Changes:
- Add
FileProvider._get_encoding()and use it forread_text(...)inJsonProviderandTomlProvider. - Add provider tests and UTF-8 fixture data covering JSON/TOML reads with non-ASCII content.
- Add new test data files for composer.json and pyproject.toml encoding scenarios.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
commitizen/providers/base_provider.py |
Reads JSON/TOML using config.settings["encoding"] via _get_encoding() |
tests/providers/test_base_provider.py |
Adds tests validating providers can read UTF-8 files with non-ASCII content |
tests/data/encoding_test_pyproject.toml |
UTF-8 TOML fixture with non-ASCII strings to reproduce/guard the bug |
tests/data/encoding_test_composer.json |
UTF-8 JSON fixture with non-ASCII strings to reproduce/guard the bug |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@bearomorphism I've opened a new pull request, #1859, to work on those changes. Once the pull request is ready, I'll request review from you. |
e994e12 to
681e8ae
Compare
Description
Checklist
Was generative AI tooling used to co-author this PR?
Used cursor to generate tests.
Code Changes
uv run poe alllocally to ensure this change passes linter check and testsDocumentation Changes
uv run poe doclocally to ensure the documentation pages renders correctlyExpected Behavior
Steps to Test This Pull Request
Check if the new tests are good enough.
https://github.com/commitizen-tools/commitizen/actions/runs/21816593200/job/62939564716?pr=1858
Additional Context
Fixes #1636