Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upMake DOMString a newtype around String, rather than a typedef. #8312
Conversation
highfive
commented
Nov 3, 2015
|
Yay, this will make experimenting with alternative DOMString representations a lot easier! A couple of comments... It would be nice to have a function for converting DOMString to String: x.flatten() or x.to_utf8() or something. At the moment there's lots of x.0 in the code, which would all need rewritten if DOMString changed internals. It might be nice to use a macro domstring!("foo") or something, so that we might be able to do compile-time magic to optimize constant DOMStrings. I can imagine the macro using a different representation for short strings or strings which are in the list of blessed atoms. |
|
Agreed, but I'd prefer to push that to a followup. |
|
Reviewed 24 of 24 files at r1, 79 of 79 files at r2, 1 of 1 files at r3. components/script/dom/bindings/trace.rs, line 287 [r2] (raw file): components/util/mem.rs, line 104 [r2] (raw file): components/util/str.rs, line 31 [r2] (raw file): Comments from the review on Reviewable.io |
|
Review status: all files reviewed at latest revision, 3 unresolved discussions. components/script/dom/bindings/trace.rs, line 287 [r2] (raw file): components/util/mem.rs, line 104 [r2] (raw file): Comments from the review on Reviewable.io |
|
@bors-servo r=jdm |
|
|
|
@bors-servo p=9 |
Make DOMString a newtype around String, rather than a typedef. <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8312) <!-- Reviewable:end -->
|
|
|
|
|
nooooo |
|
This should make it somewhat easier to experiment with alternative representations in the future. To reduce churn, this commit leaves the String field public, though. Also, this will allow us to use the default String type to represent the IDL USVString type, which explicitly forbids unpaired surrogates, ans as such is a better match to the Rust String type.
|
@bors-servo r=jdm |
|
|
Make DOMString a newtype around String, rather than a typedef. <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8312) <!-- Reviewable:end -->
|
|
Ms2ger commentedNov 3, 2015