Add Unicode Support #629

adam-grant-hendry · 2022-12-03T05:09:32Z

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 · 2022-12-03T05:27:39Z

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 · 2022-12-03T05:59:14Z

@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!

.github/workflows/pythonpackage.yml

#629 (comment)

commitizen/config/json_config.py

commitizen/defaults.py

woile · 2022-12-06T07:54:47Z

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

#629 (comment)

Lee-W · 2022-12-09T15:24:33Z

It seems CI failed on windows 🤔

#629 (comment)

adam-grant-hendry · 2022-12-31T00:12:48Z

@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 · 2022-12-31T03:28:00Z

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 · 2023-04-28T13:19:17Z

Is this still valid? Please rebase 🙏🏻

adam-grant-hendry · 2023-07-08T00:38:02Z

@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 · 2023-07-10T18:31:12Z

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

Lee-W · 2023-07-11T00:22:22Z

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

adam-grant-hendry · 2023-07-15T16:57:29Z

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!

Yes, good find: we should definitely drop those commits then
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 · 2023-07-25T20:58:23Z

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

Lee-W · 2023-07-26T00:22:09Z

@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 🙂

commitizen/bump.py

Lee-W · 2023-07-29T14:46:36Z

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

adam-grant-hendry · 2023-07-29T23:42:16Z

...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 · 2023-07-30T04:34:07Z

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!

This will allow commiting, e.g., emoji's and parsing commit messages for unicode characters when creating change logs.

Also, use `utf-8` by default on Windows in `out.py`.

See python/typeshed#3049

Map `"encoding"` and `"name"` to `encoding` and `name` variables, respectively (removes hard-coding of values).

Add `encoding` parameter to `open` and `smart_open` method calls where missed. Use `defaults.encoding` as default.

adam-grant-hendry · 2023-07-31T23:36:52Z

@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 · 2023-08-01T01:50:01Z

Many thanks! Just merged

adam-grant-hendry mentioned this pull request Dec 3, 2022

Add Unicode Support #615

Closed

4 tasks

adam-grant-hendry force-pushed the feat/unicode branch from 2133c83 to 7234936 Compare December 3, 2022 05:21

Lee-W approved these changes Dec 3, 2022

View reviewed changes

.github/workflows/pythonpackage.yml Outdated Show resolved Hide resolved

Lee-W requested a review from woile December 3, 2022 09:03

Lee-W added a commit that referenced this pull request Dec 3, 2022

ci(github-actions): change ubuntu-latest to ubuntu-20.04

619e2d2

#629 (comment)

Lee-W added the wait-for-merge label Dec 3, 2022

woile reviewed Dec 6, 2022

View reviewed changes

commitizen/config/json_config.py Outdated Show resolved Hide resolved

woile reviewed Dec 6, 2022

View reviewed changes

commitizen/defaults.py Show resolved Hide resolved

woile reviewed Dec 6, 2022

View reviewed changes

commitizen/defaults.py Outdated Show resolved Hide resolved

Lee-W added issue-status: wait-for-response and removed wait-for-merge labels Dec 6, 2022

Lee-W added a commit that referenced this pull request Dec 9, 2022

ci(github-actions): change ubuntu-latest to ubuntu-20.04

fc75449

#629 (comment)

Lee-W added a commit that referenced this pull request Dec 12, 2022

ci(github-actions): change ubuntu-latest to ubuntu-20.04

a4b8a86

#629 (comment)

Lee-W force-pushed the feat/unicode branch from 8c30aee to 5136163 Compare December 29, 2022 10:56

Lee-W added the os: Windows label Jan 5, 2023

Lee-W added the pr-status: wait-for-modification label Jan 23, 2023

adam-grant-hendry force-pushed the feat/unicode branch from 5136163 to f028055 Compare July 8, 2023 00:16

github-actions bot added the issue-status: needs-triage label Jul 8, 2023

adam-grant-hendry requested a review from woile July 10, 2023 18:30

Lee-W removed the os: Windows label Jul 11, 2023

adam-grant-hendry force-pushed the feat/unicode branch from e08d0d5 to 3383821 Compare July 25, 2023 18:59

github-actions bot added the issue-status: needs-triage label Jul 25, 2023

adam-grant-hendry requested a review from Lee-W July 25, 2023 20:57

Lee-W approved these changes Jul 29, 2023

View reviewed changes

commitizen/bump.py Outdated Show resolved Hide resolved

Lee-W removed the issue-status: needs-triage label Jul 29, 2023

woile approved these changes Jul 30, 2023

View reviewed changes

Lee-W added the pr-status: ready-to-merge almost ready to merge. just keep it for a few days for others to check label Jul 30, 2023

github-actions bot added the issue-status: needs-triage label Jul 31, 2023

adam-grant-hendry force-pushed the feat/unicode branch from 8ae25d0 to 69c90ab Compare July 31, 2023 22:31

adam-grant-hendry added 10 commits July 31, 2023 15:57

feat(unicode): add unicode support

e70d15d

This will allow commiting, e.g., emoji's and parsing commit messages for unicode characters when creating change logs.

test(unicode): add tests

d9801ae

fix: use base config for encoding

c480d89

Also, use `utf-8` by default on Windows in `out.py`.

fix: correct type hinting

be49729

feat(changelog.py): add encoding to get_metadata

3aef21d

fix(out.py): TextIOWrapper.reconfigure typing

e4611e2

See python/typeshed#3049

refactor(defaults.py): use variables in DEFAULT_SETTINGS

a588dfd

Map `"encoding"` and `"name"` to `encoding` and `name` variables, respectively (removes hard-coding of values).

docs: add encoding configuration

10c2b79

fix: add missing encoding parameter

8563db0

Add `encoding` parameter to `open` and `smart_open` method calls where missed. Use `defaults.encoding` as default.

style(bump.py): add type hinting to update_version_in_files

ecc3365

adam-grant-hendry force-pushed the feat/unicode branch from 093eb23 to ecc3365 Compare July 31, 2023 23:22

Lee-W merged commit df7acce into commitizen-tools:master Aug 1, 2023

adam-grant-hendry deleted the feat/unicode branch August 1, 2023 15:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Unicode Support #629

Add Unicode Support #629

adam-grant-hendry commented Dec 3, 2022 •

edited

Loading

codecov bot commented Dec 3, 2022 •

edited

Loading

adam-grant-hendry commented Dec 3, 2022

Lee-W left a comment

woile commented Dec 6, 2022

Lee-W commented Dec 9, 2022

adam-grant-hendry commented Dec 31, 2022

Lee-W commented Dec 31, 2022

woile commented Apr 28, 2023

adam-grant-hendry commented Jul 8, 2023

adam-grant-hendry commented Jul 10, 2023

Lee-W commented Jul 11, 2023

adam-grant-hendry commented Jul 15, 2023

adam-grant-hendry commented Jul 25, 2023

Lee-W commented Jul 26, 2023

Lee-W left a comment

Lee-W commented Jul 29, 2023

adam-grant-hendry commented Jul 29, 2023

Lee-W commented Jul 30, 2023

woile left a comment

adam-grant-hendry commented Jul 31, 2023

Lee-W commented Aug 1, 2023

Add Unicode Support #629

Add Unicode Support #629

Conversation

adam-grant-hendry commented Dec 3, 2022 • edited Loading

Checklist

codecov bot commented Dec 3, 2022 • edited Loading

Codecov Report

adam-grant-hendry commented Dec 3, 2022

Lee-W left a comment

Choose a reason for hiding this comment

woile commented Dec 6, 2022

Lee-W commented Dec 9, 2022

adam-grant-hendry commented Dec 31, 2022

Lee-W commented Dec 31, 2022

woile commented Apr 28, 2023

adam-grant-hendry commented Jul 8, 2023

adam-grant-hendry commented Jul 10, 2023

Lee-W commented Jul 11, 2023

adam-grant-hendry commented Jul 15, 2023

adam-grant-hendry commented Jul 25, 2023

Lee-W commented Jul 26, 2023

Lee-W left a comment

Choose a reason for hiding this comment

Lee-W commented Jul 29, 2023

adam-grant-hendry commented Jul 29, 2023

Lee-W commented Jul 30, 2023

woile left a comment

Choose a reason for hiding this comment

adam-grant-hendry commented Jul 31, 2023

Lee-W commented Aug 1, 2023

adam-grant-hendry commented Dec 3, 2022 •

edited

Loading

codecov bot commented Dec 3, 2022 •

edited

Loading