Clojure

Improve docstring of clojure.java.io/delete-file to be clearer about intent of silently arg

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Release 1.5
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Environment:
    any
  • Patch:
    Code
  • Approval:
    Prescreened

Description

clojure.java.io/delete-file docstring takes an optional second arg `silently` and says:

Raise an exception if it fails unless silently is true.

This is confusing. The intent is that in the case of a failure, it will by default throw an exception. Or if the optional second arg is supplied and truthy, it will be returned instead.

Initially reported: https://groups.google.com/d/msg/clojure/T9Kvr0IL0kg/wcKBfR9w_1sJ

Approach: Make docstring sentence clearer per Stu's suggestion in comments:

If silently is nil or false, raise an exception on failure, else return the value of silently.

Patch: clj-1159.patch

Activity

Hide
AtKaaZ added a comment -

I kinda just realized it affects all versions since and including 1.2, because it appears that its implementation was the same since then.

If it's not meant to return the result of the delete, maybe it should specifically return nil and/or the doc say something?

Show
AtKaaZ added a comment - I kinda just realized it affects all versions since and including 1.2, because it appears that its implementation was the same since then. If it's not meant to return the result of the delete, maybe it should specifically return nil and/or the doc say something?
Hide
Sean Corfield added a comment -

As noted in a thread on the Clojure ML, you can pass a known value in the second argument position to detect a delete that failed:

(clojure.java.io/delete-file some-file :not-deleted)

This returns true on success and :not-deleted on failure.

However the docstring could be better worded to make that intention clear. Perhaps:

Delete file f. Return true if it succeeds. If silently is nil or false, raise an exception if it fails, else return the value of silently.
This allows you to detect whether the delete succeeded without catching an exception by passing a non-true truthy value as the second argument.

Show
Sean Corfield added a comment - As noted in a thread on the Clojure ML, you can pass a known value in the second argument position to detect a delete that failed: (clojure.java.io/delete-file some-file :not-deleted) This returns true on success and :not-deleted on failure. However the docstring could be better worded to make that intention clear. Perhaps: Delete file f. Return true if it succeeds. If silently is nil or false, raise an exception if it fails, else return the value of silently. This allows you to detect whether the delete succeeded without catching an exception by passing a non-true truthy value as the second argument.
Alex Miller made changes -
Field Original Value New Value
Labels delete-file evil or errormsgs io
Hide
Stuart Halloway added a comment -

I think 'If silently is nil or false, raise an exception if it fails, else return the value of silently.' is sufficient

Show
Stuart Halloway added a comment - I think 'If silently is nil or false, raise an exception if it fails, else return the value of silently.' is sufficient
Stuart Halloway made changes -
Approval Triaged [ 10120 ]
Alex Miller made changes -
Summary clojure.java.io/delete-file doesn't return the status of the deletion(true/false) Improve docstring of clojure.java.io/delete-file to be clearer about intent of silently arg
Attachment clj-1159.patch [ 15396 ]
Description initially reported it here(somehow):
https://groups.google.com/d/msg/clojure/T9Kvr0IL0kg/wcKBfR9w_1sJ

Basically clojure.java.io/delete-file doesn't ever return false (even when silently is true, it returns the value of silently), it's due to how it's implemented - but it's obvious from the code, so I'll stop here.

Thanks.

PS: this is what I'm using as my current workaround:
(defn delete-file
"
an implementation that returns the true/false status
which clojure.java.io/delete-file doesn't do(tested in 1.5.0-RC14)
"
  [f & [silently]]
  (let [ret (.delete (clojure.java.io/file f))]
    (cond (or ret silently)
      ret
      :else
      (throw (java.io.IOException. (str "Couldn't delete " f)))
      )
    )
  )

I'm sure you guys can find a better way, but as a clojure newbie(really!) that's what I have.
clojure.java.io/delete-file docstring takes an optional second arg `silently` and says:

{code}Raise an exception if it fails unless silently is true.{code}

This is confusing. The intent is that in the case of a failure, it will by default throw an exception. Or if the optional second arg is supplied and truthy, it will be returned instead.

*Initially reported:* https://groups.google.com/d/msg/clojure/T9Kvr0IL0kg/wcKBfR9w_1sJ

*Approach:* Make docstring sentence clearer per Stu's suggestion in comments:

{code}If silently is nil or false, raise an exception on failure, else return the value of silently.{code}

*Patch:* clj-1159.patch


Approval Triaged [ 10120 ] Prescreened [ 10220 ]
Patch Code [ 10001 ]
Alex Miller made changes -
Issue Type Defect [ 1 ] Enhancement [ 4 ]

People

  • Assignee:
    Unassigned
    Reporter:
    AtKaaZ
Vote (0)
Watch (1)

Dates

  • Created:
    Updated: