Skip to content

Conversation

@chmp
Copy link
Contributor

@chmp chmp commented Feb 8, 2025

Change: Use utf8-encoding when reading files from stdin.

The current code uses stdout.buffer.read() + encode for writing, which implies Utf8 encoding, but uses stdin.read(), which uses an environment specific encoding. See docs.

I ran into this issue when using Mdformat with Helix on Windows. For files with German content and Utf8-encoding the current code mangled any non-ascii characters. The proposed code fixed this behavior.

@chmp
Copy link
Contributor Author

chmp commented Feb 9, 2025

@hukkin oh. I just executed the command itself - I will fix the test.

@chmp
Copy link
Contributor Author

chmp commented Feb 9, 2025

I fixed the tests using stdin. Locally, tests using symlinking are failing for me, but that's due to Windows, I guess.

@hukkin
Copy link
Owner

hukkin commented Feb 11, 2025

Thanks for this!

flake8 complains:

tests/test_cli.py:1:1: F401 'io.StringIO' imported but unused
tests/test_config_file.py:1:1: F401 'io.StringIO' imported but unused

Would you able to fix this?

Edit: All good actually. Seems I was able to commit the fix to your branch myself.

@codecov
Copy link

codecov bot commented Feb 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (f51432e) to head (449ee57).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #515   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          970       970           
  Branches       171       171           
=========================================
  Hits           970       970           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hukkin hukkin merged commit 10307b9 into hukkin:master Feb 13, 2025
26 checks passed
@chmp
Copy link
Contributor Author

chmp commented Feb 14, 2025

@hukkin Sorry for the late reply. I did not have access to my computer for a couple of days. Thanks a lot for fixing the last issues and merging!

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