Re: Reading directory names with read-file-name

Re: Reading directory names with read-file-name

[Stefan Monnier]

> I propose to add a sixth argument READ-DIR to read-file-name

I agree that we need some way to implement read-directory-name,
but I find those functions with never-ending lists of arguments
rather annoying and would rather not add yet-another-one.

> The completion in read-file-name-internal should (somehow)
> be aware of this and only return completions which are
> directories (i.e. having a trailing /).

The "somehow" part is rather interesting.  A seemingly trivial way
would be to use the `predicate' argument passed to completing-read
(and then to read-file-name-internal), except that that argument is
already (ab)used to pass the current-directory to use when completing.


	Stefan

Re: Reading directory names with read-file-name

[Paul Eggert]

> From: storm@cua.dk (Kim F. Storm)
> User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.2.50
> 
> I propose to add a sixth argument READ-DIR to read-file-name
> which indicates that we are reading a directory name, and
> not just any file name.

More generally, shouldn't the READ-DIR argument specify constraints on
the desired file type?  For example, in many instances you want a
non-directory.

How about allowing READ-DIR to be any predicate, so that the invoker
of read-file-name can filter file names in arbitrary ways?  I suspect
that this would subsume read-file-name's existing MUSTMATCH argument
to some extent, since MUSTMATCH=t would be roughly equivalent to
READ-DIR=file-exists-p, but I can't think of a cleaner extension
offhand.

Reading directory names with read-file-name

[Kim F. Storm]

When a directory name is read with read-file-name,
there really isn't anything which indicates that it
is a directory name rather than a file name that
is required.

Among other things, this implies that using completion
while reading a directory name returns all file _and_
directory names, rather than just the directory names.

To me, this really doesn't make sense.

I propose to add a sixth argument READ-DIR to read-file-name
which indicates that we are reading a directory name, and
not just any file name.

The completion in read-file-name-internal should (somehow)
be aware of this and only return completions which are
directories (i.e. having a trailing /).


Subsequently, I propose to implement a wrapper in lisp
around read-file-name named read-directory-name.


If we agree on this, I'll implement it.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Reading directory names with read-file-name

[Richard Stallman]

    How about allowing READ-DIR to be any predicate, so that the invoker
    of read-file-name can filter file names in arbitrary ways?

This could be made consistent with the new locate-file function.

      I suspect
    that this would subsume read-file-name's existing MUSTMATCH argument
    to some extent, since MUSTMATCH=t would be roughly equivalent to
    READ-DIR=file-exists-p, but I can't think of a cleaner extension
    offhand.

This suggests that we should redefine the MUSTMATCH argument
rather than add a new one.

Re: Reading directory names with read-file-name

[Kim F. Storm]

Richard Stallman <rms@gnu.org> writes:

>     How about allowing READ-DIR to be any predicate, so that the invoker
>     of read-file-name can filter file names in arbitrary ways?
> 
> This could be made consistent with the new locate-file function.
> 
>       I suspect
>     that this would subsume read-file-name's existing MUSTMATCH argument
>     to some extent, since MUSTMATCH=t would be roughly equivalent to
>     READ-DIR=file-exists-p, but I can't think of a cleaner extension
>     offhand.
> 
> This suggests that we should redefine the MUSTMATCH argument
> rather than add a new one.

There is one major difficult here:

The doc string states:

Fourth arg MUSTMATCH non-nil means require existing file's name.
 Non-nil and non-t means also require confirmation after completion.

I've looked through the *.el files, and I've found at least the
following non-nil and non-t values:
    lambda, confirm, must-match, yes

It seems difficult to me to extend the MUSTMATCH argument to cover
both a predicate and the current tri-state behaviour, and do it
in a way which is compatible with locate-file.

It could be a cons (PREDICATE . MUSTMATCH), but in that case, I
thing adding a sixth argument PREDICATE to read-file-name is
simpler.

The predicate need to be passed on the read-file-name-internal, but as
Stefan points out, this is difficult since the predicate argument is
already used for the directory.  Since read-file-name-internal is
overridden by several packages, I don't think changing that API 
is feasible.

Instead, read-file-name could specbind read-file-name-predicate
to the predicate argument, and read-file-name-internal could check
that predicate.

It could also be considered to add a third argument PREDICATE
to file-name-all-completions.


I'm not sure how to proceed.  Please advise!

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Reading directory names with read-file-name

[Kim F. Storm]

storm@cua.dk (Kim F. Storm) writes:

> Richard Stallman <rms@gnu.org> writes:
> 
> >     How about allowing READ-DIR to be any predicate, so that the invoker
> >     of read-file-name can filter file names in arbitrary ways?
> > 
> > This could be made consistent with the new locate-file function.
> > 
> >       I suspect
> >     that this would subsume read-file-name's existing MUSTMATCH argument
> >     to some extent, since MUSTMATCH=t would be roughly equivalent to
> >     READ-DIR=file-exists-p, but I can't think of a cleaner extension
> >     offhand.
> > 
> > This suggests that we should redefine the MUSTMATCH argument
> > rather than add a new one.
>
> [...] 
> 
> I'm not sure how to proceed.  Please advise!

I've looked some more on this, and the file-name-handlers (e.g.  used
by ange-ftp) further complicates chaning the API for read-file-name,
read-file-name-internal, and file-name-all-completions, as well as
introducting a read-directory-name function.

Hm, I see to have opened a can of worms here :-)

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Reading directory names with read-file-name

[Richard Stallman]

    Fourth arg MUSTMATCH non-nil means require existing file's name.
     Non-nil and non-t means also require confirmation after completion.

    I've looked through the *.el files, and I've found at least the
    following non-nil and non-t values:
	lambda, confirm, must-match, yes

    It seems difficult to me to extend the MUSTMATCH argument to cover
    both a predicate and the current tri-state behaviour, and do it
    in a way which is compatible with locate-file.

I overlooked that--I must have been too sleepy.  That is too bad.

It is not necessarily fatal.  We could nominate some specific value to
mean "confirm after completion too", and fix the callers.  But that is
not as clean as I hoped it would be.

    Instead, read-file-name could specbind read-file-name-predicate
    to the predicate argument, and read-file-name-internal could check
    that predicate.

That seems reasonable.

Re: Reading directory names with read-file-name

[Richard Stallman]

    I've looked some more on this, and the file-name-handlers (e.g.  used
    by ange-ftp) further complicates chaning the API for read-file-name,
    read-file-name-internal, and file-name-all-completions, as well as
    introducting a read-directory-name function.

How so?

Re: Reading directory names with read-file-name

[Stefan Monnier]

> >     I've looked some more on this, and the file-name-handlers (e.g.  used
> >     by ange-ftp) further complicates chaning the API for read-file-name,
> >     read-file-name-internal, and file-name-all-completions, as well as
> >     introducting a read-directory-name function.
> > 
> > How so?
> 
> My concern is that existing file-name-handlers for read-file-name
> are written to match the current API for that function, and
> they are expected to "mirror" the behaviour of read-file-name
> for any given set of arguments.
> 
> Now, if we change the API for read-file-name, e.g. by changing
> the interpretation of the MUSTMATCH argument or adding a PREDICATE
> argument, the existing file-name-handlers may no longer work correctly.
> 
> And if I add a new read-directory-name function, the existing
> file-name-handler for read-file-name will not be called when reading a
> directory name; instead a new file-name-handler will be needed for
> read-directory-name (which is simple), but the existing packages which
> install file-name-handlers for read-file-name will need to be extended
> with a file-name-handler for read-directory-name as well.
> 
> So it's not trivial to change the API...

I must say that the ange-ftp code confuses me:
it creates a function ange-ftp-real-read-file-name-internal
which it doesn't use.  It also announces ange-ftp-read-file-name-internal
as the handler for read-file-name-internal but never defines the
function and all this while read-file-name-internal does not check
file-name-handlers (and neither does read-file-name).

So I believe that your fear is just based on some left-over, unused
and incomplete code in ange-ftp.el but is otherwise unfounded.


	Stefan

Re: Reading directory names with read-file-name

[Kim F. Storm]

Richard Stallman <rms@gnu.org> writes:

>     I've looked some more on this, and the file-name-handlers (e.g.  used
>     by ange-ftp) further complicates chaning the API for read-file-name,
>     read-file-name-internal, and file-name-all-completions, as well as
>     introducting a read-directory-name function.
> 
> How so?

My concern is that existing file-name-handlers for read-file-name
are written to match the current API for that function, and
they are expected to "mirror" the behaviour of read-file-name
for any given set of arguments.

Now, if we change the API for read-file-name, e.g. by changing
the interpretation of the MUSTMATCH argument or adding a PREDICATE
argument, the existing file-name-handlers may no longer work correctly.

And if I add a new read-directory-name function, the existing
file-name-handler for read-file-name will not be called when reading a
directory name; instead a new file-name-handler will be needed for
read-directory-name (which is simple), but the existing packages which
install file-name-handlers for read-file-name will need to be extended
with a file-name-handler for read-directory-name as well.

So it's not trivial to change the API...

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Reading directory names with read-file-name

[Kim F. Storm]

"Stefan Monnier" <monnier+gnu/emacs@rum.cs.yale.edu> writes:

> > My concern is that existing file-name-handlers for read-file-name
> > are written to match the current API for that function, and
> > they are expected to "mirror" the behaviour of read-file-name
> > for any given set of arguments.
> 
> I must say that the ange-ftp code confuses me:

Me too :-)

> it creates a function ange-ftp-real-read-file-name-internal
> which it doesn't use.  It also announces ange-ftp-read-file-name-internal
> as the handler for read-file-name-internal but never defines the
> function and all this while read-file-name-internal does not check
> file-name-handlers (and neither does read-file-name).

I'm amazed....  I've looked at that code at least 10 times
over the last few days --- and I still managed to overlook
this (trusting the code in ange-ftp.el).  Thanks!

> 
> So I believe that your fear is just based on some left-over, unused
> and incomplete code in ange-ftp.el but is otherwise unfounded.

That's good!  I'll see what I can do to make this work.

However, one of the candidates for an improvement could be
file-name-all-completions adding a third PREDICATE arg.
However, I think that's still not feasible for the reasons
I mentioned in my previous mail.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Reading directory names with read-file-name

[Richard Stallman]

    I must say that the ange-ftp code confuses me:
    it creates a function ange-ftp-real-read-file-name-internal
    which it doesn't use.  It also announces ange-ftp-read-file-name-internal
    as the handler for read-file-name-internal but never defines the
    function and all this while read-file-name-internal does not check
    file-name-handlers (and neither does read-file-name).

It seems clear enough to me.  Someone working on ange-ftp expected
read-file-name to do file name handling, but it doesn't.
(Maybe at one point it did, or was going to.)

Could you delete that code from ange-ftp.el?

Re: Reading directory names with read-file-name

[Richard Stallman]

    However, one of the candidates for an improvement could be
    file-name-all-completions adding a third PREDICATE arg.
    However, I think that's still not feasible for the reasons
    I mentioned in my previous mail.

There is no need to get that discouraged!  You have pointed out a
complication, but a complication does not mean it is unfeasible.  It
just means a little more work is needed.

You can fix the file name handlers--in fact, they may not even need
any change.  Please take a look and see if they need any change.

Re: Reading directory names with read-file-name

[Stefan Monnier]

>     However, one of the candidates for an improvement could be
>     file-name-all-completions adding a third PREDICATE arg.
>     However, I think that's still not feasible for the reasons
>     I mentioned in my previous mail.
> 
> There is no need to get that discouraged!  You have pointed out a
> complication, but a complication does not mean it is unfeasible.  It
> just means a little more work is needed.
> 
> You can fix the file name handlers--in fact, they may not even need
> any change.  Please take a look and see if they need any change.

FWIW I don't think that file-name-all-completions and file-name-completion
need to be changed at all in order to implement the PREDICATE argument
for read-file-name.
It might be convenient to do so, but if there really is a serious
issue with the file-name-handlers, we can always revert to implementing
the PREDICATE in read-file-name-internal (I have an elisp version
of it, if you prefer using that).


	Stefan

Re: Reading directory names with read-file-name

[Stefan Monnier]

>     I must say that the ange-ftp code confuses me:
>     it creates a function ange-ftp-real-read-file-name-internal
>     which it doesn't use.  It also announces ange-ftp-read-file-name-internal
>     as the handler for read-file-name-internal but never defines the
>     function and all this while read-file-name-internal does not check
>     file-name-handlers (and neither does read-file-name).
> 
> It seems clear enough to me.  Someone working on ange-ftp expected
> read-file-name to do file name handling, but it doesn't.
> (Maybe at one point it did, or was going to.)
> 
> Could you delete that code from ange-ftp.el?

Done.


	Stefan

Re: Reading directory names with read-file-name

[Kim F. Storm]

"Stefan Monnier" <monnier+gnu/emacs@rum.cs.yale.edu> writes:

> >     However, one of the candidates for an improvement could be
> >     file-name-all-completions adding a third PREDICATE arg.
> >     However, I think that's still not feasible for the reasons
> >     I mentioned in my previous mail.
> > 
> > There is no need to get that discouraged!  You have pointed out a
> > complication, but a complication does not mean it is unfeasible.  It
> > just means a little more work is needed.
> > 
> > You can fix the file name handlers--in fact, they may not even need
> > any change.  Please take a look and see if they need any change.
> 
> FWIW I don't think that file-name-all-completions and file-name-completion
> need to be changed at all in order to implement the PREDICATE argument
> for read-file-name.

No, but it makes a significant difference in performance to check for
specific predicates like file-regular-p, file-writable-p etc
in file-name-all-completions where the file is already stat'ed.

As you point out it is not _necessary_, so we can do it later
if the current functionality is inadequate for a specific purpose.

BTW, for reading directory names, I've added specific code to
handle file-directory-p efficiently.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk