Skip to content

Conversation

@yuxiaoli
Copy link
Contributor

This PR forces UTF-8 encoding when running git subprocess commands to prevent UnicodeDecodeError on systems with different default encodings (e.g., GBK on Windows).

@tomice
Copy link
Collaborator

tomice commented Dec 16, 2025

Thank you for your contribution! It looks like the unit tests fail because of the new args you are adding not matching up with the expected args in the tests:

AssertionError: expected call not found.
Expected: run(['git', 'status'], stdout=-1, stderr=-1, text=True, check=True)
Actual: run(['git', 'status'], stdout=-1, stderr=-1, text=True, check=True, encoding='utf-8', errors='replace')

https://github.com/git-quick-stats/git-py-stats/blob/main/git_py_stats/tests/test_git_operations.py

Should we be doing errors=replace here, or do we look towards PEP323 for guidance and instead use errors=surrogateescape? I am fine with either/or, but the argument for surrogateescape seems compelling: https://docs.python.org/3/library/codecs.html

@yuxiaoli
Copy link
Contributor Author

Thank you for the feedback! I have updated the PR as suggested:

  1. Switched from \errors='replace'\ to \errors='surrogateescape'\ to properly handle non-UTF-8 bytes as recommended by PEP 383 for system interfaces.
  2. Added a code comment explaining the use of \surrogateescape\ and referencing PEP 383.
  3. Updated the unit tests in \ est_git_operations.py\ to match the new \subprocess.run\ arguments, ensuring all tests pass.

Verification:

  • Ran \python -m unittest discover git_py_stats/tests\ and confirmed all 160 tests passed.

@tomice tomice merged commit 342209c into git-quick-stats:main Dec 22, 2025
1 check passed
@tomice
Copy link
Collaborator

tomice commented Dec 22, 2025

LGTM, thank you for your contribution!

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.

2 participants