unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#28834: 25.1; Dired+ not copy file to nonexistent directory.
@ 2017-10-14 10:54 alexei28
  2017-10-14 15:50 ` Drew Adams
  2017-10-15  4:42 ` bug#28834: 25.1; dired-do-copy: allow to copy into a " Tino Calancha
  0 siblings, 2 replies; 12+ messages in thread
From: alexei28 @ 2017-10-14 10:54 UTC (permalink / raw)
  To: 28834

[-- Attachment #1: Type: text/plain, Size: 780 bytes --]

Hi!

 

Windows 10, Emacs 25.1, Dired+

 

I want to copy file from "d:/TEMP/test/test.txt" to nonexistent directory
"d:/TEMP/test/new/" in tby package Dired+

 

My steps in Dired+ buffer:

1. Run command "split-window-vertically"

2. In top and bottom buffers has directory "d:/TEMP/test/"

3. Press "Shift c" (copy file)

4. In minibuffer show message: "Copy text.txt to : d:/TEMP/test/"

5. I replace target path by: "d:/TEMP/test/new/test.txt" and press return

6. I get the next error message:

 

Copy 'd:/TEMP/test/test.txt' to 'd:/TEMP/test/new/test.txt' failed:

(file-error Copying file Operation not permitted d:/TEMP/test/test.txt
d:/TEMP/test/new/test.txt)

 

It would be good if Emacs can autocreate not exist directory when copy/move
files.

Thank you.

 

Alex


[-- Attachment #2: Type: text/html, Size: 3679 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#28834: 25.1; Dired+ not copy file to nonexistent directory.
  2017-10-14 10:54 bug#28834: 25.1; Dired+ not copy file to nonexistent directory alexei28
@ 2017-10-14 15:50 ` Drew Adams
  2017-10-15  4:42 ` bug#28834: 25.1; dired-do-copy: allow to copy into a " Tino Calancha
  1 sibling, 0 replies; 12+ messages in thread
From: Drew Adams @ 2017-10-14 15:50 UTC (permalink / raw)
  To: alexei28, 28834

To be clear, this is really about vanilla Dired, not Dired+.
The title should perhaps be changed.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#28834: 25.1; dired-do-copy: allow to copy into a nonexistent directory
  2017-10-14 10:54 bug#28834: 25.1; Dired+ not copy file to nonexistent directory alexei28
  2017-10-14 15:50 ` Drew Adams
@ 2017-10-15  4:42 ` Tino Calancha
  2017-10-15 14:21   ` Eli Zaretskii
  2017-10-15 15:43   ` Drew Adams
  1 sibling, 2 replies; 12+ messages in thread
From: Tino Calancha @ 2017-10-15  4:42 UTC (permalink / raw)
  To: 28834

"alexei28" <alexei28@gmail.com> writes:

> I want to copy file from "d:/TEMP/test/test.txt" to nonexistent directory "d:/TEMP/test/new/" with Dired
> Copy ‘d:/TEMP/test/test.txt’ to ‘d:/TEMP/test/new/test.txt’ failed:
>
> (file-error Copying file Operation not permitted d:/TEMP/test/test.txt d:/TEMP/test/new/test.txt)
>
> It would be good if Emacs can autocreate not exist directory when copy/move files.


It might has sense that Dired handle the creation of those destination dirs.
WDOT?

--8<-----------------------------cut here---------------start------------->8---
commit 1325ea21a069b5c539157390fb5df2eca76adecb
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Sun Oct 15 13:34:08 2017 +0900

    Dired: Allow to copy/rename files into a non-existent dir
    
    * lisp/dired-aux.el (dired--create-dirs): New defun.
    (dired-copy-file-recursive, dired-rename-file): Use it (Bug#28834).
    * doc/emacs/dired.texi (Operating on Files): Update manual.
    * etc/NEWS (Changes in Specialized Modes and Packages in Emacs 27.1)
    Announce feature.
    * lisp/dired-aux-tests.el (dired-test-bug28834): Add test.

diff --git a/doc/emacs/dired.texi b/doc/emacs/dired.texi
index db5dea329b..48013a3221 100644
--- a/doc/emacs/dired.texi
+++ b/doc/emacs/dired.texi
@@ -645,7 +645,8 @@ Operating on Files
 @item C @var{new} @key{RET}
 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}.
+name.  Dired creates any non-existent directories in @var{new}.  This is
+like the shell command @code{cp}.
 
 @vindex dired-copy-preserve-time
 If @code{dired-copy-preserve-time} is non-@code{nil}, then copying
@@ -674,7 +675,8 @@ Operating on Files
 @cindex moving files (in Dired)
 @item R @var{new} @key{RET}
 Rename the specified files (@code{dired-do-rename}).  If you rename a
-single file, the argument @var{new} is the new name of the file.  If
+single file, the argument @var{new} is the new name of the file.  Dired
+creates any non-existent directories in @var{new}.  If
 you rename several files, the argument @var{new} is the directory into
 which to move the files (this is like the shell command @command{mv}).
 
diff --git a/etc/NEWS b/etc/NEWS
index 716b0309a5..08291fc39f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -56,6 +56,12 @@ whether '"' is also replaced in 'electric-quote-mode'.  If non-nil,
 \f
 * Changes in Specialized Modes and Packages in Emacs 27.1
 
+** Dired
+
+---
+*** dired-do-copy and dired-rename-file now create non-existent
+directories in the destination.
+
 ** Edebug
 
 +++
diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 7e2252fcf1..fcea8f1b13 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -1548,6 +1548,10 @@ dired-copy-file
 
 (declare-function make-symbolic-link "fileio.c")
 
+(defun dired--create-dirs (dir)
+  (unless (file-exists-p dir)
+    (dired-create-directory dir)))
+
 (defun dired-copy-file-recursive (from to ok-flag &optional
 				       preserve-time top recursive)
   (when (and (eq t (car (file-attributes from)))
@@ -1564,6 +1568,7 @@ dired-copy-file-recursive
 	  (if (stringp (car attrs))
 	      ;; It is a symlink
 	      (make-symbolic-link (car attrs) to ok-flag)
+            (dired--create-dirs (file-name-directory to))
 	    (copy-file from to ok-flag preserve-time))
 	(file-date-error
 	 (push (dired-make-relative from)
@@ -1573,6 +1578,7 @@ dired-copy-file-recursive
 ;;;###autoload
 (defun dired-rename-file (file newname ok-if-already-exists)
   (dired-handle-overwrite newname)
+  (dired--create-dirs (file-name-directory newname))
   (rename-file file newname ok-if-already-exists) ; error is caught in -create-files
   ;; Silently rename the visited file of any buffer visiting this file.
   (and (get-file-buffer file)
diff --git a/test/lisp/dired-aux-tests.el b/test/lisp/dired-aux-tests.el
index d41feb1592..d8114d8401 100644
--- a/test/lisp/dired-aux-tests.el
+++ b/test/lisp/dired-aux-tests.el
@@ -40,5 +40,25 @@
           (should-not (dired-do-shell-command "ls ? ./`?`" nil files)))
       (delete-file foo))))
 
+(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)))
+    (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)))
+      ;; clean up
+      (delete-directory foo 'recursive)
+      (delete-file from))))
+
+
 (provide 'dired-aux-tests)
 ;; dired-aux-tests.el ends here

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-10-15
Repository revision: eed3a3d9e95d2c5346a23c9d92ca4e5848330183





^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#28834: 25.1; dired-do-copy: allow to copy into a nonexistent directory
  2017-10-15  4:42 ` bug#28834: 25.1; dired-do-copy: allow to copy into a " Tino Calancha
@ 2017-10-15 14:21   ` Eli Zaretskii
  2017-10-15 14:47     ` Tino Calancha
  2017-10-15 15:43   ` Drew Adams
  1 sibling, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2017-10-15 14:21 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 28834

> From: Tino Calancha <tino.calancha@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, Drew Adams <drew.adams@oracle.com>
> Date: Sun, 15 Oct 2017 13:42:48 +0900
> 
> It might has sense that Dired handle the creation of those destination dirs.
> WDOT?

I think, if we accept this, it must be an optional feature, by default
off.

Thanks.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#28834: 25.1; dired-do-copy: allow to copy into a nonexistent directory
  2017-10-15 14:21   ` Eli Zaretskii
@ 2017-10-15 14:47     ` Tino Calancha
  0 siblings, 0 replies; 12+ messages in thread
From: Tino Calancha @ 2017-10-15 14:47 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 28834, Tino Calancha



On Sun, 15 Oct 2017, Eli Zaretskii wrote:

>> From: Tino Calancha <tino.calancha@gmail.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>, Drew Adams <drew.adams@oracle.com>
>> Date: Sun, 15 Oct 2017 13:42:48 +0900
>>
>> It might has sense that Dired handle the creation of those destination dirs.
>> WDOT?
>
> I think, if we accept this, it must be an optional feature, by default
> off.
Sounds good to me.
I will prepare a new patch tomorrow.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#28834: 25.1; dired-do-copy: allow to copy into a nonexistent directory
  2017-10-15  4:42 ` bug#28834: 25.1; dired-do-copy: allow to copy into a " Tino Calancha
  2017-10-15 14:21   ` Eli Zaretskii
@ 2017-10-15 15:43   ` Drew Adams
  2017-10-16  3:47     ` Tino Calancha
  1 sibling, 1 reply; 12+ messages in thread
From: Drew Adams @ 2017-10-15 15:43 UTC (permalink / raw)
  To: Tino Calancha, 28834

> > I want to copy file from "d:/TEMP/test/test.txt" to nonexistent
> > directory "d:/TEMP/test/new/" with Dired
> > Copy ‘d:/TEMP/test/test.txt’ to ‘d:/TEMP/test/new/test.txt’ failed:
> >
> > (file-error Copying file Operation not permitted d:/TEMP/test/test.txt
> d:/TEMP/test/new/test.txt)
> >
> > It would be good if Emacs can autocreate not exist directory when
> > copy/move files.
> 
> It might has sense that Dired handle the creation of those destination
> dirs.  WDOT?

The enhancement request is "It would be good if Emacs CAN
autocreate not exist directory when copy/move files."

The operative word here is "CAN".  A change to _always_
create missing dirs would be inappropriate.  (I have
not looked at your patch in detail.)

It is important that Dired not just create dirs on its
own, especially since that would be an incompatible
behavior change, and also because it would mean also
creating "missing dirs" as a result of a user making
a typing mistake.  At the very least, users should be
able to control this by way of confirming.

What should be done is to offer users the POSSIBILITY
of having Dired create the missing directories.

This can be done in various ways:

1. Have a different command, which users can bind in
   place of the existing command.

2. Have a user option that controls whether Dired creates
   missing dirs by default, i.e., without confirmation.

3. Have Dired ask whether to create missing directories.

I think probably 2+3 would be good: have Dired prompt
when dirs are missing, but have an option that lets
users who never want to be prompted bypass prompting.
The option could be 3-valued: `always-prompt',
`always-create', `never-create' (or other names).





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#28834: 25.1; dired-do-copy: allow to copy into a nonexistent directory
  2017-10-15 15:43   ` Drew Adams
@ 2017-10-16  3:47     ` Tino Calancha
  2017-10-16 16:12       ` Eli Zaretskii
  2017-10-17  1:49       ` Drew Adams
  0 siblings, 2 replies; 12+ messages in thread
From: Tino Calancha @ 2017-10-16  3:47 UTC (permalink / raw)
  To: Drew Adams; +Cc: 28834

Drew Adams <drew.adams@oracle.com> writes:

> The enhancement request is "It would be good if Emacs CAN
> autocreate not exist directory when copy/move files."
>
> The operative word here is "CAN".
> This can be done in various ways:
>
> 1. Have a different command, which users can bind in
>    place of the existing command.
>
> 2. Have a user option that controls whether Dired creates
>    missing dirs by default, i.e., without confirmation.
>
> 3. Have Dired ask whether to create missing directories.
>
> I think probably 2+3 would be good: have Dired prompt
> when dirs are missing, but have an option that lets
> users who never want to be prompted bypass prompting.
> The option could be 3-valued: `always-prompt',
> `always-create', `never-create' (or other names).

Thank you for the explanation.  Let's discuss the updated patch:
--8<-----------------------------cut here---------------start------------->8---
commit b656651e9268f5dd646933b992bd37771c3e99ca
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Mon Oct 16 12:41:41 2017 +0900

    Allow to copy/rename file into a non-existent dir
    
    * lisp/dired-aux.el (dired-create-destination-dirs): New option.
    (dired--create-nonexistent-dirs): New defun.
    (dired-copy-file-recursive, dired-rename-file): Use it (Bug#28834).
    * lisp/dired-aux-tests.el (dired-test-bug28834): Add test.
    * doc/emacs/dired.texi (Operating on Files): Update manual.
    * etc/NEWS (Changes in Specialized Modes and Packages in Emacs 27.1)
    Announce this change.

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}.
+
+@videnx dired-create-destination-dirs
+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.
 
 @vindex dired-copy-preserve-time
 If @code{dired-copy-preserve-time} is non-@code{nil}, then copying
@@ -678,6 +688,9 @@ Operating on Files
 you rename several files, the argument @var{new} is the directory into
 which to move the files (this is like the shell command @command{mv}).
 
+The option @var{dired-create-destination-dirs} controls whether Dired
+should create non-existent directories in @var{new}.
+
 Dired automatically changes the visited file name of buffers associated
 with renamed files so that they refer to the new names.
 
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,
 \f
 * Changes in Specialized Modes and Packages in Emacs 27.1
 
+** Dired
+
+---
+*** The new user option 'dired-create-destination-dirs' controls whether
+'dired-do-copy' and 'dired-rename-file' must create non-existent
+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."
+  :type '(choice (const :tag "Never create non-existent dirs" never)
+		 (const :tag "Always create non-existent dirs" always)
+		 (const :tag "Ask for user confirmation" prompt))
+  :group 'dired
+  :version "27.1")
+
+(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))))))
+
 (defun dired-copy-file-recursive (from to ok-flag &optional
 				       preserve-time top recursive)
   (when (and (eq t (car (file-attributes from)))
@@ -1564,6 +1584,7 @@ dired-copy-file-recursive
 	  (if (stringp (car attrs))
 	      ;; It is a symlink
 	      (make-symbolic-link (car attrs) to ok-flag)
+            (dired--create-dirs (file-name-directory to))
 	    (copy-file from to ok-flag preserve-time))
 	(file-date-error
 	 (push (dired-make-relative from)
@@ -1573,6 +1594,7 @@ dired-copy-file-recursive
 ;;;###autoload
 (defun dired-rename-file (file newname ok-if-already-exists)
   (dired-handle-overwrite newname)
+  (dired--create-dirs (file-name-directory newname))
   (rename-file file newname ok-if-already-exists) ; error is caught in -create-files
   ;; Silently rename the visited file of any buffer visiting this file.
   (and (get-file-buffer file)
diff --git a/test/lisp/dired-aux-tests.el b/test/lisp/dired-aux-tests.el
index d41feb1592..7778db0ea4 100644
--- a/test/lisp/dired-aux-tests.el
+++ b/test/lisp/dired-aux-tests.el
@@ -40,5 +40,34 @@
           (should-not (dired-do-shell-command "ls ? ./`?`" nil files)))
       (delete-file foo))))
 
+(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))))
+      ;; clean up
+      (delete-directory foo 'recursive)
+      (delete-file from))))
+
+
 (provide 'dired-aux-tests)
 ;; dired-aux-tests.el ends here

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 27.0.50 (build 11, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-10-16
Repository revision: eed3a3d9e95d2c5346a23c9d92ca4e5848330183





^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#28834: 25.1; dired-do-copy: allow to copy into a nonexistent directory
  2017-10-16  3:47     ` Tino Calancha
@ 2017-10-16 16:12       ` Eli Zaretskii
  2017-10-17  4:49         ` Tino Calancha
  2017-10-17  1:49       ` Drew Adams
  1 sibling, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2017-10-16 16:12 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 28834

> 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,
>  \f
>  * 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?





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#28834: 25.1; dired-do-copy: allow to copy into a nonexistent directory
  2017-10-16  3:47     ` Tino Calancha
  2017-10-16 16:12       ` Eli Zaretskii
@ 2017-10-17  1:49       ` Drew Adams
  1 sibling, 0 replies; 12+ messages in thread
From: Drew Adams @ 2017-10-17  1:49 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 28834

Thanks for working on this, Tino.

I haven't looked at it in detail.

1. Maybe name the function `dired-maybe-create-dirs'
   (and I don't see why its name should suggest that
   it is internal).

2. I think this should use `%s', no?

  (format "Create destination dir '%s'? " dir))

(I'm also surprised that we apparently now use
'...' instead of `...' in News, but whatever...)





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#28834: 25.1; dired-do-copy: allow to copy into a nonexistent directory
  2017-10-16 16:12       ` Eli Zaretskii
@ 2017-10-17  4:49         ` Tino Calancha
  2017-10-17 16:36           ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Tino Calancha @ 2017-10-17  4:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 28834

Eli Zaretskii <eliz@gnu.org> writes:

Thank you for the comments.
* I have changed the new function name to `dired-maybe-create-dirs'
  as Drew suggested.
* I think I have addressed all your comments as well.

>> +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.
Removed that entence.

>> +@videnx dired-create-destination-dirs
>    ^^^^^^^
> A typo.
Fixed.

>> +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.
Changed to 'ask.

>> +** Dired
>> +
>> +---
>
> Since you've updated the manual, this should be "+++", not "---".
OK.

>> +'dired-do-copy' and 'dired-rename-file' must create non-existent
>                                            ^^^^
> "Should", not "must".
OK.

>> +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.
OK.
> Btw, perhaps it's better to use nil instead of 'never'.
Changed never ---> nil.

>> +(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?
I rewrote the function: now it loooks more simple.

>> +(ert-deftest dired-test-bug28834 ()
>> +  "test for https://debbugs.gnu.org/28834 ."
> This doesn't test the 3rd value.  Why is that?

Laziness? Bad memory? Busy? Stomach problems? Other not listed here?
Updated the test: now it run all cases.

--8<-----------------------------cut here---------------start------------->8---
commit fb91d159f241a6eeecc41c4101472f145a00c0d1
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Tue Oct 17 13:48:59 2017 +0900

    Allow to copy/rename file into a non-existent dir
    
    * lisp/dired-aux.el (dired-create-destination-dirs): New option.
    (dired-maybe-create-dirs): New defun.
    (dired-copy-file-recursive, dired-rename-file): Use it (Bug#28834).
    * lisp/dired-aux-tests.el (dired-test-bug28834): Add test.
    * doc/emacs/dired.texi (Operating on Files): Update manual.
    * etc/NEWS (Changes in Specialized Modes and Packages in Emacs 27.1)
    Announce this change.

diff --git a/doc/emacs/dired.texi b/doc/emacs/dired.texi
index db5dea329b..9348ef5042 100644
--- a/doc/emacs/dired.texi
+++ b/doc/emacs/dired.texi
@@ -647,6 +647,14 @@ Operating on Files
 is the directory to copy into, or (if copying a single file) the new
 name.  This is like the shell command @code{cp}.
 
+@vindex dired-create-destination-dirs
+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{nil} means Dired
+never creates such missing directories;  the value @code{always},
+means Dired automatically creates them; the value @code{ask}
+means Dired asks you for confirmation before creating them.
+
 @vindex dired-copy-preserve-time
 If @code{dired-copy-preserve-time} is non-@code{nil}, then copying
 with this command preserves the modification time of the old file in
@@ -678,6 +686,9 @@ Operating on Files
 you rename several files, the argument @var{new} is the directory into
 which to move the files (this is like the shell command @command{mv}).
 
+The option @code{dired-create-destination-dirs} controls whether Dired
+should create non-existent directories in @var{new}.
+
 Dired automatically changes the visited file name of buffers associated
 with renamed files so that they refer to the new names.
 
diff --git a/etc/NEWS b/etc/NEWS
index 716b0309a5..acfc52ab52 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -56,6 +56,13 @@ whether '"' is also replaced in 'electric-quote-mode'.  If non-nil,
 \f
 * Changes in Specialized Modes and Packages in Emacs 27.1
 
+** Dired
+
++++
+*** The new user option 'dired-create-destination-dirs' controls whether
+'dired-do-copy' and 'dired-rename-file' should create non-existent
+directories in the destination.
+
 ** Edebug
 
 +++
diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 7e2252fcf1..5e92c58d12 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -1548,6 +1548,24 @@ dired-copy-file
 
 (declare-function make-symbolic-link "fileio.c")
 
+(defcustom dired-create-destination-dirs nil
+  "Whether Dired should create destination dirs when copying/removing files.
+If nil, don't create them.
+If `always', create them without ask.
+If `ask', ask for user confirmation."
+  :type '(choice (const :tag "Never create non-existent dirs" nil)
+		 (const :tag "Always create non-existent dirs" always)
+		 (const :tag "Ask for user confirmation" ask))
+  :group 'dired
+  :version "27.1")
+
+(defun dired-maybe-create-dirs (dir)
+  "Create DIR if doesn't exist according with `dired-create-destination-dirs'."
+  (when (and dired-create-destination-dirs (not (file-exists-p dir)))
+    (if (or (eq dired-create-destination-dirs 'always)
+            (yes-or-no-p (format "Create destination dir `%s'? " dir)))
+        (dired-create-directory dir))))
+
 (defun dired-copy-file-recursive (from to ok-flag &optional
 				       preserve-time top recursive)
   (when (and (eq t (car (file-attributes from)))
@@ -1564,6 +1582,7 @@ dired-copy-file-recursive
 	  (if (stringp (car attrs))
 	      ;; It is a symlink
 	      (make-symbolic-link (car attrs) to ok-flag)
+            (dired-maybe-create-dirs (file-name-directory to))
 	    (copy-file from to ok-flag preserve-time))
 	(file-date-error
 	 (push (dired-make-relative from)
@@ -1573,6 +1592,7 @@ dired-copy-file-recursive
 ;;;###autoload
 (defun dired-rename-file (file newname ok-if-already-exists)
   (dired-handle-overwrite newname)
+  (dired-maybe-create-dirs (file-name-directory newname))
   (rename-file file newname ok-if-already-exists) ; error is caught in -create-files
   ;; Silently rename the visited file of any buffer visiting this file.
   (and (get-file-buffer file)
diff --git a/test/lisp/dired-aux-tests.el b/test/lisp/dired-aux-tests.el
index d41feb1592..9316217dd2 100644
--- a/test/lisp/dired-aux-tests.el
+++ b/test/lisp/dired-aux-tests.el
@@ -20,7 +20,7 @@
 ;;; Code:
 (require 'ert)
 (require 'dired-aux)
-
+(eval-when-compile (require 'cl-lib))
 
 (ert-deftest dired-test-bug27496 ()
   "Test for https://debbugs.gnu.org/27496 ."
@@ -40,5 +40,59 @@
           (should-not (dired-do-shell-command "ls ? ./`?`" nil files)))
       (delete-file foo))))
 
+;; Auxiliar macro for `dired-test-bug28834': it binds
+;; `dired-create-destination-dirs' to CREATE-DIRS and execute BODY.
+;; If YES-OR-NO is non-nil, it binds `yes-or-no-p' to
+;; to avoid the prompt.
+(defmacro with-dired-bug28834-test (create-dirs yes-or-no &rest body)
+  (declare ((debug form symbolp body)))
+  (let ((foo (make-symbol "foo")))
+    `(let* ((,foo (make-temp-file "foo" 'dir))
+            (dired-create-destination-dirs ,create-dirs))
+       (setq from (make-temp-file "from"))
+       (setq to-cp
+             (expand-file-name
+              "foo-cp" (file-name-as-directory (expand-file-name "bar" ,foo))))
+       (setq to-mv
+             (expand-file-name
+              "foo-mv" (file-name-as-directory (expand-file-name "qux" ,foo))))
+       (unwind-protect
+           (if ,yes-or-no
+               (cl-letf (((symbol-function 'yes-or-no-p)
+                          (lambda (prompt) (eq ,yes-or-no 'yes))))
+                 ,@body)
+             ,@body)
+         ;; clean up
+         (delete-directory ,foo 'recursive)
+         (delete-file from)))))
+
+(ert-deftest dired-test-bug28834 ()
+  "test for https://debbugs.gnu.org/28834 ."
+  (let (from to-cp to-mv)
+    ;; `dired-create-destination-dirs' set to 'always.
+    (with-dired-bug28834-test
+     'always nil
+     (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)))
+    ;; `dired-create-destination-dirs' set to nil.
+    (with-dired-bug28834-test
+     nil nil
+     (should-error (dired-copy-file-recursive from to-cp nil))
+     (should-error (dired-rename-file from to-mv nil)))
+    ;; `dired-create-destination-dirs' set to 'ask.
+    (with-dired-bug28834-test
+     'ask 'yes ; Answer `yes'
+     (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)))
+    (with-dired-bug28834-test
+     'ask 'no ; Answer `no'
+     (should-error (dired-copy-file-recursive from to-cp nil))
+     (should-error (dired-rename-file from to-mv nil)))))
+
+
 (provide 'dired-aux-tests)
 ;; dired-aux-tests.el ends here
--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-10-17
Repository revision: 94281c9a1cc0f756841fdc9b266657853df94a29





^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#28834: 25.1; dired-do-copy: allow to copy into a nonexistent directory
  2017-10-17  4:49         ` Tino Calancha
@ 2017-10-17 16:36           ` Eli Zaretskii
  2017-10-21  4:13             ` Tino Calancha
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2017-10-17 16:36 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 28834

> From: Tino Calancha <tino.calancha@gmail.com>
> Cc: 28834@debbugs.gnu.org,  drew.adams@oracle.com
> Date: Tue, 17 Oct 2017 13:49:34 +0900
> 
> * I have changed the new function name to `dired-maybe-create-dirs'
>   as Drew suggested.
> * I think I have addressed all your comments as well.

Thanks, this LGTM.  One minor comment:

> +(defun dired-maybe-create-dirs (dir)
> +  "Create DIR if doesn't exist according with `dired-create-destination-dirs'."
                                  ^^^^^^^^^^^^^^
"according to"

Please push to master in a few days, if no one raises any issues.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#28834: 25.1; dired-do-copy: allow to copy into a nonexistent directory
  2017-10-17 16:36           ` Eli Zaretskii
@ 2017-10-21  4:13             ` Tino Calancha
  0 siblings, 0 replies; 12+ messages in thread
From: Tino Calancha @ 2017-10-21  4:13 UTC (permalink / raw)
  To: 28834-done

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Tino Calancha <tino.calancha@gmail.com>
>> Cc: 28834@debbugs.gnu.org,  drew.adams@oracle.com
>> Date: Tue, 17 Oct 2017 13:49:34 +0900
>> 
>> * I have changed the new function name to `dired-maybe-create-dirs'
>>   as Drew suggested.
>> * I think I have addressed all your comments as well.
>
> Thanks, this LGTM.
Added feature into master branch as commit
"Allow to copy/rename file into a non-existent dir"
(cb29f41624e5163a0aea4bfc98591e683807a2f8)





^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-10-21  4:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-14 10:54 bug#28834: 25.1; Dired+ not copy file to nonexistent directory alexei28
2017-10-14 15:50 ` Drew Adams
2017-10-15  4:42 ` bug#28834: 25.1; dired-do-copy: allow to copy into a " Tino Calancha
2017-10-15 14:21   ` Eli Zaretskii
2017-10-15 14:47     ` Tino Calancha
2017-10-15 15:43   ` Drew Adams
2017-10-16  3:47     ` Tino Calancha
2017-10-16 16:12       ` Eli Zaretskii
2017-10-17  4:49         ` Tino Calancha
2017-10-17 16:36           ` Eli Zaretskii
2017-10-21  4:13             ` Tino Calancha
2017-10-17  1:49       ` Drew Adams

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).