Replace the unicode.data blob by a ruby constant#561
Replace the unicode.data blob by a ruby constant#561dentarg merged 1 commit intosporkmonger:mainfrom
unicode.data blob by a ruby constant#561Conversation
0211dce to
22226c2
Compare
Ref: 1eb715d It seems like this data was initially YAML, but 13 years ago various version of libyaml had issues, so Marshal was used. The problem with marshal is it's a binary format, so while this data has no reason to change, it's hard to see what's in it. I think a simple Ruby constant would work just as well. On my machine, both take as long to load, but many apps have bootsnap setup, so would probably load the ruby version faster than the marshal one.
22226c2 to
50dd71e
Compare
|
I like this change. Thanks for doing it. Hopefully people use editors with code folding if they need to interact with this file. :)
Do you mind adding a issue about it or making a PR? |
|
I'll note that I did If someone else does what I did, the naive copy-paste from
|
| UNICODE_TABLE = File.expand_path( | ||
| File.join(File.dirname(__FILE__), '../../..', 'data/unicode.data') | ||
| ) |
There was a problem hiding this comment.
I wonder if it is a breaking change that Addressable::IDNA::UNICODE_TABLE no longer exist. Looks like it only existed if you required addressable/idna/pure.
Initially I did put the constant in its own file, but then I discovered the test suite use
Will do tomorrow.
Depends how you define a breaking change. On paper it was a public constant, in practice, it had 0 usefulness, so I can't imagine its removal breaking any reasonable code. |
Ref: 1eb715d
It seems like this data was initially YAML, but 13 years ago various version of libyaml had issues, so Marshal was used.
The problem with marshal is it's a binary format, so while this data has no reason to change, it's hard to see what's in it.
I think a simple Ruby constant would work just as well. On my machine, both take as long to load, but many apps have bootsnap setup, so would probably load the ruby version faster than the marshal one.
Also as a sidenote, as far as I can tell,
COMPOSITION_TABLEis unused, getting rid of it would save a bit of memory and boot time.