Add Unicode Support

[adam-grant-hendry]

Adds unicode support by allowing configurable encodings to be specified (defaults to utf-8).

Checklist

• Add test cases to all the changes you introduce
• Run ./scripts/format and ./scripts/test locally to ensure this change passes linter check and test
• Test the changes on the local machine manually
• Update the documentation for the changes

Fixes: Issue #516

[codecov]

Codecov Report

Patch coverage: 98.02% and project coverage change: +0.02% 🎉

Comparison is base (eb39f8b) 97.31% compared to head (8ae25d0) 97.33%.
Report is 56 commits behind head on master.

❗ Current head 8ae25d0 differs from pull request most recent head ecc3365. Consider uploading reports for the commit ecc3365 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #629      +/-   ##
==========================================
+ Coverage   97.31%   97.33%   +0.02%     
==========================================
  Files          42       42              
  Lines        2045     2104      +59     
==========================================
+ Hits         1990     2048      +58     
- Misses         55       56       +1     

Flag	Coverage Δ
unittests	97.33% <98.02%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
commitizen/out.py	91.66% <50.00%> (-8.34%)	⬇️
commitizen/providers.py	97.52% <90.00%> (+0.22%)	⬆️
commitizen/commands/init.py	87.55% <95.23%> (+0.12%)	⬆️
commitizen/commands/bump.py	97.64% <96.66%> (-0.51%)	⬇️
commitizen/version_schemes.py	98.42% <98.42%> (ø)
commitizen/__version__.py	100.00% <100.00%> (ø)
commitizen/bump.py	100.00% <100.00%> (ø)
commitizen/changelog.py	99.50% <100.00%> (-0.50%)	⬇️
commitizen/changelog_parser.py	97.01% <100.00%> (+0.09%)	⬆️
commitizen/cli.py	94.20% <100.00%> (ø)
... and 17 more

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

[adam-grant-hendry]

@Lee-W @woile Please review when you get a chance. Thanks!

[Lee-W]

@adam-grant-hendry Thanks for the update! Just one minor discussion needed on my side.

@woile I'm planning on merging this next week. Let me know if you need more time to take a deeper look. Thanks!

[woile]

LGTM, the only thing missing is documentation: https://commitizen-tools.github.io/commitizen/config/

Ideally, I think the cz init should ask for the encoding if you are on windows, this would make windows users aware of potential encoding issues

[Lee-W]

It seems CI failed on windows 🤔

[adam-grant-hendry]

@Lee-W I didn't push any commits yesterday, but it is showing that I did through Unverified commits. By chance, was this you?

[Lee-W]

Hi @adam-grant-hendry , yes I tried to rebase the master branch to this one to see if the latest update fix this branch

[woile]

Is this still valid? Please rebase 🙏🏻

[adam-grant-hendry]

@woile @Lee-W Apologies for taking so long: I've been away for quite some time. My gpg signature keys are outdated, but the commits are by me.

mypy is still unhappy with line 6 of out.py (See: python/typeshed#3049). I added a #type: ignore, but Python 3.7 reads this as an unused type ignore (See: https://github.com/commitizen-tools/commitizen/actions/runs/5491623964/jobs/10008334198#step:5:251), so I'm stuck.

Only remaining item is to add documentation. I added encoding as a keyword argument to changelog.py/get_metadata so as to not break backwards-compatibility with the added scheme argument.

[adam-grant-hendry]

@woile @Lee-W Everything is good now. Could you please re-review the changes and make sure I added everything you requested?

[Lee-W]

@adam-grant-hendry Sure! I'll take a look these days.

[adam-grant-hendry]

Hi @adam-grant-hendry , sorry for the late review. Overall the changes are good, but I think we should drop commit 3430edba7a5b1b4f59e5b7aa48e03bd15ccc32a4 and 9af6c748eb1084c5239e8d8058e7eddd59abbfb5 which downgrades the version of our GitHub actions workflow. Also, I notice some open and smart_open miss this encoding feature. Is this intentional? Or just accidentally missed? Thanks!

No worries at all. Thank you for your review!

1. Yes, good find: we should definitely drop those commits then
2. That was certainly an accidental miss on my part. I’ll review, make changes, and push up the fixes

I probably can’t get to this today, but I can start tomorrow. I’ll see if I can request your re-review by Monday morning.

Thanks again!

[adam-grant-hendry]

@Lee-W I made all the changes discussed. Please re-review this PR at your earliest convenience. Thank you!

[Lee-W]

@adam-grant-hendry Sure! I'll try to take a look this weekend

[Lee-W]

Overall, I'm good with this PR. But we missed a few places missed. Wondering is it designed this way or just missed.

commitizen/commitizen/commands/init.py

Line 38 in 114501d

with open("pyproject.toml") as f:

,

commitizen/hooks/prepare-commit-msg.py

Line 63 in 114501d

with open("/dev/tty") as tty:

.

But, yep looks like they might be something not affected 🤔

I'm planning on merging this next week. @woile Please let me know if you want to take a deeper looks 🙂

[Lee-W]

@adam-grant-hendry ah, just notice a conflict. might need your help to resolve it.

[adam-grant-hendry]

...But we missed a few places missed. Wondering is it designed this way or just missed.

commitizen/commitizen/commands/init.py

Line 38 in 114501d

with open("pyproject.toml") as f:

,

commitizen/hooks/prepare-commit-msg.py

Line 63 in 114501d

with open("/dev/tty") as tty:

.
But, yep looks like they might be something not affected 🤔

I purposefully left these out as I considered them "trivial" cases. By that I mean the "pyproject.toml" case is only looking for the "[tool.poetry]" table heading, which should generally not be affected by whether the file is opened in utf-8 mode or not. As for "/dev/tty", there is no equivalent to "/dev/tty" on Windows machines, so this would only run properly from a Linux environment, in which case the encoding is already utf-8 by default.

[Lee-W]

Sounds good 👍 Just want to confirm it. I think we're good to merge it after resolving the conflict. Thanks!

[woile]

LGTM as well. Thanks!

[adam-grant-hendry]

@adam-grant-hendry ah, just notice a conflict. might need your help to resolve it.

I had to pull in the latest commits to master and rebase. It's all good to go now!

[Lee-W]

Many thanks! Just merged