bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#24441: 24.5; rename directory in dired to change case


From: Eli Zaretskii
Subject: bug#24441: 24.5; rename directory in dired to change case
Date: Fri, 11 Nov 2016 10:27:35 +0200

> Cc: 24441@debbugs.gnu.org, schwab@suse.de, brady@bradyt.com
> From: Ken Brown <kbrown@cornell.edu>
> Date: Thu, 10 Nov 2016 20:42:53 -0500
> 
> On 11/10/2016 5:25 PM, Ken Brown wrote:
> > The attached patch attempts to do this for both Cygwin and OS X.  I
> > don't have access to an OS X system, so someone else will have to test
> > the OS X part.
> 
> Here's a simpler version.

Thanks, please see a few comments below.

> +                       (if (and (file-name-case-insensitive-p
> +                                 (expand-file-name (car fn-list)))

You shouldn't need to expand-file-name here, as all primitives do that
internally.  (Yours didn't, but that's a mistake, see below.)

> +/* Filesystems are case-sensitive on all supported systems except
> +   MS-Windows, MS-DOS, Cygwin, and OS X.  They are always
> +   case-insensitive on the first two, but they may or may not be
> +   case-insensitive on Cygwin and OS X.  The following function
> +   attempts to provide a runtime test on those two systems.  If the
> +   test is not conclusive, we assume case-insensitivity on Cygwin and
> +   case-sensitivity on OS X.  */
> +static bool
> +file_name_case_insensitive_p (const char *filename)

What about filesystems mounted on Posix hosts, like Samba, NFS-mounted
Windows volumes, etc. -- do those behave as case-sensitive or not?  If
the latter, can that be detected?  Just asking, this shouldn't hold
the patch, unless incorporating that is easy (or you feel like it even
if it isn't ;-).

> +DEFUN ("file-name-case-insensitive-p", Ffile_name_case_insensitive_p,
> +       Sfile_name_case_insensitive_p, 1, 1, 0,
> +       doc: /* Return t if file FILENAME is on a case-insensitive filesystem.
> +The arg must be a string.  */)
> +  (Lisp_Object filename)
> +{
> +  CHECK_STRING (filename);
> +  return file_name_case_insensitive_p (SSDATA (filename)) ? Qt : Qnil;
> +}

This "needs work"™.  First, it should call expand-file-name, as all
file-related primitives do.  Second, it should see if there's a file
handler for this operation, and if so, call it instead (Michael,
please take note ;-).  Finally, it should run the expanded file name
through ENCODE_FILE before it calls file_name_case_insensitive_p, and
pass to the latter the result of the encoding, otherwise the call to
getattrlist will most certainly fail in some cases.

>  DEFUN ("rename-file", Frename_file, Srename_file, 2, 3,
>         "fRename file: \nGRename %s to file: \np",
>         doc: /* Rename FILE as NEWNAME.  Both args must be strings.
> @@ -2250,12 +2301,11 @@ This is what happens in interactive use with M-x.  */)
>    file = Fexpand_file_name (file, Qnil);
>  
>    if ((!NILP (Ffile_directory_p (newname)))
> -#ifdef DOS_NT
> -      /* If the file names are identical but for the case,
> -      don't attempt to move directory to itself. */
> -      && (NILP (Fstring_equal (Fdowncase (file), Fdowncase (newname))))
> -#endif
> -      )
> +      /* If the filesystem is case-insensitive and the file names are
> +      identical but for the case, don't attempt to move directory
> +      to itself.  */
> +      && (NILP (Ffile_name_case_insensitive_p (file))
> +       || NILP (Fstring_equal (Fdowncase (file), Fdowncase (newname)))))

This should call file_name_case_insensitive_p, and pass it the encoded
file names.  That's because the file handlers for rename-file were
already called, so we are now sure the file doesn't have any handlers.

> @@ -2276,14 +2326,12 @@ This is what happens in interactive use with M-x.  */)
>    encoded_file = ENCODE_FILE (file);
>    encoded_newname = ENCODE_FILE (newname);
>  
> -#ifdef DOS_NT
> -  /* If the file names are identical but for the case, don't ask for
> -     confirmation: they simply want to change the letter-case of the
> -     file name.  */
> -  if (NILP (Fstring_equal (Fdowncase (file), Fdowncase (newname))))
> -#endif
> -  if (NILP (ok_if_already_exists)
> -      || INTEGERP (ok_if_already_exists))
> +  /* If the filesystem is case-insensitive and the file names are
> +     identical but for the case, don't ask for confirmation: they
> +     simply want to change the letter-case of the file name.  */
> +  if ((NILP (Ffile_name_case_insensitive_p (file))

Likewise here.

Finally, please provide a NEWS entry for the new primitive and its
documentation in the ELisp manual.

Thanks again for working on this.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]