Re: FileSystem API - overwrite flag for copy/move?

On Thu, Sep 16, 2010 at 4:00 PM, Eric Uhrhane <ericu@google.com> wrote:
> 2010/9/16 Kinuko Yasuda <kinuko@chromium.org>:
>> On Thu, Sep 16, 2010 at 2:50 PM, Eric Uhrhane <ericu@google.com> wrote:
>>> How about this?
>>>
>>> For a move/copy of a file on top of existing file, or a directory on
>>> top of an existing empty directory, you get an automatic overwrite.
>>> A move/copy of a file on top of an existing directory, or of a
>>> directory on top of an existing file, will always fail.
>>> A move/copy of a file or directory on top of an existing non-empty
>>> directory will always fail.
>>
>> This sounds good to me.
>> I think sticking to the compatibility with Posix would make sense to
>> developers too.
>>
>>> That matches Posix[1] rename behavior, and should cover most or all of
>>> the normal use cases.
>>> If necessary, we can consider adding a "don't overwrite" flag, but
>>> that may be difficult to implement without race conditions on all
>>> platforms.
>>
>> We can achieve this in a safer way by combining getFile/getDirectory
>> with exclusive flag and move/copy, so I think it's reasonable not to
>> have "don't overwrite" flag.
>>
>>> Regarding recursive deletion of directories:
>>>
>>> One option is to add a flag to remove(); that flag will be ignored if
>>> the Entry is a file, so it's not as clean as it might be, but it keeps
>>> the interface small.
>>> Another is to add a removeRecursively() method to DirectoryEntry; this
>>> makes it really clear what's going on, and might prevent some
>>> accidental deletions.
>>
>> I have no strong opinion on this but removeRecursively might be error prone?
>> (Recursive remove can be very destructive and it should be triggered
>> with an explicit intention.)
>
> Which do you think is more error-prone, a method called
> removeRecursively, or a boolean flag to remove?

Sorry I messed up the sentence...
I meant I prefer removeRecursively.

Thanks,

>>> Which do you prefer?
>>>
>>> [1] http://www.opengroup.org/onlinepubs/009695399/functions/rename.html
>>>
>>> 2010/9/9 Kinuko Yasuda <kinuko@chromium.org>:
>>>> On Thu, Sep 9, 2010 at 12:37 AM, Ian Fette (イアンフェッティ) <ifette@google.com>
>>>> wrote:
>>>>>
>>>>> I think recursive copy/remove is a very valid use case. As for overwrite,
>>>>> is a flag necessary? On most OSes you already get overwrite as the default
>>>>> behaviour (at least from APIs, many interactive UAs such as Explorer on
>>>>> windows will prompt), is there a compelling argument why it should be
>>>>> different for a web api?
>>>>
>>>> Making overwriting mode default for copy/move sounds reasonable to me too.
>>>>  Especially if we allow recursive remove (and I think there would be more
>>>> need for this one) it'd look more consistent.
>>>> As for providing options, I was a bit concerned about the complexity in
>>>> programming with nested callbacks, but in this case it's not a big deal
>>>> (we'll need only two) and wouldn't be a problem.
>>>> I'm more concerned with recursive remove though.  The js code to do that
>>>> isn't very pretty.
>>>>
>>>>> On Thu, Sep 9, 2010 at 12:22 AM, Kinuko Yasuda <kinuko@chromium.org>
>>>>> wrote:
>>>>>>
>>>>>> On Thu, Sep 9, 2010 at 12:12 AM, Kinuko Yasuda <kinuko@chromium.org>
>>>>>> wrote:
>>>>>>>
>>>>>>> On Tue, Sep 7, 2010 at 6:12 PM, Eric Uhrhane <ericu@google.com> wrote:
>>>>>>>>
>>>>>>>> On Mon, Aug 30, 2010 at 9:27 PM, Kinuko Yasuda <kinuko@chromium.org>
>>>>>>>> wrote:
>>>>>>>> > Hi,
>>>>>>>> > I have a question about Entry.moveTo/copyTo behavior defined in
>>>>>>>> > the File API: Directories and System [1].
>>>>>>>> > Currently the API doesn't specify how Entry.moveTo() and copyTo()
>>>>>>>> > should
>>>>>>>> > behave
>>>>>>>> > when a source entry is a file and there's *already* a file at the
>>>>>>>> > destination path.
>>>>>>>> > Should UAs overwrite the existing file at the destination path or
>>>>>>>> > not?
>>>>>>>> > Or maybe we should add an 'overwrite' flag to indicate if the script
>>>>>>>> > wants
>>>>>>>> > to overwrite an existing file or not?
>>>>>>>>
>>>>>>>> I'm open to a flag.  We're already up to 4 parameters to each of those
>>>>>>>> methods, though...5 is a bit ungainly.  I'm concerned that we might
>>>>>>>> find another flag to add at some point, and we'd then be up to 6.
>>>>>>>> What about adding an flags object, as in getFile, to allow for
>>>>>>>> expansion?
>>>>>>>
>>>>>>> Adding a flag or flags object (suppose the other thread about Flags will
>>>>>>> be settled) sounds good to me.
>>>>>>> Or I think it's also ok to explicitly disallow overwriting copy/move,
>>>>>>> i.e. specify that 'it is an error to copy or move a file or directory if
>>>>>>> there's already an entry'.  In this case it might be better to have another
>>>>>>> error code like ENTRY_EXISTS_ERR so that the user script can act
>>>>>>> differently.  (But in general having a handy option would make programming
>>>>>>> much easier in async context where every operation requires one or two
>>>>>>> callbacks.)
>>>>>>> If we're going to add 'overwrite' flag, there'll be a few more things to
>>>>>>> be made clear.
>>>>>>> For example I wonder how the overwriting copy/move should behave when
>>>>>>> there's already a **directory** at the destination path/name.
>>>>>>> Should the UA remove the existing directory and create a new entry at
>>>>>>> the same path?
>>>>>>> This sounds reasonable but it'll also provide a handy alternative way to
>>>>>>> remove a directory recursively.
>>>>>>
>>>>>> By the way how do you think about recursive remove?
>>>>>> Is there a reason (or past discussion) not to have recursive option in
>>>>>> remove?  (I mean, other than the fact that adding more and more options to
>>>>>> every method doesn't look very clean.)
>>>>>> I found that it's not very easy to remove a directory when there're
>>>>>> children in it -- it requires multiple DirectoryReader.readEntries and
>>>>>> Entry.remove in a nested way.
>>>>>> Thanks,
>>>>>>>
>>>>>>> Or should the UA create a new entry *under* the directory?
>>>>>>> This behavior doesn't sound like 'overwriting'.  The resulting path will
>>>>>>> be like 'destParentPath/name/name' which doesn't sound quite consistent with
>>>>>>> the spec either.
>>>>>>>
>>>>>>>> > Similarly I wondered if we'd want to have a 'recursive' flag for
>>>>>>>> > moveTo/copyTo.
>>>>>>>> > I think for directories we can simply assume that the user wants to
>>>>>>>> > move/copy
>>>>>>>> > them recursively, but it might be good to add some notion about that
>>>>>>>> > in the
>>>>>>>> > spec.
>>>>>>>>
>>>>>>>> How about I add a note indicating that directory copies are always
>>>>>>>> recursive?
>>>>>>>> I don't think we need anything for move.
>>>>>>>
>>>>>>> This sounds good to me.  Thanks!
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> > Thanks,
>>>>>>>> > [1] http://dev.w3.org/2009/dap/file-system/file-dir-sys.html
>>>>>>>> >
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>

Received on Thursday, 16 September 2010 23:56:30 UTC