Re: Polished FileSystem API proposal

On Mon, Jul 15, 2013 at 5:58 PM, Jonas Sicking <jonas@sicking.cc> wrote:
> On Mon, Jul 15, 2013 at 4:47 PM, Eric U <ericu@google.com> wrote:
>>>> What's the purpose of DestinationDict?  It seems redundant.
>>>
>>> There's basically three potential ways to specify a copy/move destination:
>>>
>>> Specify a new name within the current directory.
>>> Specify a new directory but keep the current name.
>>> Specify a new directory as well as a new name within that directory.
>>>
>>> We obviously don't have to support them all, for example you could
>>> imagine requiring a two-step process in some cases. But we decided to
>>> try to support all three as a single operation for now.
>>>
>>> Coming up with a syntax for this is somewhat tricky. In C++ you'd
>>> simply have three overloads that take the different combinations, but
>>> that doesn't feel very "javascripty".
>>>
>>> Another is to use optional arguments like:
>>>
>>> move(..., Directory? destDir, optional DOMString destName);
>>>
>>> would work. That means that the three options would look like:
>>>
>>> dir.move("srcfile", null, "newName");
>>> dir.move("srcfile", destDir);
>>> dir.move("srcfile", destDir, "newName");
>>
>> dir.move("srcfile", "dir/and/new/name");
>>
>> Granted, that doesn't help you if you're holding a Directory object
>> and don't have a way to generate a relative path to it. But perhaps
>> that's not a big lack?  You can always munge paths yourself.
>>
>> Obviously we went with the way you'd see as iffy.
>>
>>> Unfortunately the first one there looks pretty iffy. So instead we
>>> went with a union which results in syntax like:
>>>
>>> dir.move("srcfile", "newName");
>>> dir.move("srcfile", destDir);
>>> dir.move("srcfile", { dir: destDir. name: "newName" });
>>
>> In this syntax, could you say it this way?
>>
>> dir.move("srcfile", { dir: "dirName". name: "newName" });
>
> No, but you could say
>
> dir.move("srcfile", "dirName/newName");
>
> Or you can use .get() to grab the directory object named "dirName" and use that.

So what can't you do without the DestinationDict?  You can rename the
file without moving it to a new dir, you can move it to a new dir
without renaming it, and you can rename and move it using
"dirName/newName" as the second argument.

>>>> You have MakeFileOptions.overwriteIfExists for createFile, but
>>>> CreateMode { createIfNeeded | dontcreate } for openWrite; it feels
>>>> like those should share a set of flags.  The options should probably
>>>> be {createIfNeeded, dontCreate, failIfExists}, where failIfExists
>>>> implies createIfNeeded and is the standard POSIX paradigm
>>>> [O_CREAT|O_EXCL].  It's useful for coordination between multiple pages
>>>> in the same origin, or the same page loaded twice.
>>>
>>> So you're basically proposing (modulo naming):
>>>
>>> createFile("name", { mode: "createIfNeeded" });
>>> Creates the file if it doesn't exist. Overwrites it if it does.
>>
>> Is the idea that createFile will overwrite an existing file's
>> contents, even if you don't supply any, e.g. nuking a file down to 0
>> bytes?
>
> Yes.
>
>> That might be a bit surprising.  Would you require
>> MakeFileOptions.data?
>
> In the current proposal the function is called "createFile" and if you
> want it to overwrite what's there you have to explicitly opt in to
> that using
>
> createFile("name", { overwriteIfExists: true });
>
> so I don't think the current proposal is that surprising. Especially
> since creating files usually will either error or remove what's there.
> I'd like to keep that clarity.
>
>>> createFile("name", { mode: "dontCreate" });
>>> Error if the file doesn't exist. Overwrites it if it does.
>>
>> I'm not sure why you'd ever call it this way.  Perhaps disallow it for
>> now?  Otherwise it might become a standard hack for some bizarre
>> file-based locking paradigm.
>>
>>> createFile("name", { mode: "failIfExists" });
>>> Creates the file if it doesn't exist. Error it if it does.
>>>
>>> openWrite("name", "createIfNeeded");
>>> Creates and opens the file if it doesn't exist. Opens it if it does.
>>>
>>> openWrite("name", "dontCreate");
>>> Error if the file doesn't exist. Opens it if it does.
>>>
>>> openWrite("name", "failIfExists");
>>> Creates and opens the file if it doesn't exist. Error it if it does.
>>>
>>> Is this correct? While I agree it's consistent, the number of
>>> combinations here is a bit mind boggling. It took me quite a while to
>>> write all of these out and make sure they are correct. The
>>> createFile("dontCreate") combination is especially confusing.
>>
>> I'd skip that one; I think the other 5 make sense, and are better than
>> having two different paradigms to say almost the same thing depending
>> on which function you call [boolean vs. string argument].  If you
>> decide you don't want failIfExists, that makes the list even simpler.
>
> I think the 4 modes that are important to support are:
>
> Create new file and error if there's already an existing one
> Create new file and overwrite if there's already an existing one
> Open for writing and create if doesn't exist
> Open for writing and error if doesn't exist
>
> The behavior that needs to be defined for the create function is what
> to do if the file already exists, because generally you aren't
> expecting a file to exist when you are requesting it to be created.
> The behavior that needs to be defined for the open function is what to
> do if the function does not exist.
>
> So I don't know that trying to align them too much is going to work
> very well. I do totally agree that the current setup is not good
> enough though.
>
> How about:
>
> createFile("name", { ifExists: "overwrite" });
> createFile("name", { ifExists: "fail" });
> openWrite("name", { ifNotExist: "create" });
> openWrite("name", { ifNotExist: "fail" });
>
> then we can duke it out what the defaults should be :)
>
>>>> If you don't allow ".." to traverse directories, please define using
>>>> it as a path segment as an error, to allow for future expansion and to
>>>> catch bugs for folks who didn't read the spec.
>>>
>>> Definitely.
>>>
>>>> openForWrite or openForWriting, while longer, would be clearer than
>>>> openWrite.  Heck, why not just "write"?
>>>
>>> I tried out just "write" for a while, but it seemed strange that
>>> myDir.write() actually started writing to a file.
>>
>> Doesn't seem all that worse than myDir.openForWriting() to me, but is
>> openWrite better in any way?
>> Sure, it doesn't sound like you're opening the directory for writing,
>> but that's because it doesn't make any sense at all.
>
> I've consistently gotten feedback that authors prefer shorter names
> over absolute clearness. So I still think openWrite wins over
> openForWriting. I could be convinced that "write" is better, but so
> far it doesn't seem like there's more consensus for that.
>
> / Jonas

Received on Tuesday, 16 July 2013 15:56:57 UTC