bug#56754: 28.1; Please move general stuff from `ffap.el' to `files.el'

bug#56754: 28.1; Please move general stuff from `ffap.el' to `files.el'

[Drew Adams]

File `ffap.el' includes utility functions that have nothing particular
to do with ffap.  Please consider moving at least some such functions to
`files.el', so that 3rd-party code that want so use them doesn't need to
depend on `ffap.el'.

One example of such a function: `ffap-all-subdirs' (and its internal
helper function `ffap-all-subdirs-loop').

Yes, a library that doesn't want to require `ffap.el' could always get
the same effect using more general file utilities - in this case
`directory-files-recursively' (matching a vacuous regexp and providing a
predicate).  But something like `ffap-all-subdirs' is convenient for
doing just what it does.

`ffap.el' seems to have evolved general stuff in isolation - stuff it
uses but which is by no means specific to find-file-at-point.  Maybe
start by moving a few such functions, even just `ffap-all-subdirs', if
you don't want to go to a lot of trouble immediately.

In GNU Emacs 28.1 (build 2, x86_64-w64-mingw32)
 of 2022-04-21 built on AVALON
Windowing system distributor 'Microsoft Corp.', version 10.0.19044
System Description: Microsoft Windows 10 Pro (v10.0.2009.19044.1826)

bug#56754: 28.1; Please move general stuff from `ffap.el' to `files.el'

[Lars Ingebrigtsen]

Drew Adams <drew.adams@oracle.com> writes:

> One example of such a function: `ffap-all-subdirs' (and its internal
> helper function `ffap-all-subdirs-loop').
>
> Yes, a library that doesn't want to require `ffap.el' could always get
> the same effect using more general file utilities - in this case
> `directory-files-recursively' (matching a vacuous regexp and providing a
> predicate).  But something like `ffap-all-subdirs' is convenient for
> doing just what it does.

I don't think ffap-all-subdirs is generally useful:

---
Directories beginning with \".\" are ignored, and directory symlinks
are listed but never searched (to avoid loops).
---

It's well-suited for what ffap does, though, which is why it lives in
that file.

Closing.

bug#56754: 28.1; Please move general stuff from `ffap.el' to `files.el'

[Drew Adams]

> > One example of such a function: `ffap-all-subdirs' (and its internal
> > helper function `ffap-all-subdirs-loop').
> 
> I don't think ffap-all-subdirs is generally useful:

Did you understand that the request is not about
`ffap-all-subdirs'?  If not, please read it again.

[ And see the comment in ffap.el under "File Name
  Handling": "Too many features here."  That's
  essentially a "TODO".  And the example is set
  there by commenting out the general functions
  `ffap-last' and `ffap-splice'.

  And see the comment with `ffap-locate-file',
  which cries out for `locate-library' to accept
  an optional arg to override suffixes. ]

> ---
> Directories beginning with \".\" are ignored, and directory
> symlinks are listed but never searched (to avoid loops).
> ---

Ignoring symlinks by default is what you
generally want from a function that gives you a
list of directories under/within the current one.

If you're really interested in a more general
function that lets you get or not get symlinks,
then define it.

The point is that such a general function doesn't
belong in `ffap.el'.  `ffap.el' could just make
use of it (ditto, `dired.el' or any other library).

Here's one such general function from Dired+, for
instance:

 diredp-directories-within is a Lisp function in 'dired+.el'.

 (diredp-directories-within &optional DIRECTORY NO-SYMLINKS-P
 PREDICATE)

 List of accessible directories within DIRECTORY.
 Directories in `vc-directory-exclusion-list' are skipped.

 Optional arg DIRECTORY defaults to the value of `default-directory'.
 Non-nil optional arg NO-SYMLINKS-P means do not follow symbolic links.
 Non-nil optional arg PREDICATE must be a function that accepts a
  file-name argument.  Only directories that satisfy PREDICATE are
  included in the result.

And yes, that's similarly more general than Dired.
Such a function really belongs in `files.el', not
in `ffap.el' or `dired.el'.

> It's well-suited for what ffap does, though, 
> which is why it lives in that file.

Nah.  It lives in `ffap.el' for hysterical raisins.

What about other `ffap.el' functions (and vars)
that, like `ffap-all-subdirs', aren't particularly
related to ffap?  E.g.:

 ffap-list-env
 ffap-reduce-path
 ffap-locate-file
 ffap-symbol-value
 ffap-file-suffix
 ffap-file-exists-string
 ffap-file-remote-p
 ffap-machine-p
 ffap-machine-at-point
 ffap-host-to-filename
 ffap-fixup-machine
 ffap-url-p
 ffap-url-unwrap-local
 ffap-url-unwrap-remote
 ffap-fixup-url
 ffap-read-file-or-url

There's a bunch of stuff in `ffap.el' that isn't
specifically related to picking up a file/url/*
name at point.

It's there because such picking-up needs it -
because _it isn't defined elsewhere_, in a more
general, always-loaded library.  That it's used
in `ffap.el' doesn't mean its definition belongs
there.

And why do some of the `ffap.el' commands that
open Dired have names with prefix `ffap-dired-',
but others have only prefix `dired-'?
Hysterical raisins?  

> Closing.

Too bad.  `ffap.el' needs cleanup.  Other files
could take advantage of its too-general stuff.
File-general functions belong in a library such
as `files.el'.