Fork me on GitHub
#babashka
<
2021-02-06
>
borkdude10:02:59

user=> (map str (fs/which "java" {:all true}))
("/Users/borkdude/.jenv/versions/11.0/bin/java" "/Users/borkdude/.jenv/shims/java" "/usr/bin/java" "/Users/borkdude/.jenv/shims/java")
😃

borkdude10:02:05

Oh, this is also funny:

user=> (map str (fs/real-path "."))
("Users" "borkdude" "Dropbox" "dev" "clojure" "babashka" "fs")
Path implements iterable so it works out of the box with map, etc.

👍 3
borkdude13:02:07

Alrighty, babashka.fs alpha 4: https://babashka.org/fs/babashka.fs.html Please gloss over the docs and try it out from clojars if you have some time. I think this is the first release that will be in babashka soon. Not planning any changes unless feedback.

lread16:02:36

Very cool, I will certainly make use of this!

lread16:02:23

Minor: the docstring for directory-stream states that it “Returns a stream of all files in dir” - maybe make it clearer whether or not it includes files in subdirs too?

borkdude16:02:58

Yeah, I might just make this function private since I don't think it will be useful to anyone really. It's more like an impl detail

borkdude16:02:39

I think most people want list-dir

borkdude16:02:58

It's not recursive. For recursive you want glob

borkdude16:02:05

I'll add that to the docstring

borkdude16:02:41

"Returns all paths in dir as vector. For descending into subdirectories use glob."

lread16:02:58

If find docstring for file-name a bit confusing “Returns farthest component from the root as string, if any.” The name of the fn tells me the intent but the docstring kind of makes me scratch my head. The javadoc for .getName might be closer “Returns the name of the file or directory denoted by this abstract pathname.” Oh, does that mean file-name might not be a great name for the fn?

lread16:02:20

glob looks handy!

borkdude16:02:41

Do you have a different suggestion for the name?

borkdude16:02:44

or docstring

borkdude16:02:52

it's basically foo/bar/baz -> baz

lread16:02:43

Maybe just name? Because it can be a directory name too.

lread16:02:19

And that matches Java naming so would be perhaps familiar.

borkdude16:02:23

ok. I was hoping to avoid clashes with clojure fn names, but yeah, name might be the most clear

lread16:02:31

Oh… right.

borkdude16:02:07

note that clj-commons/fs has "base-name" which I also found confusing

lread16:02:41

Yeah. To avoid a clash maybe name-from? Or is that too contrived?

borkdude16:02:55

I'll just make it name.

lread16:02:38

For docstring, maybe more like Java: “Returns the name of the file or directory for x”

borkdude16:02:32

Oh btw, no, I named it after this one: https://docs.oracle.com/javase/7/docs/api/java/nio/file/Path.html#getFileName() Note that this API is mostly based on Path, not http://java.io.File

borkdude16:02:46

so actually this name is correct

borkdude16:02:11

> Returns the name of the file or directory denoted by this path as a Path object. The file name is the farthest element from the root in the directory hierarchy.

lread16:02:11

Ah… I see where your docstring inspiration came from!

borkdude16:02:42

I'll just add an example

lread16:02:00

Yeah, so a directory has a “file name”, I guess.

lread16:02:46

Still this “The file name is the farthest element from the root in the directory hierarchy.” is hard to parse, at least for my little brain.

borkdude16:02:55

> "Returns the name of the file or directory. E.g. (file-name \"foo/bar/baz\") returns \"baz\". "

lread16:02:17

Mucho better!

lread16:02:29

which will be handy in scripts! Should docstring explain what {:keys [:all]} does?

borkdude16:02:55

Probably yes

lread16:02:29

If you were hosting docs on cljdoc I would suggest wikilinks when referring to other fns in docstrings.

borkdude16:02:25

I'm a bit tired of including cljdoc into the equation because of broken analysis and not being able to update for a given version. I haven't looked at it for a while.

lread16:02:20

After rewrite-clj v1 is out, I’m probably going to head back to cljdoc to help out with issues.

borkdude16:02:48

Updated to: "Locates a program in (exec-paths) similar to the which Unix command. The :all option will return a vec of results instead of a single result."

borkdude16:02:17

Should it be 'similarly' ?

lread16:02:02

Maybe, but either reads fine to me.

borkdude16:02:33

codox updated

lread16:02:21

I like how you handle delete for files. A clearer design, in my opinion than (delete-file f & [silently]).

borkdude16:02:50

the impl is basically dictated by java.nio

borkdude16:02:08

Alternative implementation of which using already existing ingredients of fs:

(str (first (filter fs/executable? (fs/list-dirs (filter fs/exists? (fs/exec-paths)) "java"))))

"/Users/borkdude/.jenv/versions/11.0/bin/java"

borkdude16:02:56

"java" can be a glob like "*.{.clj, cljs}" too

lread16:02:54

I like that delete-tree has an option to not follow symlinks. Nice for safer deletes. Should following links be off by default and with an option to turn it :follow-links on? Or are you just matching a Java API?

borkdude16:02:05

Just matching the API

borkdude16:02:23

I tried to make up as little as possible myself

lread16:02:54

I think you put it together very nicely, though!

borkdude17:02:32

and thanks for the review too!

lread17:02:35

my pleasure!

borkdude17:02:24

The hardest function by far was glob which is not a standard function in the Java API

lread17:02:34

Docstring for delete might need an update?

borkdude17:02:47

So I first made a function walk-file-tree which basically mirrors Files/walkFileTree and built it on top of that

borkdude17:02:15

why update, what's wrong with it?

lread17:02:49

(defn delete
  "Deletes f via Path#delete. Returns nil if the delete was succesful,
  throws otherwise."
  [dir]
  (Files/delete (as-path dir)))

lread17:02:09

Doesn’t delete via Path#delete?

lread17:02:26

Oh and small typo in succesful

lread17:02:20

Same idea for delete-if-exists docstring.

borkdude17:02:01

Both updated now, thanks

lread17:02:29

If I use :nofolow-links on delete-tree and there is a symlink found will it just skip the symlink or throw?

borkdude17:02:16

It will just delete the symlink itself, I think, but that would be good to check ...

lread17:02:56

Oh right… that makes sense.

borkdude17:02:42

I think I must make a unit test for this...

lread17:02:45

Also, do you think a delete-tree-if-exists would be useful? To match delete-if-exists?

lread17:02:15

Also no mention of what delete-tree returns in docstring.

borkdude17:02:47

It always returns nil right now

lread17:02:03

And will throw if it can’t delete something, I assume.

borkdude17:02:18

The root is deleted using fs/delete

borkdude17:02:24

and this will throw indeed

lread17:02:36

Makes sense

lread17:02:11

I’m probably getting too nit picky but (exec-paths _)`` shows in API docs, which is minor but a bit weird.

borkdude17:02:19

hmm I think we can make delete-tree take follow-links instead of no-follow-links

borkdude17:02:28

since there is no Files thing for this

lread17:02:07

if it matches existing APIs enough, it would be a safer default choice.

borkdude17:02:51

I think we can base it on walk-file-tree

borkdude17:02:55

instead of what we do now

borkdude17:02:58

and that has follow-links

lread17:02:34

Docstring, I think is incorrect here:

(defn file-time->millis
  "Converts a java.nio.file.attribute.FileTime to a java.time.Instant."
  [^FileTime ft]
  (.toMillis ft))

lread17:02:06

Probably belongs on?

(defn file-time->instant [^FileTime ft]
  (.toInstant ft))

borkdude17:02:44

Please continue. I'll first fix the delete-tree and then go through the others

lread17:02:45

That’s all I got for now! Looking awesome!

borkdude17:02:55

afk for dinner now, back in a bit

lread17:02:04

lunchtime for me!

lread19:02:12

Oh… one other small thought… should side-effecting fns have a !?

borkdude19:02:43

I think in the context of immutable data structures that might make sense, but in the context of files, the function names are pretty explanatory about their side effects? E.g. the functions in http://clojure.java.io don't have !s either

👍 3
borkdude19:02:52

Strangely enough I can't make walk-file-tree follow links...

borkdude19:02:04

o wait, it's just my test

borkdude19:02:26

so the file gets visited, but delete needs to both delete symlink and the target dir.... hmm

borkdude19:02:27

@UE21H2HHD I hope this makes sense:

(testing "symlinks"
    (let [tmp-dir1 (temp-dir)
          tmp-dir2 (temp-dir)
          tmp-file (fs/path tmp-dir2 "foo")
          _ (fs/create-file tmp-file)
          link-path (fs/path tmp-dir1 "link-to-tmp-dir2")
          link (fs/create-sym-link link-path tmp-dir2)]
      (is (fs/exists? tmp-file))
      (fs/delete-tree tmp-dir1 {:follow-links true})
      (is (not (fs/exists? link)))
      (is (not (fs/exists? tmp-file)))
      (is (not (fs/exists? tmp-dir2)))
      (is (not (fs/exists? tmp-dir1))))
    (let [tmp-dir1 (temp-dir)
          tmp-dir2 (temp-dir)
          tmp-file (fs/path tmp-dir2 "foo")
          _ (fs/create-file tmp-file)
          link-path (fs/path tmp-dir1 "link-to-tmp-dir2")
          link (fs/create-sym-link link-path tmp-dir2)]
      (is (fs/exists? tmp-file))
      (fs/delete-tree tmp-dir1 {:follow-links false})
      (is (not (fs/exists? link)))
      (is (fs/exists? tmp-file))
      (is (fs/exists? tmp-dir2))
      (is (not (fs/exists? tmp-dir1)))))

borkdude19:02:53

The first case tests that the link + the target gets deleted

👍 3
borkdude19:02:02

The second case tests that only the links get deleted

👍 3
borkdude19:02:53

We could do this similarly for delete: if you provide :follow-links there as well, both the link and the target gets deleted....

👍 3
borkdude19:02:05

but one other possibility could be that only the target gets deleted... :/

👎 3
borkdude20:02:34

but if you delete the target, then what is a symlink without a target. that seems not right ;)

👍 3
borkdude20:02:52

so yeah, let's include that option there too

👍 3
lread20:02:07

yeah, agreed, deleting target but not link is wrong

borkdude20:02:56

I wonder why Files doesn't provide this behavior of follow-link for delete. We might just support it in delete-tree. If people want to delete both, they can use that, while delete follows the normal java behavior

borkdude20:02:34

ah, in delete I think you will have a conflicting behavior when you say: delete this link, but also follow the link. If the link then is a dir, you are not delete-tree-ing, so this can't remove the entire dir

borkdude20:02:12

so what's now on master probably makes most sense. also updated codox with your other suggestions

lread20:02:45

Good observation, yeah. So delete will delete a symlink but never follow it, right?

borkdude20:02:16

I'll also doc that

borkdude09:02:15

I think it might be better to remove the follow-links behavior in delete-tree as this seems uncommon. Not even rm -rf supports it.

borkdude09:02:14

Or I don't know. I'll ask a second opinion on this. There might be use cases, but it seems dangerous

lread13:02:03

fwiw, I have a bb script that does recursive delete, and I have it check for symlinks first and fail if it finds any. This might have been before I realized a delete only deletes the link... just the same I was obviously feeling very cautious about the danger accidentally over-deleting something.

borkdude13:02:17

right. so same as rm -rf

borkdude13:02:25

not including the option anymore

lread13:02:57

cool, good thinking

Max18:02:40

Is babashka.fs usable in normal java clojure? It seems like it’d be useful in the general case