[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#28834: 25.1; dired-do-copy: allow to copy into a nonexistent directo
From: |
Eli Zaretskii |
Subject: |
bug#28834: 25.1; dired-do-copy: allow to copy into a nonexistent directory |
Date: |
Mon, 16 Oct 2017 19:12:12 +0300 |
> From: Tino Calancha <tino.calancha@gmail.com>
> Cc: 28834@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> Date: Mon, 16 Oct 2017 12:47:06 +0900
>
> Thank you for the explanation. Let's discuss the updated patch:
Thanks, I have a few comments:
> diff --git a/doc/emacs/dired.texi b/doc/emacs/dired.texi
> index db5dea329b..0f37ac60ac 100644
> --- a/doc/emacs/dired.texi
> +++ b/doc/emacs/dired.texi
> @@ -646,6 +646,16 @@ Operating on Files
> Copy the specified files (@code{dired-do-copy}). The argument @var{new}
> is the directory to copy into, or (if copying a single file) the new
> name. This is like the shell command @code{cp}.
> +The option @var{dired-create-destination-dirs} controls whether Dired
> +should create non-existent directories in @var{new}.
This sentence is redundant (repeated right after it), and also uses
@var incorrectly.
> +@videnx dired-create-destination-dirs
^^^^^^^
A typo.
> +The option @code{dired-create-destination-dirs} controls whether Dired
> +should create non-existent directories in the destination while
> +copying/renaming files. The default value @code{never} means Dired
> +never creates such missing directories; the value @code{always},
> +means Dired automatically creates them; the value @code{prompt}
> +means Dired asks you for confirmation before creating them.
I think we generally use 'ask', not 'prompt' in these cases.
> diff --git a/etc/NEWS b/etc/NEWS
> index 716b0309a5..e5cec45426 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -56,6 +56,13 @@ whether '"' is also replaced in 'electric-quote-mode'. If
> non-nil,
>
> * Changes in Specialized Modes and Packages in Emacs 27.1
>
> +** Dired
> +
> +---
Since you've updated the manual, this should be "+++", not "---".
> +*** The new user option 'dired-create-destination-dirs' controls whether
> +'dired-do-copy' and 'dired-rename-file' must create non-existent
^^^^
"Should", not "must".
> +directories in the destination.
> +
> ** Edebug
>
> +++
> diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
> index 7e2252fcf1..0e415b7738 100644
> --- a/lisp/dired-aux.el
> +++ b/lisp/dired-aux.el
> @@ -1548,6 +1548,26 @@ dired-copy-file
>
> (declare-function make-symbolic-link "fileio.c")
>
> +(defcustom dired-create-destination-dirs 'never
> + "Whether Dired should create destination dirs when copying/removing files.
> +If never, don't create them.
> +If always, create them without ask.
> +If prompt, ask for user confirmation."
The symbols should be quoted: `never', `always', etc.
Btw, perhaps it's better to use nil instead of 'never'.
> +(defun dired--create-dirs (dir)
> + (unless (file-exists-p dir)
> + (pcase dired-create-destination-dirs
> + ('never nil)
> + ('always (dired-create-directory dir))
> + ('prompt
> + (when (yes-or-no-p (format "Create destination dir '%s'? " dir))
> + (dired-create-directory dir))))))
Is use of pcase really justified here?
> +(ert-deftest dired-test-bug28834 ()
> + "test for https://debbugs.gnu.org/28834 ."
> + (let* ((from (make-temp-file "from"))
> + (foo (make-temp-file "foo" 'dir))
> + (bar (file-name-as-directory (expand-file-name "bar" foo)))
> + (qux (file-name-as-directory (expand-file-name "qux" foo)))
> + (tmpdir temporary-file-directory)
> + (to-cp (expand-file-name "foo-cp" bar))
> + (to-mv (expand-file-name "foo-mv" qux))
> + (dired-create-destination-dirs 'always))
> + (unwind-protect
> + (progn
> + (dired-copy-file-recursive from to-cp nil)
> + (should (file-exists-p to-cp))
> + (dired-rename-file from to-mv nil)
> + (should (file-exists-p to-mv))
> + ;; Repeat the same with `dired-create-destination-dirs' set to
> 'never.
> + (dired-rename-file to-mv from nil)
> + (delete-file to-cp)
> + (delete-directory bar)
> + (delete-directory qux)
> + (let ((dired-create-destination-dirs 'never))
> + (should-error (dired-copy-file-recursive from to-cp nil))
> + (should-error (dired-rename-file from to-mv nil))))
This doesn't test the 3rd value. Why is that?