Summary: | Add readDirectory command and didReadDirectory event to InspectorFileSystemAgent | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Taiju Tsuiki <tzik> | ||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Taiju Tsuiki <tzik> | ||||||||
Status: | RESOLVED DUPLICATE | ||||||||||
Severity: | Normal | CC: | apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 87724 | ||||||||||
Attachments: |
|
Description
Taiju Tsuiki
2012-05-28 23:50:17 PDT
Created attachment 144461 [details]
Patch
Comment on attachment 144461 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144461&action=review > Source/WebCore/inspector/Inspector.json:1420 > + { "name": "errorCode", "type": "integer" }, Please add a description for this and the next fields. > Source/WebCore/inspector/InspectorFileSystemAgent.cpp:80 > + m_frontend->didReadDirectory(requestId, static_cast<int>(FileError::ABORT_ERR), TypeBuilder::Array<String>::create()); You may want to make the "entries" field optional as it makes no sense in case of an error. Comment on attachment 144461 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144461&action=review Drive-by nit-picks. > Source/WebCore/inspector/InspectorFileSystemAgent.cpp:2 > + * Copyright (C) 2012 Google Inc. All rights reserved. In general we seem to just append copyright years in WebKit (rather than replacing) > Source/WebCore/inspector/InspectorFileSystemAgent.h:2 > + * Copyright (C) 2012 Google Inc. All rights reserved. ditto. > Source/WebCore/inspector/InspectorFileSystemAgent.h:49 > +typedef String ErrorString; nit: why do we suddenly need this typedef? (We seem to be already using it) Created attachment 144474 [details]
Patch
Created attachment 144476 [details]
Patch
Comment on attachment 144474 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144474&action=review > Source/WebCore/inspector/InspectorFileSystemAgent.cpp:2 > + * Copyright (C) 2011,2012 Google Inc. All rights reserved. nit: space after , > Source/WebCore/inspector/InspectorFileSystemAgent.h:2 > + * Copyright (C) 2011,2012 Google Inc. All rights reserved. nit: space after , > Source/WebCore/inspector/InspectorFileSystemAgent.h:49 > +typedef String ErrorString; No need for this as it is defined in InspectorBackendDispatcher.h Comment on attachment 144461 [details] Patch Thanks! Updated. View in context: https://bugs.webkit.org/attachment.cgi?id=144461&action=review >> Source/WebCore/inspector/Inspector.json:1420 >> + { "name": "errorCode", "type": "integer" }, > > Please add a description for this and the next fields. Done >> Source/WebCore/inspector/InspectorFileSystemAgent.cpp:2 >> + * Copyright (C) 2012 Google Inc. All rights reserved. > > In general we seem to just append copyright years in WebKit (rather than replacing) Done >> Source/WebCore/inspector/InspectorFileSystemAgent.cpp:80 >> + m_frontend->didReadDirectory(requestId, static_cast<int>(FileError::ABORT_ERR), TypeBuilder::Array<String>::create()); > > You may want to make the "entries" field optional as it makes no sense in case of an error. Done >> Source/WebCore/inspector/InspectorFileSystemAgent.h:2 >> + * Copyright (C) 2012 Google Inc. All rights reserved. > > ditto. Done >> Source/WebCore/inspector/InspectorFileSystemAgent.h:49 >> +typedef String ErrorString; > > nit: why do we suddenly need this typedef? (We seem to be already using it) It is also in InspectorFrontend.h (generated header). But I'd like to define it explicitly. Comment on attachment 144476 [details]
Patch
Please address my comments to the previous patch.
|