Skip to content

Change the remove operation to be part of a directory handle.#55

Merged
mkruisselbrink merged 1 commit into
masterfrom
change-remove-method
Jun 4, 2019
Merged

Change the remove operation to be part of a directory handle.#55
mkruisselbrink merged 1 commit into
masterfrom
change-remove-method

Conversation

@mkruisselbrink

Copy link
Copy Markdown
Contributor

Before each handle had a remove method that would remove the entry
represented by the handle. This changes it to instead have a removeEntry
method on a directory handle that will remove a child of that directory.

This means it is no longer possible to delete an individual file or
directory without having a handle to its parent, but otherwise feels
like a slightly nicer API shape to me.

This fixes#54

@pwnall pwnall left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM w/ one nit.

Thank you for the quick follow-up on the issue!

Comment thread index.bs
Promise<sequence<USVString>?> resolve(FileSystemHandle handle);

Promise<void> removeRecursively();
Promise<void> removeEntry(USVString name, optional FileSystemRemoveOptions options);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I personally like remove, the web platform seems to have settled on delete for this operation.

So, WDYT of delete, or deleteEntry?

Also fine to leave the name as-is and let the TAG decide when we submit a review.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll leave as is for now. Which web platform APIs were you referring to btw? I guess CacheStorage indeed uses delete, but for example DOM has removeChild and removeAttribute

Before each handle had a remove method that would remove the entry
represented by the handle. This changes it to instead have a removeEntry
method on a directory handle that will remove a child of that directory.

This means it is no longer possible to delete an individual file or
directory without having a handle to its parent, but otherwise feels
like a slightly nicer API shape to me.
@mkruisselbrink mkruisselbrink force-pushed the change-remove-method branch from 1a2c82b to bcd36d8 Compare June 4, 2019 23:15
@mkruisselbrink mkruisselbrink merged commit eeaffce into master Jun 4, 2019
@pwnall

pwnall commented Jun 4, 2019

Copy link
Copy Markdown
Collaborator

I was thinking primarily of whatwg/cookiestore#46. I agree we shouldn't block the API improvements in this PR on the name, though. I'll open an issue to track the name and let folks chime in there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FileSystemDirectoryHandle.removeRecursively() seems redundant with FileSystemHandle.remove()?

2 participants