search

wsick / Fayde

Inspired by Silverlight; XAML engine using Javascript and rendering to the HTML5 Canvas.
MIT License
189 stars 27 forks source link

Conversion from string to multi-numeric types should accept space as a separator #113

Closed asthomas closed 9 years ago

asthomas commented 9 years ago

String values for Point, Thickness, etc. can be specified as "10,10" or "10 10" in Silverlight. Fayde accepts comma, but not space.

BSick7 commented 9 years ago

Fixed with pull request merge 2d64420710fa1d9b3d1571d464490181b14fadac.

BSick7 commented 9 years ago

Reopening after finding issue with this merge request.

BSick7 commented 9 years ago

Fixed in commit 8f19c10e9dbe225e3686e8656b40afc18ee8655a.

asthomas commented 9 years ago

I think that splitCommaList could be reduced to:

str.split(/ *[, ] */);

The regex could be made static for speed. It would also be more correct, as it would not accept "1,,,,,2"

BSick7 commented 9 years ago

In this example, the regex was 4x slower: http://jsperf.com/split-join-vs-regex-replace

I don't think it makes sense to accept "1,,,2". This could mean "1,0,0,2" or "1,2" which are different for Thickness values.

asthomas commented 9 years ago

That's not a representative test. Try this one: http://jsperf.com/string-split-join-split-vs-regex

In Chrome the regex performs much better. In Firefox, the regex is a little bit better.