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

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

[Robert Pluim]

>>>>> On Sun,  6 Aug 2023 10:04:06 -0400 (EDT), Eli Zaretskii <eliz@gnu.org> said:

    Eli> branch: master
    Eli> commit 400df210ce0cc1ee0113b14a5ad92764d148c620
    Eli> Author: Eli Zaretskii <eliz@gnu.org>
    Eli> Commit: Eli Zaretskii <eliz@gnu.org>

    Eli>     Fix last change of 'delete-file'
    
    Eli>     * src/fileio.c (Fdelete_file_internal): Expand file name here, as
    Eli>     all primitives must.
    Eli>     (internal_delete_file): Adjust to the fact that Fdelete_file was
    Eli>     renamed.
    
    Eli>     * lisp/files.el (delete-file): Don't expand-file-name here, as
    Eli>     the called primitives already do.  Fix typo in doc string.

I donʼt know if it matters, but along with esrʼs change this means we
now use the unexpanded file name for eg `find-file-name-handler', and
donʼt expand it until (if) we reach `internal_delete_file'.

Robert
-- 

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

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>
> Date: Thu, 10 Aug 2023 15:12:01 +0200
> 
> >>>>> On Sun,  6 Aug 2023 10:04:06 -0400 (EDT), Eli Zaretskii <eliz@gnu.org> said:
> 
>     Eli> branch: master
>     Eli> commit 400df210ce0cc1ee0113b14a5ad92764d148c620
>     Eli> Author: Eli Zaretskii <eliz@gnu.org>
>     Eli> Commit: Eli Zaretskii <eliz@gnu.org>
> 
>     Eli>     Fix last change of 'delete-file'
>     
>     Eli>     * src/fileio.c (Fdelete_file_internal): Expand file name here, as
>     Eli>     all primitives must.
>     Eli>     (internal_delete_file): Adjust to the fact that Fdelete_file was
>     Eli>     renamed.
>     
>     Eli>     * lisp/files.el (delete-file): Don't expand-file-name here, as
>     Eli>     the called primitives already do.  Fix typo in doc string.
> 
> I donʼt know if it matters, but along with esrʼs change this means we
> now use the unexpanded file name for eg `find-file-name-handler'

All file handlers are perfectly equipped to deal with unexpanded file
names.  It must be so, don't you agree?  Because if it wasn't, how
could file operations deal with relative file names?

> and donʼt expand it until (if) we reach `internal_delete_file'.

Only if the caller doesn't expand by itself.  And even if it doesn't
why do you think it's a problem?  Primitives that deal with file names
must always call expand-file-name early on, for this very reason.  No
primitive should assume it will receive only absolute file names.

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

[Robert Pluim]

>>>>> On Thu, 10 Aug 2023 16:25:32 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: Eli Zaretskii <eliz@gnu.org>
    >> Date: Thu, 10 Aug 2023 15:12:01 +0200
    >> 
    >> >>>>> On Sun,  6 Aug 2023 10:04:06 -0400 (EDT), Eli Zaretskii <eliz@gnu.org> said:
    >> 
    Eli> branch: master
    Eli> commit 400df210ce0cc1ee0113b14a5ad92764d148c620
    Eli> Author: Eli Zaretskii <eliz@gnu.org>
    Eli> Commit: Eli Zaretskii <eliz@gnu.org>
    >> 
    Eli> Fix last change of 'delete-file'
    >> 
    Eli> * src/fileio.c (Fdelete_file_internal): Expand file name here, as
    Eli> all primitives must.
    Eli> (internal_delete_file): Adjust to the fact that Fdelete_file was
    Eli> renamed.
    >> 
    Eli> * lisp/files.el (delete-file): Don't expand-file-name here, as
    Eli> the called primitives already do.  Fix typo in doc string.
    >> 
    >> I donʼt know if it matters, but along with esrʼs change this means we
    >> now use the unexpanded file name for eg `find-file-name-handler'

    Eli> All file handlers are perfectly equipped to deal with unexpanded file
    Eli> names.  It must be so, don't you agree?  Because if it wasn't, how
    Eli> could file operations deal with relative file names?

I was thinking of the opposite case, where someone has a personal file
handler with a regexp with an absolute path in it.

    >> and donʼt expand it until (if) we reach `internal_delete_file'.

    Eli> Only if the caller doesn't expand by itself.  And even if it doesn't
    Eli> why do you think it's a problem?  Primitives that deal with file names
    Eli> must always call expand-file-name early on, for this very reason.  No
    Eli> primitive should assume it will receive only absolute file names.

I donʼt think itʼs a problem; I was just noting the change, and
wondering if it mattered.

Robert
-- 

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

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 10 Aug 2023 15:44:54 +0200
> 
> >>>>> On Thu, 10 Aug 2023 16:25:32 +0300, Eli Zaretskii <eliz@gnu.org> said:
> 
>     >> I donʼt know if it matters, but along with esrʼs change this means we
>     >> now use the unexpanded file name for eg `find-file-name-handler'
> 
>     Eli> All file handlers are perfectly equipped to deal with unexpanded file
>     Eli> names.  It must be so, don't you agree?  Because if it wasn't, how
>     Eli> could file operations deal with relative file names?
> 
> I was thinking of the opposite case, where someone has a personal file
> handler with a regexp with an absolute path in it.

That'd be a buggy file handler, I think, unless it also checks
default-directory.

>     Eli> Only if the caller doesn't expand by itself.  And even if it doesn't
>     Eli> why do you think it's a problem?  Primitives that deal with file names
>     Eli> must always call expand-file-name early on, for this very reason.  No
>     Eli> primitive should assume it will receive only absolute file names.
> 
> I donʼt think itʼs a problem; I was just noting the change, and
> wondering if it mattered.

I don't think it could matter.  Before the change we had Fdelete_file
which could handle relative file names perfectly well, eight?  So some
Lisp could call delete-file passing it a relative file name, and that
would work correctly, right?  Now we have the same arrangement, except
that the primitive was renamed.  In effect, the call to
expand-file-name was pushed a bit further into the processing
pipeline.  That cannot cause any harm, as long as code before
expand-file-name expects absolute file names.  But no general-purpose
Lisp code should ever expect only absolute file names, because we make
them absolute in the primitives.

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

[Robert Pluim]

>>>>> On Thu, 10 Aug 2023 16:57:43 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: emacs-devel@gnu.org
    >> Date: Thu, 10 Aug 2023 15:44:54 +0200
    >> 
    >> >>>>> On Thu, 10 Aug 2023 16:25:32 +0300, Eli Zaretskii <eliz@gnu.org> said:
    >> 
    >> >> I donʼt know if it matters, but along with esrʼs change this means we
    >> >> now use the unexpanded file name for eg `find-file-name-handler'
    >> 
    Eli> All file handlers are perfectly equipped to deal with unexpanded file
    Eli> names.  It must be so, don't you agree?  Because if it wasn't, how
    Eli> could file operations deal with relative file names?
    >> 
    >> I was thinking of the opposite case, where someone has a personal file
    >> handler with a regexp with an absolute path in it.

    Eli> That'd be a buggy file handler, I think, unless it also checks
    Eli> default-directory.

The file handler isnʼt buggy: it never gets a chance to run.

Let me illustrate. Suppose we have an entry of

("\\`/var/.*\\.txt" . my-file-handler)

in `file-name-handler-alist'.

(delete-file "foo.txt") when `default-directory' is "/var" would
previously result in `my-file-handler' being called. Now itʼs not
called at all, and the deletion is handled by `delete-file-internal'

Robert
-- 

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

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 10 Aug 2023 16:41:01 +0200
> 
>     >> I was thinking of the opposite case, where someone has a personal file
>     >> handler with a regexp with an absolute path in it.
> 
>     Eli> That'd be a buggy file handler, I think, unless it also checks
>     Eli> default-directory.
> 
> The file handler isnʼt buggy: it never gets a chance to run.
> 
> Let me illustrate. Suppose we have an entry of
> 
> ("\\`/var/.*\\.txt" . my-file-handler)
> 
> in `file-name-handler-alist'.
> 
> (delete-file "foo.txt") when `default-directory' is "/var" would
> previously result in `my-file-handler' being called. Now itʼs not
> called at all, and the deletion is handled by `delete-file-internal'

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.

But maybe I'm missing something here, so let's ask Michael (CC'ed) for
his opinion on this.

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

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli & Robert,

>>     >> I was thinking of the opposite case, where someone has a personal file
>>     >> handler with a regexp with an absolute path in it.
>> 
>>     Eli> That'd be a buggy file handler, I think, unless it also checks
>>     Eli> default-directory.
>> 
>> The file handler isnʼt buggy: it never gets a chance to run.
>> 
>> Let me illustrate. Suppose we have an entry of
>> 
>> ("\\`/var/.*\\.txt" . my-file-handler)
>> 
>> in `file-name-handler-alist'.
>> 
>> (delete-file "foo.txt") when `default-directory' is "/var" would
>> previously result in `my-file-handler' being called. Now itʼs not
>> called at all, and the deletion is handled by `delete-file-internal'
>
> 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.
>
> But maybe I'm missing something here, so let's ask Michael (CC'ed) for
> his opinion on this.

In Emacs 29, we have in Fdelete_file:

--8<---------------cut here---------------start------------->8---
  filename = Fexpand_file_name (filename, Qnil);

  handler = Ffind_file_name_handler (filename, Qdelete_file);
  if (!NILP (handler))
    return call3 (handler, Qdelete_file, filename, trash);
--8<---------------cut here---------------end--------------->8---

So the file name is always absolute. Now in Emacs 30, the file name is
not expanded prior calling find-file-name-handler in delete-file. That
is an error in the transition from C to Lisp.

Cc to Eric, who has done the transition.

Best regards, Michael.

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

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Robert Pluim <rpluim@gmail.com>,  emacs-devel@gnu.org, Eric S. Raymond
>  <esr@thyrsus.com>
> Date: Fri, 11 Aug 2023 09:33:09 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> Hi Eli & Robert,
> 
> >>     >> I was thinking of the opposite case, where someone has a personal file
> >>     >> handler with a regexp with an absolute path in it.
> >> 
> >>     Eli> That'd be a buggy file handler, I think, unless it also checks
> >>     Eli> default-directory.
> >> 
> >> The file handler isnʼt buggy: it never gets a chance to run.
> >> 
> >> Let me illustrate. Suppose we have an entry of
> >> 
> >> ("\\`/var/.*\\.txt" . my-file-handler)
> >> 
> >> in `file-name-handler-alist'.
> >> 
> >> (delete-file "foo.txt") when `default-directory' is "/var" would
> >> previously result in `my-file-handler' being called. Now itʼs not
> >> called at all, and the deletion is handled by `delete-file-internal'
> >
> > 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.
> >
> > But maybe I'm missing something here, so let's ask Michael (CC'ed) for
> > his opinion on this.
> 
> In Emacs 29, we have in Fdelete_file:
> 
> --8<---------------cut here---------------start------------->8---
>   filename = Fexpand_file_name (filename, Qnil);
> 
>   handler = Ffind_file_name_handler (filename, Qdelete_file);
>   if (!NILP (handler))
>     return call3 (handler, Qdelete_file, filename, trash);
> --8<---------------cut here---------------end--------------->8---
> 
> So the file name is always absolute. Now in Emacs 30, the file name is
> not expanded prior calling find-file-name-handler in delete-file. That
> is an error in the transition from C to Lisp.
> 
> Cc to Eric, who has done the transition.

That's my change, not Eric's.  And I still believe file handlers
should be able to deal with non-absolute file names (as they do
elsewhere).  Let's see what Michael says about this.

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

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Robert Pluim <rpluim@gmail.com>,  emacs-devel@gnu.org, Eric S. Raymond
>  <esr@thyrsus.com>
> Date: Fri, 11 Aug 2023 09:33:09 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> 
> >> (delete-file "foo.txt") when `default-directory' is "/var" would
> >> previously result in `my-file-handler' being called. Now itʼs not
> >> called at all, and the deletion is handled by `delete-file-internal'
> >
> > 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.
> >
> > But maybe I'm missing something here, so let's ask Michael (CC'ed) for
> > his opinion on this.
> 
> In Emacs 29, we have in Fdelete_file:
> 
> --8<---------------cut here---------------start------------->8---
>   filename = Fexpand_file_name (filename, Qnil);
> 
>   handler = Ffind_file_name_handler (filename, Qdelete_file);
>   if (!NILP (handler))
>     return call3 (handler, Qdelete_file, filename, trash);
> --8<---------------cut here---------------end--------------->8---
> 
> So the file name is always absolute. Now in Emacs 30, the file name is
> not expanded prior calling find-file-name-handler in delete-file. That
> is an error in the transition from C to Lisp.
> 
> Cc to Eric, who has done the transition.

Oops, sorry, I failed to see that this was from Michael.

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?

You just say this is an error, but don't answer my questions about
this.

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

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

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

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

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

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

Best regards, Michael.

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.

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

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

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

Do you want two different versions of find-file-name-handler? One used
for primitives in C core which doesn't apply expand-file-name, and
another version for file operations in Lisp?

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

It is not a recent addition, it has been there for ages. In Fdelete_file.

I vaguely remember that I've tried that approach many years ago, in my
early years working for Tramp. There were subtle bugs, hard to fix,
because of this. But I don't remember the details.

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

find-file-name-handler does not need an absolute file name in
general. Several handlers are invoked based on the file name
extensions. It are the remote file names which require absolute file
names.

If we add expand-file-name for cases it isn't applied yet, Emacs might
become slower. And perhaps we introduce new faults by this, because
everybody has arranged with the current behavior.

Best regards, Michael.

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 19:24:25 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> Hi Eli,
> 
> > 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.
> 
> Do you want two different versions of find-file-name-handler? One used
> for primitives in C core which doesn't apply expand-file-name, and
> another version for file operations in Lisp?

No, I mean single one that serves both.

> > 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.
> 
> It is not a recent addition, it has been there for ages. In Fdelete_file.

I'm talking about the situation on master now, after delete-file was
split into two parts.  After my change today, delete-file calls
expand-file-name (because find-file-name-handler needs that), and then
delete-file-internal calls expand-file-name again (because every
primitive must).

> > 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.
> 
> find-file-name-handler does not need an absolute file name in
> general. Several handlers are invoked based on the file name
> extensions. It are the remote file names which require absolute file
> names.

A caller of find-file-name-handler cannot (and should not) know which
parts of the file name the handlers look at.

> If we add expand-file-name for cases it isn't applied yet, Emacs might
> become slower. And perhaps we introduce new faults by this, because
> everybody has arranged with the current behavior.

But the situation now is bad already: if I call
file-name-as-directory, for example, with a relative file name in a
remote directory, the handler is not invoked.  Isn't that a bug?

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

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

>> > 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.
>>
>> It is not a recent addition, it has been there for ages. In Fdelete_file.
>
> I'm talking about the situation on master now, after delete-file was
> split into two parts.  After my change today, delete-file calls
> expand-file-name (because find-file-name-handler needs that), and then
> delete-file-internal calls expand-file-name again (because every
> primitive must).

But this is due to moving delete-file from C core to Lisp. We have the
same situation with other Lisp functions, which support file name
handlers and call a C-level function. For example, make-temp-file /
make-temp-file-internal, make-directory / make-directory-internal,
delete-directory / delete-directory-internal.

>> > 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.
>>
>> find-file-name-handler does not need an absolute file name in
>> general. Several handlers are invoked based on the file name
>> extensions. It are the remote file names which require absolute file
>> names.
>
> A caller of find-file-name-handler cannot (and should not) know which
> parts of the file name the handlers look at.

Yes.

>> If we add expand-file-name for cases it isn't applied yet, Emacs might
>> become slower. And perhaps we introduce new faults by this, because
>> everybody has arranged with the current behavior.
>
> But the situation now is bad already: if I call
> file-name-as-directory, for example, with a relative file name in a
> remote directory, the handler is not invoked.  Isn't that a bug?

Perhaps. But in practice, I'm not aware of bug reports about.

However, we cannot call Fexpand_file_name in Ffind_file_name_handler,
because expand-file-name supports also file name handlers. This would be
an infloop. And just binding file-name-handler-alist to nil doesn't
return the proper result, but a result which is similar only:

--8<---------------cut here---------------start------------->8---
(let ((default-directory "/ssh::")
      file-name-handler-alist)
  (expand-file-name "123"))

=> "/ssh::/123"
--8<---------------cut here---------------end--------------->8---

This might be sufficient, but there are cases where we fail. On MS
Windows, for example, we have

--8<---------------cut here---------------start------------->8---
(let ((default-directory "/ssh::")
      file-name-handler-alist)
  (expand-file-name "123"))

=> "c:/ssh::/123"
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.

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: Sat, 12 Aug 2023 11:57:19 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> Hi Eli,
> 
> >> > 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.
> >>
> >> It is not a recent addition, it has been there for ages. In Fdelete_file.
> >
> > I'm talking about the situation on master now, after delete-file was
> > split into two parts.  After my change today, delete-file calls
> > expand-file-name (because find-file-name-handler needs that), and then
> > delete-file-internal calls expand-file-name again (because every
> > primitive must).
> 
> But this is due to moving delete-file from C core to Lisp. We have the
> same situation with other Lisp functions, which support file name
> handlers and call a C-level function. For example, make-temp-file /
> make-temp-file-internal, make-directory / make-directory-internal,
> delete-directory / delete-directory-internal.

Which is a pity, don't you think?

> > But the situation now is bad already: if I call
> > file-name-as-directory, for example, with a relative file name in a
> > remote directory, the handler is not invoked.  Isn't that a bug?
> 
> Perhaps. But in practice, I'm not aware of bug reports about.

It's a ticking time bomb.

> However, we cannot call Fexpand_file_name in Ffind_file_name_handler,
> because expand-file-name supports also file name handlers. This would be
> an infloop. And just binding file-name-handler-alist to nil doesn't
> return the proper result, but a result which is similar only:

We don't need to call expand-file-name in find-file-name-handler,
unless both the file name and default-directory are non-absolute.

> --8<---------------cut here---------------start------------->8---
> (let ((default-directory "/ssh::")
>       file-name-handler-alist)
>   (expand-file-name "123"))
> 
> => "/ssh::/123"
> --8<---------------cut here---------------end--------------->8---
> 
> This might be sufficient, but there are cases where we fail. On MS
> Windows, for example, we have
> 
> --8<---------------cut here---------------start------------->8---
> (let ((default-directory "/ssh::")
>       file-name-handler-alist)
>   (expand-file-name "123"))
> 
> => "c:/ssh::/123"
> --8<---------------cut here---------------end--------------->8---

Something to fix, I guess.  I see no insoluble issues here, FWIW.

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

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

>> > But the situation now is bad already: if I call
>> > file-name-as-directory, for example, with a relative file name in a
>> > remote directory, the handler is not invoked.  Isn't that a bug?
>>
>> Perhaps. But in practice, I'm not aware of bug reports about.
>
> It's a ticking time bomb.

Perhaps. A ticking bomb for at least 34 years, when ange-ftp.el was added.

>> However, we cannot call Fexpand_file_name in Ffind_file_name_handler,
>> because expand-file-name supports also file name handlers. This would be
>> an infloop. And just binding file-name-handler-alist to nil doesn't
>> return the proper result, but a result which is similar only:
>
> We don't need to call expand-file-name in find-file-name-handler,
> unless both the file name and default-directory are non-absolute.

default-directory must be absoöute per spec. If somebody uses a relative
file name, and a relative default-directory by intention (let-bound, for
example), I don't see a way to support file name handlers which need an
absolute file name for check. I simply don't know how to expand a
relative default-directory.

>> --8<---------------cut here---------------start------------->8---
>> (let ((default-directory "/ssh::")
>>       file-name-handler-alist)
>>   (expand-file-name "123"))
>>
>> => "c:/ssh::/123"
>> --8<---------------cut here---------------end--------------->8---
>
> Something to fix, I guess.  I see no insoluble issues here, FWIW.

We could use a variation of file-name-concat inside
Ffind_file_name_handler, when FILENAME is relative.

Best regards, Michael.