Issue 9773018: Add error handling to directory

Issue 9773018: Add error handling to directory (Closed)

Created:
8 years, 9 months ago by Søren Gjesse

Modified:
8 years, 8 months ago

Reviewers:
Mads Ager (google)

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Add error handling to directory The directory object now has error handling like the rest of the dart:io. The added tests to some extend overlaps with some of the tests in DirectpryTest.dart, but I found it better to have error tests in a single place. I removed a few type checks sitting in the C++ code for the native functions as for file we currently don't have them. I have opened issue 2305 regarding where to do type checks for dart:io native functions. R=ager@google.com BUG= TEST= Committed: https://code.google.com/p/dart/source/detail?r=5866

Patch Set 1 #

Patch Set 2 : Fixed Windows and listing error handling #

Total comments: 6

Patch Set 3 : Addressed review comments #

Created: 8 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+456 lines, -178 lines)			Patch
M	runtime/bin/directory.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	runtime/bin/directory.cc	View	1	8 chunks	+64 lines, -40 lines	0 comments	Download
M	runtime/bin/directory.dart	View		2 chunks	+9 lines, -1 line	0 comments	Download
M	runtime/bin/directory_impl.dart	View	1 2	10 chunks	+109 lines, -49 lines	0 comments	Download
M	runtime/bin/directory_posix.cc	View		5 chunks	+9 lines, -24 lines	0 comments	Download
M	runtime/bin/directory_win.cc	View		4 chunks	+7 lines, -20 lines	0 comments	Download
M	runtime/bin/file_impl.dart	View	1 2	2 chunks	+8 lines, -3 lines	0 comments	Download
A	tests/standalone/src/io/DirectoryErrorTest.dart	View	1 2	1 chunk	+212 lines, -0 lines	0 comments	Download
M	tests/standalone/src/io/DirectoryInvalidArgumentsTest.dart	View	1	1 chunk	+8 lines, -3 lines	0 comments	Download
M	tests/standalone/src/io/DirectoryTest.dart	View		11 chunks	+29 lines, -37 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Mads Ager (google)

lgtm https://chromiumcodereview.appspot.com/9773018/diff/1001/runtime/bin/directory_impl.dart File runtime/bin/directory_impl.dart (right): https://chromiumcodereview.appspot.com/9773018/diff/1001/runtime/bin/directory_impl.dart#newcode183 runtime/bin/directory_impl.dart:183: new OSError(message[2][2], Maybe introduce named constants for the ...

8 years, 9 months ago (2012-03-26 16:03:05 UTC) #2

Søren Gjesse

8 years, 8 months ago (2012-04-02 07:33:02 UTC) #3

https://chromiumcodereview.appspot.com/9773018/diff/1001/runtime/bin/director...
File runtime/bin/directory_impl.dart (right):

https://chromiumcodereview.appspot.com/9773018/diff/1001/runtime/bin/director...
runtime/bin/directory_impl.dart:183: new OSError(message[2][2],
On 2012/03/26 16:03:05, Mads Ager wrote:
> Maybe introduce named constants for the indices into message. Also, maybe name
> the second and first component so this reads better?

Done.

https://chromiumcodereview.appspot.com/9773018/diff/1001/runtime/bin/director...
runtime/bin/directory_impl.dart:235: message, _path, new OSError(response[2],
response[1])));
On 2012/03/26 16:03:05, Mads Ager wrote:
> Ditto. Naming response[2] and response[1] would improve readability here.

Done.

I have put it on my TODO list that we need to see if we can make these native
port RPC calls simpler and refactor the handling on both the Dart and C++ side.

https://chromiumcodereview.appspot.com/9773018/diff/1001/tests/standalone/src...
File tests/standalone/src/io/DirectoryErrorTest.dart (right):

https://chromiumcodereview.appspot.com/9773018/diff/1001/tests/standalone/src...
tests/standalone/src/io/DirectoryErrorTest.dart:5: // Dart test program for
testing error handling in file I/O.
On 2012/03/26 16:03:05, Mads Ager wrote:
> directory?

Done.

Expand Messages | Collapse Messages