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.

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

Fix test for ISO-8859-8-I encoding and small refactoring #659

Merged
merged 6 commits into from
Oct 1, 2023

Conversation

Mingun
Copy link
Collaborator

@Mingun Mingun commented Sep 29, 2023

During work on some future PRs I found a number of mistakes and code smells. The biggest one is an invalid test for ISO-8859-8-I encoding. I noticed, that CDATA section was not a CDATA, because instead of <![CDATA[ I wrote <[[CDATA[[. When I tried to realize, how this would possible, I found that this test file does not generated by test-gen utility, because encodings/index.json does not contain a character map for it. This is unnecessary, because this encoding have the same character set as ISO-8859-8, but should be processed slightly differently.

CDATA section was formed incorrectly and instead was recognized as a Start tag.

File introduced in PR tafia#465 and was made manually and not using generator, because
WHATWG does not have definition of this encoding as a separate entry in index.json.
Actually, this encoding the same as ISO-8859-8, but influences layout direction
when render text.

Wikipedia:
The WHATWG Encoding Standard used by HTML5 treats ISO-8859-8 and ISO-8859-8-I
as distinct encodings with the same mapping due to influence on the layout direction

So generator was fixed and file regenerated
@Mingun Mingun added the encoding Issues related to support of various encodings of the XML documents label Sep 29, 2023
@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2023

Codecov Report

Merging #659 (1e19a45) into master (ede4fb9) will increase coverage by 0.00%.
The diff coverage is 66.66%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@           Coverage Diff           @@
##           master     #659   +/-   ##
=======================================
  Coverage   64.53%   64.54%           
=======================================
  Files          36       36           
  Lines       17272    17273    +1     
=======================================
+ Hits        11147    11148    +1     
  Misses       6125     6125           
Flag Coverage Δ
unittests 64.54% <66.66%> (+<0.01%) ⬆️

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

Files Coverage Δ
src/errors.rs 10.43% <ø> (ø)
src/reader/mod.rs 95.04% <ø> (ø)
src/reader/state.rs 98.71% <66.66%> (+<0.01%) ⬆️

@Mingun Mingun merged commit ae8db96 into tafia:master Oct 1, 2023
6 checks passed
@Mingun Mingun deleted the tests branch October 1, 2023 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
encoding Issues related to support of various encodings of the XML documents
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants