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

[Ken Brown]

On 11/11/2016 3:27 AM, Eli Zaretskii wrote:
> 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.)

Fixed.

> 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 ;-).

I don't have an immediate idea, but I added a "FIXME" comment and will 
think about it.

> > +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.

Fixed.

> >  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.

Actually, the handlers haven't been called yet at this point in the 
code, so I left this one alone.

> > @@ -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.

Fixed.

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

Done, but I'm not sure I found the best place for it in the manual.

Thanks for the review.  Revised patch attached.

Ken

0001-Check-case-sensitivity-when-renaming-files.patch
Description: Text document