Skip to content

Add a failing test about titles using non-Latin scripts #121

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

Merged
merged 1 commit into from
Oct 15, 2021

Conversation

javiereguiluz
Copy link
Collaborator

Title says it all. The problem is in this line:

https://github.com/doctrine/rst-parser/blob/35c61ece3a209a742f9382a4b4f5680365979750/lib/Environment.php#L492

iconv(): Detected an illegal character in input string

vendor/doctrine/rst-parser/lib/Environment.php:492
vendor/doctrine/rst-parser/lib/Nodes/TitleNode.php:32
vendor/doctrine/rst-parser/lib/NodeFactory/NodeInstantiator.php:71
vendor/doctrine/rst-parser/lib/NodeFactory/DefaultNodeFactory.php:272
vendor/doctrine/rst-parser/lib/NodeFactory/DefaultNodeFactory.php:83
vendor/doctrine/rst-parser/lib/Parser/DocumentParser.php:481
vendor/doctrine/rst-parser/lib/Parser/DocumentParser.php:395
vendor/doctrine/rst-parser/lib/Parser/DocumentParser.php:212
vendor/doctrine/rst-parser/lib/Parser/DocumentParser.php:149
vendor/doctrine/rst-parser/lib/Parser.php:178
vendor/doctrine/rst-parser/lib/Parser.php:162
vendor/doctrine/rst-parser/lib/Parser.php:200
tests/IntegrationTest.php:99

Should I create a PR in Doctrine RST parser to add Symfony String as a dependency and slugify strings with it?

Lorem ipsum dolor sit amet, consectetur adipisicing elit.

作業環境を確認する
-----------------
Copy link
Contributor

Choose a reason for hiding this comment

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

is the underline to long, or is it just because of the non latin chars?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it's not the same. The title is 9-char long, so I updated the --- to be 9-char long too. It looks very ugly now ... but in theory is fully correct now. Thanks!

@javiereguiluz javiereguiluz merged commit 5363271 into symfony-tools:main Oct 15, 2021
javiereguiluz added a commit that referenced this pull request Feb 24, 2023
This PR was merged into the main branch.

Discussion
----------

Readd an integration test

This readds the test that I disabled in #121.

Commits
-------

ffb4240 Readd an integration test
@javiereguiluz javiereguiluz deleted the non_latin_script branch April 14, 2023 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants