From: Eli Zaretskii <eliz@gnu.org>
To: Michael Albinus <michael.albinus@gmx.de>
Cc: rpluim@gmail.com, emacs-devel@gnu.org, esr@thyrsus.com
Subject: Re: master 400df210ce0: Fix last change of 'delete-file'
Date: Fri, 11 Aug 2023 15:10:18 +0300 [thread overview]
Message-ID: <834jl5hj9x.fsf@gnu.org> (raw)
In-Reply-To: <87350pu7qq.fsf@gmx.de> (message from Michael Albinus on Fri, 11 Aug 2023 13:41:01 +0200)
> 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.
next prev parent reply other threads:[~2023-08-11 12:10 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <169133064669.24990.11219399079845613336@vcs2.savannah.gnu.org>
[not found] ` <20230806140407.09E6BC038BE@vcs2.savannah.gnu.org>
2023-08-10 13:12 ` master 400df210ce0: Fix last change of 'delete-file' Robert Pluim
2023-08-10 13:25 ` Eli Zaretskii
2023-08-10 13:44 ` Robert Pluim
2023-08-10 13:57 ` Eli Zaretskii
2023-08-10 14:41 ` Robert Pluim
2023-08-10 15:00 ` Eli Zaretskii
2023-08-11 7:33 ` Michael Albinus
2023-08-11 10:59 ` Eli Zaretskii
2023-08-11 11:02 ` Eli Zaretskii
2023-08-11 11:41 ` Michael Albinus
2023-08-11 12:10 ` Eli Zaretskii [this message]
2023-08-11 17:24 ` Michael Albinus
2023-08-11 17:47 ` Eli Zaretskii
2023-08-12 9:57 ` Michael Albinus
2023-08-12 10:36 ` Eli Zaretskii
2023-08-12 11:03 ` Michael Albinus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=834jl5hj9x.fsf@gnu.org \
--to=eliz@gnu.org \
--cc=emacs-devel@gnu.org \
--cc=esr@thyrsus.com \
--cc=michael.albinus@gmx.de \
--cc=rpluim@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.