Implement The Complete Bidirectional Class Mapping - #64
Conversation
Originally, I had thought that this was covered in the code generated from "UnicodeData.txt", but after experimenting with it I realized that only a subset of code points have their bidirectional mappings present in that file. This commit adds code generation using the Unicode character data files "PropertyValueAliases.txt" and "DerivedBidiClass.txt" to generate a complete mapping of the bidirectional mapping for each Unicode code point.
Add headers, scalafix, scalafmt, and other linting for bidirectional class work.
|
@armanbilge do you you have any context on this failure? I see some talk in other repos about the native linking taking a long time, but I didn't see anything about a failure like this. I can reproduce locally on my Linux machine as well. |
|
Huh, no, this is new, never seen it. Looks like a legit bug, will take a closer look. |
|
This is the assertion that is throwing, but I don't currently have enough context to know what is going on here. |
|
Ah great, thanks for tracking that down! Big help |
|
@armanbilge I've discovered if I remove this https://github.com/typelevel/idna4s/pull/64/files#diff-517079539f982d4fe3bf3cc073c9243a819419a66f614ce24cfd3bd9f4651088R40 it appears to work. |
|
@isomarcte thanks, I minimized and reported in scala-native/scala-native#3078. It appears to be related to the size of the |
|
@armanbilge huh, that is odd. Thanks for figuring that out. I've pushed a workaround. |
Move the grouping code from UnicodeDataCodeGen to `package.scala` and re-use it in `DerivedBidiClassCodeGen.scala` to minimize the generated code size.
| l.fold(acc)(value => acc + (value -> v)) match { | ||
| case acc => | ||
| r.fold(acc)(value => acc + (value -> v)) | ||
| } |
There was a problem hiding this comment.
This was a bit of a weird use of match, i think just using a normal variable would be clearer.
| l.fold(acc)(value => acc + (value -> v)) match { | |
| case acc => | |
| r.fold(acc)(value => acc + (value -> v)) | |
| } | |
| val leftR = l.fold(acc)(value => acc + (value -> v)) | |
| r.fold(leftR)(value => leftR + (value -> v)) |
There was a problem hiding this comment.
I believe I've addressed the syntax complexity with my most recent commit. It changes the return type of difference to be a Option[Ior[CodePointRange, CodePointRange]], which in turn makes this function more simple to write.
Let me know what you think.
armanbilge
left a comment
armanbilge
left a comment
There was a problem hiding this comment.
Looked through everything, but focused on generated code and tests.
…eneratedBidirectionalClassTests.scala Co-authored-by: Arman Bilge <armanbilge@gmail.com>
This commit simplifies the `resolveMissingMapping` function in the `project` directory used for code generation. It also changes the type of the method `difference` on the `CodePointRange` (also in `project`) from `(Option[CodePointRange], Option[CodePointRange])` to `Option[Ior[CodePointRange, CodePointRange]]`. The two are isomorphic, but the new type is easier to work with. Also, due to a changes on `main` this commit updates a bunch of header files.
|
@zarthross let me know if you have any more feedback. I'll probably merge later today, unless I hear otherwise. |
…omplete-bidi-mapping
Use non-deprecated `Init.apply` overload. This was already updated on `main`, but this is a net new invocation for this branch.
3 participants
Originally, I had thought that this was covered in the code generated from "UnicodeData.txt", but after experimenting with it I realized that only a subset of code points have their bidirectional mappings present in that file.
This commit adds code generation using the Unicode character data files "PropertyValueAliases.txt" and "DerivedBidiClass.txt" to generate a complete mapping of the bidirectional mapping for each Unicode code point.