Make Utf8Decoder and Utf8Encoder's convert methods take start and end too.

Make Utf8Decoder and Utf8Encoder's convert methods take start and end too.

BUG= http://dartbug.com/21652
R=floitsch@google.com, sgjesse@google.com

Committed: https://code.google.com/p/dart/source/detail?r=41844

[Lasse Reichstein Nielsen, 2014-11-19 13:14:25]

lrn@google.com changed reviewers:
+ floitsch@google.com, sgjesse@google.com

[Lasse Reichstein Nielsen, 2014-11-19 13:14:25]

I only added the extra arguments to the convert methods of the decoder/encoder,
not to decode/encode on UTF8, since they have existing named parameters.

The functionality was already there in the underlying converters, it was just a
matter of passing the parameters through.

[floitsch, 2014-11-19 13:19:58]

LGTM, although I'm not 100% convinced that a start/end is really useful. But
apparently users encounter it...

Why not make it named and add the names to the encode/decode functions?

https://codereview.chromium.org/736583008/diff/1/sdk/lib/convert/utf.dart
File sdk/lib/convert/utf.dart (right):

https://codereview.chromium.org/736583008/diff/1/sdk/lib/convert/utf.dart#new...
sdk/lib/convert/utf.dart:85: * unsigned 8-bit integers).
Update comment.

https://codereview.chromium.org/736583008/diff/1/sdk/lib/convert/utf.dart#new...
sdk/lib/convert/utf.dart:87: List<int> convert(String string, [int start = 0,
int end]) {
Make it named?

https://codereview.chromium.org/736583008/diff/1/sdk/lib/convert/utf.dart#new...
sdk/lib/convert/utf.dart:336: String convert(List<int> codeUnits, [int start =
0, int end]) {
ditto.

[Lasse Reichstein Nielsen, 2014-11-19 13:25:00]

https://codereview.chromium.org/736583008/diff/1/sdk/lib/convert/utf.dart
File sdk/lib/convert/utf.dart (right):

https://codereview.chromium.org/736583008/diff/1/sdk/lib/convert/utf.dart#new...
sdk/lib/convert/utf.dart:87: List<int> convert(String string, [int start = 0,
int end]) {
I'd rather not use named parameters. That's a lot more inconvenient to use, and
we generally provide these "range" arguments as positional arguments.

[floitsch, 2014-11-19 13:41:25]

On 2014/11/19 13:25:00, Lasse Reichstein Nielsen wrote:
> https://codereview.chromium.org/736583008/diff/1/sdk/lib/convert/utf.dart
> File sdk/lib/convert/utf.dart (right):
> 
>
https://codereview.chromium.org/736583008/diff/1/sdk/lib/convert/utf.dart#new...
> sdk/lib/convert/utf.dart:87: List<int> convert(String string, [int start = 0,
> int end]) {
> I'd rather not use named parameters. That's a lot more inconvenient to use,
and
> we generally provide these "range" arguments as positional arguments.

Ok.
You should also update the other encodings and see if the optional name doesn't
clash with them.

[floitsch, 2014-11-19 13:41:36]

On 2014/11/19 13:41:25, floitsch wrote:
> On 2014/11/19 13:25:00, Lasse Reichstein Nielsen wrote:
> > https://codereview.chromium.org/736583008/diff/1/sdk/lib/convert/utf.dart
> > File sdk/lib/convert/utf.dart (right):
> > 
> >
>
https://codereview.chromium.org/736583008/diff/1/sdk/lib/convert/utf.dart#new...
> > sdk/lib/convert/utf.dart:87: List<int> convert(String string, [int start =
0,
> > int end]) {
> > I'd rather not use named parameters. That's a lot more inconvenient to use,
> and
> > we generally provide these "range" arguments as positional arguments.
> 
> Ok.
> You should also update the other encodings and see if the optional name
doesn't
> clash with them.

optional positional argument *

[Søren Gjesse, 2014-11-19 13:53:28]

lgtm

https://codereview.chromium.org/736583008/diff/40001/sdk/lib/convert/utf.dart
File sdk/lib/convert/utf.dart (right):

https://codereview.chromium.org/736583008/diff/40001/sdk/lib/convert/utf.dart...
sdk/lib/convert/utf.dart:90: List<int> convert(String string, [int start = 0,
int end]) {
Should these be named arguments? Otherwise we are stuck with positional
arguments for future changes.

https://codereview.chromium.org/736583008/diff/40001/sdk/lib/convert/utf.dart...
sdk/lib/convert/utf.dart:334: * If [end] is omitted, it defaults to
`codeUnits.length`.
Maybe add some more information on invalid encoding. With the ability to specify
start and end it is easier to run into, especially if you don't understand that
the indices are into the code units.

[Lasse Reichstein Nielsen, 2014-11-19 14:00:20]

On 2014/11/19 13:41:36, floitsch wrote:
> On 2014/11/19 13:41:25, floitsch wrote:
> > On 2014/11/19 13:25:00, Lasse Reichstein Nielsen wrote:
> > > https://codereview.chromium.org/736583008/diff/1/sdk/lib/convert/utf.dart
> > > File sdk/lib/convert/utf.dart (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/736583008/diff/1/sdk/lib/convert/utf.dart#new...
> > > sdk/lib/convert/utf.dart:87: List<int> convert(String string, [int start =
> 0,
> > > int end]) {
> > > I'd rather not use named parameters. That's a lot more inconvenient to
use,
> > and
> > > we generally provide these "range" arguments as positional arguments.
> > 
> > Ok.
> > You should also update the other encodings and see if the optional name
> doesn't
> > clash with them.
> 
> optional positional argument *

What other encodings?
It's not something we can add to any implementation of Converter - it requires
the input be something that can have start and end.

[floitsch, 2014-11-19 14:43:28]

On 2014/11/19 14:00:20, Lasse Reichstein Nielsen wrote:
> On 2014/11/19 13:41:36, floitsch wrote:
> > On 2014/11/19 13:41:25, floitsch wrote:
> > > On 2014/11/19 13:25:00, Lasse Reichstein Nielsen wrote:
> > > >
https://codereview.chromium.org/736583008/diff/1/sdk/lib/convert/utf.dart
> > > > File sdk/lib/convert/utf.dart (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/736583008/diff/1/sdk/lib/convert/utf.dart#new...
> > > > sdk/lib/convert/utf.dart:87: List<int> convert(String string, [int start
=
> > 0,
> > > > int end]) {
> > > > I'd rather not use named parameters. That's a lot more inconvenient to
> use,
> > > and
> > > > we generally provide these "range" arguments as positional arguments.
> > > 
> > > Ok.
> > > You should also update the other encodings and see if the optional name
> > doesn't
> > > clash with them.
> > 
> > optional positional argument *
> 
> What other encodings?
> It's not something we can add to any implementation of Converter - it requires
> the input be something that can have start and end.

https://api.dartlang.org/apidocs/channels/stable/dartdoc-viewer/dart:convert....

Ascii, Latin1 so far.

[Lasse Reichstein Nielsen, 2014-11-20 08:26:55]

Also changed ASCII, Latin-1. PTAL

[Søren Gjesse, 2014-11-20 10:41:41]

lgtm, with comments

https://codereview.chromium.org/736583008/diff/60001/sdk/lib/convert/ascii.dart
File sdk/lib/convert/ascii.dart (right):

https://codereview.chromium.org/736583008/diff/60001/sdk/lib/convert/ascii.da...
sdk/lib/convert/ascii.dart:77: List<int> convert(String string, [int start = 0,
int end]) {
Where are the new arguments documented?

https://codereview.chromium.org/736583008/diff/60001/sdk/lib/convert/ascii.da...
sdk/lib/convert/ascii.dart:78: int stringLength = string.length;
Maybe factor out argument checking.

https://codereview.chromium.org/736583008/diff/60001/sdk/lib/convert/ascii.da...
sdk/lib/convert/ascii.dart:186: * corresponding string.
Add documentation.

[Lasse Reichstein Nielsen, 2014-11-20 11:24:07]

Committed patchset #5 (id:80001) manually as 41844 (presubmit successful).

[Lasse Reichstein Nielsen, 2014-11-20 12:46:22]

https://codereview.chromium.org/736583008/diff/60001/sdk/lib/convert/ascii.dart
File sdk/lib/convert/ascii.dart (right):

https://codereview.chromium.org/736583008/diff/60001/sdk/lib/convert/ascii.da...
sdk/lib/convert/ascii.dart:77: List<int> convert(String string, [int start = 0,
int end]) {
Ack, documenting.

https://codereview.chromium.org/736583008/diff/60001/sdk/lib/convert/ascii.da...
sdk/lib/convert/ascii.dart:78: int stringLength = string.length;
Funny you should say that - I'm already doing that in another CL :)