Skip to content
This repository has been archived by the owner on Aug 20, 2021. It is now read-only.

deprecate into_path(), rename persist() #44

Closed
vitiral opened this issue Feb 6, 2018 · 4 comments
Closed

deprecate into_path(), rename persist() #44

vitiral opened this issue Feb 6, 2018 · 4 comments

Comments

@vitiral
Copy link

vitiral commented Feb 6, 2018

Feature Request

This is a feature/API change request to rename into_path() to persist(), while deprecating into_path().

Reasoning

While I understand the logic behind calling it into_path() (since it "converts" the temporary directory into an "ordinary path"), into_path() actually performs an action from the programmers point of view, namely it removes the invariant that the temporary directory will be deleted on drop.

This is made even more confusing by the documentation, which says: "This destroys the TempDir without deleting the directory represented by the returned Path." -- from a lingual point of view it sounds like we are destroying the directory

Solution

/// Consumes the `TempDir` object without deleting the directory, 
/// returning the `PathBuf` where it is located.
///
/// The directory will no longer be automatically deleted.
fn persist(self) -> PathBuf { self.into_path() }
@vitiral
Copy link
Author

vitiral commented Feb 6, 2018

For reference: the ergo_fs::PathTmp object (which wraps TempDir) has persist() instead of into_path(). If my reasonng is in error then it would be good for me to know while the library is still young!

@KodrAus
Copy link
Contributor

KodrAus commented Feb 7, 2018

Hi @vitiral!

We're actually moving the tempdir API into the tempfile crate so folks can have consistent temporary files and directories. That work has just been merged in.

Personally, I think persist is a better name, and is consistent with tempfile::NamedTempFile::persist.

Would you like to open this issue on the tempfile repo?

@vitiral
Copy link
Author

vitiral commented Feb 7, 2018

done! See the linked issue.

I didn't know about tempfile -- I will make sure it gets used in ergo_fs as well!

@KodrAus
Copy link
Contributor

KodrAus commented Feb 12, 2018

Thanks! Since we've got Stebalien/tempfile#42 to track this now I'll close the issue here.

@KodrAus KodrAus closed this as completed Feb 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants