Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Treat all YAML templates files as UTF-8 encoded #449

Merged
merged 1 commit into from
Jan 25, 2023
Merged

Treat all YAML templates files as UTF-8 encoded #449

merged 1 commit into from
Jan 25, 2023

Conversation

rmilecki
Copy link
Collaborator 

Treat all YAML templates files as UTF-8 encoded

Encoding detection based on chardet is unreliable. For UTF-8 encoded
files with few non-ASCII chars it often fails to detect UTF-8. That
results in invoices not being parsed correctly.

Read every YAML file as UTF-8 encoded. It is used by all project
internal templates. It should be used by all external ones too.

FWIW YAML documentation also says that every *processor* must support
the UTF-8, UTF-16 and UTF-32:
https://yaml.org/spec/1.2.2/#52-character-encodings

Treat all YAML templates files as UTF-8 encoded
5a69a8d 
Encoding detection based on chardet is unreliable. For UTF-8 encoded
files with few non-ASCII chars it often fails to detect UTF-8. That
results in invoices not being parsed correctly.

Read every YAML file as UTF-8 encoded. It is used by all project
internal templates. It should be used by all external ones too.

FWIW YAML documentation also says that every *processor* must support
the UTF-8, UTF-16 and UTF-32:
https://yaml.org/spec/1.2.2/#52-character-encodings

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
@rmilecki
Copy link
Collaborator Author

This fixes:

It's an alternative solution to the #446 . I'm not sure we should / have to support any other encoding.

bosd
bosd approved these changes Jan 22, 2023
View reviewed changes
Copy link
Collaborator

@bosd bosd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review, LGTM

@rmilecki rmilecki merged commit 6ab3104 into invoice-x:master Jan 25, 2023
@rmilecki rmilecki deleted the yaml-utf-8 branch January 25, 2023 10:08
This was referenced Jan 25, 2023
Closed
Closed
Closed
@tjni tjni mentioned this pull request Aug 12, 2023
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bosd bosd bosd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@rmilecki @bosd