Fix scheme's toLowerCase detection.

Fix scheme's toLowerCase detection.

BUG=
R=lrn@google.com

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

[Anders Johnsen, 2014-09-03 15:07:14]

ajohnsen@google.com changed reviewers:
+ lrn@google.com

[Lasse Reichstein Nielsen, 2014-09-04 05:57:14]

lgtm

https://codereview.chromium.org/532273002/diff/20001/sdk/lib/core/uri.dart
File sdk/lib/core/uri.dart (left):

https://codereview.chromium.org/532273002/diff/20001/sdk/lib/core/uri.dart#ol...
sdk/lib/core/uri.dart:1115: if (_LOWER_CASE_A <= char && _LOWER_CASE_Z >= char)
{
whoops.

https://codereview.chromium.org/532273002/diff/20001/sdk/lib/core/uri.dart
File sdk/lib/core/uri.dart (right):

https://codereview.chromium.org/532273002/diff/20001/sdk/lib/core/uri.dart#ne...
sdk/lib/core/uri.dart:1119: scheme = scheme.substring(0, end);
It really, really sucks that we don't have cheap string slices.

How about having:

  StringSlice String.slice(int start, int end)

where StringSlice implements almost the same interface as String, isn't actually
a String (just a CharSequence ... I spot a Java inspiration)..

Then you could do scheme.slice(0, end).toLowerCase() and get the same as
scheme.substring(0, end).toLowerCase(), but without creating the substring.

That only really makes sense if the slice object (header, reference to string,
start-index, end-index) is smaller than the actual substring (header, length,
characters), which it probably isn't here, since schemes are usually short and
already lower-case.

https://codereview.chromium.org/532273002/diff/20001/tests/corelib/uri_scheme...
File tests/corelib/uri_scheme_test.dart (right):

https://codereview.chromium.org/532273002/diff/20001/tests/corelib/uri_scheme...
tests/corelib/uri_scheme_test.dart:18: Expect.equals(expectedUri,
uri.toString());
Check that this works with Uri.parse as well:

      var uri = Uri.parse("$scheme:///");
      ... same expects.

https://codereview.chromium.org/532273002/diff/20001/tests/corelib/uri_scheme...
tests/corelib/uri_scheme_test.dart:23: test("http", "http:", "hTTP");
Please add "Http" as well.

[Anders Johnsen, 2014-09-04 06:27:58]

https://codereview.chromium.org/532273002/diff/20001/sdk/lib/core/uri.dart
File sdk/lib/core/uri.dart (left):

https://codereview.chromium.org/532273002/diff/20001/sdk/lib/core/uri.dart#ol...
sdk/lib/core/uri.dart:1115: if (_LOWER_CASE_A <= char && _LOWER_CASE_Z >= char)
{
On 2014/09/04 05:57:14, Lasse Reichstein Nielsen wrote:
> whoops.

Heh, copy paste :)

https://codereview.chromium.org/532273002/diff/20001/sdk/lib/core/uri.dart
File sdk/lib/core/uri.dart (right):

https://codereview.chromium.org/532273002/diff/20001/sdk/lib/core/uri.dart#ne...
sdk/lib/core/uri.dart:1119: scheme = scheme.substring(0, end);
On 2014/09/04 05:57:14, Lasse Reichstein Nielsen wrote:
> It really, really sucks that we don't have cheap string slices.
> 
> How about having:
> 
>   StringSlice String.slice(int start, int end)
> 
> where StringSlice implements almost the same interface as String, isn't
actually
> a String (just a CharSequence ... I spot a Java inspiration)..
> 
> Then you could do scheme.slice(0, end).toLowerCase() and get the same as
> scheme.substring(0, end).toLowerCase(), but without creating the substring.
> 
> That only really makes sense if the slice object (header, reference to string,
> start-index, end-index) is smaller than the actual substring (header, length,
> characters), which it probably isn't here, since schemes are usually short and
> already lower-case.

That's not a bad idea. Lets discuss it offline.

https://codereview.chromium.org/532273002/diff/20001/tests/corelib/uri_scheme...
File tests/corelib/uri_scheme_test.dart (right):

https://codereview.chromium.org/532273002/diff/20001/tests/corelib/uri_scheme...
tests/corelib/uri_scheme_test.dart:18: Expect.equals(expectedUri,
uri.toString());
On 2014/09/04 05:57:14, Lasse Reichstein Nielsen wrote:
> Check that this works with Uri.parse as well:
> 
>       var uri = Uri.parse("$scheme:///");
>       ... same expects.

Done.

https://codereview.chromium.org/532273002/diff/20001/tests/corelib/uri_scheme...
tests/corelib/uri_scheme_test.dart:23: test("http", "http:", "hTTP");
On 2014/09/04 05:57:14, Lasse Reichstein Nielsen wrote:
> Please add "Http" as well. 

Done.

[Anders Johnsen, 2014-09-04 06:34:42]

Committed patchset #3 (id:40001) manually as 39846 (presubmit successful).