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

feat: add with-temp-dir macro #37

Merged
merged 3 commits into from
Nov 19, 2021

Conversation

hugoduncan
Copy link
Contributor

The macro takes a sequence of binding-name, options pairs.

The body is evaluated with binding-name bound to a java.io.File path to
a temporary directory, created by passing options to create-temp-dir.

The temporary directories are deleted with delete-tree in the reverse
of the order in which they were declared.

Options are as for create-temp-dir.

Adds an exported clj-kondo config for the macro.

The macro takes a sequence of binding-name, options pairs.

The body is evaluated with binding-name bound to a java.io.File path to
a temporary directory, created by passing options to create-temp-dir.

The temporary directories are deleted with delete-tree in the reverse
of the order in which they were declared.

Options are as for create-temp-dir.

Adds an exported clj-kondo config for the macro.
(defmacro with-temp-dir
"bindings [binding-name options ...]

Evaluate body with binding-name bound to a java.io.File path to a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the binding-name is bound to a file. in babashka.fs this is either a nio Path or java io File. You should not rely on this detail but if you care specifically, use fs/file or fs/path to convert between the two. So I would just call this "file" without specifying the type.

of the order in which they were declared.

Options is a map with the keys as for create-temp-dir."
{:arglists '[[[binding-name options ...] & body]]}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the practical use case for multiple tmp dirs? Just curious about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I've ever actually used that feature. Removing would simplify the code. I'll do that.


(symbol? (bindings 0))
(with-temp-dir-fn (subvec bindings 0 2)
(if-let [more-bindings (not-empty (subvec bindings 2))]
Copy link
Contributor

@borkdude borkdude Nov 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe not-empty can just be seq here?
EDIT: oh, right, you want to preserve the type.

@borkdude
Copy link
Contributor

Added a few questions/comments. Otherwise, this looks really good!

@hugoduncan
Copy link
Contributor Author

I also wasn't sure if the path to the edm config was correct - babashka/fs/ vs babashka/babashka.fs/?

- Remove multiple dir support
- Doc string
@borkdude
Copy link
Contributor

@hugoduncan Just babashka/fs would do.

@hugoduncan
Copy link
Contributor Author

Path changed

@borkdude borkdude merged commit f4c7394 into babashka:master Nov 19, 2021
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 this pull request may close these issues.

2 participants