rejeep / f.el

Modern API for working with files and directories in Emacs
GNU General Public License v3.0
680 stars 68 forks source link

[feature-request + contribution] recursive f-mkdir #108

Closed ChauhanT closed 2 years ago

ChauhanT commented 2 years ago

Hi, I was looking for a way to implement recursive mkdir (so mkdir -p). Since the library does not have such a function out of the box, I would like to make a feature request for this. I am not sure how to do feature requests...

I can go one step further, and propose some code which does this. I have called it f-mkdir-recursive. It uses the sneaky f-traverse-upwards function and FILO (push/pop). If it is good, quality-wise, I would like to contribute it to f.el.

(defun f-mkdir-recursive (dir &optional ack-p)
  "Recursively create a directory if it doesn't already exist. The second argument prints 
   an acknowledgment with the path of the newly created directory."
  (interactive)

  (let (dirs-to-mkdir)

    (f-traverse-upwards
     (lambda (path)
       (let ((f-exists-p (f-exists? path)))
         (unless f-exists-p
           (push path dirs-to-mkdir))
         f-exists-p))
     dir)

    (dolist (dir-to-mkdir dirs-to-mkdir)
      (f-mkdir dir-to-mkdir))

    (when (and ack-p dirs-to-mkdir)
      (message "Created directory %s." dir))))

All suggestions about how to make the function more efficient are super welcome. And if the maintainers think I can contribute to f.el, I'll be happy to! I just don't know how contributing code works in terms of the git commands etc....

EDIT: I renamed the function from f-mkdir-p to f-mkdir-recursive since -p in elisp is used for other things (I am still not clear on what these things are, but they definitely don't include recursion).

Phundrak commented 2 years ago

I would rename the function with another prefix than -p'. In Elisp, this prefix is used to indicate predicates, such as withf-exists-p' which checks whether a path exists or not. Something like `f-mkdir-recursive' would be better fitted I think.

The documentation should have a sole first line no longer than 80 characters. The rest of the documentation should be its own paragraph.

I would recommend you to read this guide regarding the style to adopt when writing Elisp: https://github.com/bbatsov/emacs-lisp-style-guide

I don’t really see the point in this function being interactive. It requires an argument but you provide no way of providing it interactively, and `f-mkdir' isn’t interactive to begin with.

To create parity with f-mkdir', I believe it would be also better if the argumentdir' were a &rest' argument, and no feedback is to be given when the directory is created, unless maybe when it is called interactively, but that wouldn’t be an expected behavior regarding how f-mkdir' and `make-directory' both work.

Lastly, why didn’t you use `make-directory'? It has a much faster implementation. ,---- (let ((path "/tmp/test-a")) (benchmark-run 10000 (progn (make-directory path t) (f-delete path)))) ;; => (1.971115701 0 0.0)
(let ((path "/tmp/test-a"))
(benchmark-run 10000
(f-mkdir path)
(f-delete path)))
;; => (2.036881876 0 0.0)
;; A little bit slower
(let ((path "/tmp/test-a"))
(benchmark-run 10000
(f-mkdir-p path)
(f-delete path)))
;; => (3.6355537449999997 0 0.0)
;; Almost twice as slow

`----

I just don't know how contributing code works in terms of the git commands etc....

  1. Create your own fork of f.el
  2. (Optional, but appreciated) Create a branch prefixed with `feature/' and name it with the feature you want to implement
  3. Once your feature is ready, submit a pull request from your branch to this repository.
  4. Your code will be reviewed on the pull request. If maintainers require changes in your code, push the new changes to the same branch you’ve been working on, the new commits will get added automatically to your pull request.
  5. Once your code is deemed good enough, congrats, a maintainer will merge your code into the main repository!

-- Lucien “Phundrak” Cartier-Tilet https://phundrak.com (Français) https://phundrak.com/en (English) Sent from GNU/Emacs

ChauhanT commented 2 years ago

Yeah, I think I was editing my post just as you were writing your message. I actually went with f-mkdir-recursive!

make-directory is rather slow through recursion (about twice as slow in the following test).

#+BEGIN_SRC emacs-lisp
(let ((path "~/make-directory/sub"))
  (benchmark-run 10000
    (make-directory path t)))
#+END_SRC

#+RESULTS:
| 1.340552049 | 2 | 0.22640519299999795 |

#+BEGIN_SRC emacs-lisp
(let ((path "~/f-mkdir-recursive/sub"))
  (benchmark-run 10000
    (f-mkdir-recursive path)))
#+END_SRC

#+RESULTS:
| 0.661089928 | 1 | 0.11333148199999954 |

You are right. Probably a key idea for the library is to try and maintain parity with f-mkdir. I'll try to make some changes and send a pull request some time this week.

Thanks very much for the link to the style guide and the tips about how to submit a possible contribution!

Phundrak commented 2 years ago

Closing this due to the clarification of the behaviour of f-mkdir and the addition of f-mkdir-full-path.