Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete/Rename URIs #1509

Closed
squash opened this issue Nov 5, 2020 · 4 comments
Closed

Delete/Rename URIs #1509

squash opened this issue Nov 5, 2020 · 4 comments
Assignees
Labels
blocker Items that would block a forthcoming release enhancement New feature or request Storage changes relating to Storage Repositories and URIs

Comments

@squash
Copy link

squash commented Nov 5, 2020

Is your feature request related to a problem? Please describe:

Currently there is no straightforward way to remove or rename a file by URI.

Is it possible to construct a solution with the existing API?

I am implementing a hacky solution that breaks the URI apart into a normal file path, does the required operation, and then (for renames) parses a new URI.

Describe the solution you'd like to see:

It would be nice if this functionality were included as part of the URI type

@andydotxyz
Copy link
Member

Indeed. Let's get these in. We could not add them easily in 1.4 as it would be a breaking change in the URI type - so we will pop it in 2.0

@andydotxyz andydotxyz added the enhancement New feature or request label Nov 5, 2020
@andydotxyz andydotxyz assigned andydotxyz and unassigned andydotxyz Dec 4, 2020
@andydotxyz andydotxyz added the blocker Items that would block a forthcoming release label Dec 11, 2020
charlesdaniels added a commit to charlesdaniels/fyne that referenced this issue Dec 17, 2020
The underlying work to provide implementations of these methods has not
been done yet pending discussion, so this will break the build.

This also adds Destroy() as described in fyne-io#1509, though again no
implementation is defined yet.

This also adds detailed comments to the members of the URI interface, to
make it clear what is expected of URI implementations.
@charlesdaniels charlesdaniels mentioned this issue Dec 17, 2020
5 tasks
@charlesdaniels
Copy link
Member

I have rolled this into #1665. I think adding delete functionality via URI.Destroy() is a good idea, but I'm not sold on Rename(), as this merely composes several existing operations (create, write, delete). On the other hand, exposing this to the underlying implementation as a Rename() operation may allow for optimizations (e.g. doing the rename on the remote end, rather than pulling the data down and writing it back over the wire for a remote fs).

Thoughts?

@andydotxyz
Copy link
Member

All good points I guess.
Worth noting that if we don't add Rename now it cannot appear until 3.0 as it will break the URI API.
Doing that inside the app is indeed possible - but could produce bad intermediate states (like create, write, delete where delete fails leaves duplicates...).

Not sure that I prefer Destroy() over Delete() though (we use it internally to free memory which I think is different.
Usually the choice is between Delete and Remove.

@andydotxyz
Copy link
Member

Landed, great job thanks @charlesdaniels and the team.

@charlesdaniels charlesdaniels added the Storage changes relating to Storage Repositories and URIs label May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Items that would block a forthcoming release enhancement New feature or request Storage changes relating to Storage Repositories and URIs
Projects
None yet
Development

No branches or pull requests

3 participants