v0.19: port UTF-16, UTF-32 codecs #1482
Labels
No labels
UX
active development
backlog
blocker
bootstrap
bounty
bug
dependencies
discussion
documentation
duplicate
enhancement
flaky test
help wanted
invalid
javascript
question
release
tendentious
wontfix
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
mighty-gerbils/gerbil!1482
Loading…
Reference in a new issue
No description provided.
Delete branch "jay/gerbil:v0.19-encoding-utf"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Ports the remaining UTF-16 and UTF-32 codecs to v0.19 under
:std/encoding. Also fixes UTF-32 native-endian encoding and UTF-16 malformed-surrogate recovery.Thank you!
Can you ask the clanker to put type signatures in the internal procedures as well? It results in signficantly faster code, as a lot of runtime checks can be eliminated.
@ -0,0 +16,4 @@(def replacement-char(integer->char #xfffd))(def replacement-string(string replacement-char))While you're here, can you add tests involving surrogate pairs correctly or incorrectly used?
A cursory look at the code suggests they are supported to some degree. Notably note that Gambit will rightfully refuse to create characters with codepoints that are surrogates.
For bonus brownies (not a blocker to approval), you might want to refactor code so that std/encoding/utf16, std/encoding/json/reader and std/text/parser/char-set should share common infrastructure for handling surrogates.
If you don't want to do it or have your clanker to it—please at least add TODO to that effect.
thanks for the feedback, I addressed it in my latest two commits
also, i think it makes sense to move the modules to
std/encoding. Can you ask the clanker to move them?this looks good to me, modulo moving to std/encoding.
For the surrogate character thingie @fare pointed out, can you ask the clanker to add a test? I am fine merging as is nonetheless.
@vyzo wrote in #1482 (comment):
they already are in std/encoding unless I'm misunderstanding
@jay wrote in #1482 (comment):
ah sorry my bad, they used to be in std/text.
looks good, but it seems the clanker introduced a bit of slop.
left a couple of comments on how to unslopify.
@ -231,1 +225,3 @@(raise-invalid-json-token read-escape-char reader char)))))(def (read-json-string (reader : BufferedReader)) => :string(def (read-escape-char) => :char(using (escapedwe don't need this
usingi think, the compiler should see it's a:charand not complain.if we do, you can just cast it with
(:- ... :char)@vyzo pushed a desloppification commit
@ -249,2 +242,2 @@(raise-invalid-json-token read-escape-unicode reader lo))(surrogates->char hi lo))))))(def (read-escape-unicode) => :char(using (charsame here
@ -267,0 +274,4 @@(let* ((char (reader.read-char-utf8))(digit (unhex* char)))(if digit(using (digit digit :- :fixnum)you can just cast with
(:- digit :fixnun)looks good, thank you!