bug#36831: enhance 'directory not empty' message

2019-07-29 Thread Paul Eggert

On 7/29/19 1:28 AM, Assaf Gordon wrote:

+  if (rename_errno == ENOTEMPTY || rename_errno == EEXIST)
+{
+  error (0, 0, _("cannot move %s to %s: Target directory not empty"),
+ quoteaf_n (0, src_name), quoteaf_n (1, dst_name));


Although this is an improvement, it is not general enough, as other 
errno values are relevant only for the destination. Better would be to 
have a special case for errno values that matter only for the 
destination, and use the existing code for errno values where we don't 
know whether the problem is the source or the destination. Something 
like the attached, say.



diff --git a/src/copy.c b/src/copy.c
index 65cf65895..b1e4557e4 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2477,9 +2477,18 @@ copy_internal (char const *src_name, char const *dst_name,
  If the permissions on the directory containing the source or
  destination file are made too restrictive, the rename will
  fail.  Etc.  */
-  error (0, rename_errno,
- _("cannot move %s to %s"),
- quoteaf_n (0, src_name), quoteaf_n (1, dst_name));
+  switch (errno)
+{
+case EDQUOT: case EEXIST: case EISDIR: case ENOSPC: case ENOTEMPTY:
+  error (0, rename_errno, "%s", quotearg_colon (dst_name));
+  break;
+
+default:
+  error (0, rename_errno,
+ _("cannot move %s to %s"),
+ quoteaf_n (0, src_name), quoteaf_n (1, dst_name));
+  break;
+}
   forget_created (src_sb.st_ino, src_sb.st_dev);
   return false;
 }


bug#36831: enhance 'directory not empty' message

2019-07-29 Thread Jim Meyering
On Sun, Jul 28, 2019 at 11:29 PM Assaf Gordon  wrote:
...
> What do others think? If this is a desired improvement, I'll finish the
> patch with news/tests/etc.
...
> [PATCH] mv: improve ENOTEMPTY/EEXIST error message
>
> Suggested by Alex Mantel  in
> https://bugs.gnu.org/36831 .
>
> $ mkdir A B B/A
> $ touch A/bar B/A/foo
>
> Before:
>
> $ mv A B
> mv: cannot move 'A' to 'B/A': Directory not empty
>
> After:
>
> $ mv A B
> mv: cannot move 'A' to 'B/A': Target directory not empty
>
> * src/copy.c (copy_internal): Add special handling for ENOTEMPTY/EEXIST.
> TODO: NEWS, tests.

I like it. Thank you.