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

Add convenient method equivalent toTempDir:new('foo') #249

Closed
mamcx opened this issue Aug 15, 2023 · 8 comments · Fixed by #250
Closed

Add convenient method equivalent toTempDir:new('foo') #249

mamcx opened this issue Aug 15, 2023 · 8 comments · Fixed by #250

Comments

@mamcx
Copy link

mamcx commented Aug 15, 2023

I want to remove the dependabot for GHSA-mc8h-8q98-g5hr and I miss the old TempDir:new('foo') call.

The available constructors have different semantics and the proposed solution is kinda verbose: tempfile::Builder::new().prefix(prefix).tempdir()

@Stebalien
Copy link
Owner

You need to specify a prefix for the tempdir name? Or do you just need to specify a location. If you need to specify a location, TempDir::new_in(...) should do it.

@mamcx
Copy link
Author

mamcx commented Aug 15, 2023

TempDir::new_in(...) does not work because that needs that folder created, the old tempdir crate make it.

@Stebalien
Copy link
Owner

Hm. I don't think I'm on the same page. Could you explain exactly what you're trying to do and the desired effect?

@mamcx
Copy link
Author

mamcx commented Aug 16, 2023

The desired effect is to recover the behaviour of the old Tempdir::new, I think the equivalent now is tempfile::Builder::new().prefix(prefix).tempdir()?

It add + create a folder with the prefix...

@Stebalien
Copy link
Owner

I'll consider it, but a stronger motivation beyond "I want to do this" would be helpful. Personally, I usually care about where my temporary files go, but rarely care about their names.

@mamcx
Copy link
Author

mamcx commented Aug 16, 2023

Understood. We have a battery of test that run on parallel and we use the same temp_folder but distinct names for each, so the idea is that we have:

/tmp/test_a
/tmp/test_b
/tmp/test_c

without causing interferences, and having names help in debugging issues, that is very hard to do if the names are machine generated!

Also if used outside the test, this show up in logs and is nicer to see that this particular temp file was part of certain "context".

@Stebalien
Copy link
Owner

Yeah, that makes sense. My concern is that I could add TempDir::with_prefix(prefix), but then what about TempDir::with_prefix_in(prefix, directory)? In retrospect, new on both TempDir and NamedTempFile should have always taken a prefix (costs the user nothing to specify one) but that ship has sailed.

Yeah, ok, I should probably just do it. Now to the question of names:

  • with_prefix (with_prefix_in)
  • prefixed (prefixed_in)
  • new_with_prefix (new_with_prefix_in)
  • new_prefixed (new_prefixed_in).

I'm thinking the first one.

@mamcx
Copy link
Author

mamcx commented Aug 16, 2023

Yes with_ sounds good.

Stebalien added a commit that referenced this issue Aug 16, 2023
- `TempDir::with_prefix(prefix)`
- `TempDir::with_prefix_in(prefix, directory)`
- `TempFile::with_prefix(prefix)`
- `TempFile::with_prefix_in(prefix, directory)`

fixes #249
Stebalien added a commit that referenced this issue Aug 16, 2023
- `TempDir::with_prefix(prefix)`
- `TempDir::with_prefix_in(prefix, directory)`
- `TempFile::with_prefix(prefix)`
- `TempFile::with_prefix_in(prefix, directory)`

fixes #249
Stebalien added a commit that referenced this issue Aug 18, 2023
- `TempDir::with_prefix(prefix)`
- `TempDir::with_prefix_in(prefix, directory)`
- `TempFile::with_prefix(prefix)`
- `TempFile::with_prefix_in(prefix, directory)`

fixes #249
Stebalien added a commit that referenced this issue Aug 18, 2023
…250)

- `TempDir::with_prefix(prefix)`
- `TempDir::with_prefix_in(prefix, directory)`
- `TempFile::with_prefix(prefix)`
- `TempFile::with_prefix_in(prefix, directory)`

fixes #249
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 a pull request may close this issue.

2 participants