Re: master 400df210ce0: Fix last change of 'delete-file'

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: rpluim@gmail.com,  emacs-devel@gnu.org,  esr@thyrsus.com
> Date: Fri, 11 Aug 2023 13:41:01 +0200
> 
> > Michael, this is my change, not Eric's.  My question is above: is
> > there a requirement that file handlers are called with absolute file
> > names?  If so, where is it documented, and why some places, which I
> > mentioned above, call file handlers with file names that were not
> > passed through expand-file-name?
> 
> Ffind_file_name_handler calls fast_string_match (string, filename).
> STRING is the car of an entry in file-name-handler-alist, FILENAME is
> the file name to be checked.
> 
> If the entry in file-name-handler-alist is a regexp for the head of an
> absolute file name, the check will fail when FILENAME is relative.

Shouldn't Ffind_file_name_handler also check the default-directory, if
FILENAME is not absolute and failed to match anything in
file-name-handler-alist?  If it doesn't do that, a file "foo" in a
remote directory will fail to match a handler, and that is probably
not expected (a.k.a. "bug")?

> >> > But the same would be true for substitute-in-file-name, for example,
> >> > and for directory-file-name, and file-name-as-directory, and several
> >> > other primitives, which call Ffind_file_name_handler without calling
> >> > expand-file-name before that.
> 
> substitute-in-file-name, directory-file-name, file-name-as-directory and
> likely more primitives do not support relative file names like
> "123". They support file names relative to the remote, like
> "/ssh::123". Is this a bug?  Don't know, nobody has protested so
> far. But at least it is undocumented.

These primitives should support relative file names, and they do for
local files.  So either they should call expand-file-name, or
find-file-name-handler should check default-directory.

> Other primitives handle this themselves if necessary. Fdelete_file and
> other function expand the file name. In Fexpand_file_name itself, we have
> 
> --8<---------------cut here---------------start------------->8---
>   handler = Ffind_file_name_handler (name, Qexpand_file_name);
>   ...
>   handler = Ffind_file_name_handler (default_directory, Qexpand_file_name);
> --8<---------------cut here---------------end--------------->8---

Primitives indeed _must_ call expand-file-name.  And expand-file-name
does that with default-directory because it accepts a directory as its
2nd argument, and that argument defaults to default-directory.

I've added back expand-file-name to delete-file.  But I find this
situation unsatisfactory: Lisp code should only call expand-file-name
when it needs to analyze absolute file names for its own purposes, it
is not supposed to call expand-file-name when invoking primitives.
The addition of expand-file-name to delete-file made that function a
bit slower, and for local files that is a net loss.  We should extend
instead find-file-name-handler to work for non-absolute file names to
avoid this overhead.

> I wouldn't touch this as it works. Improvements in documentation might
> be appreciated.

Indeed, the fact that find-file-name-handler needs an absolute file
name is never mentioned anywhere in the documentation.  It is strange
this didn't pop up earlier.