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

Revert "Formatter: Escape non-printable characters in literals" #11603

Merged

Conversation

straight-shoota
Copy link
Member

Reverts #11520

This change broke the format of grapheme_spec which was merged in #11472 in the mean time: https://github.com/crystal-lang/crystal/runs/4554342641?check_suite_focus=true

grapheme_spec has string literals with graphemes containing non-printable characters. The formatter produces this diff (I isolated and reorderd the lines for better readability):

-    it_iterates_graphemes "ܐ܏ܒܓܕ ", ['\u0710', "\u070F\u0712", '\u0713', '\u0715']
+    it_iterates_graphemes "ܐ\u070Fܒܓܕ", ['\u0710', "\u070F\u0712", '\u0713', '\u0715']

-    it_iterates_graphemes "*👩‍❤️‍💋‍👩*", ['*', "\u{1F469}\u200D\u2764\uFE0F\u200D\u{1F48B}\u200D\u{1F469}", '*']
+    it_iterates_graphemes "*👩\u200D❤️\u200D💋\u200D👩*", ['*', "\u{1F469}\u200D\u2764\uFE0F\u200D\u{1F48B}\u200D\u{1F469}", '*']

-    it_iterates_graphemes "👩‍❤️‍💋‍👩", ["\u{1F469}\u200D\u2764\uFE0F\u200D\u{1F48B}\u200D\u{1F469}"]
+    it_iterates_graphemes "👩\u200D❤️\u200D💋\u200D👩", ["\u{1F469}\u200D\u2764\uFE0F\u200D\u{1F48B}\u200D\u{1F469}"]

-    it_iterates_graphemes "🏋🏽‍♀️", ["\u{1F3CB}\u{1F3FD}\u200D\u2640\uFE0F"]
+    it_iterates_graphemes "🏋🏽\u200D♀️", ["\u{1F3CB}\u{1F3FD}\u200D\u2640\uFE0F"]

-    it_iterates_graphemes "🏳️‍🌈", ["\u{1F3F3}\uFE0F\u200D\u{1F308}"]
+    it_iterates_graphemes "🏳️\u200D🌈", ["\u{1F3F3}\uFE0F\u200D\u{1F308}"]

I think that's a legitimate use case for non-printable characters in string literals. Thus we should revert #11520.
We can reconsider whether we want to improve this feature and add it again or drop it.

@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:tools:formatter kind:regression Something that used to correctly work but no longer works labels Dec 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. kind:regression Something that used to correctly work but no longer works topic:tools:formatter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants