Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(52)

Issue 22872012: Remove Encoding-enum from dart:io and add interface in dart:convert. (Closed)

Created:
7 years, 4 months ago by floitsch
Modified:
7 years, 3 months ago
CC:
reviews_dartlang.org, vm-dev_dartlang.org, hausner, Bob Nystrom, nweiz, Lasse Reichstein Nielsen, Bill Hesse
Visibility:
Public.

Description

Remove Encoding-enum from dart:io and add interface in dart:convert. BUG= http://dartbug.com/6284 BUG= http://dartbug.com/7966 R=nweiz@google.com, sgjesse@google.com Committed: https://code.google.com/p/dart/source/detail?r=26632

Patch Set 1 #

Patch Set 2 : typo. #

Patch Set 3 : Fix ddbg. #

Total comments: 10

Patch Set 4 : Address comment. #

Patch Set 5 : Move encodingFromName to Encoding.getByName. #

Patch Set 6 : Forgot to make function static. #

Patch Set 7 : Fix typo. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+417 lines, -846 lines) Patch
M pkg/analyzer_experimental/bin/analyzer.dart View 1 chunk +1 line, -1 line 0 comments Download
M pkg/analyzer_experimental/bin/formatter.dart View 1 chunk +1 line, -1 line 0 comments Download
M pkg/barback/lib/src/asset.dart View 4 chunks +5 lines, -11 lines 0 comments Download
M pkg/barback/test/asset_test.dart View 3 chunks +3 lines, -2 lines 0 comments Download
M pkg/barback/test/utils.dart View 1 chunk +1 line, -0 lines 0 comments Download
M pkg/http/lib/src/byte_stream.dart View 1 2 3 2 chunks +6 lines, -5 lines 0 comments Download
M pkg/http/lib/src/multipart_file.dart View 1 2 3 2 chunks +3 lines, -2 lines 0 comments Download
M pkg/http/lib/src/request.dart View 1 2 3 4 chunks +5 lines, -4 lines 0 comments Download
M pkg/http/lib/src/response.dart View 1 2 3 4 chunks +5 lines, -4 lines 0 comments Download
M pkg/http/lib/src/utils.dart View 1 2 3 4 3 chunks +4 lines, -16 lines 0 comments Download
M pkg/http/test/request_test.dart View 8 chunks +15 lines, -14 lines 0 comments Download
M pkg/http/test/utils.dart View 1 2 3 3 chunks +3 lines, -2 lines 0 comments Download
M pkg/http_server/lib/http_server.dart View 1 chunk +1 line, -0 lines 0 comments Download
M pkg/http_server/lib/src/http_body.dart View 3 chunks +3 lines, -3 lines 0 comments Download
M pkg/http_server/lib/src/http_body_impl.dart View 1 2 3 4 3 chunks +6 lines, -24 lines 0 comments Download
M pkg/http_server/lib/src/http_multipart_form_data_impl.dart View 1 2 3 4 1 chunk +3 lines, -3 lines 0 comments Download
M pkg/http_server/test/http_body_test.dart View 6 chunks +23 lines, -25 lines 0 comments Download
M pkg/http_server/test/utils.dart View 2 chunks +2 lines, -1 line 0 comments Download
M pkg/scheduled_test/lib/scheduled_process.dart View 3 chunks +3 lines, -3 lines 0 comments Download
M pkg/scheduled_test/test/scheduled_process_test.dart View 1 chunk +1 line, -1 line 0 comments Download
M runtime/bin/process_patch.dart View 4 chunks +6 lines, -6 lines 0 comments Download
M sdk/lib/_internal/lib/io_patch.dart View 2 chunks +4 lines, -4 lines 0 comments Download
M sdk/lib/_internal/pub/lib/src/io.dart View 2 chunks +2 lines, -1 line 0 comments Download
M sdk/lib/_internal/pub/lib/src/safe_http_server.dart View 1 chunk +1 line, -0 lines 0 comments Download
M sdk/lib/_internal/pub/test/test_pub.dart View 2 chunks +2 lines, -1 line 0 comments Download
M sdk/lib/convert/ascii.dart View 2 chunks +3 lines, -1 line 0 comments Download
M sdk/lib/convert/encoding.dart View 1 2 3 4 5 1 chunk +65 lines, -8 lines 0 comments Download
M sdk/lib/convert/latin1.dart View 2 chunks +3 lines, -1 line 0 comments Download
M sdk/lib/convert/utf.dart View 5 chunks +10 lines, -2 lines 0 comments Download
M sdk/lib/io/file.dart View 9 chunks +9 lines, -9 lines 0 comments Download
M sdk/lib/io/file_impl.dart View 7 chunks +16 lines, -16 lines 0 comments Download
M sdk/lib/io/http_impl.dart View 1 2 3 4 7 chunks +8 lines, -8 lines 0 comments Download
M sdk/lib/io/io_sink.dart View 2 chunks +2 lines, -2 lines 0 comments Download
M sdk/lib/io/process.dart View 3 chunks +5 lines, -5 lines 0 comments Download
M sdk/lib/io/stdio.dart View 2 chunks +2 lines, -2 lines 0 comments Download
M sdk/lib/io/string_transformer.dart View 1 2 3 4 5 6 1 chunk +77 lines, -309 lines 0 comments Download
M sdk/lib/io/websocket_impl.dart View 2 chunks +2 lines, -2 lines 0 comments Download
A tests/lib/convert/encoding_test.dart View 1 chunk +39 lines, -0 lines 0 comments Download
M tests/standalone/coverage_test.dart View 1 chunk +2 lines, -2 lines 0 comments Download
M tests/standalone/debugger/debug_lib.dart View 3 chunks +3 lines, -3 lines 0 comments Download
M tests/standalone/io/file_error_test.dart View 3 chunks +3 lines, -2 lines 0 comments Download
M tests/standalone/io/file_input_stream_test.dart View 2 chunks +2 lines, -2 lines 0 comments Download
M tests/standalone/io/file_test.dart View 7 chunks +22 lines, -14 lines 0 comments Download
M tests/standalone/io/process_run_output_test.dart View 2 chunks +6 lines, -5 lines 0 comments Download
M tests/standalone/io/process_std_io_script2.dart View 2 chunks +4 lines, -3 lines 0 comments Download
M tests/standalone/io/process_stdin_transform_unsubscribe_script.dart View 1 chunk +1 line, -1 line 0 comments Download
M tests/standalone/io/regress_10026_test.dart View 2 chunks +2 lines, -1 line 0 comments Download
M tests/standalone/io/secure_socket_renegotiate_client.dart View 1 chunk +1 line, -1 line 0 comments Download
M tests/standalone/io/secure_socket_renegotiate_test.dart View 1 chunk +1 line, -1 line 0 comments Download
M tests/standalone/io/stdin_sync_script.dart View 2 chunks +2 lines, -1 line 0 comments Download
M tests/standalone/io/stdin_sync_test.dart View 1 chunk +2 lines, -2 lines 0 comments Download
M tests/standalone/io/stdout_stderr_test.dart View 1 chunk +3 lines, -2 lines 0 comments Download
D tests/standalone/io/string_decoder_test.dart View 1 chunk +0 lines, -94 lines 0 comments Download
D tests/standalone/io/string_transformer_test.dart View 1 chunk +0 lines, -201 lines 0 comments Download
M tests/standalone/io/web_socket_protocol_processor_test.dart View 1 chunk +1 line, -1 line 0 comments Download
M tests/standalone/vmservice/test_helper.dart View 2 chunks +2 lines, -2 lines 0 comments Download
M tools/coverage.dart View 3 chunks +3 lines, -3 lines 0 comments Download
M tools/ddbg.dart View 1 2 3 chunks +4 lines, -3 lines 0 comments Download
M utils/testrunner/layout_test_controller.dart View 1 chunk +1 line, -1 line 0 comments Download
M utils/testrunner/pipeline_utils.dart View 1 chunk +1 line, -1 line 0 comments Download
M utils/tests/testrunner/http_client_tests/http_client_test.dart View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 10 (0 generated)
floitsch
Breaking change. - API has changed (should be covered by tests) - Encodings are now ...
7 years, 4 months ago (2013-08-23 15:05:35 UTC) #1
hausner
tools/ddbg.dart also needs to be updated.
7 years, 4 months ago (2013-08-23 15:35:52 UTC) #2
floitsch
Updated ddbg.
7 years, 4 months ago (2013-08-23 15:56:28 UTC) #3
nweiz
A couple nits, but otherwise the pkg and pub changes lgtm. It's nice to see ...
7 years, 4 months ago (2013-08-23 19:40:00 UTC) #4
floitsch
https://codereview.chromium.org/22872012/diff/7001/pkg/barback/lib/src/asset.dart File pkg/barback/lib/src/asset.dart (right): https://codereview.chromium.org/22872012/diff/7001/pkg/barback/lib/src/asset.dart#newcode8 pkg/barback/lib/src/asset.dart:8: import 'dart:convert' show Encoding, UTF8; On 2013/08/23 19:40:00, nweiz ...
7 years, 4 months ago (2013-08-24 18:56:42 UTC) #5
Søren Gjesse
LGTM! Nice change! https://codereview.chromium.org/22872012/diff/7001/sdk/lib/io/string_transformer.dart File sdk/lib/io/string_transformer.dart (right): https://codereview.chromium.org/22872012/diff/7001/sdk/lib/io/string_transformer.dart#newcode49 sdk/lib/io/string_transformer.dart:49: Encoding encodingFromName(String name) { Should this ...
7 years, 3 months ago (2013-08-26 08:03:15 UTC) #6
floitsch
https://codereview.chromium.org/22872012/diff/7001/pkg/http/lib/src/utils.dart File pkg/http/lib/src/utils.dart (right): https://codereview.chromium.org/22872012/diff/7001/pkg/http/lib/src/utils.dart#newcode86 pkg/http/lib/src/utils.dart:86: String decodeString(List<int> bytes, Encoding encoding) { On 2013/08/23 19:40:00, ...
7 years, 3 months ago (2013-08-26 09:33:39 UTC) #7
floitsch
Committed patchset #7 manually as r26632 (presubmit successful).
7 years, 3 months ago (2013-08-26 10:38:19 UTC) #8
nweiz
https://codereview.chromium.org/22872012/diff/7001/pkg/barback/lib/src/asset.dart File pkg/barback/lib/src/asset.dart (right): https://codereview.chromium.org/22872012/diff/7001/pkg/barback/lib/src/asset.dart#newcode8 pkg/barback/lib/src/asset.dart:8: import 'dart:convert' show Encoding, UTF8; On 2013/08/24 18:56:42, floitsch ...
7 years, 3 months ago (2013-08-26 17:48:43 UTC) #9
floitsch
7 years, 3 months ago (2013-08-26 17:58:42 UTC) #10
Message was sent while issue was closed.
https://codereview.chromium.org/22872012/diff/7001/pkg/barback/lib/src/asset....
File pkg/barback/lib/src/asset.dart (right):

https://codereview.chromium.org/22872012/diff/7001/pkg/barback/lib/src/asset....
pkg/barback/lib/src/asset.dart:8: import 'dart:convert' show Encoding, UTF8;
On 2013/08/26 17:48:43, nweiz wrote:
> On 2013/08/24 18:56:42, floitsch wrote:
> > On 2013/08/23 19:40:00, nweiz wrote:
> > > Style nit: we usually don't import with "show". Same goes for other
> libraries
> > > maintained by Bob and me.
> > 
> > As long as this library still uses dart:utf this is not possible. There
would
> be
> > a clash with Utf8Encoder (iirc).
> 
> Why is this library still using dart:utf? Shouldn't it be switched over
entirely
> to dart:convert?

Working on it in a separate CL. There this line is already without 'show'.

Powered by Google App Engine
This is Rietveld 408576698