Re: HTML5 Filesystem API feedback

Hi Eric,

Thanks for your reply, my comments are inline...

On Fri, Apr 15, 2011 at 11:53 AM, Eric Uhrhane <ericu@google.com> wrote:
> Robert:
>
> First of all, thanks for the feedback.
>
> On Wed, Apr 13, 2011 at 1:35 PM, Robert Ginda <rginda@chromium.org> wrote:
>> Hello public-webapps,
>>
>> I've spent a bit of time with the filesystem API recently, building out a
>> set of common file dialogs (open, save-as, etc) for ChromeOS.  We have
>> a private API call to get access to a special filesystem that contains the
>> user's downloads folder and the mounted external storage, but beyond
>> that the dialogs use the standard filesystem API calls.
>>
>> The bulk of the code is here:
>> http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/resources/file_manager/js/
>>
>> I've run into a few awkward bits that I'd like to bring up here to see what
>> everyone else thinks.
>>
>> * The async API is really awkward when you want to resolve multiple
>> entries.  I have a case where I'd like to resolve a list of
>> directories and then do something when that completes.  It would be
>> helpful if the DirectoryEntry class had a getDirectories() method.
>> I've built my own in file_manager/js/util.js in the function called
>> util.getDirectories.
>
> In general, I'm pretty happy to see this sort of thing.  You may not
> like to hear this, but any problem that can just be solved by layering
> a small javascript library on top of this API is a problem we don't
> need to fix in the first pass of the API.  We want it to start out
> powerful and simple, and eventually, if that library is something that
> absolutely everyone seems to need, we may want to incorporate it.  So
> it is very helpful to see the kinds of libraries that get built on
> top, and thanks for sending the pointer, but I suggest we wait and see
> before making any changes.
>

Sure, I see your point.  I just think this is such an obvious problem
(introduced by the necessarily asynchronous nature of the API), that
it would be nice to offer a solution up front.  Anything that isn't in
the first pass of the API is likely to stick around in shim libraries
for a long long time.

> BTW, you can make your getDirectories method faster if you don't wait
> for a response before sending off the next query [assuming no memory
> constraints].  Just fire off all the requests in one go, then keep a
> countdown of responses in your callback-wrapper.  When your countdown
> hits zero, fire the "we're done" signal.
>

Yeah, I've actually used the countdown version of this pattern in
another part of the code.  I probably should have used it here too.
FWIW, it gets a bit difficult if you care about getting called back in
the correct order.

>> * That leads to a desire to get "entries" in the general sense.  In my
>> case I knew ahead of time that I had a list of directories.  If I
>> didn't know whether I had been handed directories or files, the code
>> would get pretty hairy.  So in addition to getDirectories, a
>> getEntries method (and getEntry, too) would be super useful.
>
> This is a method that would look up, but not create, an Entry, given a
> path?  Can you outline the circumstances in which you'd use this, but
> where you wouldn't want to list the whole parent directory?
>

Say you wanted to implement the shell builtin 'mv <source> ...
<target>'.  You can have an arbitrary number of source entries, each
of which can be either a file or a directory, and can be located at an
arbitrary depth in the filesystem.  The <target>, as well, could be an
existing file or directory.

With the existing API, you would have to try each source first as one
type (say, getFile), then if that fails (with a TYPE_MISMATCH_ERR),
try again using the other getter.  A generic getEntry() would make
this considerably cleaner.

>> * Another helper I found useful was util.forEachDirEntry from the same
>> file previously mentioned.  This function invokes a callback once for
>> each entry in a directory.  This saves the caller from having to
>> create the reader and walk the results array during the callback.  It
>> makes client code significantly more readable.  I'd love to see
>> DirectoryEntry.forEach(...).
>
> I can see the utility there pretty clearly.
>
>> * The FileError object is a bit awkward to work with.  I found that I
>> frequently had every reason to expect my calls to succeed (because
>> they were a follow-on to something that already succeeded), but I
>> wanted to log the failure reason in the event they didn't.  The code
>> online suggests a switch/case statement to turn error codes into
>> mnemonic strings.  This requires a hardcoded list of all known errors,
>> and a call out to this utility function every time you want to display
>> the error reason in a readable way.  I suggest adding the mnemonic
>> string as a property of FileError, and displaying it as part of the
>> toString.  (See util.getFileErrorMnemonic() and
>> util.installFileErrorToString() for example implementations.)
>
> Hmm...I'm not sure what other APIs usually do about this.  As soon as
> you start adding strings that you intend to be human-readable, you
> start getting into all sorts of internationalization issues.  Anyone?
>

So I think there are two issues here.

The first is that it's cumbersome to map the numeric error code to the
mnemonic.  There is already a mapping from mnemonic to code
(TYPE_MISMATCH_ERR to 11, for example), and it's "possible" to compute
the reverse mapping.  Making it easier shouldn't introduce i18n
issues.  Some options are:

 * Add a 'mnemonic' property to the FileError instance.
 * Add a static FileError.getMnemonic(code).
 * Add a static array of mnemonics to FileError.

Once you have that mnemonic, you could display it in a debug log or
some last-ditch error dialog, or use it to look up a localized error
message (assuming that lookup by mnemonic is more portable than by
numeric, and that if the lookup fails you could still display the
mnemonic.)

The second issue is altering toString() to include the mnemonic.  From
my read of the spec, the toString() method is not even mentioned, so I
suppose the decision is left up to the implementor.

FWIW, Firefox's toString() method on XPConnect errors includes the
mnemonic for the error code, and it makes debugging *so* much easier
than if they did not.


Rob.

>        Eric
>

Received on Friday, 15 April 2011 21:38:46 UTC