-
Notifications
You must be signed in to change notification settings - Fork 122
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
Add NamedTempFile<F>, Builder::{make,make_in}, and NamedTempFile::from_parts #177
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jasonwhite
changed the title
Make NamedTempFile more generic
Add NamedTempFile<F>, Builder::{make,make_in}, and NamedTempFile::from_parts
Apr 9, 2022
jasonwhite
commented
Apr 9, 2022
jasonwhite
commented
Apr 9, 2022
Stebalien
approved these changes
Apr 9, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small nits, otherwise LGTM.
This enables using it with non-File types, like a `UnixListener`. This should be a backwards compatible change.
This enables the user to do more generic temporary file path creation, such as creating a temporary UNIX domain socket or creating a temporary `tokio::fs::File` (without using `tokio::fs::File::from_std`).
Thanks for the review! All review comments should be addressed. Let me know if there's anything else. Edit: FYI, I also ran |
Stebalien
approved these changes
Apr 10, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are two main parts to this PR. Unfortunately GitHub doesn't really have the ability to make PRs depend on each other. Let me know if you want this split up into separate PRs.
Fixes #175
NamedTempFile<F = File>
This adds a generic parameter to
NamedTempFile
. This enables using it with non-File types, such as:std::os::unix::net::UnixListener
,tokio::net::UnixListener
, ortokio::fs::File
This should be a backwards compatible change. To do this, some
File
-specific stuff is only implemented forNamedTempFile<File>
.Builder::{make,make_in}
This enables the user to leverage the file-path building part of the library while providing their own custom file creation callback. This is useful for creating UNIX domain sockets.
It was mentioned in #175 to provide a helper for creating sockets, but since UNIX domain sockets are only on UNIX, I believe it would be the only platform-dependent function. Thus, I held off on making that change. Happy to reconsider this though.
Note that
NamedTempFile::from_parts
was added to implementBuilder::make_in
. This could be made a private function, but it seems useful in the public API as well.