Unify most of our utf8 implementations. This takes the implementation

Unify most of our utf8 implementations. This takes the implementation
from utils/string_encoding and moves it to utils/utf . utils/utf8
goes away.
Committed: https://code.google.com/p/dart/source/detail?r=5169

[dgrove, 2012-02-24 04:38:24]

Peter - for now, please take a look at the frog pieces. I'm still trying to make
the lib/unicode pieces fit nicely together while also meeting the VM's current
requirement that all of the library be concatenated together (which means no
sub-libraries, for instance).

Once I've had a bit more of a chance to massage the lib directory, I'll send
this out to a few more reviewers.

[dgrove, 2012-02-24 19:50:09]

I needed to restructure the library a bit in order for the VM to be able to
embed it.

[ahe, 2012-02-27 06:52:52]

I don't see why the VM has a requirement that all libraries must be
concatenated.

Please file a high priority request on the VM and let's get back to this when it
has been fixed.

Also, I would really prefer if we had separate libraries for UTF-8, UTF-16, and
UTF-32. I think the library is misnamed, it doesn't provide "unicode" it merely
provides the encodings UTF-8, UTF-16, and UTF-32. Most importantly, the library
lacks these features:

* normalization

* conversions between upper case and lower case

* sorting

* character properties (isLetter, isDigit, etc)

[Ivan Posva, 2012-02-27 23:39:22]

https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_core.dart
File lib/utf/utf_core.dart (right):

https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_core.da...
lib/utf/utf_core.dart:8: List<int> stringToCodepoints(String str) {
Why are these functions exported by the dart:utf library?

String.charCodes() and String.fromCharCodes() are supposed to handle this
correctly. If there is some special handling needed in frog, then this should be
done in the frog library implementation of String?

https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_core.da...
lib/utf/utf_core.dart:270: 
Extra line?

https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_vm.dart
File lib/utf/utf_vm.dart (right):

https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_vm.dart...
lib/utf/utf_vm.dart:5: // TODO(dgrove) - once the VM has a new way to include
whole libraries,
ditto

[Ivan Posva, 2012-02-27 23:43:32]

For the record: All of the comments can be addressed in a different CL.

The changes to hook it into the VM LGTM. I have not reviewed the actual
implementation of the UTF handling code, I trust there are people on the
reviewers list better suited.

-Ivan


On 2012/02/27 23:39:22, Ivan Posva wrote:
>
https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_core.dart
> File lib/utf/utf_core.dart (right):
> 
>
https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_core.da...
> lib/utf/utf_core.dart:8: List<int> stringToCodepoints(String str) {
> Why are these functions exported by the dart:utf library?
> 
> String.charCodes() and String.fromCharCodes() are supposed to handle this
> correctly. If there is some special handling needed in frog, then this should
be
> done in the frog library implementation of String?
> 
>
https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_core.da...
> lib/utf/utf_core.dart:270: 
> Extra line?
> 
> https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_vm.dart
> File lib/utf/utf_vm.dart (right):
> 
>
https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_vm.dart...
> lib/utf/utf_vm.dart:5: // TODO(dgrove) - once the VM has a new way to include
> whole libraries,
> ditto

[ahe, 2012-02-29 13:23:02]

Frog changes look good, but please restore the explicit call to new
String.fromCharCodes.

LGTM

https://chromiumcodereview.appspot.com/9462001/diff/17001/frog/file_system_vm...
File frog/file_system_vm.dart (right):

https://chromiumcodereview.appspot.com/9462001/diff/17001/frog/file_system_vm...
frog/file_system_vm.dart:25: return decodeUtf8(buffer);
Please keep the explicit call to new String.fromCharCodes. I think it is
important that it is obvious that this method copies the input three times.

[ahe, 2012-02-29 13:24:07]

Oh, and...

Thank you for cleaning this up, Dan!

Cheers,
Peter

[dgrove, 2012-03-02 12:34:29]

https://chromiumcodereview.appspot.com/9462001/diff/17001/frog/file_system_vm...
File frog/file_system_vm.dart (right):

https://chromiumcodereview.appspot.com/9462001/diff/17001/frog/file_system_vm...
frog/file_system_vm.dart:25: return decodeUtf8(buffer);
decodeUtf8 returns a String, while string.fromCharCodes expects a List. I don't
understand why it makes sense to keep this call (not to mention the fact that it
doesn't work when I leave this call in).
On 2012/02/29 13:23:02, ahe wrote:
> Please keep the explicit call to new String.fromCharCodes. I think it is
> important that it is obvious that this method copies the input three times.

[ahe, 2012-03-05 12:55:53]

https://chromiumcodereview.appspot.com/9462001/diff/17001/frog/file_system_vm...
File frog/file_system_vm.dart (right):

https://chromiumcodereview.appspot.com/9462001/diff/17001/frog/file_system_vm...
frog/file_system_vm.dart:25: return decodeUtf8(buffer);
On 2012/03/02 12:34:30, dgrove wrote:
> decodeUtf8 returns a String, while string.fromCharCodes expects a List. I
don't
> understand why it makes sense to keep this call (not to mention the fact that
it
> doesn't work when I leave this call in).
> On 2012/02/29 13:23:02, ahe wrote:
> > Please keep the explicit call to new String.fromCharCodes. I think it is
> > important that it is obvious that this method copies the input three times.
> 

Doesn't this work:

return new String.fromCharCodes(new Utf8Decoder(list).decodeRest());

[dgrove, 2012-03-05 13:07:42]

Yes, this will work. But it's really ugly! We have decodeUTf8, which is designed
to do exactly this. Shouldn't we just use that?
On 2012/03/05 12:55:53, ahe wrote:
>
https://chromiumcodereview.appspot.com/9462001/diff/17001/frog/file_system_vm...
> File frog/file_system_vm.dart (right):
> 
>
https://chromiumcodereview.appspot.com/9462001/diff/17001/frog/file_system_vm...
> frog/file_system_vm.dart:25: return decodeUtf8(buffer);
> On 2012/03/02 12:34:30, dgrove wrote:
> > decodeUtf8 returns a String, while string.fromCharCodes expects a List. I
> don't
> > understand why it makes sense to keep this call (not to mention the fact
that
> it
> > doesn't work when I leave this call in).
> > On 2012/02/29 13:23:02, ahe wrote:
> > > Please keep the explicit call to new String.fromCharCodes. I think it is
> > > important that it is obvious that this method copies the input three
times.
> > 
> 
> Doesn't this work:
> 
> return new String.fromCharCodes(new Utf8Decoder(list).decodeRest());

[dgrove, 2012-03-05 13:35:13]

OK :)

https://chromiumcodereview.appspot.com/9462001/diff/17001/frog/file_system_vm...
File frog/file_system_vm.dart (right):

https://chromiumcodereview.appspot.com/9462001/diff/17001/frog/file_system_vm...
frog/file_system_vm.dart:25: return decodeUtf8(buffer);
On 2012/03/05 12:55:53, ahe wrote:
> On 2012/03/02 12:34:30, dgrove wrote:
> > decodeUtf8 returns a String, while string.fromCharCodes expects a List. I
> don't
> > understand why it makes sense to keep this call (not to mention the fact
that
> it
> > doesn't work when I leave this call in).
> > On 2012/02/29 13:23:02, ahe wrote:
> > > Please keep the explicit call to new String.fromCharCodes. I think it is
> > > important that it is obvious that this method copies the input three
times.
> > 
> 
> Doesn't this work:
> 
> return new String.fromCharCodes(new Utf8Decoder(list).decodeRest());

Done.

[dgrove, 2012-03-05 22:37:54]

https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_core.dart
File lib/utf/utf_core.dart (right):

https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_core.da...
lib/utf/utf_core.dart:8: List<int> stringToCodepoints(String str) {
I will take care of this in a different CL.

On 2012/02/27 23:39:22, Ivan Posva wrote:
> Why are these functions exported by the dart:utf library?
> 
> String.charCodes() and String.fromCharCodes() are supposed to handle this
> correctly. If there is some special handling needed in frog, then this should
be
> done in the frog library implementation of String?

[dgrove, 2012-03-06 12:15:44]

@dcarlson - any comments on this?

[dcarlson, 2012-03-06 15:50:01]

https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_core.dart
File lib/utf/utf_core.dart (right):

https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_core.da...
lib/utf/utf_core.dart:8: List<int> stringToCodepoints(String str) {
I don't disagree. This change would simplify the UTF code and help us close
1357.

What can I do to help? 
 
On 2012/02/27 23:39:22, Ivan Posva wrote:
> Why are these functions exported by the dart:utf library?
> 
> String.charCodes() and String.fromCharCodes() are supposed to handle this
> correctly. If there is some special handling needed in frog, then this should
be
> done in the frog library implementation of String?

[dgrove, 2012-03-06 15:51:40]

https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_core.dart
File lib/utf/utf_core.dart (right):

https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_core.da...
lib/utf/utf_core.dart:8: List<int> stringToCodepoints(String str) {
Let's get this CL in, and then if you want to do this, that would be great.
On 2012/03/06 15:50:01, dcarlson wrote:
> I don't disagree. This change would simplify the UTF code and help us close
> 1357.
> 
> What can I do to help? 
>  
> On 2012/02/27 23:39:22, Ivan Posva wrote:
> > Why are these functions exported by the dart:utf library?
> > 
> > String.charCodes() and String.fromCharCodes() are supposed to handle this
> > correctly. If there is some special handling needed in frog, then this
should
> be
> > done in the frog library implementation of String?
>

[dgrove, 2012-03-07 21:23:49]

@dcarlson - is there anything more you need on this CL? (I'd love to get this
in)

On 2012/03/06 15:51:40, dgrove wrote:
>
https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_core.dart
> File lib/utf/utf_core.dart (right):
> 
>
https://chromiumcodereview.appspot.com/9462001/diff/14043/lib/utf/utf_core.da...
> lib/utf/utf_core.dart:8: List<int> stringToCodepoints(String str) {
> Let's get this CL in, and then if you want to do this, that would be great.
> On 2012/03/06 15:50:01, dcarlson wrote:
> > I don't disagree. This change would simplify the UTF code and help us close
> > 1357.
> > 
> > What can I do to help? 
> >  
> > On 2012/02/27 23:39:22, Ivan Posva wrote:
> > > Why are these functions exported by the dart:utf library?
> > > 
> > > String.charCodes() and String.fromCharCodes() are supposed to handle this
> > > correctly. If there is some special handling needed in frog, then this
> should
> > be
> > > done in the frog library implementation of String?
> >