Add Tempfile.tempname accepting block

[straight-shoota]

This PR adds an overload to Tempfile.tempname which yields the path and ensures it is cleaned up after exiting the block.

A possible use case is detailed in #5811

Followup on #5360

[straight-shoota]

#4096 is similar but explicitly for directories and creates a directory at the path.

[woodruffw]

Awesome! I should have thought about this when I initially added tempname 🙂

[ysbaddaden]

That's weird: I'm asking for a String but suddenly the methods deletes a file or directory on disk? Requiring tempfile would now inject `FileUtils, too.

It's a good snippet to put into test helpers, but I'm not sure this should be pushed to stdlib...

[Sija]

I second @ysbaddaden here, concept itself might be useful enough to add it as a new method, or feature behind an argument (sth like Tempfile.new(cleanup: true)), though mixing it with #tempname which is supposed to return just that - name, String - seems confusing at least.

[straight-shoota]

So you'r basically objecting the name, not the concept? I see the point of confusion, but on the other hand it just follows a common idiom in Crystal, to have a yielding method overload which adds some cleanup to the non-yielding behaviour.

@ysbaddaden Removing a directory recursively should either be implemented outside FileUtils or we need to accept it being included in some other places. It's just such a common feature. We've already had this issue in other places.

[RX14]

The core problem is that we're yielding a String, not a Path object. If you consider a hypothetical future where we merge the Path object, and this PR yielded a path object and then called #delete on the path after the block ran, I don't think there would be any complaint.

The core problem with this PR is that it has to work within the confines of Crystal's existing conflation of paths and strings, which is why I'm ok with this PR. We'll fix up this PR when we merge #5550.

This PR also shows exactly why File vs FileUtils vs Dir is stupid (we don't implement recursive directory deletion in the corelib, because it's arbitrarily defined on FileUtils instead of File, and now we need it in the corelib). Another thing which will be fixed by #5550. This PR is essentially before it's time, and makes perfect sense if #5550 is merged.

[Sija]

I object. Such behaviour would be more fitting for a Tempfile#open_and_cleanup(&block) method, not one returning temporary String. Mixing generation of random String with deleting such file on disk would be just simply confusing.

[ysbaddaden]

This pull request isn't ahead of its time, it should just be a method in spec. If it happens to be very useful across many projects, then maybe we could consider moving this to stdlib, otherwise let's not bother. With a proper stdlib it's just a few lines to write anyway.

Let's fix things in the right order, instead of adding new features on a whim. Recursive deletion is indeed important: let's work on that! Forget about Tempfile and FileUtils and work on Path, rework Dir and File, so they become so good that maybe we'll just drop Tempfile and FileUtils in the end.

[straight-shoota]

I'm closing this. Since Tempfile was removed in #6485 this PR would need a refactoring. I'm not going to invest that without core members supporting this.

#5951 solved the most important use case in specs, but I still think this would also be useful as a method outside of specs. Maybe there will be a new PR at some point...